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

Fix S6605 FP: Should not fire in expressions #7508

Closed
benjiwolff opened this issue Jun 26, 2023 · 3 comments · Fixed by #7671
Closed

Fix S6605 FP: Should not fire in expressions #7508

benjiwolff opened this issue Jun 26, 2023 · 3 comments · Fixed by #7671
Assignees
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Milestone

Comments

@benjiwolff
Copy link

benjiwolff commented Jun 26, 2023

Description

As I understand, rule S6605 suggests the use of the collection-specific "Exists" method for performance reasons. It becomes a problem when the "Any" extension method is used in an expression because switching to the "Exists" method changes the expression (e. g. the C# MongoDb Driver supports Any but not Exists). There are also no performance benefits unless the expression is compiled and executed which should not be done anyways when performance is important.

Repro steps

Expression<Func<List<int>, bool>> containsThree = list => list.Any(el => el == 3);

Expected behavior

The rule should not fire.

Actual behavior

The rule fires.

Related information

  • C# Plugin version 9.4.0.72892
  • JetBrains Rider version 2023.1.1
  • MSBuild / dotnet version 7.0.305
  • Operating System macOS Ventura 13.4.1
@martin-strecker-sonarsource martin-strecker-sonarsource added Type: False Positive Rule IS triggered when it shouldn't be. Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. labels Jun 29, 2023
@martin-strecker-sonarsource
Copy link
Contributor

Thank you @DrummingBeb for bringing this up. Linq expressions are indeed a special case and we should not raise in that context as they are almost always not executed but translated.

Note to the implementer: We have the IsInExpressionTree helper methods for that.

@martin-strecker-sonarsource
Copy link
Contributor

martin-strecker-sonarsource commented Jun 29, 2023

See also #7286
@sebastien-marichal is this already fixed by #7332? If so, you may use IsInExpressionTree instead of the custom logic you used.

@sebastien-marichal
Copy link
Contributor

@martin-strecker-sonarsource The fix in #7332 only checks if Any is used within an EntityFramework context.
Using IsInExpressionTree would prevent a broader range of false positives indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants