Update SA1010 to not trigger on list patterns

[bjornhellander]

Fixes #3503

[codecov]

Codecov Report

Merging #3507 (e852d1f) into master (ccaac20) will increase coverage by 0.00%.
The diff coverage is 100.00%.

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

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3507   +/-   ##
=======================================
  Coverage   93.26%   93.26%           
=======================================
  Files        1079     1080    +1     
  Lines      113737   113762   +25     
  Branches     4027     4028    +1     
=======================================
+ Hits       106072   106097   +25     
  Misses       6630     6630           
  Partials     1035     1035           

[sharwell]

💡 This should be converted to a code fix test, which includes examples of various incorrect formatting that is then corrected (there is no need to separately test valid syntax, since all code fix tests automatically verify that the fixed code contains no undeclared diagnostics).

[bjornhellander]

This specific analyzer only suggests to remove spaces now, never to add them. I filed #3508 a couple of days ago to update SA1000/SA1003 to get a diagnostic where a space is missing, but that might be a mistake. Not sure.

[sharwell]

Interesting. Based on other analyzers (e.g. { used in property patterns), SA1010 would be the analyzer that wants to add the space here.

[bjornhellander]

Since this seems to be in line with the current implementation, would it be ok to merge this if I create another issue to investigate the resposibility of this analyzer?

[bjornhellander]

What do you want to do, @sharwell?

[sharwell]

💭 Is this analyzer not responsible for reporting a this?

x is ( [1]);
       ^

In #3511 a very similar scenario does report a diagnostic.

[bjornhellander]

The only diagnostic I would like to see in that example is SA1008. I am thinking that the list pattern's opening bracket itself should not trigger any opinion about preceding space, anymore than a literal value does. It is always something else that is causing the opinion, like a preceding open parenthesis, operator, keyword, etc. Before this PR, your example would trigger both SA1008 and SA1010. Is that a behaviour that you would like to keep? I could very well not be seeing the whole picture here, but I feel that not having an opinion at all is the correct behaviour for this rule in this specific case.

[bjornhellander]

I actually feel the same way about the property pattern changes made in SA1012, now that I think of it. I assume that both these statements will trigger two diagnostics:

_ = x is ( { P: 0 }, { P: 0 }) // SA1008 and SA1012
_ = x is [ { P: 0 }, { P: 0 }] // SA1010 and SA1012

With my thinking, only SA1008 and SA1010 should be triggered.

[bjornhellander]

Returning to SA1010 again, there was already a similar case implemented where this code only triggers SA1012 and not SA1010:
var test = new Dictionary<ulong, string>() {[100] = "100" };

[bjornhellander]

@sharwell?

[jnm2]

Would you be interested in fixing #3509 at the same time?

[bjornhellander]

I rebased this since it was getting old and turned the test into a theory to make it easier to debug

[bjornhellander]

Thanks!