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

StringPlusAny still problematic #288

Closed
ritschwumm opened this issue Nov 8, 2016 · 5 comments
Closed

StringPlusAny still problematic #288

ritschwumm opened this issue Nov 8, 2016 · 5 comments

Comments

@ritschwumm
Copy link
Contributor

ritschwumm commented Nov 8, 2016

"" + { val x = ""; if (true) x else x }

and

Set("") + ""

both trigger

Implicit conversion to string is disabled
@ritschwumm ritschwumm changed the title if-clauses still problematic string addition still problematic Nov 8, 2016
@ritschwumm ritschwumm changed the title string addition still problematic StringPlusAny still problematic Nov 8, 2016
@Fristi
Copy link

Fristi commented Nov 21, 2016

Can confirm Implicit conversion to string is disabled is problematic on string sets

@seanmcl
Copy link
Contributor

seanmcl commented Nov 23, 2016

+1

ritschwumm pushed a commit to ritschwumm/wartremover that referenced this issue Nov 23, 2016
@ritschwumm
Copy link
Contributor Author

i played with this for a bit..

Set("") + "" may be fixable by having a better mechanism to figure out the type of Set(""). at the moment it seems to be stumbling over the fact that the owner of the apply method in Set.apply("") is GenericCompanion and looks there for an overloaded + method.

"" + { val x = ""; if (true) x else x } looks more difficult - for some reason, the type of { val x = ""; if (true) x else x } is reported as Any...

@ClaireNeveu
Copy link
Collaborator

I preserved the old Any2StringAdd behavior in the wart, so we should be able to fix the Set problem by just checking when the left-hand type is String and ignoring cases where only the right-hand type is String.

As for the other issue, we probably need more investigation into what is wrong with the reflection api here but users can fix it with explicit typing.

@ritschwumm
Copy link
Contributor Author

i probably should have opened two separate issues...

tim-zh added a commit to tim-zh/wartremover that referenced this issue Dec 17, 2016
tim-zh added a commit to tim-zh/wartremover that referenced this issue Dec 21, 2016
ritschwumm pushed a commit to ritschwumm/wartremover that referenced this issue Dec 23, 2016
tim-zh added a commit to tim-zh/wartremover that referenced this issue Dec 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants