From 9e9451df8260db80f72386e13597e32ea3ad3438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Wed, 12 Jul 2023 11:09:17 +0200 Subject: [PATCH] Add `-no-relative-path:` flag to control indexing of generated files Previously, the SemanticDB compiler plugin errored when indexing auto-generated files outside of the configured `-sourceroot` directory (which is automatically inferred for Bazel builds). This behavior was undesirable because: - There's no good workaround for the issue - The error message was cryptic making it difficult to understand what went wrong For some cases, we were able to detect this situation for Bazel and ignore the indexed file while printing an informative message, but this behavior was also undesirable because we skipping these files means that we can't render hover messages for symbols in those generated files. This PR fixes the issue by adding a configurable `-no-relative-path:` flag with the following valid options: - `index_anyways` (default): indexes the file but with no guarantee that it's possible to recover the location of the original generated file. This allows us to display accurate hover tooltips for symbols in these files even if "Go to definition" won't work. - `skip`: silently ignored these files. - `warning`: ignore these files and print a message explaining it was skipped. - `error`: fail the compilation process (old default). --- semanticdb-javac/defs.bzl | 22 +++--- .../semanticdb_javac/NoRelativePathMode.java | 28 ++++++++ .../SemanticdbJavacOptions.java | 28 ++++++++ .../SemanticdbTaskListener.java | 68 +++++++++++-------- .../semanticdb_javac/SemanticdbVisitor.java | 11 ++- 5 files changed, 117 insertions(+), 40 deletions(-) create mode 100644 semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/NoRelativePathMode.java diff --git a/semanticdb-javac/defs.bzl b/semanticdb-javac/defs.bzl index 8890eb97..1323219b 100644 --- a/semanticdb-javac/defs.bzl +++ b/semanticdb-javac/defs.bzl @@ -1,18 +1,20 @@ """Java rules that automatically register the SemanticDB compiler plugin based on the presence of a string flag.""" -load("@rules_java//java:defs.bzl", native_java_library="java_library", native_java_binary="java_binary") -def java_library(javacopts=[], plugins=[],**kwargs): - native_java_library( - javacopts=_actual_javacopts(javacopts), - plugins=_actual_plugins(plugins), - **kwargs) +load("@rules_java//java:defs.bzl", native_java_binary = "java_binary", native_java_library = "java_library") +def java_library(javacopts = [], plugins = [], **kwargs): + native_java_library( + javacopts = _actual_javacopts(javacopts), + plugins = _actual_plugins(plugins), + **kwargs + ) -def java_binary(javacopts=[], plugins=[],**kwargs): +def java_binary(javacopts = [], plugins = [], **kwargs): native_java_binary( - javacopts=_actual_javacopts(javacopts), - plugins=_actual_plugins(plugins), - **kwargs) + javacopts = _actual_javacopts(javacopts), + plugins = _actual_plugins(plugins), + **kwargs + ) def _actual_javacopts(javacopts): return select({ diff --git a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/NoRelativePathMode.java b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/NoRelativePathMode.java new file mode 100644 index 00000000..89277ea9 --- /dev/null +++ b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/NoRelativePathMode.java @@ -0,0 +1,28 @@ +package com.sourcegraph.semanticdb_javac; + +/** + * Configuration options to determine how semanticdb-javac should handle files that have no good + * relative paths. + * + *

When indexing a file at an absolute path /project/src/main/example/Foo.java, SemanticDB needs + * to know the "sourceroot" path /project in order to relativize the path of the source file into + * src/main/example/Foo.java. This configuration option determines what the compiler plugin should + * do when it's not able to find a good relative path. + */ +public enum NoRelativePathMode { + + /** + * Come up with a unique relative path for the SemanticDB path with no guarantee that this path + * corresponds to the original path of the generated source file. + */ + INDEX_ANYWAY, + + /** Silently ignore the file, don't index it. */ + SKIP, + + /** Report an error message and fail the compilation. */ + ERROR, + + /** Ignore the file, but print a message explaining it's ignored. */ + WARNING +} diff --git a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbJavacOptions.java b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbJavacOptions.java index e6ed2c16..da03ffaf 100644 --- a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbJavacOptions.java +++ b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbJavacOptions.java @@ -5,6 +5,8 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; +import java.util.stream.Collectors; import javax.tools.FileObject; import javax.tools.JavaFileManager; import javax.tools.JavaFileObject; @@ -26,6 +28,7 @@ public class SemanticdbJavacOptions { public final ArrayList errors; public boolean alreadyReportedErrors = false; public UriScheme uriScheme = UriScheme.DEFAULT; + public NoRelativePathMode noRelativePath = NoRelativePathMode.INDEX_ANYWAY; public Path generatedTargetRoot; public static String stubClassName = "META-INF-semanticdb-stub"; @@ -60,6 +63,31 @@ public static SemanticdbJavacOptions parse(String[] args, Context ctx) { result.sourceroot = Paths.get(arg.substring("-sourceroot:".length())).normalize(); } else if (arg.equals("-build-tool:sbt") || arg.equals("-build-tool:mill")) { result.uriScheme = UriScheme.ZINC; + } else if (arg.startsWith("-no-relative-path:")) { + String value = arg.substring("-no-relative-path:".length()); + switch (value) { + case "index_anyway": + result.noRelativePath = NoRelativePathMode.INDEX_ANYWAY; + break; + case "skip": + result.noRelativePath = NoRelativePathMode.SKIP; + break; + case "error": + result.noRelativePath = NoRelativePathMode.ERROR; + break; + case "warning": + result.noRelativePath = NoRelativePathMode.WARNING; + break; + default: + String validValues = + Arrays.stream(NoRelativePathMode.values()) + .map(NoRelativePathMode::toString) + .collect(Collectors.joining(", ")); + result.errors.add( + String.format( + "unknown -no-relative-path mode '%s'. Valid values are %s.", + value, validValues)); + } } else if (arg.equals("-build-tool:bazel")) { result.uriScheme = UriScheme.BAZEL; useJavacClassesDir = true; diff --git a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java index f46df649..e7b7a11e 100644 --- a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java +++ b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java @@ -6,7 +6,6 @@ import com.sun.source.util.Trees; import com.sun.tools.javac.model.JavacTypes; -import javax.tools.Diagnostic; import javax.tools.JavaFileObject; import java.io.IOException; import java.net.URI; @@ -25,7 +24,7 @@ public final class SemanticdbTaskListener implements TaskListener { private final SemanticdbReporter reporter; private final JavacTypes javacTypes; private final Trees trees; - private boolean sourceGeneratorsMessageIsLogged = false; + private int noRelativePathCounter = 0; public SemanticdbTaskListener( SemanticdbJavacOptions options, @@ -130,7 +129,11 @@ public static Path absolutePathFromUri(SemanticdbJavacOptions options, JavaFileO new String[] {"SimpleFileObject[", "DirectoryFileObject["}; for (String pattern : knownBazelToStringPatterns) { if (toString.startsWith(pattern) && toString.endsWith("]")) { - return Paths.get(toString.substring(pattern.length(), toString.length() - 1)); + Path path = Paths.get(toString.substring(pattern.length(), toString.length() - 1)); + if (path.isAbsolute()) { + return path; + } + return options.sourceroot.resolve(path); } } throw new IllegalArgumentException("unsupported source file: " + toString); @@ -190,32 +193,41 @@ private Result semanticdbOutputPath(SemanticdbJavacOptions options .resolve(relativePath) .resolveSibling(filename); return Result.ok(semanticdbOutputPath); - } else { - - if (options.uriScheme == UriScheme.BAZEL && options.generatedTargetRoot != null) { - try { - if (absolutePath.toRealPath().startsWith(options.generatedTargetRoot)) { - if (!sourceGeneratorsMessageIsLogged) { - sourceGeneratorsMessageIsLogged = true; - reporter.info( - "Usage of source generators detected - scip-java does not produce SemanticDB files for generated files.\n" - + "This message is logged only once", - e); - } - - return null; - } - } catch (IOException exc) { - reporter.exception(exc, e); - return null; - } - } + } - return Result.error( - String.format( - "sourceroot '%s does not contain path '%s'. To fix this problem, update the -sourceroot flag to " - + "be a parent directory of this source file.", - options.sourceroot, absolutePath)); + switch (options.noRelativePath) { + case INDEX_ANYWAY: + // Come up with a unique relative path for this file even if it's not under the sourceroot. + // By indexing auto-generated files, we collect SymbolInformation for auto-generated symbol, + // which results in more useful hover tooltips in the editor. + // In the future, we may want to additionally embed the full text contents of these files + // so that it's possible to browse generated files with precise code navigation. + String uniqueFilename = + String.format("%d.%s.semanticdb", ++noRelativePathCounter, absolutePath.getFileName()); + Path semanticdbOutputPath = + options + .targetroot + .resolve("META-INF") + .resolve("semanticdb") + .resolve("no-relative-path") + .resolve(uniqueFilename); + return Result.ok(semanticdbOutputPath); + case WARNING: + reporter.info( + String.format( + "Skipping file '%s' because it is not under the sourceroot '%s'", + absolutePath, options.sourceroot), + e); + case SKIP: + return null; + case ERROR: + default: + return Result.error( + String.format( + "Unable to detect the relative path of '%s'. A common reason for this error is that the file is that this file is auto-generated. " + + "To fix this problem, either configure the -sourceroot:PATH flag to be the parent directory of all indexed files, or " + + "configure -no-relative-path:VALUE flag to have one of the following values: index_anyway, skip, warning.", + absolutePath)); } } } diff --git a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbVisitor.java b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbVisitor.java index 120b449e..efbdf3fe 100644 --- a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbVisitor.java +++ b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbVisitor.java @@ -436,9 +436,12 @@ private Semanticdb.Access semanticdbAccess(Symbol sym) { private String semanticdbUri() { Path absolutePath = SemanticdbTaskListener.absolutePathFromUri(options, event.getSourceFile()); - Path relativePath = options.sourceroot.relativize(absolutePath); + Path uriPath = + absolutePath.startsWith(options.sourceroot) + ? options.sourceroot.relativize(absolutePath) + : absolutePath; StringBuilder out = new StringBuilder(); - Iterator it = relativePath.iterator(); + Iterator it = uriPath.iterator(); if (it.hasNext()) out.append(it.next().getFileName().toString()); while (it.hasNext()) { Path part = it.next(); @@ -447,6 +450,10 @@ private String semanticdbUri() { return out.toString(); } + private Path semanticdbPath() { + return SemanticdbTaskListener.absolutePathFromUri(options, event.getSourceFile()); + } + private Semanticdb.Documentation semanticdbDocumentation(Symbol sym) { try { Elements elements = task.getElements();