From f79fc10dbb58df5b272cd0c417d26df6f9527c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Mon, 6 Jul 2026 15:36:32 +0200 Subject: [PATCH 1/9] Add suppress warnings quick-fix code action --- .../metals/CompilerConfiguration.scala | 9 +- .../meta/internal/metals/Compilers.scala | 31 +- .../internal/metals/UserConfiguration.scala | 48 + .../codeactions/CodeActionProvider.scala | 1 + .../metals/codeactions/SuppressWarnings.scala | 328 +++++++ .../meta/internal/parsing/JavaTrees.scala | 15 +- .../src/main/scala/tests/BaseLspSuite.scala | 2 + .../src/test/scala/tests/InfraSuite.scala | 5 +- .../scala/tests/UserConfigurationSuite.scala | 32 + .../SuppressWarningsLspSuite.scala | 833 ++++++++++++++++++ 10 files changed, 1284 insertions(+), 20 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala create mode 100644 tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala index 1d120947044..6561dc539d0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala @@ -434,12 +434,12 @@ class CompilerConfiguration( } case class JavaLazyCompiler( - javaTarget: JvmTarget, + target: JvmTarget, search: SymbolSearch, completionItemPriority: CompletionItemPriority, ) extends LazyCompiler { - def buildTargetId: BuildTargetIdentifier = javaTarget.id + def buildTargetId: BuildTargetIdentifier = target.id protected def newCompiler( classpath: Seq[Path], @@ -449,10 +449,13 @@ class CompilerConfiguration( val shouldUseOpts = featureFlags .readBoolean(FeatureFlag.JAVAC_OPTIONS) .orElse(false) - val options = javaTarget match { + val buildOptions = target match { case j: JavaTarget if shouldUseOpts => j.options case _ => Nil } + val lintOptions = + userConfig().javaLintOptions.values.map(option => s"-Xlint:$option") + val options = buildOptions ++ lintOptions configure(pc, search, completionItemPriority) .newInstance( buildTargetId.getUri(), diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala index c9d9147ae76..082cdc5c813 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala @@ -1765,20 +1765,23 @@ class Compilers( private def loadJavaCompiler( targetId: BuildTargetIdentifier ): Option[PresentationCompiler] = { - buildTargets.jvmTarget(targetId).map { javaTarget => - jcache - .computeIfAbsent( - PresentationCompilerKey.JavaBuildTarget(targetId), - { _ => - workDoneProgress.trackBlocking( - s"${config.icons().sync}Loading presentation compiler" - ) { - JavaLazyCompiler(javaTarget, search, completionItemPriority()) - } - }, - ) - .await - } + buildTargets + .javaTarget(targetId) + .orElse(buildTargets.jvmTarget(targetId)) + .map { javaTarget => + jcache + .computeIfAbsent( + PresentationCompilerKey.JavaBuildTarget(targetId), + { _ => + workDoneProgress.trackBlocking( + s"${config.icons().sync}Loading presentation compiler" + ) { + JavaLazyCompiler(javaTarget, search, completionItemPriority()) + } + }, + ) + .await + } } private def protoCompiler: PresentationCompiler = { diff --git a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala index 37721afbeb0..a452bb80f75 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -44,6 +44,37 @@ case class JavaFormatConfig( eclipseFormatProfile: Option[String], ) +case class JavaLintOptions(values: List[String]) + +object JavaLintOptions { + val allValues: List[String] = List( + "cast", "deprecation", "dep-ann", "divzero", "empty", "fallthrough", + "finally", "lossy-conversions", "overloads", "overrides", "rawtypes", + "removal", "serial", "static", "strictfp", "synchronization", "text-blocks", + "this-escape", "try", "unchecked", "varargs", + ) + + val default: JavaLintOptions = JavaLintOptions(allValues) + + private val allowed: Set[String] = + (allValues :+ "preview").toSet + + def fromConfig( + values: Option[List[String]] + ): Either[String, JavaLintOptions] = + values match { + case None => Right(default) + case Some(values) => + values.find(!allowed.contains(_)) match { + case Some(invalid) => + Left( + s"invalid config value '$invalid' for javaLintOptions. Valid values are ${allowed.toSeq.sorted.map(value => s""""$value"""").mkString(", ")}" + ) + case None => Right(JavaLintOptions(values)) + } + } +} + /** * Configuration that the user can override via workspace/didChangeConfiguration. */ @@ -82,6 +113,7 @@ case class UserConfiguration( javaFormatter: Option[JavaFormatterConfig] = None, scalafixRulesDependencies: List[String] = Nil, scalafixLintEnabled: Boolean = false, + javaLintOptions: JavaLintOptions = JavaLintOptions.default, customProjectRoot: Option[String] = None, verboseCompilation: Boolean = false, automaticImportBuild: AutoImportBuildKind = AutoImportBuildKind.Off, @@ -220,6 +252,7 @@ case class UserConfiguration( Some(scalafixRulesDependencies), ), Some(("scalafixLintEnabled", scalafixLintEnabled)), + listField("javaLintOptions", Some(javaLintOptions.values)), optStringField("customProjectRoot", customProjectRoot), Some(("verboseCompilation", verboseCompilation)), Some( @@ -521,6 +554,16 @@ object UserConfiguration { |""".stripMargin, isBoolean = true, ), + UserConfigurationOption( + "java-lint-options", + JavaLintOptions.default.values.mkString("[", ",", "]"), + """["deprecation", "unchecked"]""", + "Java lint diagnostics", + """Javac `-Xlint` options passed to the Java presentation compiler. + |Use an empty array to disable Java lint diagnostics. + |""".stripMargin, + isArray = true, + ), UserConfigurationOption( "excluded-packages", "[]", @@ -1308,6 +1351,10 @@ object UserConfiguration { val scalafixLintEnabled = getBooleanKey("scalafix-lint-enabled").getOrElse(false) + val javaLintOptions = getParsedArrayKey( + "java-lint-options", + JavaLintOptions.fromConfig, + ).getOrElse(JavaLintOptions.default) val customProjectRoot = getStringKey("custom-project-root") val verboseCompilation = @@ -1506,6 +1553,7 @@ object UserConfiguration { javaFormatter, scalafixRulesDependencies, scalafixLintEnabled, + javaLintOptions, customProjectRoot, verboseCompilation, autoImportBuilds, diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala index e2bcf5b5dc5..3aa51b461c4 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala @@ -63,6 +63,7 @@ final class CodeActionProvider( new ConvertToNamedLambdaParameters(trees, compilers), new AddMissingOverrideAnnotation(javaTrees, buffers), new RemoveUnusedJavaImport(buffers), + new SuppressWarnings(javaTrees, buffers), new GenerateConstructors(javaTrees, buffers), new GenerateGettersSetters(javaTrees, buffers), new GenerateEqualsHashCodeToString(javaTrees, buffers), diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala new file mode 100644 index 00000000000..3d4ff865749 --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala @@ -0,0 +1,328 @@ +package scala.meta.internal.metals.codeactions + +import scala.concurrent.ExecutionContext +import scala.concurrent.Future + +import scala.meta.internal.metals.Buffers +import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.parsing.JavaMember +import scala.meta.internal.parsing.JavaRange +import scala.meta.internal.parsing.JavaTrees +import scala.meta.io.AbsolutePath +import scala.meta.pc.CancelToken + +import org.eclipse.{lsp4j => l} + +class SuppressWarnings( + javaTrees: JavaTrees, + buffers: Buffers, +) extends CodeAction { + import SuppressWarnings._ + + override def kind: String = l.CodeActionKind.QuickFix + override def isScala: Boolean = false + override def isJava: Boolean = true + + override def contribute( + params: l.CodeActionParams, + token: CancelToken, + )(implicit ec: ExecutionContext): Future[Seq[l.CodeAction]] = Future { + val path = params.getTextDocument().getUri().toAbsolutePath + val range = params.getRange() + + val actions = for { + text <- buffers.get(path).orElse(path.readTextOpt).toSeq + diagnostic <- params.getContext().getDiagnostics().asScala.toSeq + warningName <- warningName(diagnostic).toSeq + if range.overlapsWith(diagnostic.getRange()) || + isZeroRange(diagnostic.getRange()) + position = + if (range.overlapsWith(diagnostic.getRange())) + diagnostic.getRange().getStart() + else range.getStart() + member <- enclosingMember(path, position).toSeq + edit <- suppressEdit(text, member, warningName).toSeq + } yield CodeActionBuilder.build( + title(warningName), + kind, + diagnostics = List(diagnostic), + changes = Seq(path -> Seq(edit)), + ) + actions.distinctBy(_.getTitle()) + } + + private def enclosingMember( + path: AbsolutePath, + position: l.Position, + ): Option[SuppressTarget] = + javaTrees + .findEnclosingJavaVariable(path, position, onNameOnly = false) + .map(variable => SuppressTarget(variable, variable.nameRange)) + .orElse( + javaTrees + .findEnclosingJavaMethod(path, position) + .map(method => SuppressTarget(method, method.nameRange)) + ) + .orElse( + javaTrees + .findEnclosingJavaClass(path, position) + .map(cls => SuppressTarget(cls, cls.nameRange)) + ) +} + +object SuppressWarnings { + def title(warningName: String): String = + s"""Add @SuppressWarnings("$warningName")""" + + private val SuppressWarningsName = "@SuppressWarnings" + + private val WarningNames = + List( + "auxiliaryclass", "cast", "classfile", "deprecation", "dep-ann", + "divzero", "empty", "exports", "fallthrough", "finally", + "lossy-conversions", "missing-explicit-ctor", "module", "opens", + "options", "output-file-clash", "overloads", "overrides", "path", + "processing", "rawtypes", "removal", "requires-automatic", + "requires-transitive-automatic", "serial", "static", "strictfp", + "synchronization", "text-blocks", "this-escape", "try", "unchecked", + "varargs", "preview", + ) + + private def warningName(diagnostic: l.Diagnostic): Option[String] = + if (diagnostic.getSource() == "javac") { + val code = Option(diagnostic.getCode()) + .collect { case code if code.isLeft() => code.getLeft() } + .getOrElse("") + .toLowerCase() + val isWarning = code.startsWith("compiler.warn.") || + diagnostic.getSeverity() == l.DiagnosticSeverity.Warning + if (isWarning) { + val message = + Option(diagnostic.getMessage()) + .map(_.toString()) + .getOrElse("") + .toLowerCase() + warningNameFrom(code).orElse(warningNameFrom(message)) + } else None + } else None + + private def warningNameFrom(text: String): Option[String] = { + val matched = WarningNames.filter(name => containsWholeWord(text, name)) + matched.maxByOption(_.length).orElse { + if (text.contains("missing.deprecated.annotation")) Some("dep-ann") + else if (text.contains("loss.of.precision")) Some("lossy-conversions") + else if (text.contains("fall-through")) Some("fallthrough") + else if (text.contains("ambiguous.overload")) Some("overloads") + else if (text.contains("override.equals")) Some("overrides") + else if (text.contains("trailing.white.space")) Some("text-blocks") + else if (text.contains("this.escape")) Some("this-escape") + else if (text.contains("synchronize")) Some("synchronization") + else if (text.contains("deprecated")) Some("deprecation") + else if (text.contains("raw.class")) Some("rawtypes") + else if (text.contains("serialversionuid") || text.contains("svuid")) + Some("serial") + else None + } + } + + private def containsWholeWord(text: String, name: String): Boolean = { + var index = text.indexOf(name) + var found = false + while (!found && index >= 0) { + val end = index + name.length + val beforeOk = index == 0 || !isWarningNameChar(text.charAt(index - 1)) + val afterOk = end >= text.length || !isWarningNameChar(text.charAt(end)) + if (beforeOk && afterOk) found = true + else index = text.indexOf(name, index + 1) + } + found + } + + private def isWarningNameChar(ch: Char): Boolean = + Character.isLetterOrDigit(ch) || ch == '-' + + private def isZeroRange(range: l.Range): Boolean = + range.getStart().getLine() == 0 && + range.getStart().getCharacter() == 0 && + range.getEnd().getLine() == 0 && + range.getEnd().getCharacter() == 0 + + private def suppressEdit( + text: String, + target: SuppressTarget, + warningName: String, + ): Option[l.TextEdit] = + existingSuppressWarnings(text, target) match { + case Some(existing) => appendWarningEdit(text, existing, warningName) + case None => Some(insertSuppressWarningsEdit(text, target, warningName)) + } + + private def insertSuppressWarningsEdit( + text: String, + target: SuppressTarget, + warningName: String, + ): l.TextEdit = { + val declarationOffset = declarationStartOffset(text, target) + val declarationStart = positionAtOffset(text, declarationOffset) + val linePrefix = + JavaMemberInsertion.linePrefix(text, declarationOffset) + val (position, newText) = + if (linePrefix.forall(_.isWhitespace)) + ( + new l.Position(declarationStart.getLine(), 0), + s"""$linePrefix@SuppressWarnings("$warningName") + |""".stripMargin, + ) + else (declarationStart, s"""@SuppressWarnings("$warningName") """) + + new l.TextEdit(new l.Range(position, position), newText) + } + + /** + * The offset where the member declaration proper starts, skipping any + * annotations that precede it, so that `@SuppressWarnings` lands after + * existing annotations like `@Override`. + */ + private def declarationStartOffset( + text: String, + target: SuppressTarget, + ): Int = { + val end = target.nameRange.startOffset.min(text.length) + var offset = target.member.range.startOffset.max(0) + var continue = true + while (continue) { + var annotationStart = offset + while (annotationStart < end && text.charAt(annotationStart).isWhitespace) + annotationStart += 1 + if (annotationStart < end && text.charAt(annotationStart) == '@') { + var nameEnd = annotationStart + 1 + while ( + nameEnd < end && + (Character.isJavaIdentifierPart(text.charAt(nameEnd)) || + text.charAt(nameEnd) == '.') + ) nameEnd += 1 + // `@interface` is an annotation-type declaration, not an annotation. + if (text.substring(annotationStart + 1, nameEnd) == "interface") + continue = false + else { + var argsStart = nameEnd + while (argsStart < end && text.charAt(argsStart).isWhitespace) + argsStart += 1 + if (argsStart < end && text.charAt(argsStart) == '(') + matchingCloseParen(text, argsStart, end) match { + case Some(close) => offset = close + 1 + case None => continue = false + } + else offset = nameEnd + } + } else continue = false + } + var declarationStart = offset + while (declarationStart < end && text.charAt(declarationStart).isWhitespace) + declarationStart += 1 + declarationStart + } + + private def existingSuppressWarnings( + text: String, + target: SuppressTarget, + ): Option[ExistingSuppressWarnings] = { + val prefixStart = target.member.range.startOffset + val prefixEnd = target.nameRange.startOffset + if (prefixStart < 0 || prefixEnd <= prefixStart || prefixEnd > text.length) + None + else { + val prefix = text.substring(prefixStart, prefixEnd) + val annotationStartInPrefix = prefix.indexOf(SuppressWarningsName) + if (annotationStartInPrefix < 0) None + else { + val annotationStart = prefixStart + annotationStartInPrefix + val open = text.indexOf('(', annotationStart) + if (open < 0 || open >= prefixEnd) None + else + matchingCloseParen(text, open, prefixEnd).map { close => + ExistingSuppressWarnings(open, close) + } + } + } + } + + private def appendWarningEdit( + text: String, + existing: ExistingSuppressWarnings, + warningName: String, + ): Option[l.TextEdit] = { + val insideStart = existing.openParenOffset + 1 + val insideEnd = existing.closeParenOffset + val inside = text.substring(insideStart, insideEnd) + if (inside.contains(s""""$warningName"""")) None + else { + val trimmed = inside.trim() + val (range, newText) = + if (trimmed.startsWith("{") && trimmed.endsWith("}")) { + val closeBrace = insideEnd - inside.reverse.indexOf('}') - 1 + ( + new l.Range( + positionAtOffset(text, closeBrace), + positionAtOffset(text, closeBrace), + ), + s""", "$warningName"""", + ) + } else { + ( + new l.Range( + positionAtOffset(text, insideStart), + positionAtOffset(text, insideEnd), + ), + s"""{$trimmed, "$warningName"}""", + ) + } + Some(new l.TextEdit(range, newText)) + } + } + + private def matchingCloseParen( + text: String, + open: Int, + end: Int, + ): Option[Int] = { + var offset = open + var depth = 0 + while (offset < end) { + text.charAt(offset) match { + case '(' => depth += 1 + case ')' => + depth -= 1 + if (depth == 0) return Some(offset) + case _ => + } + offset += 1 + } + None + } + + private def positionAtOffset(text: String, offset: Int): l.Position = { + val safeOffset = offset.max(0).min(text.length) + var line = 0 + var lineStart = 0 + var index = 0 + while (index < safeOffset) { + if (text.charAt(index) == '\n') { + line += 1 + lineStart = index + 1 + } + index += 1 + } + new l.Position(line, safeOffset - lineStart) + } + + private case class SuppressTarget( + member: JavaMember, + nameRange: JavaRange, + ) + + private case class ExistingSuppressWarnings( + openParenOffset: Int, + closeParenOffset: Int, + ) +} diff --git a/metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala b/metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala index c458e5b4a66..7cc8c716513 100644 --- a/metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala +++ b/metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala @@ -90,15 +90,22 @@ class JavaTrees(buffers: Buffers) { } } yield result + /** + * Finds the variable declaration enclosing `pos`. + * + * @param onNameOnly when true, `pos` must be on the variable's name + * identifier; when false, anywhere within the declaration counts. + */ def findEnclosingJavaVariable( source: AbsolutePath, pos: l.Position, + onNameOnly: Boolean = true, ): Option[JavaVariable] = for { text <- text(source) tree <- get(source) result <- { - val visitor = new EnclosingVariableFinder(tree, text, pos) + val visitor = new EnclosingVariableFinder(tree, text, pos, onNameOnly) visitor.scan(tree, ()) visitor.result } @@ -327,6 +334,7 @@ class JavaTrees(buffers: Buffers) { cu: CompilationUnitTree, text: String, targetPos: l.Position, + onNameOnly: Boolean, ) extends EnclosingFinder[JavaVariable](cu, text, targetPos) { override def visitVariable( @@ -341,7 +349,10 @@ class JavaTrees(buffers: Buffers) { findNameOffset(text, nodeStart, nodeEnd, name) .getOrElse(nodeStart) val actualNodeEnd = actualNodeStart + name.length() - if (positionContains(targetOffset, actualNodeStart, actualNodeEnd)) { + if ( + !onNameOnly || + positionContains(targetOffset, actualNodeStart, actualNodeEnd) + ) { _result = javaVariable(node) } } diff --git a/tests/unit/src/main/scala/tests/BaseLspSuite.scala b/tests/unit/src/main/scala/tests/BaseLspSuite.scala index 44e15b2b283..c169543d2ef 100644 --- a/tests/unit/src/main/scala/tests/BaseLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseLspSuite.scala @@ -26,6 +26,7 @@ import scala.meta.internal.metals.MtagsResolver import scala.meta.internal.metals.RecursivelyDelete import scala.meta.internal.metals.Time import scala.meta.internal.metals.Trace +import scala.meta.internal.metals.JavaLintOptions import scala.meta.internal.metals.UserConfiguration import scala.meta.internal.metals.debug.DebugProtocol import scala.meta.internal.metals.logging.MetalsLogger @@ -51,6 +52,7 @@ abstract class BaseLspSuite( buildChangedAction = BuildChangedAction.prompt, fallbackScalaVersion = Some(BuildInfo.scalaVersion), presentationCompilerDiagnostics = false, + javaLintOptions = JavaLintOptions(Nil), definitionIndexStrategy = Configs.DefinitionIndexStrategy.classpath, // Legacy settings that are enabled for tests only. We should eventually diff --git a/tests/unit/src/test/scala/tests/InfraSuite.scala b/tests/unit/src/test/scala/tests/InfraSuite.scala index 32c69975fe2..3d2bd645e72 100644 --- a/tests/unit/src/test/scala/tests/InfraSuite.scala +++ b/tests/unit/src/test/scala/tests/InfraSuite.scala @@ -17,6 +17,8 @@ object TestingInfra { val events: mutable.ArrayBuffer[Event] = mutable.ArrayBuffer[Event]() val testFlags: mutable.ArrayBuffer[FeatureFlag] = mutable.ArrayBuffer[FeatureFlag]() + + val enabledFlags: mutable.Set[FeatureFlag] = mutable.Set[FeatureFlag]() } class TestingMonitoringClient extends MonitoringClient { override def recordUsage(metric: Metric): Unit = @@ -29,7 +31,8 @@ class TestingMonitoringClient extends MonitoringClient { class TestingFeatureFlagProvider extends FeatureFlagProvider { override def readBoolean(flag: FeatureFlag): Optional[java.lang.Boolean] = { TestingInfra.testFlags.append(flag) - Optional.empty() + if (TestingInfra.enabledFlags.contains(flag)) Optional.of(true) + else Optional.empty() } override def readInt(flag: FeatureFlag, default: Integer): Optional[Integer] = diff --git a/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala b/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala index 7a32c01c173..43556707200 100644 --- a/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala +++ b/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala @@ -20,6 +20,7 @@ import scala.meta.internal.metals.InlayHintsOption import scala.meta.internal.metals.InlayHintsOptions import scala.meta.internal.metals.JavaFormatConfig import scala.meta.internal.metals.JavaFormatterConfig +import scala.meta.internal.metals.JavaLintOptions import scala.meta.internal.metals.JsonParser._ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MetalsServerConfig @@ -189,6 +190,31 @@ class UserConfigurationSuite extends BaseSuite { """.stripMargin, ) + checkError( + "java-lint-options-invalid", + """ + |{ + | "java-lint-options": ["deprecation", "typo"] + |} + """.stripMargin, + "json error: invalid config value 'typo' for javaLintOptions. Valid values are " + + "\"cast\", \"dep-ann\", \"deprecation\", \"divzero\", \"empty\", \"fallthrough\", \"finally\", " + + "\"lossy-conversions\", \"overloads\", \"overrides\", \"preview\", \"rawtypes\", \"removal\", " + + "\"serial\", \"static\", \"strictfp\", \"synchronization\", \"text-blocks\", \"this-escape\", " + + "\"try\", \"unchecked\", \"varargs\"", + ) + + checkOK( + "java-lint-options-empty", + """ + |{ + | "java-lint-options": [] + |} + """.stripMargin, + ) { obtained => + assertEquals(obtained.javaLintOptions, JavaLintOptions(Nil)) + } + checkError( "symbol-prefixes", """ @@ -398,6 +424,7 @@ class UserConfigurationSuite extends BaseSuite { javacServicesOverrides = JavacServicesOverrides.default.copy(names = false), scalafixRulesDependencies = List("rule1", "rule2"), + javaLintOptions = JavaLintOptions(List("deprecation", "unchecked")), customProjectRoot = Some("customs"), workspaceSymbolProvider = WorkspaceSymbolProviderConfig("mbt"), javaTurbineRecompileDelay = TurbineRecompileDelayConfig.testing, @@ -478,6 +505,10 @@ class UserConfigurationSuite extends BaseSuite { "rule2" ], "scalafixLintEnabled": false, + "javaLintOptions": [ + "deprecation", + "unchecked" + ], "customProjectRoot": "customs", "verboseCompilation": true, "autoImportBuilds": "all", @@ -563,6 +594,7 @@ class UserConfigurationSuite extends BaseSuite { |shim-globs string `{}`. Shim file globs |scalafix-rules-dependencies array [] Scalafix rules dependencies |scalafix-lint-enabled boolean false Enable Scalafix lint diagnostics + |java-lint-options array [cast,deprecation,dep-ann,divzero,empty,fallthrough,finally,lossy-conversions,overloads,overrides,rawtypes,removal,serial,static,strictfp,synchronization,text-blocks,this-escape,try,unchecked,varargs] Java lint diagnostics |excluded-packages array [] Excluded Packages |bloop-sbt-already-installed boolean false Don't generate Bloop plugin file for sbt |bloop-version string $bloopVersionPadded Version of Bloop diff --git a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala new file mode 100644 index 00000000000..8b99f75b912 --- /dev/null +++ b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala @@ -0,0 +1,833 @@ +package tests.codeactions + +import scala.meta.internal.metals.JavaLintOptions +import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.codeactions.SuppressWarnings + +import munit.Location +import tests.MbtTestInitializer + +class SuppressWarningsLspSuite + extends BaseCodeActionLspSuite( + "suppress-warnings", + MbtTestInitializer, + useMbtLayout = true, + ) { + + override def userConfig: UserConfiguration = + super.userConfig.copy( + presentationCompilerDiagnostics = true, + javaLintOptions = JavaLintOptions.default, + ) + + override protected def toPath( + fileName: String, + isSource: Boolean = true, + ): String = + if (isSource) s"a/src/main/java/a/$fileName" + else s"a/$fileName" + + checkSuppressWarnings( + "rawtypes-method", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + |<< public List names() { + | return new ArrayList(); + | }>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings("rawtypes") + | public List names() { + | return new ArrayList(); + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "deprecation-method", + """|package a; + | + |public class Example { + |<< public void run() { + | DeprecatedApi.old(); + | }>> + |} + |""".stripMargin, + "compiler.warn.has.been.deprecated", + "deprecation", + """|package a; + | + |public class Example { + | @SuppressWarnings("deprecation") + | public void run() { + | DeprecatedApi.old(); + | } + |} + |""".stripMargin, + extraLayout = """|/a/src/main/java/a/DeprecatedApi.java + |package a; + | + |class DeprecatedApi { + | @Deprecated + | static void old() {} + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "serial-class", + """|package a; + | + |import java.io.Serializable; + | + |public class <> implements Serializable { + |} + |""".stripMargin, + "compiler.warn.missing.SVUID", + "serial", + """|package a; + | + |import java.io.Serializable; + | + |@SuppressWarnings("serial") + |public class Example implements Serializable { + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "append-existing", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings("unchecked") + |<< public List names() { + | return new ArrayList(); + | }>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings({"unchecked", "rawtypes"}) + | public List names() { + | return new ArrayList(); + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "local-variable", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | public void run() { + |<< List list = new ArrayList<>();>> + | } + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | public void run() { + | @SuppressWarnings("rawtypes") + | List list = new ArrayList<>(); + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "class-field", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + |<< private List names = new ArrayList<>();>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings("rawtypes") + | private List names = new ArrayList<>(); + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "constructor", + """|package a; + | + |public class Example { + |<< public Example() { + | DeprecatedApi.old(); + | }>> + |} + |""".stripMargin, + "compiler.warn.has.been.deprecated", + "deprecation", + """|package a; + | + |public class Example { + | @SuppressWarnings("deprecation") + | public Example() { + | DeprecatedApi.old(); + | } + |} + |""".stripMargin, + extraLayout = """|/a/src/main/java/a/DeprecatedApi.java + |package a; + | + |class DeprecatedApi { + | @Deprecated + | static void old() {} + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "append-existing-array", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings({"unchecked", "serial"}) + |<< public List names() { + | return new ArrayList(); + | }>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings({"unchecked", "serial", "rawtypes"}) + | public List names() { + | return new ArrayList(); + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "multiple-annotations", + """|package a; + | + |public class Example implements Runnable { + | @Override + |<< public void run() { + | DeprecatedApi.old(); + | }>> + |} + |""".stripMargin, + "compiler.warn.has.been.deprecated", + "deprecation", + """|package a; + | + |public class Example implements Runnable { + | @Override + | @SuppressWarnings("deprecation") + | public void run() { + | DeprecatedApi.old(); + | } + |} + |""".stripMargin, + extraLayout = """|/a/src/main/java/a/DeprecatedApi.java + |package a; + | + |class DeprecatedApi { + | @Deprecated + | static void old() {} + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "cast-method", + """|package a; + | + |public class Example { + |<< public void run() { + | String value = (String) "value"; + | }>> + |} + |""".stripMargin, + "compiler.warn.redundant.cast", + "cast", + """|package a; + | + |public class Example { + | @SuppressWarnings("cast") + | public void run() { + | String value = (String) "value"; + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "dep-ann-class", + """|package a; + | + |/** + | * @deprecated use something else + | */ + |<> { + |} + |""".stripMargin, + "compiler.warn.missing.deprecated.annotation", + "dep-ann", + """|package a; + | + |/** + | * @deprecated use something else + | */ + |@SuppressWarnings("dep-ann") + |public class Example { + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "divzero-method", + """|package a; + | + |public class Example { + |<< public int run() { + | return 1 / 0; + | }>> + |} + |""".stripMargin, + "compiler.warn.div.zero", + "divzero", + """|package a; + | + |public class Example { + | @SuppressWarnings("divzero") + | public int run() { + | return 1 / 0; + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "empty-method", + """|package a; + | + |public class Example { + |<< public void run(boolean ok) { + | if (ok); + | }>> + |} + |""".stripMargin, + "compiler.warn.empty.if", + "empty", + """|package a; + | + |public class Example { + | @SuppressWarnings("empty") + | public void run(boolean ok) { + | if (ok); + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "fallthrough-method", + """|package a; + | + |public class Example { + |<< public void run(int value) { + | switch (value) { + | case 0: + | value++; + | case 1: + | value++; + | default: + | value++; + | } + | }>> + |} + |""".stripMargin, + "compiler.warn.possible.fall-through.into.case", + "fallthrough", + """|package a; + | + |public class Example { + | @SuppressWarnings("fallthrough") + | public void run(int value) { + | switch (value) { + | case 0: + | value++; + | case 1: + | value++; + | default: + | value++; + | } + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "finally-method", + """|package a; + | + |public class Example { + |<< public int run() { + | try { + | return 1; + | } finally { + | return 2; + | } + | }>> + |} + |""".stripMargin, + "compiler.warn.finally.cannot.complete", + "finally", + """|package a; + | + |public class Example { + | @SuppressWarnings("finally") + | public int run() { + | try { + | return 1; + | } finally { + | return 2; + | } + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "lossy-conversions-method", + """|package a; + | + |public class Example { + |<< public void run() { + | short value = 0; + | value += 100000; + | }>> + |} + |""".stripMargin, + "compiler.warn.possible.loss.of.precision", + "lossy-conversions", + """|package a; + | + |public class Example { + | @SuppressWarnings("lossy-conversions") + | public void run() { + | short value = 0; + | value += 100000; + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "overloads-method", + """|package a; + | + |import java.util.function.Consumer; + |import java.util.function.Function; + | + |public class Example { + | public void run(Consumer consumer) { + | } + | + |<< public void run(Function function) { + | }>> + |} + |""".stripMargin, + "compiler.warn.potentially.ambiguous.overload", + "overloads", + """|package a; + | + |import java.util.function.Consumer; + |import java.util.function.Function; + | + |public class Example { + | public void run(Consumer consumer) { + | } + | + | @SuppressWarnings("overloads") + | public void run(Function function) { + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "overrides-class", + """|package a; + | + |<> { + | @Override + | public boolean equals(Object other) { + | return other instanceof Example; + | } + |} + |""".stripMargin, + "compiler.warn.override.equals.but.not.hashcode", + "overrides", + """|package a; + | + |@SuppressWarnings("overrides") + |public class Example { + | @Override + | public boolean equals(Object other) { + | return other instanceof Example; + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "removal-method", + """|package a; + | + |public class Example { + |<< public void run() { + | RemovedApi.old(); + | }>> + |} + |""".stripMargin, + "compiler.warn.has.been.deprecated.for.removal", + "removal", + """|package a; + | + |public class Example { + | @SuppressWarnings("removal") + | public void run() { + | RemovedApi.old(); + | } + |} + |""".stripMargin, + extraLayout = """|/a/src/main/java/a/RemovedApi.java + |package a; + | + |class RemovedApi { + | @Deprecated(forRemoval = true) + | static void old() {} + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "unchecked-method", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + |<< public void run() { + | List raw = new ArrayList(); + | List strings = raw; + | }>> + |} + |""".stripMargin, + "compiler.warn.prob.found.req", + "unchecked", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings("unchecked") + | public void run() { + | List raw = new ArrayList(); + | List strings = raw; + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "static-method", + """|package a; + | + |public class Example { + | static int count; + | + |<< public void run(Example other) { + | other.count++; + | }>> + |} + |""".stripMargin, + "compiler.warn.static.not.qualified.by.type", + "static", + """|package a; + | + |public class Example { + | static int count; + | + | @SuppressWarnings("static") + | public void run(Example other) { + | other.count++; + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "strictfp-method", + """|package a; + | + |public class Example { + |<< public strictfp void run() { + | }>> + |} + |""".stripMargin, + "compiler.warn.strictfp", + "strictfp", + """|package a; + | + |public class Example { + | @SuppressWarnings("strictfp") + | public strictfp void run() { + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "synchronization-method", + """|package a; + | + |public class Example { + |<< public void run() { + | synchronized (Integer.valueOf(1)) { + | } + | }>> + |} + |""".stripMargin, + "compiler.warn.attempt.to.synchronize.on.instance.of.value.based.class", + "synchronization", + """|package a; + | + |public class Example { + | @SuppressWarnings("synchronization") + | public void run() { + | synchronized (Integer.valueOf(1)) { + | } + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "text-blocks-method", + s"""|package a; + | + |public class Example { + |<< public void run() { + | String text = ""\" + | trailing${" "} + | ""\"; + | }>> + |} + |""".stripMargin, + "compiler.warn.trailing.white.space.will.be.removed", + "text-blocks", + s"""|package a; + | + |public class Example { + | @SuppressWarnings("text-blocks") + | public void run() { + | String text = ""\" + | trailing${" "} + | ""\"; + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "this-escape-constructor", + """|package a; + | + |public class Example { + |<< public Example() { + | overridable(); + | }>> + | + | public void overridable() {} + |} + |""".stripMargin, + "compiler.warn.possible.this.escape", + "this-escape", + """|package a; + | + |public class Example { + | @SuppressWarnings("this-escape") + | public Example() { + | overridable(); + | } + | + | public void overridable() {} + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "try-method", + """|package a; + | + |import java.io.Closeable; + |import java.io.IOException; + | + |public class Example { + |<< public void run() throws IOException { + | try (Closeable closeable = null) { + | } + | }>> + |} + |""".stripMargin, + "compiler.warn.try.resource.not.referenced", + "try", + """|package a; + | + |import java.io.Closeable; + |import java.io.IOException; + | + |public class Example { + | @SuppressWarnings("try") + | public void run() throws IOException { + | try (Closeable closeable = null) { + | } + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "varargs-method", + """|package a; + | + |import java.util.List; + | + |public class Example { + |<< public void run(List... lists) { + | }>> + |} + |""".stripMargin, + "compiler.warn.unchecked.varargs.non.reifiable.type", + "varargs", + """|package a; + | + |import java.util.List; + | + |public class Example { + | @SuppressWarnings("varargs") + | public void run(List... lists) { + | } + |} + |""".stripMargin, + ) + + private def checkSuppressWarnings( + name: String, + original: String, + diagnosticCode: String, + warningName: String, + expected: String, + extraLayout: String = "", + )(implicit loc: Location): Unit = + test(name) { + val fileName = "Example.java" + val path = toPath(fileName) + val code = original.replace("<<", "").replace(">>", "") + + cleanWorkspace() + for { + _ <- initialize( + s"""|/.metals/mbt.json + |{ + | "namespaces": { + | "a": { + | "sources": ["a/src/main/java/**", "a/src/main/scala/**"] + | } + | } + |} + |/$path + |$code + |$extraLayout""".stripMargin + ) + _ <- server.didOpen(path) + diagnosticsPublished = + server.awaitNextDiagnostics( + path, + _.exists(diagnostic => + Option(diagnostic.getCode()).exists(code => + code.isLeft() && code.getLeft() == diagnosticCode + ) + ), + ) + _ <- server.didFocus(path) + _ <- diagnosticsPublished + codeActions <- server.assertCodeAction( + path, + original, + s"""|${SuppressWarnings.title(warningName)} + |""".stripMargin, + kind = Nil, + filterAction = _.getTitle() == SuppressWarnings.title(warningName), + ) + _ <- client.applyCodeAction(0, codeActions, server) + _ <- server.didChange(path) { _ => + server.bufferContents(path) + } + _ <- server.didSave(path) + _ = assertNoDiff(server.bufferContents(path), expected) + } yield () + } +} From cbbcbb8f6e039429a4b085ba5ca20825543392b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Fri, 10 Jul 2026 14:14:51 +0200 Subject: [PATCH 2/9] Remove redundant cast --- .../internal/metals/UserConfiguration.scala | 2 +- .../codeactions/CodeActionProvider.scala | 1 + .../codeactions/RemoveRedundantCast.scala | 157 +++++++ .../scala/tests/UserConfigurationSuite.scala | 7 +- .../RemoveRedundantCastLspSuite.scala | 385 ++++++++++++++++++ 5 files changed, 546 insertions(+), 6 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/internal/metals/codeactions/RemoveRedundantCast.scala create mode 100644 tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala index a452bb80f75..e386ef2c0cf 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -65,7 +65,7 @@ object JavaLintOptions { values match { case None => Right(default) case Some(values) => - values.find(!allowed.contains(_)) match { + values.find(value => !allowed(value)) match { case Some(invalid) => Left( s"invalid config value '$invalid' for javaLintOptions. Valid values are ${allowed.toSeq.sorted.map(value => s""""$value"""").mkString(", ")}" diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala index 3aa51b461c4..08796c81769 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala @@ -63,6 +63,7 @@ final class CodeActionProvider( new ConvertToNamedLambdaParameters(trees, compilers), new AddMissingOverrideAnnotation(javaTrees, buffers), new RemoveUnusedJavaImport(buffers), + new RemoveRedundantCast(buffers), new SuppressWarnings(javaTrees, buffers), new GenerateConstructors(javaTrees, buffers), new GenerateGettersSetters(javaTrees, buffers), diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/RemoveRedundantCast.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/RemoveRedundantCast.scala new file mode 100644 index 00000000000..7bef7b3a2cf --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/RemoveRedundantCast.scala @@ -0,0 +1,157 @@ +package scala.meta.internal.metals.codeactions + +import scala.concurrent.ExecutionContext +import scala.concurrent.Future + +import scala.meta.internal.metals.Buffers +import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.pc.CancelToken + +import org.eclipse.{lsp4j => l} + +class RemoveRedundantCast(buffers: Buffers) extends CodeAction { + import RemoveRedundantCast._ + + override def kind: String = l.CodeActionKind.QuickFix + override def isScala: Boolean = false + override def isJava: Boolean = true + + override def contribute( + params: l.CodeActionParams, + token: CancelToken, + )(implicit ec: ExecutionContext): Future[Seq[l.CodeAction]] = Future { + val path = params.getTextDocument().getUri().toAbsolutePath + val range = params.getRange() + + for { + text <- buffers.get(path).orElse(path.readTextOpt).toSeq + diagnostic <- params.getContext().getDiagnostics().asScala.toSeq + if isRedundantCast(diagnostic) + if range.overlapsWith(diagnostic.getRange()) + edit <- removeCastEdit(text, diagnostic.getRange()).toSeq + } yield CodeActionBuilder.build( + title, + kind, + diagnostics = List(diagnostic), + changes = Seq(path -> Seq(edit)), + ) + } +} + +object RemoveRedundantCast { + val title = "Remove redundant cast" + + private val RedundantCastCode = "compiler.warn.redundant.cast" + + private def isRedundantCast(diagnostic: l.Diagnostic): Boolean = + Option(diagnostic.getCode()).exists(code => + code.isLeft() && code.getLeft() == RedundantCastCode + ) + + private def removeCastEdit( + text: String, + range: l.Range, + ): Option[l.TextEdit] = { + val start = positionToOffset(text, range.getStart()) + for { + open <- findOpenParen(text, start) + close <- findCloseParen(text, open + 1) + if close > open + } yield { + val editEnd = skipHorizontalWhitespace(text, close + 1) + val editStart = + if ( + editEnd >= text.length || text + .charAt(editEnd) == '\n' || text.charAt(editEnd) == '\r' + ) + skipHorizontalWhitespaceBackwards(text, open) + else + open + new l.TextEdit( + new l.Range( + text.indexToLspPosition(editStart), + text.indexToLspPosition(editEnd), + ), + "", + ) + } + } + + private def findOpenParen(text: String, from: Int): Option[Int] = { + var index = from.min(text.length - 1) + var result = -1 + var continue = true + while (continue && index >= 0) { + text.charAt(index) match { + case '(' => + result = index + continue = false + case '\n' | '\r' | ';' | '=' | ',' => + continue = false + case _ => + index -= 1 + } + } + if (result >= 0) Some(result) else None + } + + private def findCloseParen(text: String, from: Int): Option[Int] = { + var index = from.max(0) + var result = -1 + var continue = true + while (continue && index < text.length) { + text.charAt(index) match { + case ')' => + result = index + continue = false + case '\n' | '\r' | ';' => + continue = false + case _ => + index += 1 + } + } + if (result >= 0) Some(result) else None + } + + private def skipHorizontalWhitespace(text: String, from: Int): Int = { + var index = from + while ( + index < text.length && { + val ch = text.charAt(index) + ch == ' ' || ch == '\t' + } + ) index += 1 + index + } + + private def skipHorizontalWhitespaceBackwards( + text: String, + from: Int, + ): Int = { + var index = from - 1 + while ( + index >= 0 && (text.charAt(index) == ' ' || text.charAt(index) == '\t') + ) + index -= 1 + index + 1 + } + + private def positionToOffset(text: String, position: l.Position): Int = { + var line = 0 + var character = 0 + var offset = 0 + while ( + offset < text.length && + (line < position.getLine() || character < position.getCharacter()) + ) { + if (text.charAt(offset) == '\n') { + line += 1 + character = 0 + } else { + character += 1 + } + offset += 1 + } + offset + } +} diff --git a/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala b/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala index 43556707200..449e17e1f79 100644 --- a/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala +++ b/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala @@ -424,7 +424,7 @@ class UserConfigurationSuite extends BaseSuite { javacServicesOverrides = JavacServicesOverrides.default.copy(names = false), scalafixRulesDependencies = List("rule1", "rule2"), - javaLintOptions = JavaLintOptions(List("deprecation", "unchecked")), + javaLintOptions = JavaLintOptions(Nil), customProjectRoot = Some("customs"), workspaceSymbolProvider = WorkspaceSymbolProviderConfig("mbt"), javaTurbineRecompileDelay = TurbineRecompileDelayConfig.testing, @@ -505,10 +505,7 @@ class UserConfigurationSuite extends BaseSuite { "rule2" ], "scalafixLintEnabled": false, - "javaLintOptions": [ - "deprecation", - "unchecked" - ], + "javaLintOptions": [], "customProjectRoot": "customs", "verboseCompilation": true, "autoImportBuilds": "all", diff --git a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala new file mode 100644 index 00000000000..86af285c621 --- /dev/null +++ b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala @@ -0,0 +1,385 @@ +package tests.codeactions + +import scala.meta.internal.metals.JavaLintOptions +import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.codeactions.RemoveRedundantCast + +import tests.MbtTestInitializer + +class RemoveRedundantCastLspSuite + extends BaseCodeActionLspSuite( + "remove-redundant-cast", + MbtTestInitializer, + useMbtLayout = true, + ) { + + override def userConfig: UserConfiguration = + super.userConfig.copy( + presentationCompilerDiagnostics = true, + javaLintOptions = JavaLintOptions(List("cast")), + ) + + override protected def toPath( + fileName: String, + isSource: Boolean = true, + ): String = + if (isSource) s"a/src/main/java/a/$fileName" + else s"a/$fileName" + + private val onlyRemoveRedundantCast: org.eclipse.lsp4j.CodeAction => Boolean = + _.getTitle() == RemoveRedundantCast.title + + private def title: String = + s"""|${RemoveRedundantCast.title} + |""".stripMargin + + check( + "basic", + """|package a; + | + |public class Example { + | public String run() { + | return <<(String) "value">>; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public String run() { + | return "value"; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "without-space", + """|package a; + | + |public class Example { + | public String run(String value) { + | return <<(String)value>>; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public String run(String value) { + | return value; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "nested-parentheses", + """|package a; + | + |public class Example { + | public String run(String value) { + | return (<<(String) value>>); + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public String run(String value) { + | return (value); + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "generic-type", + """|package a; + | + |import java.util.List; + | + |public class Example { + | public String run(List list) { + | return (<<(List) list>>).get(0); + | } + |} + |""".stripMargin, + title, + """|package a; + | + |import java.util.List; + | + |public class Example { + | public String run(List list) { + | return (list).get(0); + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "array-type", + """|package a; + | + |public class Example { + | public String run(String[] array) { + | return (<<(String[]) array>>)[0]; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public String run(String[] array) { + | return (array)[0]; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "primitive-type", + """|package a; + | + |public class Example { + | public int run() { + | return <<(int) 5>>; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public int run() { + | return 5; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "method-invocation", + """|package a; + | + |public class Example { + | public String run(Object obj) { + | return <<(String) obj.toString()>>; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public String run(Object obj) { + | return obj.toString(); + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "extra-spaces", + """|package a; + | + |public class Example { + | public String run(String value) { + | return <<( String ) value>>; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public String run(String value) { + | return value; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "newline-after-cast", + """|package a; + | + |public class Example { + | public String run(String value) { + | return <<(String) + | value>>; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public String run(String value) { + | return + | value; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "variable-declaration", + """|package a; + | + |public class Example { + | public String run() { + | String s = <<(String) "value">>; + | return s; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public String run() { + | String s = "value"; + | return s; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "intersection-type", + """|package a; + | + |import java.io.Serializable; + | + |public class Example { + | public String run(T t) { + | return <<(String & Serializable) t>>; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |import java.io.Serializable; + | + |public class Example { + | public String run(T t) { + | return t; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "this-cast", + """|package a; + | + |public class Example { + | public Example run() { + | return <<(Example) this>>; + | } + |} + |""".stripMargin, + title, + """|package a; + | + |public class Example { + | public Example run() { + | return this; + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "lambda-cast", + """|package a; + | + |import java.util.List; + |import java.util.stream.Collectors; + | + |public class Example { + | public List run(List list) { + | return list.stream().map(x -> <<(String) x>>).collect(Collectors.toList()); + | } + |} + |""".stripMargin, + title, + """|package a; + | + |import java.util.List; + |import java.util.stream.Collectors; + | + |public class Example { + | public List run(List list) { + | return list.stream().map(x -> x).collect(Collectors.toList()); + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) + + check( + "lambda-return-cast", + """|package a; + | + |import java.util.List; + |import java.util.stream.Collectors; + | + |public class Example { + | public List run(List list) { + | return list.stream().map(x -> { + | return <<(String) x>>; + | }).collect(Collectors.toList()); + | } + |} + |""".stripMargin, + title, + """|package a; + | + |import java.util.List; + |import java.util.stream.Collectors; + | + |public class Example { + | public List run(List list) { + | return list.stream().map(x -> { + | return x; + | }).collect(Collectors.toList()); + | } + |} + |""".stripMargin, + fileName = "Example.java", + filterAction = onlyRemoveRedundantCast, + ) +} From 301267ba4c924c5dd4cde7f109de7ccd57400476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Thu, 6 Aug 2026 22:53:01 +0200 Subject: [PATCH 3/9] cleanup --- .../internal/metals/MetalsEnrichments.scala | 19 ++ .../internal/metals/UserConfiguration.scala | 2 +- .../codeactions/CodeActionProvider.scala | 2 +- .../codeactions/RemoveRedundantCast.scala | 137 ++------ .../metals/codeactions/SuppressWarnings.scala | 312 +++++++----------- .../meta/internal/parsing/JavaTrees.scala | 134 +++++++- .../scala/tests/UserConfigurationSuite.scala | 2 +- .../RemoveRedundantCastLspSuite.scala | 29 +- .../SuppressWarningsLspSuite.scala | 4 +- 9 files changed, 318 insertions(+), 323 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala index a0ff0444bcd..8caa9596f27 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala @@ -847,6 +847,25 @@ object MetalsEnrichments new l.Position(lineCount, index - lineStartIdx) } + def lspPositionToIndex(position: l.Position): Int = { + var line = 0 + var character = 0 + var offset = 0 + while ( + offset < value.length && + (line < position.getLine() || character < position.getCharacter()) + ) { + if (value.charAt(offset) == '\n') { + line += 1 + character = 0 + } else { + character += 1 + } + offset += 1 + } + offset + } + def replaceAllBetween(start: String, end: String)( replacement: String ): String = diff --git a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala index e386ef2c0cf..5153de2cacb 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -54,7 +54,7 @@ object JavaLintOptions { "this-escape", "try", "unchecked", "varargs", ) - val default: JavaLintOptions = JavaLintOptions(allValues) + val default: JavaLintOptions = JavaLintOptions(Nil) private val allowed: Set[String] = (allValues :+ "preview").toSet diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala index 08796c81769..1be44112fb2 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala @@ -63,7 +63,7 @@ final class CodeActionProvider( new ConvertToNamedLambdaParameters(trees, compilers), new AddMissingOverrideAnnotation(javaTrees, buffers), new RemoveUnusedJavaImport(buffers), - new RemoveRedundantCast(buffers), + new RemoveRedundantCast(javaTrees, buffers), new SuppressWarnings(javaTrees, buffers), new GenerateConstructors(javaTrees, buffers), new GenerateGettersSetters(javaTrees, buffers), diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/RemoveRedundantCast.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/RemoveRedundantCast.scala index 7bef7b3a2cf..3026482b920 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/RemoveRedundantCast.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/RemoveRedundantCast.scala @@ -5,11 +5,14 @@ import scala.concurrent.Future import scala.meta.internal.metals.Buffers import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.parsing.JavaTrees +import scala.meta.internal.parsing.JavaTypeCast import scala.meta.pc.CancelToken import org.eclipse.{lsp4j => l} -class RemoveRedundantCast(buffers: Buffers) extends CodeAction { +class RemoveRedundantCast(javaTrees: JavaTrees, buffers: Buffers) + extends CodeAction { import RemoveRedundantCast._ override def kind: String = l.CodeActionKind.QuickFix @@ -28,12 +31,14 @@ class RemoveRedundantCast(buffers: Buffers) extends CodeAction { diagnostic <- params.getContext().getDiagnostics().asScala.toSeq if isRedundantCast(diagnostic) if range.overlapsWith(diagnostic.getRange()) - edit <- removeCastEdit(text, diagnostic.getRange()).toSeq + cast <- javaTrees + .findTypeCast(path, diagnostic.getRange().getStart()) + .toSeq } yield CodeActionBuilder.build( title, kind, diagnostics = List(diagnostic), - changes = Seq(path -> Seq(edit)), + changes = Seq(path -> Seq(removeCastEdit(text, cast))), ) } } @@ -48,110 +53,32 @@ object RemoveRedundantCast { code.isLeft() && code.getLeft() == RedundantCastCode ) - private def removeCastEdit( - text: String, - range: l.Range, - ): Option[l.TextEdit] = { - val start = positionToOffset(text, range.getStart()) - for { - open <- findOpenParen(text, start) - close <- findCloseParen(text, open + 1) - if close > open - } yield { - val editEnd = skipHorizontalWhitespace(text, close + 1) - val editStart = - if ( - editEnd >= text.length || text - .charAt(editEnd) == '\n' || text.charAt(editEnd) == '\r' - ) - skipHorizontalWhitespaceBackwards(text, open) - else - open - new l.TextEdit( - new l.Range( - text.indexToLspPosition(editStart), - text.indexToLspPosition(editEnd), - ), - "", + private def removeCastEdit(text: String, cast: JavaTypeCast): l.TextEdit = { + val castStart = cast.typeRange.startOffset + val typeEnd = cast.typeRange.endOffset + val editEnd = typeEnd + text + .substring(typeEnd, cast.exprRange.startOffset) + .takeWhile(ch => ch == ' ' || ch == '\t') + .length + val editStart = + if ( + editEnd >= text.length || text.charAt(editEnd) == '\n' || text + .charAt(editEnd) == '\r' ) - } - } - - private def findOpenParen(text: String, from: Int): Option[Int] = { - var index = from.min(text.length - 1) - var result = -1 - var continue = true - while (continue && index >= 0) { - text.charAt(index) match { - case '(' => - result = index - continue = false - case '\n' | '\r' | ';' | '=' | ',' => - continue = false - case _ => - index -= 1 - } - } - if (result >= 0) Some(result) else None - } - - private def findCloseParen(text: String, from: Int): Option[Int] = { - var index = from.max(0) - var result = -1 - var continue = true - while (continue && index < text.length) { - text.charAt(index) match { - case ')' => - result = index - continue = false - case '\n' | '\r' | ';' => - continue = false - case _ => - index += 1 - } - } - if (result >= 0) Some(result) else None - } - - private def skipHorizontalWhitespace(text: String, from: Int): Int = { - var index = from - while ( - index < text.length && { - val ch = text.charAt(index) - ch == ' ' || ch == '\t' - } - ) index += 1 - index - } - - private def skipHorizontalWhitespaceBackwards( - text: String, - from: Int, - ): Int = { - var index = from - 1 - while ( - index >= 0 && (text.charAt(index) == ' ' || text.charAt(index) == '\t') + castStart - JavaMemberInsertion + .linePrefix(text, castStart) + .reverse + .takeWhile(ch => ch == ' ' || ch == '\t') + .length + else + castStart + new l.TextEdit( + new l.Range( + text.indexToLspPosition(editStart), + text.indexToLspPosition(editEnd), + ), + "", ) - index -= 1 - index + 1 } - private def positionToOffset(text: String, position: l.Position): Int = { - var line = 0 - var character = 0 - var offset = 0 - while ( - offset < text.length && - (line < position.getLine() || character < position.getCharacter()) - ) { - if (text.charAt(offset) == '\n') { - line += 1 - character = 0 - } else { - character += 1 - } - offset += 1 - } - offset - } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala index 3d4ff865749..1cd4c1049e1 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala @@ -5,6 +5,7 @@ import scala.concurrent.Future import scala.meta.internal.metals.Buffers import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.parsing.JavaAnnotation import scala.meta.internal.parsing.JavaMember import scala.meta.internal.parsing.JavaRange import scala.meta.internal.parsing.JavaTrees @@ -40,8 +41,8 @@ class SuppressWarnings( if (range.overlapsWith(diagnostic.getRange())) diagnostic.getRange().getStart() else range.getStart() - member <- enclosingMember(path, position).toSeq - edit <- suppressEdit(text, member, warningName).toSeq + member <- enclosingMember(text, path, position).toSeq + edit <- suppressEdit(text, path, member, warningName).toSeq } yield CodeActionBuilder.build( title(warningName), kind, @@ -52,11 +53,17 @@ class SuppressWarnings( } private def enclosingMember( + text: String, path: AbsolutePath, position: l.Position, - ): Option[SuppressTarget] = + ): Option[SuppressTarget] = { + val positionOffset = text.lspPositionToIndex(position) javaTrees .findEnclosingJavaVariable(path, position, onNameOnly = false) + .filter(variable => + variable.isStandaloneDeclaration && + positionOffset <= variable.nameRange.endOffset + ) .map(variable => SuppressTarget(variable, variable.nameRange)) .orElse( javaTrees @@ -68,102 +75,30 @@ class SuppressWarnings( .findEnclosingJavaClass(path, position) .map(cls => SuppressTarget(cls, cls.nameRange)) ) -} - -object SuppressWarnings { - def title(warningName: String): String = - s"""Add @SuppressWarnings("$warningName")""" - - private val SuppressWarningsName = "@SuppressWarnings" - - private val WarningNames = - List( - "auxiliaryclass", "cast", "classfile", "deprecation", "dep-ann", - "divzero", "empty", "exports", "fallthrough", "finally", - "lossy-conversions", "missing-explicit-ctor", "module", "opens", - "options", "output-file-clash", "overloads", "overrides", "path", - "processing", "rawtypes", "removal", "requires-automatic", - "requires-transitive-automatic", "serial", "static", "strictfp", - "synchronization", "text-blocks", "this-escape", "try", "unchecked", - "varargs", "preview", - ) - - private def warningName(diagnostic: l.Diagnostic): Option[String] = - if (diagnostic.getSource() == "javac") { - val code = Option(diagnostic.getCode()) - .collect { case code if code.isLeft() => code.getLeft() } - .getOrElse("") - .toLowerCase() - val isWarning = code.startsWith("compiler.warn.") || - diagnostic.getSeverity() == l.DiagnosticSeverity.Warning - if (isWarning) { - val message = - Option(diagnostic.getMessage()) - .map(_.toString()) - .getOrElse("") - .toLowerCase() - warningNameFrom(code).orElse(warningNameFrom(message)) - } else None - } else None - - private def warningNameFrom(text: String): Option[String] = { - val matched = WarningNames.filter(name => containsWholeWord(text, name)) - matched.maxByOption(_.length).orElse { - if (text.contains("missing.deprecated.annotation")) Some("dep-ann") - else if (text.contains("loss.of.precision")) Some("lossy-conversions") - else if (text.contains("fall-through")) Some("fallthrough") - else if (text.contains("ambiguous.overload")) Some("overloads") - else if (text.contains("override.equals")) Some("overrides") - else if (text.contains("trailing.white.space")) Some("text-blocks") - else if (text.contains("this.escape")) Some("this-escape") - else if (text.contains("synchronize")) Some("synchronization") - else if (text.contains("deprecated")) Some("deprecation") - else if (text.contains("raw.class")) Some("rawtypes") - else if (text.contains("serialversionuid") || text.contains("svuid")) - Some("serial") - else None - } } - private def containsWholeWord(text: String, name: String): Boolean = { - var index = text.indexOf(name) - var found = false - while (!found && index >= 0) { - val end = index + name.length - val beforeOk = index == 0 || !isWarningNameChar(text.charAt(index - 1)) - val afterOk = end >= text.length || !isWarningNameChar(text.charAt(end)) - if (beforeOk && afterOk) found = true - else index = text.indexOf(name, index + 1) - } - found - } - - private def isWarningNameChar(ch: Char): Boolean = - Character.isLetterOrDigit(ch) || ch == '-' - - private def isZeroRange(range: l.Range): Boolean = - range.getStart().getLine() == 0 && - range.getStart().getCharacter() == 0 && - range.getEnd().getLine() == 0 && - range.getEnd().getCharacter() == 0 - private def suppressEdit( text: String, + path: scala.meta.io.AbsolutePath, target: SuppressTarget, warningName: String, - ): Option[l.TextEdit] = - existingSuppressWarnings(text, target) match { + ): Option[l.TextEdit] = { + val annotations = javaTrees.memberAnnotations(path, target.member) + existingSuppressWarnings(annotations) match { case Some(existing) => appendWarningEdit(text, existing, warningName) - case None => Some(insertSuppressWarningsEdit(text, target, warningName)) + case None => + Some(insertSuppressWarningsEdit(text, target, warningName, annotations)) } + } private def insertSuppressWarningsEdit( text: String, target: SuppressTarget, warningName: String, + annotations: List[JavaAnnotation], ): l.TextEdit = { - val declarationOffset = declarationStartOffset(text, target) - val declarationStart = positionAtOffset(text, declarationOffset) + val declarationOffset = declarationStartOffset(text, target, annotations) + val declarationStart = text.indexToLspPosition(declarationOffset) val linePrefix = JavaMemberInsertion.linePrefix(text, declarationOffset) val (position, newText) = @@ -178,74 +113,118 @@ object SuppressWarnings { new l.TextEdit(new l.Range(position, position), newText) } - /** - * The offset where the member declaration proper starts, skipping any - * annotations that precede it, so that `@SuppressWarnings` lands after - * existing annotations like `@Override`. - */ private def declarationStartOffset( text: String, target: SuppressTarget, + annotations: List[JavaAnnotation], ): Int = { - val end = target.nameRange.startOffset.min(text.length) - var offset = target.member.range.startOffset.max(0) - var continue = true - while (continue) { - var annotationStart = offset - while (annotationStart < end && text.charAt(annotationStart).isWhitespace) - annotationStart += 1 - if (annotationStart < end && text.charAt(annotationStart) == '@') { - var nameEnd = annotationStart + 1 - while ( - nameEnd < end && - (Character.isJavaIdentifierPart(text.charAt(nameEnd)) || - text.charAt(nameEnd) == '.') - ) nameEnd += 1 - // `@interface` is an annotation-type declaration, not an annotation. - if (text.substring(annotationStart + 1, nameEnd) == "interface") - continue = false - else { - var argsStart = nameEnd - while (argsStart < end && text.charAt(argsStart).isWhitespace) - argsStart += 1 - if (argsStart < end && text.charAt(argsStart) == '(') - matchingCloseParen(text, argsStart, end) match { - case Some(close) => offset = close + 1 - case None => continue = false - } - else offset = nameEnd - } - } else continue = false - } - var declarationStart = offset - while (declarationStart < end && text.charAt(declarationStart).isWhitespace) - declarationStart += 1 - declarationStart + val afterAnnotations = annotations + .maxByOption(_.range.endOffset) + .map(_.range.endOffset) + .getOrElse(target.member.range.startOffset) + var offset = afterAnnotations + while ( + offset < target.nameRange.startOffset && text.charAt(offset).isWhitespace + ) + offset += 1 + offset } +} + +object SuppressWarnings { + def title(warningName: String): String = + s"""Add @SuppressWarnings("$warningName")""" + + private val DiagnosticSubstrings: List[(String, String)] = List( + "missing.deprecated.annotation" -> "dep-ann", + "deprecated.for.removal" -> "removal", + "requires-transitive-automatic" -> "requires-transitive-automatic", + "requires-automatic" -> "requires-automatic", + "output-file-clash" -> "output-file-clash", + "missing-explicit-ctor" -> "missing-explicit-ctor", + "loss.of.precision" -> "lossy-conversions", + "fall-through" -> "fallthrough", + "ambiguous.overload" -> "overloads", + "override.equals" -> "overrides", + "trailing.white.space" -> "text-blocks", + "this.escape" -> "this-escape", + "synchronize" -> "synchronization", + "auxiliaryclass" -> "auxiliaryclass", + "raw.class" -> "rawtypes", + "div.zero" -> "divzero", + "serialversionuid" -> "serial", + "svuid" -> "serial", + "classfile" -> "classfile", + "varargs" -> "varargs", + "unchecked" -> "unchecked", + "deprecated" -> "deprecation", + "cast" -> "cast", + "divzero" -> "divzero", + "empty" -> "empty", + "exports" -> "exports", + "fallthrough" -> "fallthrough", + "finally" -> "finally", + "lossy-conversions" -> "lossy-conversions", + "module" -> "module", + "opens" -> "opens", + "options" -> "options", + "overloads" -> "overloads", + "overrides" -> "overrides", + "path" -> "path", + "preview" -> "preview", + "processing" -> "processing", + "rawtypes" -> "rawtypes", + "removal" -> "removal", + "serial" -> "serial", + "static" -> "static", + "strictfp" -> "strictfp", + "synchronization" -> "synchronization", + "text-blocks" -> "text-blocks", + "this-escape" -> "this-escape", + "try" -> "try", + ) + + private def warningName(diagnostic: l.Diagnostic): Option[String] = + if (diagnostic.getSource() == "javac") { + val code = Option(diagnostic.getCode()) + .collect { case code if code.isLeft() => code.getLeft() } + .getOrElse("") + .toLowerCase() + val isWarning = code.startsWith("compiler.warn.") || + diagnostic.getSeverity() == l.DiagnosticSeverity.Warning + if (isWarning) { + val message = + Option(diagnostic.getMessage()) + .map(_.toString()) + .getOrElse("") + .toLowerCase() + warningNameFrom(code).orElse(warningNameFrom(message)) + } else None + } else None + + private def warningNameFrom(text: String): Option[String] = + DiagnosticSubstrings.collectFirst { + case (substr, name) if text.contains(substr) => name + } + + private def isZeroRange(range: l.Range): Boolean = + range.getStart().getLine() == 0 && + range.getStart().getCharacter() == 0 && + range.getEnd().getLine() == 0 && + range.getEnd().getCharacter() == 0 private def existingSuppressWarnings( - text: String, - target: SuppressTarget, - ): Option[ExistingSuppressWarnings] = { - val prefixStart = target.member.range.startOffset - val prefixEnd = target.nameRange.startOffset - if (prefixStart < 0 || prefixEnd <= prefixStart || prefixEnd > text.length) - None - else { - val prefix = text.substring(prefixStart, prefixEnd) - val annotationStartInPrefix = prefix.indexOf(SuppressWarningsName) - if (annotationStartInPrefix < 0) None - else { - val annotationStart = prefixStart + annotationStartInPrefix - val open = text.indexOf('(', annotationStart) - if (open < 0 || open >= prefixEnd) None - else - matchingCloseParen(text, open, prefixEnd).map { close => - ExistingSuppressWarnings(open, close) - } + annotations: List[JavaAnnotation] + ): Option[ExistingSuppressWarnings] = + annotations + .collectFirst { + case ann + if ann.name == "SuppressWarnings" || + ann.name.endsWith(".SuppressWarnings") => + ann.argsRange } - } - } + .flatten + .map { case (open, close) => ExistingSuppressWarnings(open, close) } private def appendWarningEdit( text: String, @@ -263,16 +242,16 @@ object SuppressWarnings { val closeBrace = insideEnd - inside.reverse.indexOf('}') - 1 ( new l.Range( - positionAtOffset(text, closeBrace), - positionAtOffset(text, closeBrace), + text.indexToLspPosition(closeBrace), + text.indexToLspPosition(closeBrace), ), s""", "$warningName"""", ) } else { ( new l.Range( - positionAtOffset(text, insideStart), - positionAtOffset(text, insideEnd), + text.indexToLspPosition(insideStart), + text.indexToLspPosition(insideEnd), ), s"""{$trimmed, "$warningName"}""", ) @@ -281,41 +260,6 @@ object SuppressWarnings { } } - private def matchingCloseParen( - text: String, - open: Int, - end: Int, - ): Option[Int] = { - var offset = open - var depth = 0 - while (offset < end) { - text.charAt(offset) match { - case '(' => depth += 1 - case ')' => - depth -= 1 - if (depth == 0) return Some(offset) - case _ => - } - offset += 1 - } - None - } - - private def positionAtOffset(text: String, offset: Int): l.Position = { - val safeOffset = offset.max(0).min(text.length) - var line = 0 - var lineStart = 0 - var index = 0 - while (index < safeOffset) { - if (text.charAt(index) == '\n') { - line += 1 - lineStart = index + 1 - } - index += 1 - } - new l.Position(line, safeOffset - lineStart) - } - private case class SuppressTarget( member: JavaMember, nameRange: JavaRange, diff --git a/metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala b/metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala index 7cc8c716513..755e2f375e2 100644 --- a/metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala +++ b/metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala @@ -17,12 +17,14 @@ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals._ import scala.meta.io.AbsolutePath +import com.sun.source.tree.AnnotationTree import com.sun.source.tree.ClassTree import com.sun.source.tree.CompilationUnitTree import com.sun.source.tree.IdentifierTree import com.sun.source.tree.LineMap import com.sun.source.tree.MethodTree import com.sun.source.tree.Tree +import com.sun.source.tree.TypeCastTree import com.sun.source.tree.VariableTree import com.sun.source.util.TreePathScanner import com.sun.tools.javac.file.JavacFileManager @@ -111,6 +113,95 @@ class JavaTrees(buffers: Buffers) { } } yield result + def findTypeCast( + source: AbsolutePath, + pos: l.Position, + ): Option[JavaTypeCast] = + for { + text <- text(source) + cu <- get(source) + castTree <- { + val visitor = new TypeCastFinder(cu, text, pos) + visitor.scan(cu, ()) + visitor.result + } + } yield { + val treePos = new TreePositions(cu) + val lineMap = + JavacPosition.makeLineMap(text.toCharArray(), text.length(), false) + val castStart = treePos.startPos(castTree) + val castEnd = treePos.endPos(castTree) + val typeEnd = treePos.endPos(castTree.getType()) + val exprStart = treePos.startPos(castTree.getExpression()) + val exprEnd = treePos.endPos(castTree.getExpression()) + var closeParenPos = typeEnd + while (closeParenPos < exprStart && text.charAt(closeParenPos) != ')') + closeParenPos += 1 + val typeRangeEnd = closeParenPos + 1 + JavaTypeCast( + range = JavaRange( + Positions.toLspRange(lineMap, castStart, castEnd, text), + castStart, + castEnd, + ), + typeRange = JavaRange( + Positions.toLspRange(lineMap, castStart, typeRangeEnd, text), + castStart, + typeRangeEnd, + ), + exprRange = JavaRange( + Positions.toLspRange(lineMap, exprStart, exprEnd, text), + exprStart, + exprEnd, + ), + ) + } + + def memberAnnotations( + source: AbsolutePath, + member: JavaMember, + ): List[JavaAnnotation] = + (for { + text <- text(source) + cu <- get(source) + } yield { + val treePos = new TreePositions(cu) + val lineMap = + JavacPosition.makeLineMap(text.toCharArray(), text.length(), false) + val rawAnnotations: Iterable[AnnotationTree] = member.tree match { + case m: MethodTree => m.getModifiers().getAnnotations().asScala + case v: VariableTree => v.getModifiers().getAnnotations().asScala + case c: ClassTree => c.getModifiers().getAnnotations().asScala + case _ => Iterable.empty + } + rawAnnotations.flatMap { ann => + val start = treePos.startPos(ann) + val end = treePos.endPos(ann) + if (start < 0 || end < 0) None + else { + val range = JavaRange( + Positions.toLspRange(lineMap, start, end, text), + startOffset = start, + endOffset = end, + ) + val nameStr = ann.getAnnotationType().toString() + val argsRange = + if (ann.getArguments().isEmpty()) None + else { + var openParen = start + 1 + while (openParen < end && text.charAt(openParen) != '(') + openParen += 1 + var closeParen = end - 1 + while (closeParen > openParen && text.charAt(closeParen) != ')') + closeParen -= 1 + if (openParen < closeParen) Some((openParen, closeParen)) + else None + } + Some(JavaAnnotation(nameStr, range, argsRange)) + } + }.toList + }).getOrElse(Nil) + private def text(source: AbsolutePath): Option[String] = buffers.get(source).orElse(source.readTextOpt) @@ -247,7 +338,10 @@ class JavaTrees(buffers: Buffers) { } .toList - protected def javaVariable(node: VariableTree): Option[JavaVariable] = { + protected def javaVariable( + node: VariableTree, + ownerKind: Tree.Kind, + ): Option[JavaVariable] = { val variableName = node.getName().toString() treeRange(node).map { range => JavaVariable( @@ -269,6 +363,7 @@ class JavaTrees(buffers: Buffers) { "var" }, modifiers = node.getModifiers().getFlags().asScala.toSet, + ownerKind = ownerKind, ) } } @@ -353,7 +448,11 @@ class JavaTrees(buffers: Buffers) { !onNameOnly || positionContains(targetOffset, actualNodeStart, actualNodeEnd) ) { - _result = javaVariable(node) + val parentPath = getCurrentPath().getParentPath() + val ownerKind = + if (parentPath == null) Tree.Kind.COMPILATION_UNIT + else parentPath.getLeaf().getKind() + _result = javaVariable(node, ownerKind) } } super.visitVariable(node, p) @@ -444,7 +543,7 @@ class JavaTrees(buffers: Buffers) { ) } case field: VariableTree => - javaVariable(field) + javaVariable(field, Tree.Kind.CLASS) } .flatten .toList @@ -472,6 +571,20 @@ class JavaTrees(buffers: Buffers) { } } + private class TypeCastFinder( + cu: CompilationUnitTree, + text: String, + targetPos: l.Position, + ) extends EnclosingFinder[TypeCastTree](cu, text, targetPos) { + override def visitTypeCast(node: TypeCastTree, p: Unit): Unit = { + val nodeStart = pos.startPos(node) + val nodeEnd = pos.endPos(node) + if (positionContains(targetOffset, nodeStart, nodeEnd)) + _result = Some(node) + super.visitTypeCast(node, p) + } + } + private class TreePositions(cu: CompilationUnitTree) { private val endPosTable = cu.asInstanceOf[JavacJCTree.JCCompilationUnit].endPositions @@ -784,7 +897,22 @@ case class JavaVariable( nameRange: JavaRange, typ: String, modifiers: Set[Modifier], + ownerKind: Tree.Kind, ) extends JavaMember with HasModifiers { def hasInitializer: Boolean = tree.getInitializer() != null + def isStandaloneDeclaration: Boolean = + ownerKind != Tree.Kind.METHOD && ownerKind != Tree.Kind.TRY } + +case class JavaTypeCast( + range: JavaRange, + typeRange: JavaRange, + exprRange: JavaRange, +) + +case class JavaAnnotation( + name: String, + range: JavaRange, + argsRange: Option[(Int, Int)], +) diff --git a/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala b/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala index 449e17e1f79..71102f7d912 100644 --- a/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala +++ b/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala @@ -591,7 +591,7 @@ class UserConfigurationSuite extends BaseSuite { |shim-globs string `{}`. Shim file globs |scalafix-rules-dependencies array [] Scalafix rules dependencies |scalafix-lint-enabled boolean false Enable Scalafix lint diagnostics - |java-lint-options array [cast,deprecation,dep-ann,divzero,empty,fallthrough,finally,lossy-conversions,overloads,overrides,rawtypes,removal,serial,static,strictfp,synchronization,text-blocks,this-escape,try,unchecked,varargs] Java lint diagnostics + |java-lint-options array [] Java lint diagnostics |excluded-packages array [] Excluded Packages |bloop-sbt-already-installed boolean false Don't generate Bloop plugin file for sbt |bloop-version string $bloopVersionPadded Version of Bloop diff --git a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala index 86af285c621..0dcb63c84ec 100644 --- a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala @@ -1,9 +1,11 @@ package tests.codeactions import scala.meta.internal.metals.JavaLintOptions +import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.UserConfiguration import scala.meta.internal.metals.codeactions.RemoveRedundantCast +import org.eclipse.{lsp4j => l} import tests.MbtTestInitializer class RemoveRedundantCastLspSuite @@ -271,33 +273,6 @@ class RemoveRedundantCastLspSuite filterAction = onlyRemoveRedundantCast, ) - check( - "intersection-type", - """|package a; - | - |import java.io.Serializable; - | - |public class Example { - | public String run(T t) { - | return <<(String & Serializable) t>>; - | } - |} - |""".stripMargin, - title, - """|package a; - | - |import java.io.Serializable; - | - |public class Example { - | public String run(T t) { - | return t; - | } - |} - |""".stripMargin, - fileName = "Example.java", - filterAction = onlyRemoveRedundantCast, - ) - check( "this-cast", """|package a; diff --git a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala index 8b99f75b912..6c5526e3d47 100644 --- a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala @@ -1,10 +1,12 @@ package tests.codeactions import scala.meta.internal.metals.JavaLintOptions +import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.UserConfiguration import scala.meta.internal.metals.codeactions.SuppressWarnings import munit.Location +import org.eclipse.{lsp4j => l} import tests.MbtTestInitializer class SuppressWarningsLspSuite @@ -17,7 +19,7 @@ class SuppressWarningsLspSuite override def userConfig: UserConfiguration = super.userConfig.copy( presentationCompilerDiagnostics = true, - javaLintOptions = JavaLintOptions.default, + javaLintOptions = JavaLintOptions(JavaLintOptions.allValues), ) override protected def toPath( From ddbc842825f1b4d1c24c929ccc1b771a03404306 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Fri, 7 Aug 2026 00:30:15 +0200 Subject: [PATCH 4/9] address review --- .../metals/codeactions/SuppressWarnings.scala | 22 ++- .../RemoveRedundantCastLspSuite.scala | 2 +- .../SuppressWarningsLspSuite.scala | 127 ++++++++++++++---- 3 files changed, 124 insertions(+), 27 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala index 1cd4c1049e1..890fd63b53a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala @@ -237,8 +237,24 @@ object SuppressWarnings { if (inside.contains(s""""$warningName"""")) None else { val trimmed = inside.trim() + val (namedValuePrefix, value) = trimmed match { + case NamedValueArgument(prefix, value) => (prefix, value.trim()) + case _ => ("", trimmed) + } + val isArray = value.startsWith("{") && value.endsWith("}") val (range, newText) = - if (trimmed.startsWith("{") && trimmed.endsWith("}")) { + if ( + isArray && + value.substring(1, value.length() - 1).trim().isEmpty() + ) { + ( + new l.Range( + text.indexToLspPosition(insideStart), + text.indexToLspPosition(insideEnd), + ), + s"""$namedValuePrefix{"$warningName"}""", + ) + } else if (isArray) { val closeBrace = insideEnd - inside.reverse.indexOf('}') - 1 ( new l.Range( @@ -253,13 +269,15 @@ object SuppressWarnings { text.indexToLspPosition(insideStart), text.indexToLspPosition(insideEnd), ), - s"""{$trimmed, "$warningName"}""", + s"""$namedValuePrefix{$value, "$warningName"}""", ) } Some(new l.TextEdit(range, newText)) } } + private val NamedValueArgument = """(?s)(value\s*=\s*)(.*)""".r + private case class SuppressTarget( member: JavaMember, nameRange: JavaRange, diff --git a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala index 0dcb63c84ec..796e8b4ea6d 100644 --- a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala @@ -28,7 +28,7 @@ class RemoveRedundantCastLspSuite if (isSource) s"a/src/main/java/a/$fileName" else s"a/$fileName" - private val onlyRemoveRedundantCast: org.eclipse.lsp4j.CodeAction => Boolean = + private val onlyRemoveRedundantCast: l.CodeAction => Boolean = _.getTitle() == RemoveRedundantCast.title private def title: String = diff --git a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala index 6c5526e3d47..11c1b82a773 100644 --- a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala @@ -29,6 +29,16 @@ class SuppressWarningsLspSuite if (isSource) s"a/src/main/java/a/$fileName" else s"a/$fileName" + private val deprecatedApiLayout = + """|/a/src/main/java/a/DeprecatedApi.java + |package a; + | + |class DeprecatedApi { + | @Deprecated + | static void old() {} + |} + |""".stripMargin + checkSuppressWarnings( "rawtypes-method", """|package a; @@ -79,14 +89,7 @@ class SuppressWarningsLspSuite | } |} |""".stripMargin, - extraLayout = """|/a/src/main/java/a/DeprecatedApi.java - |package a; - | - |class DeprecatedApi { - | @Deprecated - | static void old() {} - |} - |""".stripMargin, + extraLayout = deprecatedApiLayout, ) checkSuppressWarnings( @@ -215,14 +218,7 @@ class SuppressWarningsLspSuite | } |} |""".stripMargin, - extraLayout = """|/a/src/main/java/a/DeprecatedApi.java - |package a; - | - |class DeprecatedApi { - | @Deprecated - | static void old() {} - |} - |""".stripMargin, + extraLayout = deprecatedApiLayout, ) checkSuppressWarnings( @@ -255,6 +251,96 @@ class SuppressWarningsLspSuite |""".stripMargin, ) + checkSuppressWarnings( + "append-existing-empty-array", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings({}) + |<< public List names() { + | return new ArrayList(); + | }>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings({"rawtypes"}) + | public List names() { + | return new ArrayList(); + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "append-existing-named-scalar", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings(value = "unchecked") + |<< public List names() { + | return new ArrayList(); + | }>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings(value = {"unchecked", "rawtypes"}) + | public List names() { + | return new ArrayList(); + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "append-existing-named-array", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings(value = {"unchecked", "serial"}) + |<< public List names() { + | return new ArrayList(); + | }>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings(value = {"unchecked", "serial", "rawtypes"}) + | public List names() { + | return new ArrayList(); + | } + |} + |""".stripMargin, + ) + checkSuppressWarnings( "multiple-annotations", """|package a; @@ -278,14 +364,7 @@ class SuppressWarningsLspSuite | } |} |""".stripMargin, - extraLayout = """|/a/src/main/java/a/DeprecatedApi.java - |package a; - | - |class DeprecatedApi { - | @Deprecated - | static void old() {} - |} - |""".stripMargin, + extraLayout = deprecatedApiLayout, ) checkSuppressWarnings( From b5dcdaf0f19ce61eb42174fdc9efa5be929103c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Fri, 7 Aug 2026 00:50:38 +0200 Subject: [PATCH 5/9] Use Java lint options from build target --- .../meta/internal/metals/BuildTargets.scala | 7 +- .../metals/CompilerConfiguration.scala | 24 +++++-- .../meta/internal/metals/Compilers.scala | 31 ++++---- .../meta/internal/metals/ModuleStatus.scala | 2 +- .../meta/internal/metals/TargetData.scala | 8 ++- .../internal/metals/UserConfiguration.scala | 48 ------------- .../debug/server/DebugeeParamsCreator.scala | 2 +- .../internal/metals/mcp/MetalsMcpTools.scala | 2 +- .../src/main/scala/tests/BaseLspSuite.scala | 2 - .../src/main/scala/tests/MbtJsonBuilder.scala | 3 +- .../codeactions/BaseCodeActionLspSuite.scala | 7 +- .../metals/CompilerConfigurationSuite.scala | 22 ++++++ .../src/test/scala/tests/InfraSuite.scala | 5 +- .../scala/tests/UserConfigurationSuite.scala | 29 -------- .../RemoveRedundantCastLspSuite.scala | 7 +- .../SuppressWarningsLspSuite.scala | 71 +++---------------- 16 files changed, 87 insertions(+), 183 deletions(-) create mode 100644 tests/unit/src/test/scala/scala/meta/internal/metals/CompilerConfigurationSuite.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala b/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala index 34e01968e64..06937e06caa 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala @@ -156,8 +156,11 @@ final class BuildTargets private ( def javaTarget(id: BuildTargetIdentifier): Option[JavaTarget] = data.fromOptions(_.javaTarget(id)) - def jvmTarget(id: BuildTargetIdentifier): Option[JvmTarget] = - data.fromOptions(_.jvmTarget(id)) + def jvmTarget( + id: BuildTargetIdentifier, + scalaPreferred: Boolean = true, + ): Option[JvmTarget] = + data.fromOptions(_.jvmTarget(id, scalaPreferred)) def fullClasspath( id: BuildTargetIdentifier, diff --git a/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala index a619be222a0..b06b5d64e87 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala @@ -439,12 +439,12 @@ class CompilerConfiguration( } case class JavaLazyCompiler( - target: JvmTarget, + javaTarget: JvmTarget, search: SymbolSearch, completionItemPriority: CompletionItemPriority, ) extends LazyCompiler { - def buildTargetId: BuildTargetIdentifier = target.id + def buildTargetId: BuildTargetIdentifier = javaTarget.id protected def newCompiler( classpath: Seq[Path], @@ -454,13 +454,11 @@ class CompilerConfiguration( val shouldUseOpts = featureFlags .readBoolean(FeatureFlag.JAVAC_OPTIONS) .orElse(false) - val buildOptions = target match { - case j: JavaTarget if shouldUseOpts => j.options + val options = javaTarget match { + case j: JavaTarget => + CompilerConfiguration.javaPcOptions(j.options, shouldUseOpts) case _ => Nil } - val lintOptions = - userConfig().javaLintOptions.values.map(option => s"-Xlint:$option") - val options = buildOptions ++ lintOptions configure(pc, search, completionItemPriority) .newInstance( buildTargetId.getUri(), @@ -681,3 +679,15 @@ class CompilerConfiguration( Nil } } + +object CompilerConfiguration { + private[metals] def javaPcOptions( + options: List[String], + includeAll: Boolean, + ): List[String] = + if (includeAll) options + else + options.filter(option => + option == "-Xlint" || option.startsWith("-Xlint:") + ) +} diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala index e7af1466f16..29c6d39817d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala @@ -1795,23 +1795,20 @@ class Compilers( private def loadJavaCompiler( targetId: BuildTargetIdentifier ): Option[PresentationCompiler] = { - buildTargets - .javaTarget(targetId) - .orElse(buildTargets.jvmTarget(targetId)) - .map { javaTarget => - jcache - .computeIfAbsent( - PresentationCompilerKey.JavaBuildTarget(targetId), - { _ => - workDoneProgress.trackBlocking( - s"${config.icons().sync}Loading presentation compiler" - ) { - JavaLazyCompiler(javaTarget, search, completionItemPriority()) - } - }, - ) - .await - } + buildTargets.jvmTarget(targetId, scalaPreferred = false).map { javaTarget => + jcache + .computeIfAbsent( + PresentationCompilerKey.JavaBuildTarget(targetId), + { _ => + workDoneProgress.trackBlocking( + s"${config.icons().sync}Loading presentation compiler" + ) { + JavaLazyCompiler(javaTarget, search, completionItemPriority()) + } + }, + ) + .await + } } private def protoCompiler: PresentationCompiler = { diff --git a/metals/src/main/scala/scala/meta/internal/metals/ModuleStatus.scala b/metals/src/main/scala/scala/meta/internal/metals/ModuleStatus.scala index c440df855ad..fe793c67334 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ModuleStatus.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ModuleStatus.scala @@ -54,7 +54,7 @@ class ModuleStatus( case Some(buildTarget) => handler.diagnostics .upstreamTargetsWithCompilationErrors(buildTarget.id) - .flatMap(handler.buildTargets.jvmTarget) + .flatMap(handler.buildTargets.jvmTarget(_)) .headOption match { case Some(buildTargetWithError) => client.metalsStatus( diff --git a/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala b/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala index 0fa213b26f0..763d4aaea30 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala @@ -147,8 +147,12 @@ final class TargetData() { scalaTargetInfo.get(id) def javaTarget(id: BuildTargetIdentifier): Option[JavaTarget] = javaTargetInfo.get(id) - def jvmTarget(id: BuildTargetIdentifier): Option[JvmTarget] = - scalaTarget(id).orElse(javaTarget(id)) + def jvmTarget( + id: BuildTargetIdentifier, + scalaPreferred: Boolean = true, + ): Option[JvmTarget] = + if (scalaPreferred) scalaTarget(id).orElse(javaTarget(id)) + else javaTarget(id).orElse(scalaTarget(id)) def jvmTargets(id: BuildTargetIdentifier): List[JvmTarget] = List(scalaTarget(id), javaTarget(id)).flatten diff --git a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala index 5153de2cacb..37721afbeb0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -44,37 +44,6 @@ case class JavaFormatConfig( eclipseFormatProfile: Option[String], ) -case class JavaLintOptions(values: List[String]) - -object JavaLintOptions { - val allValues: List[String] = List( - "cast", "deprecation", "dep-ann", "divzero", "empty", "fallthrough", - "finally", "lossy-conversions", "overloads", "overrides", "rawtypes", - "removal", "serial", "static", "strictfp", "synchronization", "text-blocks", - "this-escape", "try", "unchecked", "varargs", - ) - - val default: JavaLintOptions = JavaLintOptions(Nil) - - private val allowed: Set[String] = - (allValues :+ "preview").toSet - - def fromConfig( - values: Option[List[String]] - ): Either[String, JavaLintOptions] = - values match { - case None => Right(default) - case Some(values) => - values.find(value => !allowed(value)) match { - case Some(invalid) => - Left( - s"invalid config value '$invalid' for javaLintOptions. Valid values are ${allowed.toSeq.sorted.map(value => s""""$value"""").mkString(", ")}" - ) - case None => Right(JavaLintOptions(values)) - } - } -} - /** * Configuration that the user can override via workspace/didChangeConfiguration. */ @@ -113,7 +82,6 @@ case class UserConfiguration( javaFormatter: Option[JavaFormatterConfig] = None, scalafixRulesDependencies: List[String] = Nil, scalafixLintEnabled: Boolean = false, - javaLintOptions: JavaLintOptions = JavaLintOptions.default, customProjectRoot: Option[String] = None, verboseCompilation: Boolean = false, automaticImportBuild: AutoImportBuildKind = AutoImportBuildKind.Off, @@ -252,7 +220,6 @@ case class UserConfiguration( Some(scalafixRulesDependencies), ), Some(("scalafixLintEnabled", scalafixLintEnabled)), - listField("javaLintOptions", Some(javaLintOptions.values)), optStringField("customProjectRoot", customProjectRoot), Some(("verboseCompilation", verboseCompilation)), Some( @@ -554,16 +521,6 @@ object UserConfiguration { |""".stripMargin, isBoolean = true, ), - UserConfigurationOption( - "java-lint-options", - JavaLintOptions.default.values.mkString("[", ",", "]"), - """["deprecation", "unchecked"]""", - "Java lint diagnostics", - """Javac `-Xlint` options passed to the Java presentation compiler. - |Use an empty array to disable Java lint diagnostics. - |""".stripMargin, - isArray = true, - ), UserConfigurationOption( "excluded-packages", "[]", @@ -1351,10 +1308,6 @@ object UserConfiguration { val scalafixLintEnabled = getBooleanKey("scalafix-lint-enabled").getOrElse(false) - val javaLintOptions = getParsedArrayKey( - "java-lint-options", - JavaLintOptions.fromConfig, - ).getOrElse(JavaLintOptions.default) val customProjectRoot = getStringKey("custom-project-root") val verboseCompilation = @@ -1553,7 +1506,6 @@ object UserConfiguration { javaFormatter, scalafixRulesDependencies, scalafixLintEnabled, - javaLintOptions, customProjectRoot, verboseCompilation, autoImportBuilds, diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/server/DebugeeParamsCreator.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/server/DebugeeParamsCreator.scala index 71c9696914e..bc2e8bbc095 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/server/DebugeeParamsCreator.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/server/DebugeeParamsCreator.scala @@ -56,7 +56,7 @@ class DebugeeParamsCreator(buildTargetClasses: BuildTargetClasses) { val modules = buildTargets .buildTargetTransitiveDependencies(id) - .flatMap(buildTargets.jvmTarget) + .flatMap(buildTargets.jvmTarget(_)) .map(createModule(_)) .toSeq diff --git a/metals/src/main/scala/scala/meta/internal/metals/mcp/MetalsMcpTools.scala b/metals/src/main/scala/scala/meta/internal/metals/mcp/MetalsMcpTools.scala index a0a5367fc38..14c6cab89da 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/mcp/MetalsMcpTools.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/mcp/MetalsMcpTools.scala @@ -378,7 +378,7 @@ trait MetalsMcpTools extends Cancelable { .upstreamTargetsWithCompilationErrors(buildTarget) if (upstreamModules.nonEmpty) { val modules = upstreamModules - .flatMap(buildTargets.jvmTarget) + .flatMap(buildTargets.jvmTarget(_)) .map(_.displayName) .mkString("\n", "\n", "") Some( diff --git a/tests/unit/src/main/scala/tests/BaseLspSuite.scala b/tests/unit/src/main/scala/tests/BaseLspSuite.scala index bab203e17d9..44e15b2b283 100644 --- a/tests/unit/src/main/scala/tests/BaseLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseLspSuite.scala @@ -20,7 +20,6 @@ import scala.meta.internal.metals.Debug import scala.meta.internal.metals.ExecuteClientCommandConfig import scala.meta.internal.metals.Icons import scala.meta.internal.metals.InitializationOptions -import scala.meta.internal.metals.JavaLintOptions import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MetalsServerConfig import scala.meta.internal.metals.MtagsResolver @@ -52,7 +51,6 @@ abstract class BaseLspSuite( buildChangedAction = BuildChangedAction.prompt, fallbackScalaVersion = Some(BuildInfo.scalaVersion), presentationCompilerDiagnostics = false, - javaLintOptions = JavaLintOptions(Nil), definitionIndexStrategy = Configs.DefinitionIndexStrategy.classpath, // Legacy settings that are enabled for tests only. We should eventually diff --git a/tests/unit/src/main/scala/tests/MbtJsonBuilder.scala b/tests/unit/src/main/scala/tests/MbtJsonBuilder.scala index 37659450e4e..1cbf2c332f2 100644 --- a/tests/unit/src/main/scala/tests/MbtJsonBuilder.scala +++ b/tests/unit/src/main/scala/tests/MbtJsonBuilder.scala @@ -80,12 +80,13 @@ case class MbtJsonBuilder( sources: List[String], dependsOn: List[String] = Nil, customScalaVersion: Option[String] = None, + javacOptions: List[String] = Nil, ): MbtJsonBuilder = { val distinctDeps = dependencyModules.distinctBy(_.id).reverse val namespace = MbtNamespace( sources = sources.asJava, scalacOptions = null, - javacOptions = null, + javacOptions = if (javacOptions.isEmpty) null else javacOptions.asJava, dependencyModules = distinctDeps.map(_.id).asJava, scalaVersion = customScalaVersion.getOrElse(scalaVersion), javaHome = null, diff --git a/tests/unit/src/main/scala/tests/codeactions/BaseCodeActionLspSuite.scala b/tests/unit/src/main/scala/tests/codeactions/BaseCodeActionLspSuite.scala index 5e43427abda..7230007306a 100644 --- a/tests/unit/src/main/scala/tests/codeactions/BaseCodeActionLspSuite.scala +++ b/tests/unit/src/main/scala/tests/codeactions/BaseCodeActionLspSuite.scala @@ -22,6 +22,7 @@ abstract class BaseCodeActionLspSuite( ) extends BaseLspSuite(suiteName, initializer) { protected val scalaVersion: String = V.scala213 + protected def javacOptions: List[String] = Nil /** * When set, `check` waits for a matching diagnostics publication before @@ -121,7 +122,11 @@ abstract class BaseCodeActionLspSuite( val layout = overrideLayout.getOrElse { if (useMbtLayout) { val mbtJson = new MbtJsonBuilder(scalaVersion) - .addNamespace("a", List("a/src/main/java/**", "a/src/main/scala/**")) + .addNamespace( + "a", + List("a/src/main/java/**", "a/src/main/scala/**"), + javacOptions = javacOptions, + ) .build() s"""/.metals/mbt.json |$mbtJson diff --git a/tests/unit/src/test/scala/scala/meta/internal/metals/CompilerConfigurationSuite.scala b/tests/unit/src/test/scala/scala/meta/internal/metals/CompilerConfigurationSuite.scala new file mode 100644 index 00000000000..104223f49ff --- /dev/null +++ b/tests/unit/src/test/scala/scala/meta/internal/metals/CompilerConfigurationSuite.scala @@ -0,0 +1,22 @@ +package scala.meta.internal.metals + +import tests.BaseSuite + +class CompilerConfigurationSuite extends BaseSuite { + + test("java-pc-options") { + val options = List( + "--release", "21", "-Xlint", "-Xlint:deprecation,unchecked", "-Xlintfile", + "-Werror", + ) + + assertEquals( + CompilerConfiguration.javaPcOptions(options, includeAll = false), + List("-Xlint", "-Xlint:deprecation,unchecked"), + ) + assertEquals( + CompilerConfiguration.javaPcOptions(options, includeAll = true), + options, + ) + } +} diff --git a/tests/unit/src/test/scala/tests/InfraSuite.scala b/tests/unit/src/test/scala/tests/InfraSuite.scala index 3d2bd645e72..32c69975fe2 100644 --- a/tests/unit/src/test/scala/tests/InfraSuite.scala +++ b/tests/unit/src/test/scala/tests/InfraSuite.scala @@ -17,8 +17,6 @@ object TestingInfra { val events: mutable.ArrayBuffer[Event] = mutable.ArrayBuffer[Event]() val testFlags: mutable.ArrayBuffer[FeatureFlag] = mutable.ArrayBuffer[FeatureFlag]() - - val enabledFlags: mutable.Set[FeatureFlag] = mutable.Set[FeatureFlag]() } class TestingMonitoringClient extends MonitoringClient { override def recordUsage(metric: Metric): Unit = @@ -31,8 +29,7 @@ class TestingMonitoringClient extends MonitoringClient { class TestingFeatureFlagProvider extends FeatureFlagProvider { override def readBoolean(flag: FeatureFlag): Optional[java.lang.Boolean] = { TestingInfra.testFlags.append(flag) - if (TestingInfra.enabledFlags.contains(flag)) Optional.of(true) - else Optional.empty() + Optional.empty() } override def readInt(flag: FeatureFlag, default: Integer): Optional[Integer] = diff --git a/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala b/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala index 71102f7d912..7a32c01c173 100644 --- a/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala +++ b/tests/unit/src/test/scala/tests/UserConfigurationSuite.scala @@ -20,7 +20,6 @@ import scala.meta.internal.metals.InlayHintsOption import scala.meta.internal.metals.InlayHintsOptions import scala.meta.internal.metals.JavaFormatConfig import scala.meta.internal.metals.JavaFormatterConfig -import scala.meta.internal.metals.JavaLintOptions import scala.meta.internal.metals.JsonParser._ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MetalsServerConfig @@ -190,31 +189,6 @@ class UserConfigurationSuite extends BaseSuite { """.stripMargin, ) - checkError( - "java-lint-options-invalid", - """ - |{ - | "java-lint-options": ["deprecation", "typo"] - |} - """.stripMargin, - "json error: invalid config value 'typo' for javaLintOptions. Valid values are " + - "\"cast\", \"dep-ann\", \"deprecation\", \"divzero\", \"empty\", \"fallthrough\", \"finally\", " + - "\"lossy-conversions\", \"overloads\", \"overrides\", \"preview\", \"rawtypes\", \"removal\", " + - "\"serial\", \"static\", \"strictfp\", \"synchronization\", \"text-blocks\", \"this-escape\", " + - "\"try\", \"unchecked\", \"varargs\"", - ) - - checkOK( - "java-lint-options-empty", - """ - |{ - | "java-lint-options": [] - |} - """.stripMargin, - ) { obtained => - assertEquals(obtained.javaLintOptions, JavaLintOptions(Nil)) - } - checkError( "symbol-prefixes", """ @@ -424,7 +398,6 @@ class UserConfigurationSuite extends BaseSuite { javacServicesOverrides = JavacServicesOverrides.default.copy(names = false), scalafixRulesDependencies = List("rule1", "rule2"), - javaLintOptions = JavaLintOptions(Nil), customProjectRoot = Some("customs"), workspaceSymbolProvider = WorkspaceSymbolProviderConfig("mbt"), javaTurbineRecompileDelay = TurbineRecompileDelayConfig.testing, @@ -505,7 +478,6 @@ class UserConfigurationSuite extends BaseSuite { "rule2" ], "scalafixLintEnabled": false, - "javaLintOptions": [], "customProjectRoot": "customs", "verboseCompilation": true, "autoImportBuilds": "all", @@ -591,7 +563,6 @@ class UserConfigurationSuite extends BaseSuite { |shim-globs string `{}`. Shim file globs |scalafix-rules-dependencies array [] Scalafix rules dependencies |scalafix-lint-enabled boolean false Enable Scalafix lint diagnostics - |java-lint-options array [] Java lint diagnostics |excluded-packages array [] Excluded Packages |bloop-sbt-already-installed boolean false Don't generate Bloop plugin file for sbt |bloop-version string $bloopVersionPadded Version of Bloop diff --git a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala index 796e8b4ea6d..a9e0ba26073 100644 --- a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala @@ -1,7 +1,5 @@ package tests.codeactions -import scala.meta.internal.metals.JavaLintOptions -import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.UserConfiguration import scala.meta.internal.metals.codeactions.RemoveRedundantCast @@ -17,10 +15,11 @@ class RemoveRedundantCastLspSuite override def userConfig: UserConfiguration = super.userConfig.copy( - presentationCompilerDiagnostics = true, - javaLintOptions = JavaLintOptions(List("cast")), + presentationCompilerDiagnostics = true ) + override protected def javacOptions: List[String] = List("-Xlint:cast") + override protected def toPath( fileName: String, isSource: Boolean = true, diff --git a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala index 11c1b82a773..47f41ae9847 100644 --- a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala @@ -1,12 +1,9 @@ package tests.codeactions -import scala.meta.internal.metals.JavaLintOptions -import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.UserConfiguration import scala.meta.internal.metals.codeactions.SuppressWarnings import munit.Location -import org.eclipse.{lsp4j => l} import tests.MbtTestInitializer class SuppressWarningsLspSuite @@ -18,8 +15,7 @@ class SuppressWarningsLspSuite override def userConfig: UserConfiguration = super.userConfig.copy( - presentationCompilerDiagnostics = true, - javaLintOptions = JavaLintOptions(JavaLintOptions.allValues), + presentationCompilerDiagnostics = true ) override protected def toPath( @@ -527,31 +523,6 @@ class SuppressWarningsLspSuite |""".stripMargin, ) - checkSuppressWarnings( - "lossy-conversions-method", - """|package a; - | - |public class Example { - |<< public void run() { - | short value = 0; - | value += 100000; - | }>> - |} - |""".stripMargin, - "compiler.warn.possible.loss.of.precision", - "lossy-conversions", - """|package a; - | - |public class Example { - | @SuppressWarnings("lossy-conversions") - | public void run() { - | short value = 0; - | value += 100000; - | } - |} - |""".stripMargin, - ) - checkSuppressWarnings( "overloads-method", """|package a; @@ -560,11 +531,11 @@ class SuppressWarningsLspSuite |import java.util.function.Function; | |public class Example { - | public void run(Consumer consumer) { - | } - | - |<< public void run(Function function) { + |<< public void run(Consumer consumer) { | }>> + | + | public void run(Function function) { + | } |} |""".stripMargin, "compiler.warn.potentially.ambiguous.overload", @@ -575,10 +546,10 @@ class SuppressWarningsLspSuite |import java.util.function.Function; | |public class Example { + | @SuppressWarnings("overloads") | public void run(Consumer consumer) { | } | - | @SuppressWarnings("overloads") | public void run(Function function) { | } |} @@ -772,33 +743,6 @@ class SuppressWarningsLspSuite |""".stripMargin, ) - checkSuppressWarnings( - "this-escape-constructor", - """|package a; - | - |public class Example { - |<< public Example() { - | overridable(); - | }>> - | - | public void overridable() {} - |} - |""".stripMargin, - "compiler.warn.possible.this.escape", - "this-escape", - """|package a; - | - |public class Example { - | @SuppressWarnings("this-escape") - | public Example() { - | overridable(); - | } - | - | public void overridable() {} - |} - |""".stripMargin, - ) - checkSuppressWarnings( "try-method", """|package a; @@ -875,7 +819,8 @@ class SuppressWarningsLspSuite |{ | "namespaces": { | "a": { - | "sources": ["a/src/main/java/**", "a/src/main/scala/**"] + | "sources": ["a/src/main/java/**", "a/src/main/scala/**"], + | "javacOptions": ["-Xlint:all"] | } | } |} From 639391149fabe52849ba4fbb3b0a9d40baa81995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Tue, 11 Aug 2026 11:38:48 +0200 Subject: [PATCH 6/9] address review --- .../metals/CompilerConfiguration.scala | 17 +-- .../internal/metals/MetalsEnrichments.scala | 19 --- .../metals/codeactions/SuppressWarnings.scala | 120 +++++------------- .../meta/internal/jpc/JavaDiagnostics.scala | 13 +- .../metals/CompilerConfigurationSuite.scala | 22 ---- .../RemoveRedundantCastLspSuite.scala | 43 ++++--- .../SuppressWarningsLspSuite.scala | 37 +----- 7 files changed, 74 insertions(+), 197 deletions(-) delete mode 100644 tests/unit/src/test/scala/scala/meta/internal/metals/CompilerConfigurationSuite.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala index b06b5d64e87..438fadaaec0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala @@ -455,8 +455,11 @@ class CompilerConfiguration( .readBoolean(FeatureFlag.JAVAC_OPTIONS) .orElse(false) val options = javaTarget match { + case j: JavaTarget if shouldUseOpts => j.options case j: JavaTarget => - CompilerConfiguration.javaPcOptions(j.options, shouldUseOpts) + j.options.filter(option => + option == "-Xlint" || option.startsWith("-Xlint:") + ) case _ => Nil } configure(pc, search, completionItemPriority) @@ -679,15 +682,3 @@ class CompilerConfiguration( Nil } } - -object CompilerConfiguration { - private[metals] def javaPcOptions( - options: List[String], - includeAll: Boolean, - ): List[String] = - if (includeAll) options - else - options.filter(option => - option == "-Xlint" || option.startsWith("-Xlint:") - ) -} diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala index 8caa9596f27..a0ff0444bcd 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala @@ -847,25 +847,6 @@ object MetalsEnrichments new l.Position(lineCount, index - lineStartIdx) } - def lspPositionToIndex(position: l.Position): Int = { - var line = 0 - var character = 0 - var offset = 0 - while ( - offset < value.length && - (line < position.getLine() || character < position.getCharacter()) - ) { - if (value.charAt(offset) == '\n') { - line += 1 - character = 0 - } else { - character += 1 - } - offset += 1 - } - offset - } - def replaceAllBetween(start: String, end: String)( replacement: String ): String = diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala index 890fd63b53a..45c222897a4 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala @@ -3,6 +3,7 @@ package scala.meta.internal.metals.codeactions import scala.concurrent.ExecutionContext import scala.concurrent.Future +import scala.meta.inputs.Input import scala.meta.internal.metals.Buffers import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.parsing.JavaAnnotation @@ -12,6 +13,7 @@ import scala.meta.internal.parsing.JavaTrees import scala.meta.io.AbsolutePath import scala.meta.pc.CancelToken +import com.google.gson.JsonPrimitive import org.eclipse.{lsp4j => l} class SuppressWarnings( @@ -56,26 +58,26 @@ class SuppressWarnings( text: String, path: AbsolutePath, position: l.Position, - ): Option[SuppressTarget] = { - val positionOffset = text.lspPositionToIndex(position) - javaTrees - .findEnclosingJavaVariable(path, position, onNameOnly = false) - .filter(variable => - variable.isStandaloneDeclaration && - positionOffset <= variable.nameRange.endOffset - ) - .map(variable => SuppressTarget(variable, variable.nameRange)) - .orElse( - javaTrees - .findEnclosingJavaMethod(path, position) - .map(method => SuppressTarget(method, method.nameRange)) - ) - .orElse( - javaTrees - .findEnclosingJavaClass(path, position) - .map(cls => SuppressTarget(cls, cls.nameRange)) - ) - } + ): Option[SuppressTarget] = + position.toMeta(Input.String(text)).map(_.start).flatMap { positionOffset => + javaTrees + .findEnclosingJavaVariable(path, position, onNameOnly = false) + .filter(variable => + variable.isStandaloneDeclaration && + positionOffset <= variable.nameRange.endOffset + ) + .map(variable => SuppressTarget(variable, variable.nameRange)) + .orElse( + javaTrees + .findEnclosingJavaMethod(path, position) + .map(method => SuppressTarget(method, method.nameRange)) + ) + .orElse( + javaTrees + .findEnclosingJavaClass(path, position) + .map(cls => SuppressTarget(cls, cls.nameRange)) + ) + } private def suppressEdit( text: String, @@ -135,77 +137,15 @@ object SuppressWarnings { def title(warningName: String): String = s"""Add @SuppressWarnings("$warningName")""" - private val DiagnosticSubstrings: List[(String, String)] = List( - "missing.deprecated.annotation" -> "dep-ann", - "deprecated.for.removal" -> "removal", - "requires-transitive-automatic" -> "requires-transitive-automatic", - "requires-automatic" -> "requires-automatic", - "output-file-clash" -> "output-file-clash", - "missing-explicit-ctor" -> "missing-explicit-ctor", - "loss.of.precision" -> "lossy-conversions", - "fall-through" -> "fallthrough", - "ambiguous.overload" -> "overloads", - "override.equals" -> "overrides", - "trailing.white.space" -> "text-blocks", - "this.escape" -> "this-escape", - "synchronize" -> "synchronization", - "auxiliaryclass" -> "auxiliaryclass", - "raw.class" -> "rawtypes", - "div.zero" -> "divzero", - "serialversionuid" -> "serial", - "svuid" -> "serial", - "classfile" -> "classfile", - "varargs" -> "varargs", - "unchecked" -> "unchecked", - "deprecated" -> "deprecation", - "cast" -> "cast", - "divzero" -> "divzero", - "empty" -> "empty", - "exports" -> "exports", - "fallthrough" -> "fallthrough", - "finally" -> "finally", - "lossy-conversions" -> "lossy-conversions", - "module" -> "module", - "opens" -> "opens", - "options" -> "options", - "overloads" -> "overloads", - "overrides" -> "overrides", - "path" -> "path", - "preview" -> "preview", - "processing" -> "processing", - "rawtypes" -> "rawtypes", - "removal" -> "removal", - "serial" -> "serial", - "static" -> "static", - "strictfp" -> "strictfp", - "synchronization" -> "synchronization", - "text-blocks" -> "text-blocks", - "this-escape" -> "this-escape", - "try" -> "try", - ) - private def warningName(diagnostic: l.Diagnostic): Option[String] = - if (diagnostic.getSource() == "javac") { - val code = Option(diagnostic.getCode()) - .collect { case code if code.isLeft() => code.getLeft() } - .getOrElse("") - .toLowerCase() - val isWarning = code.startsWith("compiler.warn.") || - diagnostic.getSeverity() == l.DiagnosticSeverity.Warning - if (isWarning) { - val message = - Option(diagnostic.getMessage()) - .map(_.toString()) - .getOrElse("") - .toLowerCase() - warningNameFrom(code).orElse(warningNameFrom(message)) - } else None - } else None - - private def warningNameFrom(text: String): Option[String] = - DiagnosticSubstrings.collectFirst { - case (substr, name) if text.contains(substr) => name - } + Option + .when(diagnostic.getSource() == "javac")(diagnostic.getData()) + .flatMap { + case value: String => Some(value) + case value: JsonPrimitive if value.isString() => + Some(value.getAsString()) + case _ => None + } private def isZeroRange(range: l.Range): Boolean = range.getStart().getLine() == 0 && diff --git a/mtags-java/src/main/scala/scala/meta/internal/jpc/JavaDiagnostics.scala b/mtags-java/src/main/scala/scala/meta/internal/jpc/JavaDiagnostics.scala index b8f12fb97c9..538121590d3 100644 --- a/mtags-java/src/main/scala/scala/meta/internal/jpc/JavaDiagnostics.scala +++ b/mtags-java/src/main/scala/scala/meta/internal/jpc/JavaDiagnostics.scala @@ -9,6 +9,8 @@ import javax.tools.Diagnostic.Kind.WARNING import javax.tools.JavaFileObject import com.sun.source.tree.LineMap +import com.sun.tools.javac.api.ClientCodeWrapper +import com.sun.tools.javac.util.JCDiagnostic import org.eclipse.{lsp4j => l} object JavaDiagnostics { @@ -28,7 +30,7 @@ object JavaDiagnostics { d.getEndPosition(), text ) - new l.Diagnostic( + val diagnostic = new l.Diagnostic( range, d.getMessage(null), d.getKind() match { @@ -45,5 +47,14 @@ object JavaDiagnostics { "javac", d.getCode() ) + val javacDiagnostic = d match { + case value: JCDiagnostic => Some(value) + case value: ClientCodeWrapper#DiagnosticSourceUnwrapper => Some(value.d) + case _ => None + } + javacDiagnostic + .filter(_.hasLintCategory()) + .foreach(value => diagnostic.setData(value.getLintCategory().option)) + diagnostic } } diff --git a/tests/unit/src/test/scala/scala/meta/internal/metals/CompilerConfigurationSuite.scala b/tests/unit/src/test/scala/scala/meta/internal/metals/CompilerConfigurationSuite.scala deleted file mode 100644 index 104223f49ff..00000000000 --- a/tests/unit/src/test/scala/scala/meta/internal/metals/CompilerConfigurationSuite.scala +++ /dev/null @@ -1,22 +0,0 @@ -package scala.meta.internal.metals - -import tests.BaseSuite - -class CompilerConfigurationSuite extends BaseSuite { - - test("java-pc-options") { - val options = List( - "--release", "21", "-Xlint", "-Xlint:deprecation,unchecked", "-Xlintfile", - "-Werror", - ) - - assertEquals( - CompilerConfiguration.javaPcOptions(options, includeAll = false), - List("-Xlint", "-Xlint:deprecation,unchecked"), - ) - assertEquals( - CompilerConfiguration.javaPcOptions(options, includeAll = true), - options, - ) - } -} diff --git a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala index a9e0ba26073..6aad64435af 100644 --- a/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/RemoveRedundantCastLspSuite.scala @@ -30,10 +30,6 @@ class RemoveRedundantCastLspSuite private val onlyRemoveRedundantCast: l.CodeAction => Boolean = _.getTitle() == RemoveRedundantCast.title - private def title: String = - s"""|${RemoveRedundantCast.title} - |""".stripMargin - check( "basic", """|package a; @@ -44,7 +40,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -67,7 +64,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -90,7 +88,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -115,7 +114,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |import java.util.List; @@ -140,7 +140,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -163,7 +164,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -186,7 +188,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -209,7 +212,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -233,7 +237,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -258,7 +263,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -282,7 +288,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |public class Example { @@ -308,7 +315,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |import java.util.List; @@ -339,7 +347,8 @@ class RemoveRedundantCastLspSuite | } |} |""".stripMargin, - title, + s"""|${RemoveRedundantCast.title} + |""".stripMargin, """|package a; | |import java.util.List; diff --git a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala index 47f41ae9847..ea7301cfb93 100644 --- a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala @@ -523,39 +523,6 @@ class SuppressWarningsLspSuite |""".stripMargin, ) - checkSuppressWarnings( - "overloads-method", - """|package a; - | - |import java.util.function.Consumer; - |import java.util.function.Function; - | - |public class Example { - |<< public void run(Consumer consumer) { - | }>> - | - | public void run(Function function) { - | } - |} - |""".stripMargin, - "compiler.warn.potentially.ambiguous.overload", - "overloads", - """|package a; - | - |import java.util.function.Consumer; - |import java.util.function.Function; - | - |public class Example { - | @SuppressWarnings("overloads") - | public void run(Consumer consumer) { - | } - | - | public void run(Function function) { - | } - |} - |""".stripMargin, - ) - checkSuppressWarnings( "overrides-class", """|package a; @@ -786,13 +753,13 @@ class SuppressWarningsLspSuite |} |""".stripMargin, "compiler.warn.unchecked.varargs.non.reifiable.type", - "varargs", + "unchecked", """|package a; | |import java.util.List; | |public class Example { - | @SuppressWarnings("varargs") + | @SuppressWarnings("unchecked") | public void run(List... lists) { | } |} From 67465e5a0c517479fa1635074d12264fc4c18989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Tue, 11 Aug 2026 15:46:18 +0200 Subject: [PATCH 7/9] address review --- .../metals/codeactions/SuppressWarnings.scala | 28 ++++--- .../SuppressWarningsLspSuite.scala | 76 ++++++++++++++++++- 2 files changed, 89 insertions(+), 15 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala index 45c222897a4..ab442aa2c6b 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala @@ -33,7 +33,7 @@ class SuppressWarnings( val path = params.getTextDocument().getUri().toAbsolutePath val range = params.getRange() - val actions = for { + val actionsWithKeys = for { text <- buffers.get(path).orElse(path.readTextOpt).toSeq diagnostic <- params.getContext().getDiagnostics().asScala.toSeq warningName <- warningName(diagnostic).toSeq @@ -45,13 +45,16 @@ class SuppressWarnings( else range.getStart() member <- enclosingMember(text, path, position).toSeq edit <- suppressEdit(text, path, member, warningName).toSeq - } yield CodeActionBuilder.build( - title(warningName), - kind, - diagnostics = List(diagnostic), - changes = Seq(path -> Seq(edit)), + } yield ( + (member.nameRange.startOffset, warningName), + CodeActionBuilder.build( + title(warningName), + kind, + diagnostics = List(diagnostic), + changes = Seq(path -> Seq(edit)), + ), ) - actions.distinctBy(_.getTitle()) + actionsWithKeys.distinctBy(_._1).map(_._2) } private def enclosingMember( @@ -182,11 +185,11 @@ object SuppressWarnings { case _ => ("", trimmed) } val isArray = value.startsWith("{") && value.endsWith("}") + val arrayContents = + if (isArray) value.substring(1, value.length() - 1).trim() + else "" val (range, newText) = - if ( - isArray && - value.substring(1, value.length() - 1).trim().isEmpty() - ) { + if (isArray && arrayContents.isEmpty()) { ( new l.Range( text.indexToLspPosition(insideStart), @@ -196,12 +199,13 @@ object SuppressWarnings { ) } else if (isArray) { val closeBrace = insideEnd - inside.reverse.indexOf('}') - 1 + val separator = if (arrayContents.endsWith(",")) " " else ", " ( new l.Range( text.indexToLspPosition(closeBrace), text.indexToLspPosition(closeBrace), ), - s""", "$warningName"""", + s"""$separator"$warningName"""", ) } else { ( diff --git a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala index ea7301cfb93..bae955b7ae1 100644 --- a/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala +++ b/tests/unit/src/test/scala/tests/codeactions/SuppressWarningsLspSuite.scala @@ -247,6 +247,73 @@ class SuppressWarningsLspSuite |""".stripMargin, ) + checkSuppressWarnings( + "append-existing-array-trailing-comma", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings({"unchecked",}) + |<< public List names() { + | return new ArrayList(); + | }>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.ArrayList; + |import java.util.List; + | + |public class Example { + | @SuppressWarnings({"unchecked", "rawtypes"}) + | public List names() { + | return new ArrayList(); + | } + |} + |""".stripMargin, + ) + + checkSuppressWarnings( + "same-warning-multiple-declarations", + """|package a; + | + |import java.util.List; + | + |public class Example { + |<< public List first() { + | return List.of(); + | } + | + | public List second() { + | return List.of(); + | }>> + |} + |""".stripMargin, + "compiler.warn.raw.class.use", + "rawtypes", + """|package a; + | + |import java.util.List; + | + |public class Example { + | public List first() { + | return List.of(); + | } + | + | @SuppressWarnings("rawtypes") + | public List second() { + | return List.of(); + | } + |} + |""".stripMargin, + expectedActionCount = 2, + selectedActionIndex = 1, + ) + checkSuppressWarnings( "append-existing-empty-array", """|package a; @@ -773,6 +840,8 @@ class SuppressWarningsLspSuite warningName: String, expected: String, extraLayout: String = "", + expectedActionCount: Int = 1, + selectedActionIndex: Int = 0, )(implicit loc: Location): Unit = test(name) { val fileName = "Example.java" @@ -810,12 +879,13 @@ class SuppressWarningsLspSuite codeActions <- server.assertCodeAction( path, original, - s"""|${SuppressWarnings.title(warningName)} - |""".stripMargin, + List + .fill(expectedActionCount)(SuppressWarnings.title(warningName)) + .mkString("\n"), kind = Nil, filterAction = _.getTitle() == SuppressWarnings.title(warningName), ) - _ <- client.applyCodeAction(0, codeActions, server) + _ <- client.applyCodeAction(selectedActionIndex, codeActions, server) _ <- server.didChange(path) { _ => server.bufferContents(path) } From 6d545b79fa434f88f4ac81fc3cb7938a68082013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Wed, 12 Aug 2026 01:06:54 +0200 Subject: [PATCH 8/9] cleanup --- .../metals/codeactions/SuppressWarnings.scala | 66 ++++++++----------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala index ab442aa2c6b..a3f2aa47a57 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/SuppressWarnings.scala @@ -3,7 +3,6 @@ package scala.meta.internal.metals.codeactions import scala.concurrent.ExecutionContext import scala.concurrent.Future -import scala.meta.inputs.Input import scala.meta.internal.metals.Buffers import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.parsing.JavaAnnotation @@ -33,7 +32,7 @@ class SuppressWarnings( val path = params.getTextDocument().getUri().toAbsolutePath val range = params.getRange() - val actionsWithKeys = for { + val actions = for { text <- buffers.get(path).orElse(path.readTextOpt).toSeq diagnostic <- params.getContext().getDiagnostics().asScala.toSeq warningName <- warningName(diagnostic).toSeq @@ -43,44 +42,38 @@ class SuppressWarnings( if (range.overlapsWith(diagnostic.getRange())) diagnostic.getRange().getStart() else range.getStart() - member <- enclosingMember(text, path, position).toSeq + member <- enclosingMember(path, position).toSeq edit <- suppressEdit(text, path, member, warningName).toSeq - } yield ( - (member.nameRange.startOffset, warningName), - CodeActionBuilder.build( - title(warningName), - kind, - diagnostics = List(diagnostic), - changes = Seq(path -> Seq(edit)), - ), + } yield CodeActionBuilder.build( + title(warningName), + kind, + diagnostics = List(diagnostic), + changes = Seq(path -> Seq(edit)), ) - actionsWithKeys.distinctBy(_._1).map(_._2) + actions.distinctBy(_.getEdit()) } private def enclosingMember( - text: String, path: AbsolutePath, position: l.Position, ): Option[SuppressTarget] = - position.toMeta(Input.String(text)).map(_.start).flatMap { positionOffset => - javaTrees - .findEnclosingJavaVariable(path, position, onNameOnly = false) - .filter(variable => - variable.isStandaloneDeclaration && - positionOffset <= variable.nameRange.endOffset - ) - .map(variable => SuppressTarget(variable, variable.nameRange)) - .orElse( - javaTrees - .findEnclosingJavaMethod(path, position) - .map(method => SuppressTarget(method, method.nameRange)) - ) - .orElse( - javaTrees - .findEnclosingJavaClass(path, position) - .map(cls => SuppressTarget(cls, cls.nameRange)) - ) - } + javaTrees + .findEnclosingJavaVariable(path, position, onNameOnly = false) + .filter(variable => + variable.isStandaloneDeclaration && + position <= variable.nameRange.getEnd() + ) + .map(variable => SuppressTarget(variable, variable.nameRange)) + .orElse( + javaTrees + .findEnclosingJavaMethod(path, position) + .map(method => SuppressTarget(method, method.nameRange)) + ) + .orElse( + javaTrees + .findEnclosingJavaClass(path, position) + .map(cls => SuppressTarget(cls, cls.nameRange)) + ) private def suppressEdit( text: String, @@ -151,10 +144,9 @@ object SuppressWarnings { } private def isZeroRange(range: l.Range): Boolean = - range.getStart().getLine() == 0 && - range.getStart().getCharacter() == 0 && - range.getEnd().getLine() == 0 && - range.getEnd().getCharacter() == 0 + range.isOffset && + range.getStart().getLine() == 0 && + range.getStart().getCharacter() == 0 private def existingSuppressWarnings( annotations: List[JavaAnnotation] @@ -198,7 +190,7 @@ object SuppressWarnings { s"""$namedValuePrefix{"$warningName"}""", ) } else if (isArray) { - val closeBrace = insideEnd - inside.reverse.indexOf('}') - 1 + val closeBrace = insideStart + inside.lastIndexOf('}') val separator = if (arrayContents.endsWith(",")) " " else ", " ( new l.Range( From b10a7ca462c4ea332ec140b6404a8265f3e5394e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Zieli=C5=84ski?= Date: Wed, 12 Aug 2026 11:42:35 +0200 Subject: [PATCH 9/9] change default javac_options value from feature flags --- .../metals/CompilerConfiguration.scala | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala index 438fadaaec0..55398c8bafc 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/CompilerConfiguration.scala @@ -453,13 +453,10 @@ class CompilerConfiguration( val pc = JavaPresentationCompiler() val shouldUseOpts = featureFlags .readBoolean(FeatureFlag.JAVAC_OPTIONS) - .orElse(false) + .orElse(true) val options = javaTarget match { - case j: JavaTarget if shouldUseOpts => j.options - case j: JavaTarget => - j.options.filter(option => - option == "-Xlint" || option.startsWith("-Xlint:") - ) + case j: JavaTarget if shouldUseOpts => + CompilerConfiguration.filterJavaPcOptions(j.options) case _ => Nil } configure(pc, search, completionItemPriority) @@ -682,3 +679,17 @@ class CompilerConfiguration( Nil } } + +object CompilerConfiguration { + private val excludedJavaPcOptionPrefixes = List( + "-J", + "@", + "-Xplugin:", + "-proc:", + ) + + private[metals] def filterJavaPcOptions(options: List[String]): List[String] = + options.filterNot(option => + excludedJavaPcOptionPrefixes.exists(option.startsWith) + ) +}