-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
.NET 8 and C# 12: fix repros and UTs for rules starting with P in SonarWay #8118
.NET 8 and C# 12: fix repros and UTs for rules starting with P in SonarWay #8118
Conversation
bd6f392
to
745fa94
Compare
b0fb546
to
c3eb751
Compare
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParameterValidationInYieldShouldBeWrapped.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp11.cs
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp11.cs
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs
Show resolved
Hide resolved
@@ -91,22 +72,22 @@ public class DefaultLambdaParameters | |||
{ | |||
void InvokedFromAnotherLambda() | |||
{ | |||
var f1 = (int a, int b) => a + b; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests were not supposed to be duplicates of the ones for C# 11, but rather tests with the same structure, but having default lambda parameters set, that makes them C# 12 specific.
I have kept the C# 12 testa and added the missing default values to the lambda parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to mention the default lambda parameters in #8072 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I have added a sentence in the description of the GitHub issue.
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -91,22 +72,22 @@ public class DefaultLambdaParameters | |||
{ | |||
void InvokedFromAnotherLambda() | |||
{ | |||
var f1 = (int a, int b) => a + b; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to mention the default lambda parameters in #8072 ?
Due to a glitch in the review process described here and here, we need a second pass, to address the following new comments on rules starting with P: