Skip to content

Commit

Permalink
Use pos.unit.source instead of currentUnit.source in quick fixes (#10682
Browse files Browse the repository at this point in the history
)

Type completion can lead to currentUnit being different from
tree.pos.unit.
  • Loading branch information
lrytz committed Feb 6, 2024
1 parent 75f762d commit e8e95f3
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/typechecker/Adaptations.scala
Expand Up @@ -102,7 +102,7 @@ trait Adaptations {
s"adapted the argument list to the expected ${args.size}-tuple: add additional parens instead")
val pos = wrappingPos(args)
context.warning(t.pos, msg, WarningCategory.LintAdaptedArgs,
runReporting.codeAction("add wrapping parentheses", pos, s"(${currentUnit.source.sourceAt(pos)})", msg))
runReporting.codeAction("add wrapping parentheses", pos, s"(${pos.source.sourceAt(pos)})", msg))
}
true // keep adaptation
}
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Expand Up @@ -467,7 +467,7 @@ abstract class RefChecks extends Transform {
val msg = s"$mbr without a parameter list overrides ${other.fullLocationString} defined with a single empty parameter list"
val namePos = member.pos
val action =
if (namePos.isDefined && currentUnit.source.sourceAt(namePos) == member.decodedName)
if (namePos.isDefined && namePos.source.sourceAt(namePos) == member.decodedName)
runReporting.codeAction("add empty parameter list", namePos.focusEnd, "()", msg)
else Nil
overrideErrorOrNullaryWarning(msg, action)
Expand All @@ -477,7 +477,7 @@ abstract class RefChecks extends Transform {
val msg = s"$mbr with a single empty parameter list overrides ${other.fullLocationString} defined without a parameter list"
val namePos = member.pos
val action =
if (namePos.isDefined && currentUnit.source.sourceAt(namePos) == member.decodedName)
if (namePos.isDefined && namePos.source.sourceAt(namePos) == member.decodedName)
runReporting.codeAction("remove empty parameter list", namePos.focusEnd.withEnd(namePos.end + 2), "", msg, expected = Some(("()", currentUnit)))
else Nil
overrideErrorOrNullaryWarning(msg, action)
Expand Down Expand Up @@ -1811,7 +1811,7 @@ abstract class RefChecks extends Transform {
val msg = s"side-effecting nullary methods are discouraged: suggest defining as `def ${sym.name.decode}()` instead"
val namePos = sym.pos.focus.withEnd(sym.pos.point + sym.decodedName.length)
val action =
if (currentUnit.source.sourceAt(namePos) == sym.decodedName)
if (namePos.source.sourceAt(namePos) == sym.decodedName)
runReporting.codeAction("add empty parameter list", namePos.focusEnd, "()", msg)
else Nil
refchecksWarning(sym.pos, msg, WarningCategory.LintNullaryUnit, action)
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -1181,7 +1181,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
if (isInharmonic) {
// not `context.deprecationWarning` because they are not buffered in silent mode
val msg = s"Widening conversion from ${tpSym.name} to ${ptSym.name} is deprecated because it loses precision. Write `.to${ptSym.name}` instead."
val orig = currentUnit.source.sourceAt(tree.pos)
val orig = tree.pos.source.sourceAt(tree.pos)
context.warning(tree.pos, msg, WarningCategory.Deprecation,
runReporting.codeAction("add conversion", tree.pos, s"${CodeAction.maybeWrapInParens(orig)}.to${ptSym.name}", msg))
} else {
Expand All @@ -1191,7 +1191,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
case Apply(Select(q, nme.DIV), _) if isInt(q) =>
val msg = s"integral division is implicitly converted (widened) to floating point. Add an explicit `.to${ptSym.name}`."
context.warning(tree.pos, msg, WarningCategory.LintIntDivToFloat,
runReporting.codeAction("add conversion", tree.pos, s"(${currentUnit.source.sourceAt(tree.pos)}).to${ptSym.name}", msg))
runReporting.codeAction("add conversion", tree.pos, s"(${tree.pos.source.sourceAt(tree.pos)}).to${ptSym.name}", msg))
case Apply(Select(a1, _), List(a2)) if isInt(tree) && isInt(a1) && isInt(a2) => traverse(a1); traverse(a2)
case Select(q, _) if isInt(tree) && isInt(q) => traverse(q)
case _ =>
Expand Down Expand Up @@ -5040,8 +5040,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper

val action = {
val etaPos = pos.withEnd(pos.end + 2)
if (currentUnit.source.sourceAt(etaPos).endsWith(" _"))
runReporting.codeAction("replace by function literal", etaPos, s"() => ${currentUnit.source.sourceAt(pos)}", msg)
if (pos.source.sourceAt(etaPos).endsWith(" _"))
runReporting.codeAction("replace by function literal", etaPos, s"() => ${pos.source.sourceAt(pos)}", msg)
else Nil
}

Expand Down
84 changes: 71 additions & 13 deletions test/junit/scala/tools/nsc/QuickfixTest.scala
Expand Up @@ -6,25 +6,30 @@ import org.junit.Test
import java.nio.file.Files
import scala.reflect.internal.util.BatchSourceFile
import scala.reflect.io.AbstractFile
import scala.tools.nsc.reporters.StoreReporter
import scala.tools.testkit.BytecodeTesting
import scala.tools.testkit.ReleasablePath._
import scala.util.Using

import reporters.StoreReporter

class QuickfixTest extends BytecodeTesting {
def testQuickfix(a: String, b: String, args: String, checkInfo: StoreReporter.Info => Boolean = _ => true): Unit = if (!scala.util.Properties.isWin) {
Using.resource(Files.createTempFile("unitSource", "scala")) { src =>
Files.write(src, a.getBytes)
val c = BytecodeTesting.newCompiler(extraArgs = args)
val r = c.newRun()
val f = AbstractFile.getFile(src.toFile.getAbsolutePath)
r.compileSources(List(new BatchSourceFile(f)))
assertEquals(b, new String(Files.readAllBytes(src)))
for (info <- c.global.reporter.asInstanceOf[StoreReporter].infos)
assert(checkInfo(info), info)

def testQuickfixs(as: List[String], b: String, args: String, checkInfo: StoreReporter.Info => Boolean = _ => true): Unit =
if (!scala.util.Properties.isWin) {
Using.resource(Files.createTempDirectory("quickfixTest")) { tmpDir =>
val srcs = as.indices.map(i => tmpDir.resolve(s"unitSource$i.scala")).toList
as.lazyZip(srcs).foreach { case (a, src) => Files.write(src, a.getBytes) }
val c = BytecodeTesting.newCompiler(extraArgs = args)
val r = c.newRun()
val fs = srcs.map(src => AbstractFile.getFile(src.toFile.getAbsolutePath))
r.compileSources(fs.map(new BatchSourceFile(_)))
assertEquals(b.replaceAll("\n+", "\n"), srcs.map(src => new String(Files.readAllBytes(src))).mkString("\n").replaceAll("\n+", "\n"))
for (info <- c.global.reporter.asInstanceOf[StoreReporter].infos)
assert(checkInfo(info), info)
}
}
}

def testQuickfix(a: String, b: String, args: String, checkInfo: StoreReporter.Info => Boolean = _ => true): Unit =
testQuickfixs(List(a), b, args, checkInfo)

def comp(args: String) = BytecodeTesting.newCompiler(extraArgs = args)

Expand Down Expand Up @@ -90,4 +95,57 @@ class QuickfixTest extends BytecodeTesting {
val a = "import foo.bar"
testQuickfix(a, a, "-quickfix:any", !_.msg.contains("[rewritten by -quickfix]"))
}

// https://github.com/scala/bug/issues/12941
@Test def correctSourceInTypeCompleters(): Unit = {
val a1s = List(
"""trait Trait[A] {
| def foo: Option[A]
|}
|object Test1 extends Trait[Any] {
| def foo = None
|}
|""".stripMargin,
"""object Test2 {
| def one = Test1.foo
|}
|""".stripMargin)
val b1 =
"""trait Trait[A] {
| def foo: Option[A]
|}
|object Test1 extends Trait[Any] {
| def foo: None.type = None
|}
|object Test2 {
| def one = Test1.foo
|}
|""".stripMargin
testQuickfixs(a1s, b1, "-Xsource:3 -quickfix:any")

val a2s = List(
"""object Test2 {
| def one = Test1.foo
|}
|""".stripMargin,
"""trait Trait[A] {
| def foo: Option[A]
|}
|object Test1 extends Trait[Any] {
| def foo = None
|}
|""".stripMargin)
val b2 =
"""object Test2 {
| def one = Test1.foo
|}
|trait Trait[A] {
| def foo: Option[A]
|}
|object Test1 extends Trait[Any] {
| def foo: None.type = None
|}
|""".stripMargin
testQuickfixs(a2s, b2, "-Xsource:3 -quickfix:any")
}
}

0 comments on commit e8e95f3

Please sign in to comment.