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
New rule S3878: Arrays should not be created for params parameters #6666
Conversation
analyzers/src/SonarAnalyzer.Common/Rules/ArrayPassedAsParamsBase.cs
Outdated
Show resolved
Hide resolved
.../its/expected/Ember-MM/Ember Media Manager-{9B57D3AB-AF12-4012-B945-284C2448DC81}-S3878.json
Show resolved
Hide resolved
analyzers/its/expected/Nancy/Nancy.ViewEngines.Razor.BuildProviders--net452-S3878.json
Show resolved
Hide resolved
@@ -0,0 +1,31 @@ | |||
<p>There’s no point in creating an array solely for the purpose of passing it as a params (<code>...</code>) argument; params keyword allow to pass a |
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.
a params (
...
) argument
This refers to varargs in java. This needs to be adopted. Make sure it is fixed in the RSpec PR.
@@ -0,0 +1,30 @@ | |||
<p>There’s no point in creating an array solely for the purpose of passing it as a ParamArray (<code>...</code>) argument; ParamArray keyword allow to |
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.
Same here.
analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpConstructorParameterLookup.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/ArrayPassedAsParamsBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
.../its/expected/Ember-MM/Ember Media Manager-{9B57D3AB-AF12-4012-B945-284C2448DC81}-S3878.json
Show resolved
Hide resolved
.../its/expected/Ember-MM/Ember Media Manager-{9B57D3AB-AF12-4012-B945-284C2448DC81}-S3878.json
Show resolved
Hide resolved
.../its/expected/Ember-MM/Ember Media Manager-{9B57D3AB-AF12-4012-B945-284C2448DC81}-S3878.json
Show resolved
Hide resolved
.../its/expected/Ember-MM/Ember Media Manager-{9B57D3AB-AF12-4012-B945-284C2448DC81}-S3878.json
Show resolved
Hide resolved
.../its/expected/Ember-MM/Ember Media Manager-{9B57D3AB-AF12-4012-B945-284C2448DC81}-S3878.json
Show resolved
Hide resolved
analyzers/its/expected/Nancy/Nancy.ViewEngines.Razor.BuildProviders--net452-S3878.json
Show resolved
Hide resolved
analyzers/its/expected/Nancy/Nancy.ViewEngines.Razor.BuildProviders--net452-S3878.json
Show resolved
Hide resolved
analyzers/its/expected/akka.net/Akka.DistributedData--netstandard2.0-S3878.json
Show resolved
Hide resolved
analyzers/its/expected/akka.net/Akka.DistributedData--netstandard2.0-S3878.json
Show resolved
Hide resolved
ab3294a
to
5517d4f
Compare
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.
Only minor changes. I think you are done! Good job. Please have a look at the code smell as well.
analyzers/src/SonarAnalyzer.CSharp/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Helpers/MethodParameterLookupBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/ArrayPassedAsParamsBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/ArrayPassedAsParamsBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/ArrayPassedAsParamsBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
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.
The default cases need to be added back and the new test case for the exception should be standalone.
analyzers/src/SonarAnalyzer.CSharp/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Helpers/MethodParameterLookupTest.cs
Outdated
Show resolved
Hide resolved
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.
I think we can improve precision in C# and VB a bit. There are some confusing cases with VB array initializers that need some more effort.
analyzers/tests/SonarAnalyzer.UnitTest/Helpers/MethodParameterLookupTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Helpers/MethodParameterLookupTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
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.
VB behavior looks good now. Most remaining work is more stylistic. I'm not sure about removing one restriction in C#.
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/ArrayPassedAsParamsTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/ArrayPassedAsParamsTest.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
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.
Just some pattern magic left.
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ArrayPassedAsParams.cs
Outdated
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! |
SonarCloud Quality Gate failed. |
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. @pavel-mikula-sonarsource please merge. Code coverage can not be improved because the default case is untestable. It is there for defensive coding.
Fixes #6657