Skip to content

feature: Add scalafix imports rules for completions, quickfix #7490

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.

Sign up for GitHub

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

Draft
theBird2Bones wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feature: Add scalafix imports rules for completions, quickfix #7490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ lazy val interfaces = project
),
crossPaths := false,
libraryDependencies ++= List(
V.lsp4j
V.lsp4j,
"ch.epfl.scala" % "scalafix-interfaces" % V.scalafix,
),
javacOptions := Seq("--release", "8"),
crossVersion := CrossVersion.disabled,
Expand Down
22 changes: 20 additions & 2 deletions metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import scala.meta.pc.SymbolSearch
import ch.epfl.scala.bsp4j.BuildTargetIdentifier
import org.eclipse.lsp4j.InitializeParams

import scalafix.interfaces.imports.OrganizeImportsDirect

class CompilerConfiguration(
workspace: AbsolutePath,
config: ClientConfiguration,
Expand Down Expand Up @@ -53,6 +55,7 @@ class CompilerConfiguration(
symbolSearch: SymbolSearch,
classpath: Seq[Path],
referenceCounter: CompletionItemPriority,
organizeImportsDirect: OrganizeImportsDirect,
) extends MtagsPresentationCompiler {
private val mtags =
mtagsResolver.resolve(scalaVersion).getOrElse(MtagsBinaries.BuildIn)
Expand All @@ -65,6 +68,7 @@ class CompilerConfiguration(
"default",
symbolSearch,
referenceCounter,
organizeImportsDirect,
)

def shutdown(): Unit = standalone.shutdown()
Expand All @@ -79,6 +83,7 @@ class CompilerConfiguration(
sources: Seq[Path],
workspaceFallback: Option[SymbolSearch],
referenceCounter: CompletionItemPriority,
organizeImportsDirect: OrganizeImportsDirect,
): StandaloneCompiler = {
val search =
createStandaloneSearch(classpath, sources, workspaceFallback)
Expand All @@ -87,6 +92,7 @@ class CompilerConfiguration(
search,
classpath,
referenceCounter,
organizeImportsDirect,
)
}
}
Expand Down Expand Up @@ -160,6 +166,7 @@ class CompilerConfiguration(
mtags: MtagsBinaries,
search: SymbolSearch,
referenceCounter: CompletionItemPriority,
organizeImportsDirect: OrganizeImportsDirect,
additionalClasspath: Seq[Path] = Nil,
) extends LazyCompiler {

Expand Down Expand Up @@ -199,6 +206,7 @@ class CompilerConfiguration(
name,
search,
referenceCounter,
organizeImportsDirect,
)
.withBuildTargetName(scalaTarget.displayName)
}
Expand All @@ -210,6 +218,7 @@ class CompilerConfiguration(
Nil,
Some(search),
referenceCounter,
organizeImportsDirect,
).standalone

}
Expand All @@ -233,6 +242,7 @@ class CompilerConfiguration(
mtags,
worksheetSearch,
referenceCounter,
OrganizeImportsDirect.noopInstance(),
classpath,
)
}
Expand All @@ -248,7 +258,12 @@ class CompilerConfiguration(

protected def newCompiler(classpath: Seq[Path]): PresentationCompiler = {
val pc = JavaPresentationCompiler()
configure(pc, search, completionItemPriority)
configure(
pc,
search,
completionItemPriority,
OrganizeImportsDirect.noopInstance(),
)
.newInstance(
targetId.getUri(),
classpath.asJava,
Expand All @@ -264,10 +279,12 @@ class CompilerConfiguration(
pc: PresentationCompiler,
search: SymbolSearch,
completionItemPriority: CompletionItemPriority,
organizeImportsDirect: OrganizeImportsDirect,
): PresentationCompiler =
pc.withSearch(search)
.withExecutorService(ec)
.withCompletionItemPriority(completionItemPriority)
.withOrganizeImports(organizeImportsDirect)
.withWorkspace(workspace.toNIO)
.withScheduledExecutorService(sh)
.withReportsLoggerLevel(MetalsServerConfig.default.loglevel)
Expand All @@ -293,6 +310,7 @@ class CompilerConfiguration(
name: String,
symbolSearch: SymbolSearch,
referenceCounter: CompletionItemPriority,
organizeImportsDirect: OrganizeImportsDirect,
): PresentationCompiler = {
val pc = mtags match {
case MtagsBinaries.BuildIn => new ScalaPresentationCompiler()
Expand All @@ -301,7 +319,7 @@ class CompilerConfiguration(
}

val filteredOptions = plugins.filterSupportedOptions(options)
configure(pc, symbolSearch, referenceCounter)
configure(pc, symbolSearch, referenceCounter, organizeImportsDirect)
.newInstance(
name,
classpathSeq.asJava,
Expand Down
26 changes: 26 additions & 0 deletions metals/src/main/scala/scala/meta/internal/metals/Compilers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import java.{util => ju}
import scala.annotation.nowarn
import scala.annotation.tailrec
import scala.collection.concurrent.TrieMap
import scala.collection.mutable.ArrayBuffer
import scala.concurrent.ExecutionContextExecutorService
import scala.concurrent.Future
import scala.util.control.NonFatal

import scala.meta._
import scala.meta.inputs.Input
import scala.meta.inputs.Position
import scala.meta.internal
Expand Down Expand Up @@ -90,6 +92,7 @@ class Compilers(
sourceMapper: SourceMapper,
worksheetProvider: WorksheetProvider,
completionItemPriority: () => CompletionItemPriority,
scalafixProvider: () => ScalafixProvider,
)(implicit ec: ExecutionContextExecutorService, rc: ReportContext)
extends Cancelable {

Expand Down Expand Up @@ -154,19 +157,29 @@ class Compilers(
lazyPc
} else {
presentationCompiler.shutdown()
val scalafix = scala.concurrent.Await.result(
scalafixProvider().getScalafix(scalaVersion),
scala.concurrent.duration.Duration.Inf,
)
StandaloneCompiler(
scalaVersion,
search,
Nil,
completionItemPriority(),
scalafix.loadOrganizeImports(java.util.Optional.empty()),
)
}
case None =>
val scalafix = scala.concurrent.Await.result(
scalafixProvider().getScalafix(scalaVersion),
scala.concurrent.duration.Duration.Inf,
)
StandaloneCompiler(
scalaVersion,
search,
Nil,
completionItemPriority(),
scalafix.loadOrganizeImports(java.util.Optional.empty()),
)
}
},
Expand Down Expand Up @@ -673,6 +686,7 @@ class Compilers(
outlineFilesProvider.getOutlineFiles(pc.buildTargetId())
val offsetParams =
CompilerOffsetParamsUtils.fromPos(pos, token, outlineFiles)

pc.complete(offsetParams)
.asScala
.map { list =>
Expand Down Expand Up @@ -1263,12 +1277,18 @@ class Compilers(
path, {
val scalaVersion =
scalaVersionSelector.fallbackScalaVersion(isAmmonite = false)

val scalafix = scala.concurrent.Await.result(
scalafixProvider().getScalafix(scalaVersion),
scala.concurrent.duration.Duration.Inf,
)
StandaloneCompiler(
scalaVersion,
classpath,
sources,
Some(search),
completionItemPriority(),
scalafix.loadOrganizeImports(java.util.Optional.empty()),
)
},
)
Expand Down Expand Up @@ -1312,11 +1332,17 @@ class Compilers(
workDoneProgress.trackBlocking(
s"${config.icons().sync}Loading presentation compiler"
) {
// TODO
val scalafix = scala.concurrent.Await.result(
scalafixProvider().getScalafix(scalaVersion),
scala.concurrent.duration.Duration.Inf,
)
Comment on lines +1335 to +1339
Copy link
Copy Markdown
Member

@kasiaMarek kasiaMarek May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may actually be useful to supply scalafix rules to the presentation compiler at any point. So pc it can start with empty rules, so it works even before scalafix is fetched and later if rules change, that could also be easily updated.

Even if you don't want to go with this design, infinite await isn't a good idea, but I'm guessing, that is what the TODO is about.

ScalaLazyCompiler(
scalaTarget,
mtags,
search,
completionItemPriority(),
scalafix.loadOrganizeImports(java.util.Optional.empty()),
)
}
val key =
Expand Down
6 changes: 5 additions & 1 deletion metals/src/main/scala/scala/meta/internal/metals/Configs.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import scala.meta.pc.PresentationCompilerConfig.OverrideDefFormat
import org.eclipse.lsp4j.DidChangeWatchedFilesRegistrationOptions
import org.eclipse.lsp4j.FileSystemWatcher
import org.eclipse.lsp4j.jsonrpc.messages.Either
import metaconfig._
import metaconfig.generic._
import scala.reflect.ClassTag
import scala.meta.Importer
import scala.util.matching.Regex
Comment on lines +13 to +17
Copy link
Copy Markdown
Member

@kasiaMarek kasiaMarek May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused?


object Configs {

Expand Down Expand Up @@ -115,5 +120,4 @@ object Configs {
)
}
}

}
1 change: 1 addition & 0 deletions metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ abstract class MetalsLspService(
sourceMapper,
worksheetProvider,
() => referencesProvider,
() => scalafixProvider,
)
)

Expand Down
1 change: 1 addition & 0 deletions metals/src/main/scala/scala/meta/internal/metals/PresentationCompilerClassLoader.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class PresentationCompilerClassLoader(parent: ClassLoader)
name.startsWith("org.eclipse.lsp4j") ||
name.startsWith("com.google.gson") ||
name.startsWith("scala.meta.pc") ||
name.startsWith("scalafix") ||
name.startsWith("javax")
if (isShared) {
parent.loadClass(name)
Expand Down
7 changes: 6 additions & 1 deletion metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,12 +574,13 @@ case class ScalafixProvider(
}
}

private def getScalafix(
def getScalafix(
scalaVersion: ScalaVersion
): Future[Scalafix] = Future {
scalafixCache.getOrElseUpdate(
scalaVersion, {
workDoneProgress.trackBlocking("Downloading scalafix") {
scribe.info(s"GET SCALAFIX")
val scalafix =
if (scalaVersion.startsWith("2.11")) scala211Fallback
else
Expand Down Expand Up @@ -633,6 +634,7 @@ case class ScalafixProvider(
Embedded.rulesClasspath(
rulesDependencies.toList ++ organizeImportRule
)
scribe.info(s"RULE CLASSLOADER: ${paths}")
val classloader = Embedded.toClassLoader(
Classpath(paths.map(AbsolutePath(_))),
scalafixClassLoader,
Expand Down Expand Up @@ -712,6 +714,9 @@ object ScalafixProvider {
userConfig: UserConfiguration,
rules: List[String],
): Set[Dependency] = {
scribe.info(
s"here is scalafixRulesDependencies: ${userConfig.scalafixRulesDependencies}"
)
val fromSettings =
userConfig.scalafixRulesDependencies.flatMap { dependencyString =>
Try {
Expand Down
5 changes: 5 additions & 0 deletions mtags-interfaces/src/main/java/scala/meta/pc/PresentationCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ScheduledExecutorService;

import scalafix.interfaces.imports.OrganizeImportsDirect;
/**
* The public API of the presentation compiler.
*
Expand Down Expand Up @@ -308,6 +309,10 @@ public PresentationCompiler withCompletionItemPriority(CompletionItemPriority pr
return this;
}

public PresentationCompiler withOrganizeImports(OrganizeImportsDirect organize) {
return this;
}

/**
* Construct a new presentation compiler with the given parameters.
*
Expand Down
12 changes: 7 additions & 5 deletions mtags/src/main/scala-2/scala/meta/internal/pc/AutoImports.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,13 @@ trait AutoImports { this: MetalsGlobal =>
.lastOption
val padTop = lastImportOpt.isEmpty
val lastImportOrPkg = lastImportOpt.getOrElse(pkg.pid)
new AutoImportPosition(
pos.source.lineToOffset(lastImportOrPkg.pos.focusEnd.line),
text,
padTop
)
val aipos =
new AutoImportPosition(
pos.source.lineToOffset(lastImportOrPkg.pos.focusEnd.line),
text,
padTop
)
aipos
}

def forScript(isAmmonite: Boolean) = {
Expand Down
7 changes: 6 additions & 1 deletion mtags/src/main/scala-2/scala/meta/internal/pc/InferredMethodProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import scala.meta.internal.metals.PcQueryContext
import scala.meta.pc.OffsetParams

import org.eclipse.lsp4j.TextEdit
import scalafix.interfaces.imports.OrganizeImportsDirect

/**
* Tries to calculate edits needed to create a method that will fix missing symbol
Expand Down Expand Up @@ -144,7 +145,11 @@ final class InferredMethodProvider(
// existing symbols in scope, so we just do nothing
Nil
case Some(importPosition) =>
history.autoImports(pos, importPosition)
history.autoImports(
pos,
importPosition,
OrganizeImportsDirect.noopInstance()
)
}

private def prettyType(tpe: Type) =
Expand Down
7 changes: 6 additions & 1 deletion mtags/src/main/scala-2/scala/meta/internal/pc/InferredTypeProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import scala.meta.tokens.{Token => T}

import org.eclipse.lsp4j.TextEdit
import org.eclipse.{lsp4j => l}
import scalafix.interfaces.imports.OrganizeImportsDirect

/**
* Tries to calculate edits needed to insert the inferred type annotation
Expand Down Expand Up @@ -66,7 +67,11 @@ final class InferredTypeProvider(
// existing symbols in scope, so we just do nothing
Nil
case Some(importPosition) =>
history.autoImports(pos, importPosition)
history.autoImports(
pos,
importPosition,
OrganizeImportsDirect.noopInstance()
)
}

def prettyType(tpe: Type) =
Expand Down
5 changes: 4 additions & 1 deletion mtags/src/main/scala-2/scala/meta/internal/pc/MetalsGlobal.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import scala.meta.pc.PresentationCompilerConfig
import scala.meta.pc.SymbolDocumentation
import scala.meta.pc.SymbolSearch

import scalafix.interfaces.imports.OrganizeImportsDirect

import org.eclipse.{lsp4j => l}

class MetalsGlobal(
Expand All @@ -40,7 +42,8 @@ class MetalsGlobal(
val buildTargetIdentifier: String,
val metalsConfig: PresentationCompilerConfig,
val workspace: Option[Path],
val completionItemPriority: CompletionItemPriority
val completionItemPriority: CompletionItemPriority,
val orgImports: OrganizeImportsDirect
) extends Global(settings, reporter)
with completions.Completions
with completions.AmmoniteFileCompletions
Expand Down
Loading