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

RemoveUnusedImports not working #592

Closed
avdv opened this issue Feb 1, 2018 · 18 comments
Closed

RemoveUnusedImports not working #592

avdv opened this issue Feb 1, 2018 · 18 comments

Comments

@avdv
Copy link

avdv commented Feb 1, 2018

Hi.

I am trying to use scalafix 0.5.10 with a SBT project. A quick test with scalafix ProcedureSyntax was successful.

But running scalafix RemoveUnusedImports does not change anything, although the Scala compiler spits out 11 "Unused import" warnings:

[warn] /home/claudio/src/sbt-eve-play/src/main/scala/com/redbull/eve/sbt/EveCommonPlugin.scala:81:5: Unused import
[warn]     commandLineReleaseVersion,  useDefaults, versions
[warn]     ^
[warn] /home/claudio/src/sbt-eve-play/src/main/scala/com/redbull/eve/sbt/EveCommonPlugin.scala:81:33: Unused import
[warn]     commandLineReleaseVersion,  useDefaults, versions
[warn]                                 ^
[warn] /home/claudio/src/sbt-eve-play/src/main/scala/com/redbull/eve/sbt/EveCommonPlugin.scala:81:46: Unused import
[warn]     commandLineReleaseVersion,  useDefaults, versions
...

The imports are still there.

I also tried to run scalafix manually, using the --stdout option, but with the same result. The files are printed as they are processed, but still contain the unused imports.

Using --verbose does not help either. What can I do to debug this?

@MasseGuillaume
Copy link
Contributor

Do you have scalafix enabled in your "build build"?

project/project/plugins.sbt must contain addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.5.10")

@MasseGuillaume
Copy link
Contributor

Hmm nevermind, I thought those warnings are from the build itself, they are from an sbt plugin.

@olafurpg
Copy link
Contributor

olafurpg commented Feb 1, 2018

@avdv can you provide a minimal reproduction? Ideally from the standard library. Is this a regression from 0.5.7? What happens if you run scalafixCli --rules RemoveUnusedImports?

@avdv
Copy link
Author

avdv commented Feb 1, 2018

Sorry, should have been more specific. Yes, this is from an SBT plugin, but I also tried it in another SBT project.

I am using SBT 1.1.0, Scala 2.12.4. The scalacOptions are looking OK and the semanticdb files are generated.

@olafurpg
Copy link
Contributor

olafurpg commented Feb 1, 2018

What happens if you run scalafixCli --rules RemoveUnusedImports?

@olafurpg
Copy link
Contributor

olafurpg commented Feb 1, 2018

Is this a regression from 0.5.7?

@olafurpg
Copy link
Contributor

olafurpg commented Feb 1, 2018

Can you provide a small source file that reproduces the bug? :)

@avdv
Copy link
Author

avdv commented Feb 1, 2018

Hi @olafurpg,

I have tried with 0.5.7, but still no dice. I have prepared a fresh SBT project with sbt new scala/hello-world.g8, set the Scala version to 2.12.4, SBT version to 1.1.0 and added the scalafix plugin. It did not work.

Also, I had used scalafix downloaded with coursier manually to no avail.

But then, I ran SBT with -sbt-dir /tmp/sbt and low and behold, this time it worked. Seems like scalafix conflicts with some global SBT plugins (namely coursier?!) I have activated... I'll look into this.

@avdv
Copy link
Author

avdv commented Feb 1, 2018

OK, seems the culprit is not coursier as suspected, but clippy:

addSbtPlugin("com.softwaremill.clippy" % "plugin-sbt" % "0.5.3")

I still do not understand why running scalafix manually did not work either. (I just ran ~/.coursier/coursier bootstrap ch.epfl.scala:scalafix-cli_2.12.4:0.5.10 -f --main scalafix.cli.Cli -o scalafix to create an executable). Does coursier use SBT under the hood - ie. also the global plugins?

@olafurpg
Copy link
Contributor

olafurpg commented Feb 1, 2018

Thanks for investigating @avdv Interesting, I suspect there is a bad interplay between the clippy compiler plugin and semanticdb-scalac. RemoveUnusedImports uses the messages reported by the compiler so this bug makes sense if clippy is changing the reporter

@olafurpg
Copy link
Contributor

olafurpg commented Feb 1, 2018

clippy looks btw super cool! Want to try it out 😄

@ShaneDelmore
Copy link
Contributor

Clippy does hijack the reporter and inject new messages but it also leaves the original message so it should be possible to make them play well together.

@avdv
Copy link
Author

avdv commented Feb 1, 2018

thanks @ShaneDelmore, that makes total sense now, I compiled (read: generated the semanticdb's) with clippy on, so running scalafix on the command line later did still not recognize the import errors -- same input -> same output.

clippy looks btw super cool! Want to try it out 😄

Oh yes, I guess I'm so used to its output that I forgot that the messages look different usually... 😉

@avdv
Copy link
Author

avdv commented Feb 5, 2018

There is not much that could be done by scalafix about this, I think. Should we close this issue?

@MasseGuillaume
Copy link
Contributor

@avdv we should add a note about this in the documentation. Maybe open an issue on the clippy side. I'm not sure why we are not being able to consume those errors.

@avdv
Copy link
Author

avdv commented Feb 5, 2018

Yes, I did open an issue here: softwaremill/scala-clippy#61

Somehow clippy does not leave the original message in the semanticdb file.

@olafurpg
Copy link
Contributor

The relevant piece of code in scalameta is https://github.com/scalameta/scalameta/blob/2bc7cfd081b45bfbb2e16c60eb906fa208a597cb/semanticdb/scalac/library/src/main/scala/scala/meta/internal/semanticdb/scalac/HijackReporter.scala#L12 We set the reporter to our own custom implementation that records all messages and forwards them to the underlying reporter.

The ordering of the compiler plugins may impact if it works or not. You can try scalafixEnabled := false and manually addCompilerPlugin("org.scalameta" % "semanticdb-scalac" ....). Not sure whether it's better to enable semanticdb before or after clippy.

@olafurpg
Copy link
Contributor

I'm not sure what can be done on the scalafix side to improve this situation. The bug either needs to be addressed in clippy or semanticdb-scalac.

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

No branches or pull requests

4 participants