Skip to content

CC0118 Remove to string only if it is save to do#939

Merged
giggio merged 9 commits intocode-cracker:masterfrom
MaStr11:CC0118RemoveToStringOnlyIfItIsSaveToDo
Jun 24, 2017
Merged

CC0118 Remove to string only if it is save to do#939
giggio merged 9 commits intocode-cracker:masterfrom
MaStr11:CC0118RemoveToStringOnlyIfItIsSaveToDo

Conversation

@MaStr11
Copy link
Copy Markdown
Contributor

@MaStr11 MaStr11 commented Jun 23, 2017

Fixes #866.

I implemented option 2 as requested. This fix now looks up the types of both sides of the plus operator and tries removes those candidates, that might break things.

It turned out, that some of the checks in CheckAddOperationOverloadsOfTypes are not needed (After the second check every other check returns false). Essentially right now the removal of "ToString" is only save if either the underlying type is a string (as in 1 + "a".ToString()) or the type on the other side of the plus operator is a string (as in "a" + 1.ToString()). I kept all the other checks for future enhancements to make clear that there are several cases to consider. Further I added lots of unit tests to cover these cases. The idea is that CheckAddOperationOverloadsOfTypes can easily be expanded in the future to support more cases without breaking anything. Those redundant checks are also very cheap and are essentially just simple comparisons.

I also had some troubles with the merging (started of the master branch, tried to re-base my changes on the V1.0.x branch, discovered that you removed this analyzer there, went back to master). So I'm sorry for the complicated commits in this pull request.

@MaStr11
Copy link
Copy Markdown
Contributor Author

MaStr11 commented Jun 23, 2017

Currently the build fails because I have used some C#7 features:

if (toStringCall.Expression is MemberAccessExpressionSyntax memberAccess)

Should I remove those features or can you upgrade AppVeyor?

@giggio giggio self-assigned this Jun 24, 2017
@giggio giggio self-requested a review June 24, 2017 02:27
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 91.587% when pulling df2d6c3 on MaStr11:CC0118RemoveToStringOnlyIfItIsSaveToDo into d6d7bdf on code-cracker:master.

@giggio
Copy link
Copy Markdown
Member

giggio commented Jun 24, 2017

The problem was in psake. I wrote a workaround now on 8f48dfe. More info: psake/psake#201
The build passed now. Now I can check the PR.

@giggio giggio merged commit 6de6d07 into code-cracker:master Jun 24, 2017
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.

BUG: CC0118 - Unnecessary '.ToString()' call in string concatenation

3 participants