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 S2201: FP for string.Intern which has side effect #2382

Closed
andrei-epure-sonarsource opened this issue Apr 15, 2019 · 3 comments · Fixed by #2415
Closed

Fix S2201: FP for string.Intern which has side effect #2382

andrei-epure-sonarsource opened this issue Apr 15, 2019 · 3 comments · Fixed by #2415
Assignees
Labels
Area: C# C# rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
Milestone

Comments

@andrei-epure-sonarsource
Copy link
Contributor

Description

string.Intern has a side-effect: it actually interns the string if it’s not interned already. Hence, this should not be marked as a bug by S2201.

Repro steps

Add string.Intern("foo"); inside ReturnValueIgnored.cs and run the ReturnValueIgnored UT.

Expected behavior

Should not raise an issue.

Actual behavior

Raises an issue 'Use the return value of method 'Intern', which has no side effect.'

Related information

  • SonarC# Version: 7.12

Originally reported on community.sonarsource.com

@andrei-epure-sonarsource andrei-epure-sonarsource added Type: False Positive Rule IS triggered when it shouldn't be. Status: Confirmed Area: C# C# rules related issues. labels Apr 15, 2019
@andrei-epure-sonarsource andrei-epure-sonarsource added this to the 7.14 milestone Apr 17, 2019
@andrei-epure-sonarsource andrei-epure-sonarsource changed the title S2201 FP: issue is raised for string.Intern which has side effect Fix S2201: FP for string.Intern which has side effect Apr 17, 2019
@christophe-zurn-sonarsource
Copy link
Contributor

Looking at the documentation for string.Intern, the method does not seem to have any side effect.
Only the string returned by the method will be interned, the string given as parameter will not be have been interned, so its reference might still be different than the interned one.

@christophe-zurn-sonarsource
Copy link
Contributor

Scratch that, it does have a side effect:

If the string does not exist, a reference to str is added to the intern pool, then that reference is returned.

Although I don't see why one would not use the return value that holds the reference to the interned string, since it has a side effect, we can definitely add an exception for this rule.

@christophe-zurn-sonarsource
Copy link
Contributor

As discussed, we will keep raising issue on unused string.Intern function call, but updated message and description

@christophe-zurn-sonarsource christophe-zurn-sonarsource moved this from In progress to Review in progress in Best Kanban Jun 12, 2019
Best Kanban automation moved this from Review in progress to Done Jun 13, 2019
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. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
Best Kanban
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants