bug icon indicating copy to clipboard operation
bug copied to clipboard

Missing accurate position of the unused import

Open adpi2 opened this issue 2 years ago • 8 comments

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.

adpi2 avatar Feb 20 '23 09:02 adpi2

(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.)

SethTisue avatar Aug 08 '23 22:08 SethTisue

The position should include just the selector.

I fixed a similar issue IIRC.

som-snytt avatar Aug 08 '23 22:08 som-snytt

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

joroKr21 avatar Aug 10 '23 18:08 joroKr21

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.

som-snytt avatar Aug 10 '23 18:08 som-snytt

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
  )

franciscolopezsancho avatar Sep 22 '24 04:09 franciscolopezsancho

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.)

som-snytt avatar Sep 22 '24 05:09 som-snytt

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

franciscolopezsancho avatar Sep 22 '24 21:09 franciscolopezsancho

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.

som-snytt avatar Sep 22 '24 21:09 som-snytt