From cf77817f7186d6e19b7ad83323488a39b9a94225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 19 Jul 2022 13:05:32 +0200 Subject: [PATCH 1/3] Link references for fields, fixes #413 Previously, scip-java only linked references for method symbols. This commit changes the behavior to additionally include fields so that "find references" for fields shows usages of all implementations of the field (abstract super field and concrete implementations). --- project/build.properties | 2 +- .../commands/IndexSemanticdbCommand.scala | 8 ++++++-- .../scip_semanticdb/ScipSemanticdb.java | 4 ++-- .../semanticdb_javac/SemanticdbSymbols.java | 4 ++-- .../src/main/scala/minimized/Issue413.scala | 9 +++++++++ .../main/generated/minimized/Issue413.scala | 18 ++++++++++++++++++ 6 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 tests/minimized-scala/src/main/scala/minimized/Issue413.scala create mode 100644 tests/snapshots/src/main/generated/minimized/Issue413.scala diff --git a/project/build.properties b/project/build.properties index 10fd9eee..22af2628 100644 --- a/project/build.properties +++ b/project/build.properties @@ -1 +1 @@ -sbt.version=1.5.5 +sbt.version=1.7.1 diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala b/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala index 102e57cc..6c3fd2aa 100644 --- a/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala +++ b/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala @@ -48,7 +48,11 @@ final case class IndexSemanticdbCommand( ) extends Command { def sourceroot: Path = AbsolutePath.of(app.env.workingDirectory) def absoluteTargetroots: List[Path] = - targetroot.map(AbsolutePath.of(_, app.env.workingDirectory)) + if (targetroot.isEmpty) + List(sourceroot) + else + targetroot.map(AbsolutePath.of(_, sourceroot)) + def run(): Int = { val reporter = new ConsoleScipSemanticdbReporter(app) val outputFilename = output.getFileName.toString @@ -68,7 +72,7 @@ final case class IndexSemanticdbCommand( .toList val options = new ScipSemanticdbOptions( - targetroot.map(ts => AbsolutePath.of(ts, sourceroot)).asJava, + absoluteTargetroots.asJava, AbsolutePath.of(output, sourceroot), sourceroot, reporter, diff --git a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdb.java b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdb.java index db813fd6..802c3ca4 100644 --- a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdb.java +++ b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdb.java @@ -131,7 +131,7 @@ private void processTypedDocument(Path path, PackageTable packages) { Scip.Relationship.newBuilder() .setSymbol(typedSymbol(overriddenSymbol, overriddenSymbolPkg)) .setIsImplementation(true) - .setIsReference(SemanticdbSymbols.isMethod(info.getSymbol()))); + .setIsReference(SemanticdbSymbols.isMethodOrField(info.getSymbol()))); } if (info.hasSignature()) { String language = @@ -284,7 +284,7 @@ private Integer processDocumentUnsafe( // Overrides if (symbolInformation.getOverriddenSymbolsCount() > 0 - && SemanticdbSymbols.isMethod(symbolInformation.getSymbol()) + && SemanticdbSymbols.isMethodOrField(symbolInformation.getSymbol()) && occ.getRole() == Role.DEFINITION) { List overriddenReferenceResultIds = new ArrayList<>(symbolInformation.getOverriddenSymbolsCount()); diff --git a/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbSymbols.java b/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbSymbols.java index d11b0e55..2ec0a2be 100644 --- a/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbSymbols.java +++ b/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbSymbols.java @@ -35,8 +35,8 @@ public static boolean isGlobal(String symbol) { return !isLocal(symbol); } - public static boolean isMethod(String symbol) { - return symbol.endsWith(")."); + public static boolean isMethodOrField(String symbol) { + return symbol.endsWith("."); } /** diff --git a/tests/minimized-scala/src/main/scala/minimized/Issue413.scala b/tests/minimized-scala/src/main/scala/minimized/Issue413.scala new file mode 100644 index 00000000..6579cf79 --- /dev/null +++ b/tests/minimized-scala/src/main/scala/minimized/Issue413.scala @@ -0,0 +1,9 @@ +package minimized + +trait Issue413 { + val b: Int +} + +class Issue413Subclass extends Issue413 { + override val b = 10 +} diff --git a/tests/snapshots/src/main/generated/minimized/Issue413.scala b/tests/snapshots/src/main/generated/minimized/Issue413.scala new file mode 100644 index 00000000..d98c3391 --- /dev/null +++ b/tests/snapshots/src/main/generated/minimized/Issue413.scala @@ -0,0 +1,18 @@ +package minimized +// ^^^^^^^^^ definition minimized/ + +trait Issue413 { +// ^^^^^^^^ definition minimized/Issue413# trait Issue413 + val b: Int +// ^ definition minimized/Issue413#b. val b: Int +// ^^^ reference scala/Int# +} + +class Issue413Subclass extends Issue413 { +// ^^^^^^^^^^^^^^^^ definition minimized/Issue413Subclass# class Issue413Subclass +// definition minimized/Issue413Subclass#``(). def this() +// ^^^^^^^^ reference minimized/Issue413# +// reference java/lang/Object#``(). + override val b = 10 +// ^ definition minimized/Issue413Subclass#b. val b: Int +} From eaa1cacc71fe2c07fb1488c87aac4e9a70eca9bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 19 Jul 2022 15:56:10 +0200 Subject: [PATCH 2/3] Use more robust logic to determine reference relationship Previously, we used the symbol itself, which was not reliable because it didn't distinguish between fields an objects. This commit changes the logic to be based on `SymbolInformation.kind` from SemanticDB, that is a more robust and idiomatic solution. Fixes #414. --- .../scip_semanticdb/ScipSemanticdb.java | 16 ++++++++-- .../semanticdb_javac/SemanticdbSymbols.java | 3 +- .../src/main/scala/minimized/Issue414.scala | 14 +++++++++ .../main/generated/minimized/Issue414.scala | 30 +++++++++++++++++++ 4 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 tests/minimized-scala/src/main/scala/minimized/Issue414.scala create mode 100644 tests/snapshots/src/main/generated/minimized/Issue414.scala diff --git a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdb.java b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdb.java index 802c3ca4..3545a42d 100644 --- a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdb.java +++ b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdb.java @@ -10,7 +10,6 @@ import com.sourcegraph.Scip; import java.io.IOException; -import java.io.InputStream; import java.net.URI; import java.nio.file.*; import java.util.*; @@ -131,7 +130,7 @@ private void processTypedDocument(Path path, PackageTable packages) { Scip.Relationship.newBuilder() .setSymbol(typedSymbol(overriddenSymbol, overriddenSymbolPkg)) .setIsImplementation(true) - .setIsReference(SemanticdbSymbols.isMethodOrField(info.getSymbol()))); + .setIsReference(supportsReferenceRelationship(info))); } if (info.hasSignature()) { String language = @@ -284,7 +283,7 @@ private Integer processDocumentUnsafe( // Overrides if (symbolInformation.getOverriddenSymbolsCount() > 0 - && SemanticdbSymbols.isMethodOrField(symbolInformation.getSymbol()) + && supportsReferenceRelationship(symbolInformation) && occ.getRole() == Role.DEFINITION) { List overriddenReferenceResultIds = new ArrayList<>(symbolInformation.getOverriddenSymbolsCount()); @@ -310,6 +309,17 @@ private Integer processDocumentUnsafe( return documentId; } + private static boolean supportsReferenceRelationship(SymbolInformation info) { + switch (info.getKind()) { + case CLASS: + case OBJECT: + case PACKAGE_OBJECT: + return false; + default: + return true; + } + } + private Stream parseTextDocument(Path semanticdbPath) { try { return textDocumentsParseFrom(semanticdbPath).getDocumentsList().stream() diff --git a/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbSymbols.java b/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbSymbols.java index 2ec0a2be..94ca1695 100644 --- a/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbSymbols.java +++ b/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbSymbols.java @@ -1,7 +1,6 @@ package com.sourcegraph.semanticdb_javac; import java.util.Objects; -import java.util.Optional; /** * Utilities to construct SemanticDB symbols. @@ -36,7 +35,7 @@ public static boolean isGlobal(String symbol) { } public static boolean isMethodOrField(String symbol) { - return symbol.endsWith("."); + return symbol.endsWith("#"); } /** diff --git a/tests/minimized-scala/src/main/scala/minimized/Issue414.scala b/tests/minimized-scala/src/main/scala/minimized/Issue414.scala new file mode 100644 index 00000000..e6c61705 --- /dev/null +++ b/tests/minimized-scala/src/main/scala/minimized/Issue414.scala @@ -0,0 +1,14 @@ +package minimized + +object Issue414 { + trait A { + def b(): Unit + } + val a: A = + new A { + override def b(): Unit = { + print("Hello") + } + } + println(a.b()) +} diff --git a/tests/snapshots/src/main/generated/minimized/Issue414.scala b/tests/snapshots/src/main/generated/minimized/Issue414.scala new file mode 100644 index 00000000..00656231 --- /dev/null +++ b/tests/snapshots/src/main/generated/minimized/Issue414.scala @@ -0,0 +1,30 @@ +package minimized +// ^^^^^^^^^ definition minimized/ + +object Issue414 { +// ^^^^^^^^ definition minimized/Issue414. object Issue414 + trait A { +// ^ definition minimized/Issue414.A# trait A + def b(): Unit +// ^ definition minimized/Issue414.A#b(). def b(): Unit +// ^^^^ reference scala/Unit# + } + val a: A = +// ^ definition minimized/Issue414.a. val a: A +// ^ reference minimized/Issue414.A# + new A { +// definition local0 final class $anon +// ^ reference minimized/Issue414.A# +// reference java/lang/Object#``(). + override def b(): Unit = { +// ^ definition local1 def b(): Unit +// ^^^^ reference scala/Unit# + print("Hello") +// ^^^^^ reference scala/Predef.print(). + } + } + println(a.b()) +//^^^^^^^ reference scala/Predef.println(+1). +// ^ reference minimized/Issue414.a. +// ^ reference minimized/Issue414.A#b(). +} From e60a08097c760b6ad516ef322ff7c8e324bd4b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 19 Jul 2022 17:04:21 +0200 Subject: [PATCH 3/3] Add test case to reproduce reflective access issue --- .../main/scala/minimized/ReflectiveCall.scala | 12 ++++++++++ .../generated/minimized/ReflectiveCall.scala | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/minimized-scala/src/main/scala/minimized/ReflectiveCall.scala create mode 100644 tests/snapshots/src/main/generated/minimized/ReflectiveCall.scala diff --git a/tests/minimized-scala/src/main/scala/minimized/ReflectiveCall.scala b/tests/minimized-scala/src/main/scala/minimized/ReflectiveCall.scala new file mode 100644 index 00000000..0b1c0253 --- /dev/null +++ b/tests/minimized-scala/src/main/scala/minimized/ReflectiveCall.scala @@ -0,0 +1,12 @@ +package minimized + +import scala.language.reflectiveCalls + +class ReflectiveCall { + // Reproduction for https://github.com/scalameta/scalameta/issues/2788 + val a = + new { + val b = 1 + } + println(a.b) +} diff --git a/tests/snapshots/src/main/generated/minimized/ReflectiveCall.scala b/tests/snapshots/src/main/generated/minimized/ReflectiveCall.scala new file mode 100644 index 00000000..6b132a25 --- /dev/null +++ b/tests/snapshots/src/main/generated/minimized/ReflectiveCall.scala @@ -0,0 +1,23 @@ +package minimized +// ^^^^^^^^^ definition minimized/ + +import scala.language.reflectiveCalls +// ^^^^^ reference scala/ +// ^^^^^^^^ reference scala/language. +// ^^^^^^^^^^^^^^^ reference scala/language.reflectiveCalls. + +class ReflectiveCall { +// ^^^^^^^^^^^^^^ definition minimized/ReflectiveCall# class ReflectiveCall +// definition minimized/ReflectiveCall#``(). def this() + // Reproduction for https://github.com/scalameta/scalameta/issues/2788 + val a = +// ^ definition minimized/ReflectiveCall#a. val a: { val b: Int } + new { +// definition local0 final class $anon + val b = 1 +// ^ definition local1 val b: Int + } + println(a.b) +//^^^^^^^ reference scala/Predef.println(+1). +// ^ reference minimized/ReflectiveCall#a. +}