Issue #2801: SA1500 fires for the while clause of do/while statement

[Kevin-Andrew]

Provide new configuration setting, layoutRules.allowDoWhileOnClosingBrace such that when enabled, the following will be allowed:

do
{
    Console.WriteLine("test");
} while (false);

See Issue #2801.

[codecov]

Codecov Report

Merging #3196 (e77e66a) into master (e7af4b3) will decrease coverage by 0.03%.
The diff coverage is 99.29%.

❗ Current head e77e66a differs from pull request most recent head c58307c. Consider uploading reports for the commit c58307c to get more accurate results

@@            Coverage Diff             @@
##           master    #3196      +/-   ##
==========================================
- Coverage   93.44%   93.40%   -0.04%     
==========================================
  Files        1029     1029              
  Lines      111035   111170     +135     
  Branches     3934     3940       +6     
==========================================
+ Hits       103754   103841      +87     
- Misses       6266     6268       +2     
- Partials     1015     1061      +46     

[Kevin-Andrew]

This line shows only partial code coverage from the Unit Tests and would be the reason for the decrease in Codecov.

The reason is because there isn't a test in which the braceToken.Parent is null. I'm not completely familiar with the flow of execution and what is possible and what isn't, so I wrote this just being safe. I suppose, given the SyntaxTree, it may be impossible for braceToken.Parent to be null, but I just don't know. I tried to write some Unit Tests that had malformed syntax and they would fail with a compiler error.

If someone is sure that braceToken.Parent can never be null, then I'd be happy to change it. I know the line of code right above this also uses braceToken.Parent without the Null-Conditional operator, but I wasn't sure if the fact that it is applicable to a Multi-Dimensional Array Initializer, that the syntax may be different. So I just wanted to be extra safe.

But we can remove it if we want the Codcov percentage to go back up.

[Kevin-Andrew]

I see that there is a 1.2.0-beta in the works:

So I put the version number here as >1.2.0 because I don't know how to predict what version this might end up in.

We can change it to something else, or omit these changes from this PR and make another one or something later.

[sharwell]

@Kevin-Andrew I may still merge this. Let me give it some thought today.

[Kevin-Andrew]

Since I don't know the syntax tree well enough, I just want to make sure, as I had similar code and this same question elsewhere, "Is using .Parent.Parent safe here?" We know that openBraceToken.Parent is not null and is of kind Block, but does that guarantee that .Parent.Parent will never be null? For example what if I have code like the following that uses some extra curly braces for scoping:

{ // Some braces for scoping
   var x = 0;
   do
   {
      x = 1;
   } while (x == 0);
}

[sharwell]

IsKind allows null, and openBraceToken.Parent.IsKind(SyntaxKind.Block) is only true if openBraceToken.Parent is not null.

[Kevin-Andrew]

Thanks for looking at this @sharwell. I see that one of the required automation builds is failing.

I've looked at the information/logs in the build pipeline, but I'm not familiar with it enough to tell what the actual problem might be. But if there is anything more I can help with, I'd be happy to do so.

[sharwell]

I restarted the build, and it should pass this time. I'm guessing our timeout is too short to handle all cases, but I can increase it.