Skip to content
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

Modify rule S3878: Add collection expression and collection params to the description #4430

Merged
merged 5 commits into from
Oct 23, 2024

Conversation

martin-strecker-sonarsource
Copy link
Contributor

Review

A dedicated reviewer checked the rule description successfully for:

  • logical errors and incorrect information
  • information gaps and missing content
  • text style and tone
  • PR summary and labels follow the guidelines

Copy link
Contributor

@CristianAmbrosini CristianAmbrosini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of LAYC

Comment on lines 41 to 45
== Resources

* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/method-parameters#params-modifier[`params` modifier]
* Microsoft Learn - C# 13 https://learn.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-13#params-collections[`params` collections]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/collection-expressions[Collection expressions]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
== Resources
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/method-parameters#params-modifier[`params` modifier]
* Microsoft Learn - C# 13 https://learn.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-13#params-collections[`params` collections]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/collection-expressions[Collection expressions]
== Resources
=== Documentation
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/method-parameters#params-modifier[`params` modifier]
* Microsoft Learn - C# 13 https://learn.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-13#params-collections[`params` collections]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/collection-expressions[Collection expressions]

rules/S3878/csharp/rule.adoc Show resolved Hide resolved
rules/S3878/csharp/rule.adoc Show resolved Hide resolved
@@ -1,6 +1,6 @@
== Why is this an issue?

There's no point in creating an array solely for the purpose of passing it to a `params` parameter. Simply pass the elements directly. They will be consolidated into an array automatically.
Creating an array or using a collection expression solely for the purpose of passing it to a `params` parameter is unnecessary. Simply pass the elements directly, and they will be automatically consolidated into the appropriate collection type.

=== Noncompliant code example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
=== Noncompliant code example
== How to fix it
=== Code examples
==== Noncompliant code example

rules/S3878/csharp/rule.adoc Show resolved Hide resolved
Copy link
Contributor

@CristianAmbrosini CristianAmbrosini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please address this comment before merging

Copy link

Quality Gate passed Quality Gate passed for 'rspec-tools'

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Copy link

Quality Gate passed Quality Gate passed for 'rspec-frontend'

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@martin-strecker-sonarsource martin-strecker-sonarsource merged commit f4c4dd5 into master Oct 23, 2024
8 of 9 checks passed
@martin-strecker-sonarsource martin-strecker-sonarsource deleted the Martin/S3878_ParamsCollections branch October 23, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants