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

suppressing the warning but not the error #7

Closed
fommil opened this issue Apr 20, 2018 · 11 comments
Closed

suppressing the warning but not the error #7

fommil opened this issue Apr 20, 2018 · 11 comments

Comments

@fommil
Copy link

fommil commented Apr 20, 2018

In this code https://github.com/scalaz/ioeffect/blob/6f57538969e38f62f942a05d0b7734483629dc62/src/main/scala/scalaz/ioeffect/Void.scala#L34-L35

we are able to suppress this warning

[error] src/main/scala/scalaz/ioeffect/Void.scala:33:26: dead code following this construct
[error] private[ioeffect] object VoidImpl extends VoidModule with VoidSyntax {
[error]                          ^

but when scalacOptions in Compile += "-Xfatal-warnings" is added, we get

[info] Compiling 13 Scala sources to /tmp/fommil-sbt/home/fommil/Projects/scalaz-ioeffect/target/scala-2.12/classes ...
[error] No warnings can be incurred under -Xfatal-warnings.
[error] one error found
@ghik
Copy link
Owner

ghik commented Apr 20, 2018

Hello,

It looks like something changed in how -Xfatal-warnings is handled in Scala 2.12.5. This should probably work fine with 2.12.4. I'll dig into it and see if I can make it compatible.

@fommil
Copy link
Author

fommil commented Apr 20, 2018

this is scala 2.12.4... we can't upgrade to 2.12.5 because of compiler plugin incompatibilities.

@ghik
Copy link
Owner

ghik commented Apr 21, 2018

Can't reproduce this with a synthetic test, which means it must be related to SBT and the way it modifies the default scalac reporter. I need to dig more into this.

@ghik
Copy link
Owner

ghik commented Apr 21, 2018

OK, I think I nailed it.

There is a clash between silencer and semanticdb compiler plugin. Both have their own error/warning reporter which wraps the original reporter. They should totally be able to wrap each other and work together just fine but it's important to ensure that the silencer reporter wraps the semanticdb reporter and not the other way. This means that silencer plugin must be applied after semanticdb plugin, which can be forced using this workaround:

def ensureSilencerIsLast(options: Seq[String]): Seq[String] = {
  val (Seq(silencer), rest) = options.partition(o => o.startsWith("-Xplugin:") && o.contains("silencer"))
  rest :+ silencer
}

scalacOptions in Compile := ensureSilencerIsLast((scalacOptions in Compile).value)
scalacOptions in Test := ensureSilencerIsLast((scalacOptions in Test).value)

@fommil
Copy link
Author

fommil commented Apr 21, 2018

@olafurpg @xeno-by FYI...

@olafurpg
Copy link

there are similar conflicts with clippy, I'm not sure what's going on because semanticdb forwards all messages to the underlying reporter so silencer should get messages even if semanticdb is enabled last.

@ghik
Copy link
Owner

ghik commented Apr 21, 2018

@olafurpg semanticdb reporter might properly forward all the messages to its underlying reporter but it still maintains its own error/warning count. This means its warning count will also include warnings suppressed by silencer and when the compiler asks it "are there warnings?" it will answer "yes" even though they were suppressed.

One way to fix this is to synchronize error/warning counts with the underlying reporter. This is what silencer's SuppressingReporter does.

However, I think it would still be better if silencer was the last compiler plugin applied because semanticdb reporter still sees and stores the suppressed warnings. Only by making silencers reporter the toplevel one we can make sure that truly nobody ever sees warnings suppressed by it.

@fommil
Copy link
Author

fommil commented Apr 21, 2018

I think it would be good if all plugins tried to be good citizens to the best of their ability, with silencer at the end of the list as the nuclear option.

@olafurpg
Copy link

@ghik aah, good catch! I've opened scalameta/scalameta#1505 to track this, I suspect synchronizing info/warn/error count should be a simple fix.

I think it would actually be great to have silenced messages in SemanticDB since those can be useful for tooling. I've always wanted a mode to store in SemanticDB "Unused import" warnings without reporting them in the console since scalafix can automatically fix them.

@ghik
Copy link
Owner

ghik commented Apr 21, 2018

Ok, I think things are clear now and there isn't anything to do about it in silencer itself, so I'm closing this issue.

@ghik ghik closed this as completed Apr 21, 2018
@fommil
Copy link
Author

fommil commented Apr 21, 2018

thanks to you both!

rysh pushed a commit to rysh/example-scalafix-with-fatalwarnings that referenced this issue Oct 26, 2019
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

3 participants