scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Missing accurate position of the unused import #12734

Open adpi2 opened 1 year ago

adpi2 commented 1 year ago

Reproduction steps

Scala version: 2.13.8 (also in 2.12.15) scalacOptions: -Wunused:imports (or -Xlint:_)

package example

import scala.annotation.{nowarn, showAsInfix}

@nowarn
class Example(unusedVar: String)

Problem

The position of the unused import warning spans the entire line: the start column is 0 and the end column is 45. The pointer accurately points to 33, the beginning of the unused import.

This is a problem in LSP because the LSP position only contains the start column and end column, not the pointer. Hence we don't know which import is unused.

image

Expected result.

The start column should be 33 and end column should be 44.

SethTisue commented 1 year ago

(I've optimistically added the "good first issue" as often improving position information isn't too hard, but it's conceivable that if someone investigated, it would turn out that it isn't so easy to get the specific information to where it's needed in this case.)

som-snytt commented 1 year ago

The position should include just the selector.

I fixed a similar issue IIRC.

joroKr21 commented 1 year ago

case class ImportSelector(name: Name, namePos: Int, rename: Name, renamePos: Int) - it's not a tree

som-snytt commented 1 year ago

To remind myself, the similar issue was the "NamePos" business.

The selector not being a tree is a nuisance. I implemented rewrite for unused imports on Scala 3 somehow, on my branch for that, the details of course I have suppressed.

franciscolopezsancho commented 1 week ago

It seems it's working now

[info] Non-compiled module 'compiler-bridge_2.13' for Scala 2.13.8. Compiling...
[info]   Compilation completed in 3.379s.
[warn] /Users/francisco/scala_issue_12734/src/main/scala/example/Hello.scala:3:34: Unused import
[warn] import scala.annotation.{nowarn, showAsInfix}
[warn]                                  ^
[warn] one warning found
[success] Total time: 7 s, completed 22 Sept 2024, 11:39:24

Project build.sbt:

import Dependencies._

ThisBuild / scalaVersion     := "2.13.8"
ThisBuild / version          := "0.1.0-SNAPSHOT"
ThisBuild / organization     := "com.example"
ThisBuild / organizationName := "example"
ThisBuild / scalacOptions ++= Seq("-Wunused:imports")

lazy val root = (project in file("."))
  .settings(
    name := "scala_issue_12734",
    libraryDependencies += munit % Test
  )
som-snytt commented 1 week ago

It was reported on 2.13.8, so it can't be fixed on that version. Regular output just shows the point, as in OP.

I happen to have upgraded to WSL Ubuntu-24.04, so I can try things requiring modern glibc.

image

I got a squiggle before I added the lint. It's the same afterward. I'll try its auto-suggestion after I'm done here, but I suspect I won't like it.

image

I'm just getting accustomed to finding info in vscode, it's been a while. Showing that it is in column 1:

image

(Previous art alluded to above is at https://github.com/scala/scala3/pull/14524/files to support -rewrite.)

(By way of a typo, the NamePos ticket alluded to a year ago was 12735.)

franciscolopezsancho commented 1 week ago

I see. Thanks for reaching out. I see you have a fix (which I can learn from) 👍

som-snytt commented 1 week ago

Thanks for the prompt. I'd forgotten this issue. I also learned from it, namely about type completion of the import.

it isn't so easy to get the specific information to where it's needed

In particular, the selector is typechecked at completion, but the context doesn't give access to the import context at that time. That's OK because it's just a function of the tree position, the selector position, and the selected name.

I was not emboldened to offer -rewrite (as CodeAction); maybe I will check the baseball scores first.