Fix S2234 FN: primary constructors for records, classes and structs

[martin-strecker-sonarsource]

Fixes #8070
Fixes #8071

The original implementation was rewritten from scratch. The reasons are

• The implementation didn't follow our principals
• The analyzer wasn't derived from SonarDiagnosticAnalyzer<TSyntaxKind>
• The registration happened in the derived classes instead of the base class
• LanguageFacade wasn't used
• There was a mixture of abstract methods (e.g., GetArgumentTypeSymbolInfo) and passed delegates (e.g., getLocationToReport parameter in ReportIncorrectlyOrderedParameters) which are both used as language-specific holes for the language-neutral implementation
• The ArgumentIdentifier private class and derived classes are not needed
• The logic was complex to follow

The current implementation follows our standards more closely and moves a lot of logic to the facade.

[martin-strecker-sonarsource]

DerivesOrImplements misses a whole lot of conversions:
Identity conversions
Implicit numeric conversions
Implicit enumeration conversions
Implicit interpolated string conversions
Implicit reference conversions
Boxing conversions
Implicit dynamic conversions
Implicit type parameter conversions
Implicit constant expression conversions
User-defined implicit conversions
Anonymous function conversions
Method group conversions
Null literal conversions
Implicit nullable conversions
Implicit tuple conversions
Lifted user-defined implicit conversions
Default literal conversions
Implicit throw conversion

The best way to handle this is by using ClassifyConversion(SemanticModel, ExpressionSyntax, ITypeSymbol, Boolean) but this requires even more changes. I would propose to create an issue about adding something like ILanguageFace.IsAssignableTo(SyntaxNode expression, ITypeSymbol destination) to the facade, bring it under test, and use it here. We also should have a look at how Roslyn handles this.

[martin-strecker-sonarsource]

ArgumentName() already returns names for all kinds of expressions:

• M(A.B) -> B
• M(A?.B) -> B
• M(A())-> A
• M(A) -> A (this might be unsupported at the moment)

and more could be added when this is fixed: #8241

Given that, the comparison should be replaced by StringComparison.OrdinalIgnoreCase for all languages.

[cristian-ambrosini-sonarsource]

That's a lot better!

[cristian-ambrosini-sonarsource]

Suggested change

	&& methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName) && argumentType.DerivesOrImplements(p.Type)) is { IsParams: false }
	&& methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName)
	&& argumentType.DerivesOrImplements(p.Type)) is { IsParams: false }

[martin-strecker-sonarsource]

Why?

[cristian-ambrosini-sonarsource]

you inlined that AND condition, all the others are listed one after the other

[cristian-ambrosini-sonarsource]

LGTM! Please fix the last unresolved comment before merging

[martin-strecker-sonarsource]

Can you please have a second look? I used the enhanced "NodeIdentifier" method of the facade for the primary location in CS:
0ebf903

and here I fixed the outstanding issue:
a9fe51c

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

98.3% Coverage
0.0% Duplication

[cristian-ambrosini-sonarsource]

Looks good! I think you can merge this one

[martin-strecker-sonarsource]

Peach validation:

Lost one issue, which was an invocation of Func<string, object, object, bool> with (name, arg1, arg2). The delegate is defined as public delegate TResult Func<in T1,in T2,in T3,out TResult>(T1 arg1, T2 arg2, T3 arg3); and therefore this seems to be a FN. On the other hand arg1, .. are generic names of the Func delegate so in this case I would say it is a TN.
This might not be the case for "custom" delegates.

Three new issues:

https://peach.sonarsource.com/project/issues?issues=AYtq0ttJgPIhmeQQhQ_D&open=AYtq0ttJgPIhmeQQhQ_D&id=dotnet-runtime

https://peach.sonarsource.com/project/issues?issues=AYtpf6OqhLL7WIb3wRCY&open=AYtpf6OqhLL7WIb3wRCY&id=wcf

Both examples are identical (copy/paste).

TP: GetIdentifier() for the argument expression supports more than just simple IdentifierNameSyntax and more arguments are considered to be named (see also #8253 on how to extend support even more here)

https://peach.sonarsource.com/project/issues?issues=AYtpjmQYUJUfslckQTtU&open=AYtpjmQYUJUfslckQTtU&id=omnisharp-server

TP: this() object initializer call support