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 S2259 FP: SE engine doesn't take into account element existence collection methods #9505

Merged
merged 8 commits into from
Jul 5, 2024

Conversation

mary-georgiou-sonarsource
Copy link
Contributor

Fixes #8266

Comment on lines 142 to 146
symbol.IsExtensionMethod switch
{
true => symbol.Parameters.Length > 1,
false => !symbol.Parameters.IsEmpty
};
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
symbol.IsExtensionMethod switch
{
true => symbol.Parameters.Length > 1,
false => !symbol.Parameters.IsEmpty
};
symbol.IsExtensionMethod
? symbol.Parameters.Length > 1
: !symbol.Parameters.IsEmpty;

Copy link
Contributor

@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource left a comment

Choose a reason for hiding this comment

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

LGTM (review was mostly done sync with @mary-georgiou-sonarsource and @Tim-Pohlmann)

Copy link

sonarcloud bot commented Jul 5, 2024

Copy link

sonarcloud bot commented Jul 5, 2024

Tag("Value", value);
""";
var validator = SETestContext.CreateCS(code, $"List<object> collection").Validator;
validator.TagValue("Value").Should().HaveOnlyConstraint(ObjectConstraint.Null);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can merge these tests into one if you extract the collection type into a parameter:

  • "List<object>"
  • "Dictionary<int, object>"
  • etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes but I'm testing different things - in one case it's a reference that can return null and in the other case it never returns cause the keyvalue pair is a struct.

@mary-georgiou-sonarsource mary-georgiou-sonarsource changed the title Fix S2259 FP: SE engine doesn't take into account collection extension method Any Fix S2259 FP: SE engine doesn't take into account element existence collection methods Jul 5, 2024
@mary-georgiou-sonarsource mary-georgiou-sonarsource merged commit 138deb7 into master Jul 5, 2024
30 checks passed
@mary-georgiou-sonarsource mary-georgiou-sonarsource deleted the mary/se-fp branch July 5, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix S2259 FP: SE engine doesn't take into account element existence collection methods
2 participants