scalameta / metals-feature-requests

Issue tracker for Metals feature requests
37 stars 4 forks source link

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

Open curtisfennergivery opened 1 year ago

curtisfennergivery commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I think this issue should be fixed at the compiler side, so I opened https://github.com/scala/bug/issues/12735.