From 0520f5a7862eb1332659cb8f218e7f222d47e06e Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 17 Jan 2024 17:46:51 +0000 Subject: [PATCH 1/8] Fix export go-to-definition [Cherry-picked 32b00f4dc108ba55564bcfccecdc826dcf4e5075] --- .../src/dotty/tools/dotc/core/Symbols.scala | 5 +++++ .../tools/dotc/interactive/SourceTree.scala | 7 +++++- .../dotty/tools/pc/MetalsInteractive.scala | 22 +++++-------------- .../tests/definition/PcDefinitionSuite.scala | 18 +++++++++++++++ 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index aca718322af0..88f8d9d5fb59 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -321,6 +321,11 @@ object Symbols extends SymUtils { } else if (denot.isPrimaryConstructor) denot.owner.sourceSymbol + else if denot.is(Exported) then + denot.info.finalResultType match + case TypeAlias(target: NamedType) => target.symbol.sourceSymbol // exported type + case target: NamedType => target.symbol.sourceSymbol // exported term + case info => this else this /** The position of this symbol, or NoSpan if the symbol was not loaded diff --git a/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala b/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala index 5480d4a43043..258d92a2d1a8 100644 --- a/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala +++ b/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala @@ -42,7 +42,12 @@ case class SourceTree(tree: tpd.Import | tpd.NameTree, source: SourceFile) { (treeSpan.end - nameLength, treeSpan.end) Span(start, end, start) } - source.atSpan(position) + // Don't widen the span, only narrow. + // E.g. The star in a wildcard export is 1 character, + // and that is the span of the type alias that results from it + // but the name may very well be larger, which we don't want. + val span1 = if treeSpan.contains(position) then position else treeSpan + source.atSpan(span1) } case _ => NoSourcePosition diff --git a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala index 0e64a6c839ab..6133389945c6 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala @@ -109,7 +109,6 @@ object MetalsInteractive: * Returns the list of tuple enclosing symbol and * the symbol's expression type if possible. */ - @tailrec def enclosingSymbolsWithExpressionType( path: List[Tree], pos: SourcePosition, @@ -117,7 +116,7 @@ object MetalsInteractive: skipCheckOnName: Boolean = false ): List[(Symbol, Type)] = import indexed.ctx - path match + @tailrec def go(path: List[Tree]): List[(Symbol, Type)] = path match // For a named arg, find the target `DefDef` and jump to the param case NamedArg(name, _) :: Apply(fn, _) :: _ => val funSym = fn.symbol @@ -206,12 +205,7 @@ object MetalsInteractive: case path @ head :: tail => if head.symbol.is(Synthetic) then - enclosingSymbolsWithExpressionType( - tail, - pos, - indexed, - skipCheckOnName - ) + go(tail) else if head.symbol != NoSymbol then if skipCheckOnName || MetalsInteractive.isOnName( @@ -225,21 +219,17 @@ object MetalsInteractive: * https://github.com/lampepfl/dotty/issues/15937 */ else if head.isInstanceOf[TypeTree] then - enclosingSymbolsWithExpressionType(tail, pos, indexed) + go(tail) else Nil else val recovered = recoverError(head, indexed) if recovered.isEmpty then - enclosingSymbolsWithExpressionType( - tail, - pos, - indexed, - skipCheckOnName - ) + go(tail) else recovered.map(sym => (sym, sym.info)) end if case Nil => Nil - end match + end go + go(path).map((sym, tp) => (sym.sourceSymbol, tp)) end enclosingSymbolsWithExpressionType import dotty.tools.pc.utils.MtagsEnrichments.* diff --git a/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala index 358e159eb539..360bc885e134 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala @@ -199,6 +199,24 @@ class PcDefinitionSuite extends BasePcDefinitionSuite: |""".stripMargin ) + @Test def `export` = + check( + """object enumerations: + | trait <> + | trait CymbalKind + | + |object all: + | export enumerations.* + | + |@main def hello = + | import all.SymbolKind + | import enumerations.CymbalKind + | + | val x = new Symbo@@lKind {} + | val y = new CymbalKind {} + |""".stripMargin + ) + @Test def `named-arg-local` = check( """| From 4042f9fe9b0796fd07e07e16b38096df422300ad Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 19 Jan 2024 11:56:28 +0000 Subject: [PATCH 2/8] Par back sourceSymbol usage, to keep some non-source Given: class Foo(name: String, age: Int) case class Person(name: String) Is the Foo constructor symbol synthetic? Being the primary constructor, sourceSymbol considers it should be substituted for the Foo class symbol. Similarly, the Person apply method symbol is replaced by the class symbol for Person (via the Person module class). Doing so leads the presentation compiler to show details about the class, rather than the constructor or the apply method intially targeted. [Cherry-picked 6792fc798752545a129399fa2851a9b179c11413] --- compiler/src/dotty/tools/dotc/core/Symbols.scala | 2 +- .../src/main/dotty/tools/pc/MetalsInteractive.scala | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index 88f8d9d5fb59..43d16af742c5 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -305,7 +305,7 @@ object Symbols extends SymUtils { /** A symbol related to `sym` that is defined in source code. * - * @see enclosingSourceSymbols + * @see [[interactive.Interactive.enclosingSourceSymbols]] */ @annotation.tailrec final def sourceSymbol(using Context): Symbol = if (!denot.exists) diff --git a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala index 6133389945c6..00609e1435c5 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala @@ -229,7 +229,14 @@ object MetalsInteractive: end if case Nil => Nil end go - go(path).map((sym, tp) => (sym.sourceSymbol, tp)) + go(path).map { (sym, tp) => + if sym.is(Synthetic) && sym.name == StdNames.nme.apply then + (sym, tp) // return synthetic apply, rather than the apply's owner + else if sym.isClassConstructor && sym.isPrimaryConstructor then + (sym, tp) // return the class constructor, rather than the class (so skip trait constructors) + else + (sym.sourceSymbol, tp) + } end enclosingSymbolsWithExpressionType import dotty.tools.pc.utils.MtagsEnrichments.* From 92d2266b1b922b797bf5ac6bd60ae8abcd13b3e1 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 19 Jan 2024 14:53:11 +0000 Subject: [PATCH 3/8] Alternatively, make export method synthetic & handle that [Cherry-picked 6aadd94b6fbfafc160489f6d81a6cbcc7b08a51b] --- .../src/dotty/tools/dotc/core/Symbols.scala | 5 --- .../tools/dotc/interactive/SourceTree.scala | 7 +-- .../src/dotty/tools/dotc/typer/Namer.scala | 2 +- .../dotty/tools/pc/MetalsInteractive.scala | 44 ++++++++++++------- tests/neg/export-leaks.check | 12 +++++ tests/neg/export-leaks.scala | 16 +++++-- tests/neg/exports.check | 35 +++++---------- tests/neg/exports.scala | 8 ++-- 8 files changed, 72 insertions(+), 57 deletions(-) create mode 100644 tests/neg/export-leaks.check diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index 43d16af742c5..6acee8aaa836 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -321,11 +321,6 @@ object Symbols extends SymUtils { } else if (denot.isPrimaryConstructor) denot.owner.sourceSymbol - else if denot.is(Exported) then - denot.info.finalResultType match - case TypeAlias(target: NamedType) => target.symbol.sourceSymbol // exported type - case target: NamedType => target.symbol.sourceSymbol // exported term - case info => this else this /** The position of this symbol, or NoSpan if the symbol was not loaded diff --git a/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala b/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala index 258d92a2d1a8..5480d4a43043 100644 --- a/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala +++ b/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala @@ -42,12 +42,7 @@ case class SourceTree(tree: tpd.Import | tpd.NameTree, source: SourceFile) { (treeSpan.end - nameLength, treeSpan.end) Span(start, end, start) } - // Don't widen the span, only narrow. - // E.g. The star in a wildcard export is 1 character, - // and that is the span of the type alias that results from it - // but the name may very well be larger, which we don't want. - val span1 = if treeSpan.contains(position) then position else treeSpan - source.atSpan(span1) + source.atSpan(position) } case _ => NoSourcePosition diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index ff140e30a2ae..077b8199295a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -1244,7 +1244,7 @@ class Namer { typer: Typer => (EmptyFlags, mbrInfo) var flagMask = RetainedExportFlags if sym.isTerm then flagMask |= HasDefaultParams | NoDefaultParams - var mbrFlags = Exported | Method | Final | maybeStable | sym.flags & flagMask + var mbrFlags = Exported | Synthetic | Method | Final | maybeStable | sym.flags & flagMask if sym.is(ExtensionMethod) || pathMethod.exists then mbrFlags |= ExtensionMethod val forwarderName = checkNoConflict(alias, isPrivate = false, span) diff --git a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala index 00609e1435c5..cf7c10470bf2 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala @@ -10,7 +10,7 @@ import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.Names.Name import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.core.Symbols.* -import dotty.tools.dotc.core.Types.Type +import dotty.tools.dotc.core.Types.* import dotty.tools.dotc.interactive.SourceTree import dotty.tools.dotc.util.SourceFile import dotty.tools.dotc.util.SourcePosition @@ -109,6 +109,7 @@ object MetalsInteractive: * Returns the list of tuple enclosing symbol and * the symbol's expression type if possible. */ + @tailrec def enclosingSymbolsWithExpressionType( path: List[Tree], pos: SourcePosition, @@ -116,7 +117,7 @@ object MetalsInteractive: skipCheckOnName: Boolean = false ): List[(Symbol, Type)] = import indexed.ctx - @tailrec def go(path: List[Tree]): List[(Symbol, Type)] = path match + path match // For a named arg, find the target `DefDef` and jump to the param case NamedArg(name, _) :: Apply(fn, _) :: _ => val funSym = fn.symbol @@ -204,8 +205,24 @@ object MetalsInteractive: Nil case path @ head :: tail => - if head.symbol.is(Synthetic) then - go(tail) + if head.symbol.is(Exported) then + head.symbol.info match + case TypeAlias(target: NamedType) => + val ss = target.symbol.sourceSymbol // exported type + List((ss, ss.info)) + case info => info.finalResultType match + case target: NamedType => + val ss = target.symbol.sourceSymbol // exported term + List((ss, ss.info)) + case _ => + enclosingSymbolsWithExpressionType(tail, pos, indexed, skipCheckOnName) + else if head.symbol.is(Synthetic) then + enclosingSymbolsWithExpressionType( + tail, + pos, + indexed, + skipCheckOnName + ) else if head.symbol != NoSymbol then if skipCheckOnName || MetalsInteractive.isOnName( @@ -219,24 +236,21 @@ object MetalsInteractive: * https://github.com/lampepfl/dotty/issues/15937 */ else if head.isInstanceOf[TypeTree] then - go(tail) + enclosingSymbolsWithExpressionType(tail, pos, indexed) else Nil else val recovered = recoverError(head, indexed) if recovered.isEmpty then - go(tail) + enclosingSymbolsWithExpressionType( + tail, + pos, + indexed, + skipCheckOnName + ) else recovered.map(sym => (sym, sym.info)) end if case Nil => Nil - end go - go(path).map { (sym, tp) => - if sym.is(Synthetic) && sym.name == StdNames.nme.apply then - (sym, tp) // return synthetic apply, rather than the apply's owner - else if sym.isClassConstructor && sym.isPrimaryConstructor then - (sym, tp) // return the class constructor, rather than the class (so skip trait constructors) - else - (sym.sourceSymbol, tp) - } + end match end enclosingSymbolsWithExpressionType import dotty.tools.pc.utils.MtagsEnrichments.* diff --git a/tests/neg/export-leaks.check b/tests/neg/export-leaks.check new file mode 100644 index 000000000000..bd63b06e2703 --- /dev/null +++ b/tests/neg/export-leaks.check @@ -0,0 +1,12 @@ +-- [E006] Not Found Error: tests/neg/export-leaks.scala:24:13 ---------------------------------------------------------- +24 | val t1 = bar // error: Not found: bar + | ^^^ + | Not found: bar + | + | longer explanation available when compiling with `-explain` +-- [E006] Not Found Error: tests/neg/export-leaks.scala:25:13 ---------------------------------------------------------- +25 | val t2 = foo // error: Not found: foo + | ^^^ + | Not found: foo + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/export-leaks.scala b/tests/neg/export-leaks.scala index fcc63b0781fc..ebb1474dd963 100644 --- a/tests/neg/export-leaks.scala +++ b/tests/neg/export-leaks.scala @@ -6,10 +6,20 @@ object Signature { object O1 { private[Signature] def bar: T = ??? } - export O1._ // error: non-private method bar refers to private type T + export O1._ object O2 { private[Signature] val foo: T = ??? } - export O2._ // error: non-private method foo refers to private type T -} \ No newline at end of file + export O2._ + + object PosTest: + def main(args: Array[String]): Unit = + val t1 = bar // ok + val t2 = foo // ok +} + +object Test: + def main(args: Array[String]): Unit = + val t1 = bar // error: Not found: bar + val t2 = foo // error: Not found: foo diff --git a/tests/neg/exports.check b/tests/neg/exports.check index 79951cebfc39..b0fbfda24509 100644 --- a/tests/neg/exports.check +++ b/tests/neg/exports.check @@ -7,16 +7,21 @@ | ^^^^^^^^^^^^^^ | no eligible member scanAll at this.scanUnit | this.scanUnit.scanAll cannot be exported because it is not accessible --- Error: tests/neg/exports.scala:25:21 -------------------------------------------------------------------------------- -25 | export printUnit.bitmap // error: no eligible member - | ^ - | non-private given instance bitmap in class Copier refers to private value printUnit - | in its type signature => object Copier.this.printUnit.bitmap +-- [E120] Naming Error: tests/neg/exports.scala:28:8 ------------------------------------------------------------------- +28 | def status: List[String] = printUnit.status ++ scanUnit.status // error: double definition w/ printUnit.status + | ^ + | Double definition: + | final def status: List[String] in class Copier at line 23 and + | def status: List[String] in class Copier at line 28 + | have the same type after erasure. + | + | Consider adding a @targetName annotation to one of the conflicting definitions + | for disambiguation. -- [E120] Naming Error: tests/neg/exports.scala:23:33 ------------------------------------------------------------------ 23 | export printUnit.{stat => _, _} // error: double definition | ^ | Double definition: - | def status: List[String] in class Copier at line 28 and + | final def status: List[String] in class Copier at line 24 and | final def status: List[String] in class Copier at line 23 | have the same type after erasure. | @@ -26,22 +31,12 @@ 24 | export scanUnit._ // error: double definition | ^ | Double definition: - | final def status: List[String] in class Copier at line 23 and + | final def status: List[String] in class Copier at line 26 and | final def status: List[String] in class Copier at line 24 | have the same type after erasure. | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. --- [E120] Naming Error: tests/neg/exports.scala:26:21 ------------------------------------------------------------------ -26 | export printUnit.status // error: double definition - | ^ - | Double definition: - | final def status: List[String] in class Copier at line 24 and - | final def status: List[String] in class Copier at line 26 - | have the same type after erasure. - | - | Consider adding a @targetName annotation to one of the conflicting definitions - | for disambiguation. -- Error: tests/neg/exports.scala:35:24 -------------------------------------------------------------------------------- 35 | export this.{concat => ++} // error: no eligible member | ^^^^^^^^^^^^ @@ -52,12 +47,6 @@ | ^^^ | no eligible member foo at this.foo | this.foo.foo cannot be exported because it is already a member of class Foo --- [E120] Naming Error: tests/neg/exports.scala:46:15 ------------------------------------------------------------------ -46 | export bar._ // error: double definition - | ^ - | Double definition: - | val bar: Bar in class Baz at line 45 and - | final def bar: (Baz.this.bar.bar : => (Baz.this.bar.baz.bar : Bar)) in class Baz at line 46 -- [E083] Type Error: tests/neg/exports.scala:57:11 -------------------------------------------------------------------- 57 | export printer.* // error: not stable | ^^^^^^^ diff --git a/tests/neg/exports.scala b/tests/neg/exports.scala index c187582c940d..8889efdd917c 100644 --- a/tests/neg/exports.scala +++ b/tests/neg/exports.scala @@ -22,10 +22,10 @@ export scanUnit.{scanAll => foo} // error: no eligible member export printUnit.{stat => _, _} // error: double definition export scanUnit._ // error: double definition - export printUnit.bitmap // error: no eligible member - export printUnit.status // error: double definition + export printUnit.bitmap + export printUnit.status - def status: List[String] = printUnit.status ++ scanUnit.status + def status: List[String] = printUnit.status ++ scanUnit.status // error: double definition w/ printUnit.status } trait IterableOps[+A, +CC[_], +C] { @@ -43,7 +43,7 @@ class Baz { val bar: Bar = new Bar - export bar._ // error: double definition + export bar._ } class Bar { val baz: Baz = new Baz From a25c33fb56297ede8998808bba3bfbbb202b887b Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 19 Jan 2024 18:25:11 +0000 Subject: [PATCH 4/8] Back to non-synthetic, handle only exported types [Cherry-picked 03c9b3e5be52c95ef92270f1d3d36b4b11b61627] --- .../src/dotty/tools/dotc/core/Flags.scala | 2 +- .../src/dotty/tools/dotc/core/Symbols.scala | 2 + .../tools/dotc/interactive/SourceTree.scala | 7 +- .../src/dotty/tools/dotc/typer/Namer.scala | 2 +- .../dotty/tools/pc/MetalsInteractive.scala | 16 ++--- .../tests/definition/PcDefinitionSuite.scala | 70 ++++++++++++++++++- tests/neg/export-leaks.check | 12 ---- tests/neg/export-leaks.scala | 16 +---- tests/neg/exports.check | 35 ++++++---- tests/neg/exports.scala | 8 +-- 10 files changed, 113 insertions(+), 57 deletions(-) delete mode 100644 tests/neg/export-leaks.check diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 8100bea374eb..5cff44ecfc4b 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -363,7 +363,7 @@ object Flags { val (Enum @ _, EnumVal @ _, _) = newFlags(40, "enum") /** An export forwarder */ - val (Exported @ _, _, _) = newFlags(41, "exported") + val (Exported @ _, ExportedTerm @ _, ExportedType @ _) = newFlags(41, "exported") /** Labeled with `erased` modifier (erased value or class) */ val (Erased @ _, _, _) = newFlags(42, "erased") diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index 6acee8aaa836..baec4fdba27d 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -312,6 +312,8 @@ object Symbols extends SymUtils { this else if (denot.is(ModuleVal)) this.moduleClass.sourceSymbol // The module val always has a zero-extent position + else if denot.is(ExportedType) then + denot.info.dropAlias.asInstanceOf[NamedType].symbol.sourceSymbol else if (denot.is(Synthetic)) { val linked = denot.linkedClass if (linked.exists && !linked.is(Synthetic)) diff --git a/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala b/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala index 5480d4a43043..258d92a2d1a8 100644 --- a/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala +++ b/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala @@ -42,7 +42,12 @@ case class SourceTree(tree: tpd.Import | tpd.NameTree, source: SourceFile) { (treeSpan.end - nameLength, treeSpan.end) Span(start, end, start) } - source.atSpan(position) + // Don't widen the span, only narrow. + // E.g. The star in a wildcard export is 1 character, + // and that is the span of the type alias that results from it + // but the name may very well be larger, which we don't want. + val span1 = if treeSpan.contains(position) then position else treeSpan + source.atSpan(span1) } case _ => NoSourcePosition diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 077b8199295a..ff140e30a2ae 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -1244,7 +1244,7 @@ class Namer { typer: Typer => (EmptyFlags, mbrInfo) var flagMask = RetainedExportFlags if sym.isTerm then flagMask |= HasDefaultParams | NoDefaultParams - var mbrFlags = Exported | Synthetic | Method | Final | maybeStable | sym.flags & flagMask + var mbrFlags = Exported | Method | Final | maybeStable | sym.flags & flagMask if sym.is(ExtensionMethod) || pathMethod.exists then mbrFlags |= ExtensionMethod val forwarderName = checkNoConflict(alias, isPrivate = false, span) diff --git a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala index cf7c10470bf2..c213acef5fe8 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala @@ -10,7 +10,7 @@ import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.Names.Name import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.core.Symbols.* -import dotty.tools.dotc.core.Types.* +import dotty.tools.dotc.core.Types.Type import dotty.tools.dotc.interactive.SourceTree import dotty.tools.dotc.util.SourceFile import dotty.tools.dotc.util.SourcePosition @@ -205,17 +205,9 @@ object MetalsInteractive: Nil case path @ head :: tail => - if head.symbol.is(Exported) then - head.symbol.info match - case TypeAlias(target: NamedType) => - val ss = target.symbol.sourceSymbol // exported type - List((ss, ss.info)) - case info => info.finalResultType match - case target: NamedType => - val ss = target.symbol.sourceSymbol // exported term - List((ss, ss.info)) - case _ => - enclosingSymbolsWithExpressionType(tail, pos, indexed, skipCheckOnName) + if head.symbol.is(ExportedType) then + val sym = head.symbol.sourceSymbol + List((sym, sym.info)) else if head.symbol.is(Synthetic) then enclosingSymbolsWithExpressionType( tail, diff --git a/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala index 360bc885e134..f763702cffdf 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala @@ -199,7 +199,7 @@ class PcDefinitionSuite extends BasePcDefinitionSuite: |""".stripMargin ) - @Test def `export` = + @Test def `exportType1` = check( """object enumerations: | trait <> @@ -217,6 +217,74 @@ class PcDefinitionSuite extends BasePcDefinitionSuite: |""".stripMargin ) + @Test def `exportType1Wild` = + check( + """object enumerations: + | trait <> + | trait CymbalKind + | + |object all: + | export enumerations.SymbolKind + | + |@main def hello = + | import all.SymbolKind + | import enumerations.CymbalKind + | + | val x = new Symbo@@lKind {} + | val y = new CymbalKind {} + |""".stripMargin + ) + + @Test def `exportTerm1` = + check( + """class BitMap + |class Scanner: + | def scan(): BitMap = ??? + |class Copier: + | private val scanUnit = new Scanner + | export scanUnit.<> + | def t1 = sc@@an() + |""".stripMargin + ) + + @Test def `exportTerm2` = + check( + """class BitMap + |class Scanner: + | def scan(): BitMap = ??? + |class Copier: + | private val scanUnit = new Scanner + | export scanUnit.<> + |class Test: + | def t2(cpy: Copier) = cpy.sc@@an() + |""".stripMargin + ) + + @Test def `exportTerm1Wild` = + check( + """class BitMap + |class Scanner: + | def scan(): BitMap = ??? + |class Copier: + | private val scanUnit = new Scanner + | export scanUnit.<<*>> + | def t1 = sc@@an() + |""".stripMargin + ) + + @Test def `exportTerm2Wild` = + check( + """class BitMap + |class Scanner: + | def scan(): BitMap = ??? + |class Copier: + | private val scanUnit = new Scanner + | export scanUnit.<<*>> + |class Test: + | def t2(cpy: Copier) = cpy.sc@@an() + |""".stripMargin + ) + @Test def `named-arg-local` = check( """| diff --git a/tests/neg/export-leaks.check b/tests/neg/export-leaks.check deleted file mode 100644 index bd63b06e2703..000000000000 --- a/tests/neg/export-leaks.check +++ /dev/null @@ -1,12 +0,0 @@ --- [E006] Not Found Error: tests/neg/export-leaks.scala:24:13 ---------------------------------------------------------- -24 | val t1 = bar // error: Not found: bar - | ^^^ - | Not found: bar - | - | longer explanation available when compiling with `-explain` --- [E006] Not Found Error: tests/neg/export-leaks.scala:25:13 ---------------------------------------------------------- -25 | val t2 = foo // error: Not found: foo - | ^^^ - | Not found: foo - | - | longer explanation available when compiling with `-explain` diff --git a/tests/neg/export-leaks.scala b/tests/neg/export-leaks.scala index ebb1474dd963..fcc63b0781fc 100644 --- a/tests/neg/export-leaks.scala +++ b/tests/neg/export-leaks.scala @@ -6,20 +6,10 @@ object Signature { object O1 { private[Signature] def bar: T = ??? } - export O1._ + export O1._ // error: non-private method bar refers to private type T object O2 { private[Signature] val foo: T = ??? } - export O2._ - - object PosTest: - def main(args: Array[String]): Unit = - val t1 = bar // ok - val t2 = foo // ok -} - -object Test: - def main(args: Array[String]): Unit = - val t1 = bar // error: Not found: bar - val t2 = foo // error: Not found: foo + export O2._ // error: non-private method foo refers to private type T +} \ No newline at end of file diff --git a/tests/neg/exports.check b/tests/neg/exports.check index b0fbfda24509..79951cebfc39 100644 --- a/tests/neg/exports.check +++ b/tests/neg/exports.check @@ -7,21 +7,16 @@ | ^^^^^^^^^^^^^^ | no eligible member scanAll at this.scanUnit | this.scanUnit.scanAll cannot be exported because it is not accessible --- [E120] Naming Error: tests/neg/exports.scala:28:8 ------------------------------------------------------------------- -28 | def status: List[String] = printUnit.status ++ scanUnit.status // error: double definition w/ printUnit.status - | ^ - | Double definition: - | final def status: List[String] in class Copier at line 23 and - | def status: List[String] in class Copier at line 28 - | have the same type after erasure. - | - | Consider adding a @targetName annotation to one of the conflicting definitions - | for disambiguation. +-- Error: tests/neg/exports.scala:25:21 -------------------------------------------------------------------------------- +25 | export printUnit.bitmap // error: no eligible member + | ^ + | non-private given instance bitmap in class Copier refers to private value printUnit + | in its type signature => object Copier.this.printUnit.bitmap -- [E120] Naming Error: tests/neg/exports.scala:23:33 ------------------------------------------------------------------ 23 | export printUnit.{stat => _, _} // error: double definition | ^ | Double definition: - | final def status: List[String] in class Copier at line 24 and + | def status: List[String] in class Copier at line 28 and | final def status: List[String] in class Copier at line 23 | have the same type after erasure. | @@ -31,12 +26,22 @@ 24 | export scanUnit._ // error: double definition | ^ | Double definition: - | final def status: List[String] in class Copier at line 26 and + | final def status: List[String] in class Copier at line 23 and | final def status: List[String] in class Copier at line 24 | have the same type after erasure. | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. +-- [E120] Naming Error: tests/neg/exports.scala:26:21 ------------------------------------------------------------------ +26 | export printUnit.status // error: double definition + | ^ + | Double definition: + | final def status: List[String] in class Copier at line 24 and + | final def status: List[String] in class Copier at line 26 + | have the same type after erasure. + | + | Consider adding a @targetName annotation to one of the conflicting definitions + | for disambiguation. -- Error: tests/neg/exports.scala:35:24 -------------------------------------------------------------------------------- 35 | export this.{concat => ++} // error: no eligible member | ^^^^^^^^^^^^ @@ -47,6 +52,12 @@ | ^^^ | no eligible member foo at this.foo | this.foo.foo cannot be exported because it is already a member of class Foo +-- [E120] Naming Error: tests/neg/exports.scala:46:15 ------------------------------------------------------------------ +46 | export bar._ // error: double definition + | ^ + | Double definition: + | val bar: Bar in class Baz at line 45 and + | final def bar: (Baz.this.bar.bar : => (Baz.this.bar.baz.bar : Bar)) in class Baz at line 46 -- [E083] Type Error: tests/neg/exports.scala:57:11 -------------------------------------------------------------------- 57 | export printer.* // error: not stable | ^^^^^^^ diff --git a/tests/neg/exports.scala b/tests/neg/exports.scala index 8889efdd917c..c187582c940d 100644 --- a/tests/neg/exports.scala +++ b/tests/neg/exports.scala @@ -22,10 +22,10 @@ export scanUnit.{scanAll => foo} // error: no eligible member export printUnit.{stat => _, _} // error: double definition export scanUnit._ // error: double definition - export printUnit.bitmap - export printUnit.status + export printUnit.bitmap // error: no eligible member + export printUnit.status // error: double definition - def status: List[String] = printUnit.status ++ scanUnit.status // error: double definition w/ printUnit.status + def status: List[String] = printUnit.status ++ scanUnit.status } trait IterableOps[+A, +CC[_], +C] { @@ -43,7 +43,7 @@ class Baz { val bar: Bar = new Bar - export bar._ + export bar._ // error: double definition } class Bar { val baz: Baz = new Baz From de1b7155a670dfe054e229b67a17550610d38d94 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sat, 20 Jan 2024 14:19:52 +0000 Subject: [PATCH 5/8] Handle eta-expanded type exports [Cherry-picked 4e532e90d7b0713375191198e643c19107df9faa] --- compiler/src/dotty/tools/dotc/core/Symbols.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index baec4fdba27d..d9a746b03a0e 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -313,7 +313,9 @@ object Symbols extends SymUtils { else if (denot.is(ModuleVal)) this.moduleClass.sourceSymbol // The module val always has a zero-extent position else if denot.is(ExportedType) then - denot.info.dropAlias.asInstanceOf[NamedType].symbol.sourceSymbol + denot.info.dropAlias.finalResultType.typeConstructor match + case tp: NamedType => tp.symbol.sourceSymbol + case _ => this else if (denot.is(Synthetic)) { val linked = denot.linkedClass if (linked.exists && !linked.is(Synthetic)) From 7fa5efd7c9487041eb7fd1c04cd6b815b4970185 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 21 Jan 2024 22:43:36 +0000 Subject: [PATCH 6/8] Support goto-def on exported methods too [Cherry-picked 050d11dd1a438913d4519d7f6fca9ab545755efd] --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 5 + .../src/dotty/tools/dotc/core/Symbols.scala | 13 ++- .../dotty/tools/pc/MetalsInteractive.scala | 23 ++-- .../tests/definition/PcDefinitionSuite.scala | 106 +++++++----------- 4 files changed, 66 insertions(+), 81 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index a1066a6131fc..a74cd543a8eb 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -1135,6 +1135,11 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { buf.toList } + def collectSubTrees[A](f: PartialFunction[Tree, A])(using Context): List[A] = + val buf = mutable.ListBuffer[A]() + foreachSubTree(f.runWith(buf += _)(_)) + buf.toList + /** Set this tree as the `defTree` of its symbol and return this tree */ def setDefTree(using Context): ThisTree = { val sym = tree.symbol diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index d9a746b03a0e..2980aef8ed78 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -19,9 +19,7 @@ import DenotTransformers.* import StdNames.* import NameOps.* import NameKinds.LazyImplicitName -import ast.tpd -import tpd.{Tree, TreeProvider, TreeOps} -import ast.TreeTypeMap +import ast.*, tpd.* import Constants.Constant import Variances.Variance import reporting.Message @@ -316,6 +314,15 @@ object Symbols extends SymUtils { denot.info.dropAlias.finalResultType.typeConstructor match case tp: NamedType => tp.symbol.sourceSymbol case _ => this + else if denot.is(ExportedTerm) then + val root = denot.maybeOwner match + case cls: ClassSymbol => cls.rootTreeContaining(name.toString) + case _ => EmptyTree + val targets = root.collectSubTrees: + case tree: DefDef if tree.name == name => methPart(tree.rhs).tpe + targets.match + case (tp: NamedType) :: _ => tp.symbol.sourceSymbol + case _ => this else if (denot.is(Synthetic)) { val linked = denot.linkedClass if (linked.exists && !linked.is(Synthetic)) diff --git a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala index c213acef5fe8..2c2897e401a1 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala @@ -1,19 +1,14 @@ -package dotty.tools.pc +package dotty.tools +package pc import scala.annotation.tailrec -import dotty.tools.dotc.ast.tpd -import dotty.tools.dotc.ast.tpd.* -import dotty.tools.dotc.ast.untpd -import dotty.tools.dotc.core.Contexts.* -import dotty.tools.dotc.core.Flags.* -import dotty.tools.dotc.core.Names.Name -import dotty.tools.dotc.core.StdNames -import dotty.tools.dotc.core.Symbols.* -import dotty.tools.dotc.core.Types.Type -import dotty.tools.dotc.interactive.SourceTree -import dotty.tools.dotc.util.SourceFile -import dotty.tools.dotc.util.SourcePosition +import dotc.* +import ast.*, tpd.* +import core.*, Contexts.*, Decorators.*, Flags.*, Names.*, Symbols.*, Types.* +import interactive.* +import util.* +import util.SourcePosition object MetalsInteractive: @@ -205,7 +200,7 @@ object MetalsInteractive: Nil case path @ head :: tail => - if head.symbol.is(ExportedType) then + if head.symbol.is(Exported) then val sym = head.symbol.sourceSymbol List((sym, sym.info)) else if head.symbol.is(Synthetic) then diff --git a/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala index f763702cffdf..c05545124495 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala @@ -199,89 +199,67 @@ class PcDefinitionSuite extends BasePcDefinitionSuite: |""".stripMargin ) - @Test def `exportType1` = - check( - """object enumerations: - | trait <> - | trait CymbalKind - | - |object all: - | export enumerations.* - | - |@main def hello = - | import all.SymbolKind - | import enumerations.CymbalKind - | - | val x = new Symbo@@lKind {} - | val y = new CymbalKind {} + @Test def exportType0 = + check( + """object Foo: + | trait <> + |object Bar: + | export Foo.* + |class Test: + | import Bar.* + | def test = new Ca@@t {} |""".stripMargin ) - @Test def `exportType1Wild` = - check( - """object enumerations: - | trait <> - | trait CymbalKind - | - |object all: - | export enumerations.SymbolKind - | - |@main def hello = - | import all.SymbolKind - | import enumerations.CymbalKind - | - | val x = new Symbo@@lKind {} - | val y = new CymbalKind {} + @Test def exportType1 = + check( + """object Foo: + | trait <>[A] + |object Bar: + | export Foo.* + |class Test: + | import Bar.* + | def test = new Ca@@t[Int] {} |""".stripMargin ) - @Test def `exportTerm1` = + @Test def exportTerm0Nullary = check( - """class BitMap - |class Scanner: - | def scan(): BitMap = ??? - |class Copier: - | private val scanUnit = new Scanner - | export scanUnit.<> - | def t1 = sc@@an() + """trait Foo: + | def <>: Int + |class Bar(val foo: Foo): + | export foo.* + | def test(bar: Bar) = bar.me@@th |""".stripMargin ) - @Test def `exportTerm2` = + @Test def exportTerm0 = check( - """class BitMap - |class Scanner: - | def scan(): BitMap = ??? - |class Copier: - | private val scanUnit = new Scanner - | export scanUnit.<> - |class Test: - | def t2(cpy: Copier) = cpy.sc@@an() + """trait Foo: + | def <>(): Int + |class Bar(val foo: Foo): + | export foo.* + | def test(bar: Bar) = bar.me@@th() |""".stripMargin ) - @Test def `exportTerm1Wild` = + @Test def exportTerm1 = check( - """class BitMap - |class Scanner: - | def scan(): BitMap = ??? - |class Copier: - | private val scanUnit = new Scanner - | export scanUnit.<<*>> - | def t1 = sc@@an() + """trait Foo: + | def <>(x: Int): Int + |class Bar(val foo: Foo): + | export foo.* + | def test(bar: Bar) = bar.me@@th(0) |""".stripMargin ) - @Test def `exportTerm2Wild` = + @Test def exportTerm1Poly = check( - """class BitMap - |class Scanner: - | def scan(): BitMap = ??? - |class Copier: - | private val scanUnit = new Scanner - | export scanUnit.<<*>> - |class Test: - | def t2(cpy: Copier) = cpy.sc@@an() + """trait Foo: + | def <>[A](x: A): A + |class Bar(val foo: Foo): + | export foo.* + | def test(bar: Bar) = bar.me@@th(0) |""".stripMargin ) From fe6b1b461b38566ce7fa510242ec54bfeb551ec0 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 31 Jan 2024 12:54:04 +0000 Subject: [PATCH 7/8] Use Symbol not name!! [Cherry-picked 2a0277305d86515182dd9bf376f3c949278bf780] --- compiler/src/dotty/tools/dotc/core/Symbols.scala | 2 +- .../tools/pc/tests/definition/PcDefinitionSuite.scala | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index 2980aef8ed78..eb60d76c77c4 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -319,7 +319,7 @@ object Symbols extends SymUtils { case cls: ClassSymbol => cls.rootTreeContaining(name.toString) case _ => EmptyTree val targets = root.collectSubTrees: - case tree: DefDef if tree.name == name => methPart(tree.rhs).tpe + case tree: DefDef if tree.symbol == denot.symbol => methPart(tree.rhs).tpe targets.match case (tp: NamedType) :: _ => tp.symbol.sourceSymbol case _ => this diff --git a/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala index c05545124495..9636aea77c2e 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/definition/PcDefinitionSuite.scala @@ -263,6 +263,17 @@ class PcDefinitionSuite extends BasePcDefinitionSuite: |""".stripMargin ) + @Test def exportTerm1Overload = + check( + """trait Foo: + | def <>(x: Int): Int + | def meth(x: String): String + |class Bar(val foo: Foo): + | export foo.* + | def test(bar: Bar) = bar.me@@th(0) + |""".stripMargin + ) + @Test def `named-arg-local` = check( """| From f8f98ccfb4f32540bf16c6f920b61c9c7ce6d91c Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 31 Jan 2024 12:56:02 +0000 Subject: [PATCH 8/8] Filter findDefinitions, to avoid using the wrong one Tested by giving exportTerm0 and exportTerm1 difference filenames, resulting in different locations and breakage. [Cherry-picked 0ee03ef7ba6c105d88d7b701cbbcc60d1e1a3afd] --- .../src/main/dotty/tools/pc/PcDefinitionProvider.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/PcDefinitionProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/PcDefinitionProvider.scala index c4266ce5d709..0de81ec39711 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/PcDefinitionProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/PcDefinitionProvider.scala @@ -124,7 +124,7 @@ class PcDefinitionProvider( val isLocal = sym.source == pos.source if isLocal then val defs = - Interactive.findDefinitions(List(sym), driver, false, false) + Interactive.findDefinitions(List(sym), driver, false, false).filter(_.source == sym.source) defs.headOption match case Some(srcTree) => val pos = srcTree.namePos