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

Unused method warning from bloop highlights entire method body, rather than just method name #330

Open
curtisfennergivery opened this issue Feb 7, 2023 · 3 comments

Comments

@curtisfennergivery
Copy link

Describe the bug

When the -Xlint:unused Scala compiler flag is enabled, Metals correctly indicates that a private function with no uses is unused:

Screenshot of below code running in VSCode with entire `thisPrivateMethodIsUnused` highlighted with yellow squiggly underlines

class Basic {
  private def thisPrivateMethodIsUnused(): Unit = {
    println("Do a thing")
  }

  def publicMethod(): Unit = {
    println("Do a thing")
  }
}
ThisBuild / version := "0.1.0-SNAPSHOT"

ThisBuild / scalaVersion := "2.13.10"

lazy val root = (project in file("."))
  .settings(
    name := "minimalmetals"
  )
Compile / scalacOptions ++= Seq(
  "-Xlint:unused"
)

However, the annotation spans the entire method implementation, rather than just the name. This can obscure other useful annotations on the method body, making it harder to implement functions. When writing a function, it's generally initially unused, so this is a frustrating experience.

A similar issue with deprecation warnings was reported in scalameta/metals#719

Expected behavior

Only the method name, rather than the entire body, should be highlighted by the unused warning.

For example, IntelliJ IDEA indicates an unused function by displaying the name of the function in italic gray, and does not annotate the function's body.

Operating system

macOS

Editor/Extension

VS Code

Version of Metals

v0.11.10

Extra context or search terms

warning, squiggle, unused, annotation, entire, body

@tgodzik
Copy link
Contributor

tgodzik commented Feb 7, 2023

Thanks for reporting! This is a warning coming directly from the compiler, so Metals doesn't really control how they are rendered, but it depends on the editor. We would either:

  1. Need to modify the compiler to show the warnings only on the name

  2. Or do some heuristics on the Metals side to calculate the name based on the warning range

  3. might not be very difficult, but not sure if it's something that we want. Ideally this should be solved in the compiler.

I will move it to the feature requests since it's not really a bug, rather current limitation of LSP

@tgodzik tgodzik transferred this issue from scalameta/metals Feb 7, 2023
@tgodzik
Copy link
Contributor

tgodzik commented Feb 7, 2023

Btw. I do understand your issue and it does annoy mi a bit to see it unused, when in reality sometimes it's just a mismatch of named or non compiling code.

@adpi2
Copy link
Member

adpi2 commented Feb 20, 2023

I think this issue should be fixed at the compiler side, so I opened scala/bug#12735.

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