Skip to content

Commit ed6fb66

Browse files
committed
fix(semanticdb-javac): report analysis exceptions as WARNING instead of ERROR
reportException() was calling reporter.error() which uses Diagnostic.Kind.ERROR, causing javac to exit non-zero and fail the build. The surrounding catch block explicitly states 'we don't want to stop the compilation' — this change makes the implementation match the intent. Adds SemanticdbReporter.warning() for plugin-internal failures, and updates SemanticdbTaskListener.reportException() to use it. Intentional error paths (e.g. -no-relative-path:error mode) are unchanged. Fixes builds for projects with partial classpaths (e.g. Apache Spark) where CompletionFailure for anonymous inner classes like DataType was breaking compileJava entirely. Fixes #861 Signed-off-by: Roberto Perez Alcolea <rperezalcolea@netflix.com>
1 parent 4486a05 commit ed6fb66

File tree

3 files changed

+216
-7
lines changed

3 files changed

+216
-7
lines changed

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbReporter.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ public void exception(Throwable e, Tree tree, CompilationUnitTree root) {
2828
e.printStackTrace(writer);
2929
writer.println(
3030
"Please report a bug to https://github.com/sourcegraph/semanticdb-java with the stack trace above.");
31-
trees.printMessage(Diagnostic.Kind.ERROR, baos.toString(), tree, root);
31+
// Use WARNING so that internal exceptions never fail the compilation.
32+
// The full stack trace is preserved for bug reports.
33+
trees.printMessage(Diagnostic.Kind.WARNING, baos.toString(), tree, root);
3234
}
3335

3436
public void exception(Throwable e, TaskEvent task) {
@@ -59,4 +61,14 @@ public void error(String message, Tree tree, CompilationUnitTree root) {
5961
trees.printMessage(
6062
Diagnostic.Kind.ERROR, String.format("semanticdb-javac: %s", message), tree, root);
6163
}
64+
65+
/**
66+
* Reports a warning diagnostic. Use this for internal plugin failures (e.g. exceptions during
67+
* analysis) that should not fail the compilation — the build continues with partial semanticdb
68+
* output rather than aborting.
69+
*/
70+
public void warning(String message, Tree tree, CompilationUnitTree root) {
71+
trees.printMessage(
72+
Diagnostic.Kind.WARNING, String.format("semanticdb-javac: %s", message), tree, root);
73+
}
6274
}

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,20 @@ public void finished(TaskEvent e) {
9494
}
9595
}
9696

97-
// Uses reporter.error with the full stack trace of the exception instead of
98-
// reporter.exception
99-
// because reporter.exception doesn't seem to print any meaningful information
100-
// about the
101-
// exception, it just prints the location with an empty message.
97+
// Uses reporter.warning with the full stack trace of the exception instead of
98+
// reporter.exception because reporter.exception doesn't seem to print any meaningful
99+
// information about the exception, it just prints the location with an empty message.
100+
//
101+
// WARNING (not ERROR) is intentional: the catch block above says "we don't want to stop
102+
// the compilation", but Kind.ERROR was causing javac to exit non-zero and fail the build.
103+
// Reporting as a warning preserves the full stack trace for bug reports while allowing
104+
// compilation to succeed with partial semanticdb output.
102105
private void reportException(Throwable exception, TaskEvent e) {
103106
ByteArrayOutputStream baos = new ByteArrayOutputStream();
104107
PrintWriter pw = new PrintWriter(baos);
105108
exception.printStackTrace(pw);
106109
pw.close();
107-
reporter.error(baos.toString(), e.getCompilationUnit(), e.getCompilationUnit());
110+
reporter.warning(baos.toString(), e.getCompilationUnit(), e.getCompilationUnit());
108111
}
109112

110113
private void onFinishedAnalyze(TaskEvent e) {
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
package tests
2+
3+
import java.nio.file.Files
4+
import javax.tools.StandardLocation
5+
import javax.tools.ToolProvider
6+
7+
import scala.jdk.CollectionConverters._
8+
import scala.meta.Input
9+
10+
import munit.FunSuite
11+
12+
/**
13+
* Regression test for https://github.com/sourcegraph/scip-java/issues/861.
14+
*
15+
* When semanticdb-javac encounters a CompletionFailure (e.g. a missing anonymous inner
16+
* class from a Scala-compiled JAR), reportException() was calling reporter.error() which
17+
* uses Diagnostic.Kind.ERROR — causing javac to exit non-zero and fail the build.
18+
*
19+
* The fix changes reportException() to use reporter.warning() (Kind.WARNING) so the build
20+
* always succeeds, with partial semanticdb output and a warning in the compiler output.
21+
*/
22+
class PartialClasspathSuite extends FunSuite with TempDirectories {
23+
24+
val targetroot = new DirectoryFixture()
25+
26+
override def munitFixtures: Seq[Fixture[_]] =
27+
super.munitFixtures ++ List(targetroot)
28+
29+
/**
30+
* Builds an incomplete classpath directory that will trigger a CompletionFailure inside
31+
* semanticdb-javac's override-resolution logic.
32+
*
33+
* SemanticdbVisitor.semanticdbOverrides() walks the supertype chain and calls
34+
* superElement.getEnclosedElements(), which forces javac to complete ALL enclosed types —
35+
* including inner/anonymous classes. This mirrors the production failure in bdc-catalogs
36+
* where DataType$1 (a Scala-compiled anonymous inner class) was not on the Java classpath.
37+
*
38+
* Setup:
39+
* - B has an inner class B.Inner (generates B$Inner.class) and a method doSomething()
40+
* - A extends B and overrides doSomething()
41+
* - B$Inner.class is deleted after compilation
42+
*
43+
* When semanticdb processes a class that extends A and overrides doSomething(), it walks:
44+
* UsesA → A.getEnclosedElements() → B.getEnclosedElements() → tries to complete B$Inner
45+
* → CompletionFailure: class file for pkg.B$Inner not found
46+
*/
47+
private def buildIncompleteClasspath(): java.nio.file.Path = {
48+
val classDir = Files.createTempDirectory("partial-classpath-classes")
49+
val srcDir = Files.createTempDirectory("partial-classpath-sources")
50+
Files.createDirectories(classDir.resolve("pkg"))
51+
Files.createDirectories(srcDir.resolve("pkg"))
52+
53+
// B has an inner class (B$Inner.class will be generated) and an overridable method.
54+
Files.writeString(
55+
srcDir.resolve("pkg/B.java"),
56+
"""|package pkg;
57+
|public class B {
58+
| public static class Inner {}
59+
| public void doSomething() {}
60+
|}
61+
|""".stripMargin
62+
)
63+
// A extends B and overrides the method so the override chain goes through B.
64+
Files.writeString(
65+
srcDir.resolve("pkg/A.java"),
66+
"""|package pkg;
67+
|public class A extends B {
68+
| @Override public void doSomething() {}
69+
|}
70+
|""".stripMargin
71+
)
72+
73+
val javac = ToolProvider.getSystemJavaCompiler
74+
val fm = javac.getStandardFileManager(null, null, null)
75+
fm.setLocation(StandardLocation.CLASS_OUTPUT, List(classDir.toFile).asJava)
76+
val units = fm.getJavaFileObjects(
77+
srcDir.resolve("pkg/B.java").toFile,
78+
srcDir.resolve("pkg/A.java").toFile
79+
)
80+
javac.getTask(null, fm, null, null, null, units).call()
81+
fm.close()
82+
83+
// Delete B$Inner.class — simulates anonymous/inner class from Scala compilation
84+
// that is not on the Java classpath (e.g. DataType$1 in Apache Spark).
85+
Files.delete(classDir.resolve("pkg/B$Inner.class"))
86+
87+
classDir
88+
}
89+
90+
/**
91+
* Triggers an IOException inside writeSemanticdb() by pre-creating the semanticdb output
92+
* directory path as a regular file. When semanticdb-javac tries to call
93+
* Files.createDirectories() on a path that already exists as a file, it throws
94+
* FileAlreadyExistsException (a subtype of IOException). This IOException is caught by the
95+
* writeSemanticdb() catch block, which calls reportException() → reporter.warning().
96+
*
97+
* This tests the core invariant: any internal exception in semanticdb-javac must surface as a
98+
* compiler warning (not error), so the build always succeeds with partial output.
99+
*
100+
* Note: The original test used a missing inner class (B$Inner.class) to trigger
101+
* CompletionFailure. That approach no longer works on Java 21+ because javac handles
102+
* missing inner class files gracefully without throwing to user-facing plugin code.
103+
*/
104+
private def buildTargetrootWithBlockedOutputDir(): java.nio.file.Path = {
105+
val tr = Files.createTempDirectory("semanticdb-javac-blocked")
106+
// The output path for "example/UsesA.java" is:
107+
// tr/META-INF/semanticdb/example/UsesA.java.semanticdb
108+
// We create tr/META-INF/semanticdb/example as a regular FILE (not a dir).
109+
// When SemanticdbTaskListener calls Files.createDirectories(output.getParent()),
110+
// it finds a file at "example" and throws FileAlreadyExistsException.
111+
val semanticdbBase = tr.resolve("META-INF").resolve("semanticdb")
112+
Files.createDirectories(semanticdbBase)
113+
Files.createFile(semanticdbBase.resolve("example")) // regular file, blocks dir creation
114+
tr
115+
}
116+
117+
test("compilation succeeds with warning when semanticdb-javac encounters an internal exception") {
118+
// Use a targetroot where the output directory path is blocked (a file exists where a
119+
// directory is needed). This reliably triggers an IOException in writeSemanticdb(),
120+
// regardless of JDK version.
121+
val blockedTargetroot = buildTargetrootWithBlockedOutputDir()
122+
123+
val compiler = new TestCompiler(
124+
classpath = TestCompiler.PROCESSOR_PATH,
125+
javacOptions = Nil,
126+
scalacOptions = Nil,
127+
targetroot = blockedTargetroot
128+
)
129+
130+
val result = compiler.compileSemanticdb(List(
131+
Input.VirtualFile(
132+
"example/UsesA.java",
133+
"""|package example;
134+
|public class UsesA {
135+
| public void doSomething() {}
136+
|}
137+
|""".stripMargin
138+
)
139+
))
140+
141+
// The build must succeed — semanticdb-javac errors must never fail compilation.
142+
assert(result.isSuccess, s"Expected build success but got:\n${result.stdout}")
143+
144+
// The IOException must surface as a warning, not an error.
145+
assert(
146+
result.stdout.contains("warning:"),
147+
s"Expected a warning in compiler output but got:\n${result.stdout}"
148+
)
149+
assert(
150+
!result.stdout.contains("\nerror:"),
151+
s"Expected no errors in compiler output but got:\n${result.stdout}"
152+
)
153+
}
154+
155+
test("semanticdb files are still produced for healthy files when another file triggers an exception") {
156+
val incompleteClasspath = buildIncompleteClasspath()
157+
158+
val compiler = new TestCompiler(
159+
classpath = incompleteClasspath.toString,
160+
javacOptions = Nil,
161+
scalacOptions = Nil,
162+
targetroot = targetroot()
163+
)
164+
165+
val result = compiler.compileSemanticdb(List(
166+
Input.VirtualFile(
167+
"example/UsesA.java",
168+
"""|package example;
169+
|import pkg.A;
170+
|public class UsesA extends A {
171+
| @Override public void doSomething() {}
172+
|}
173+
|""".stripMargin
174+
),
175+
Input.VirtualFile(
176+
"example/Healthy.java",
177+
"""|package example;
178+
|public class Healthy {
179+
| public String hello() { return "hello"; }
180+
|}
181+
|""".stripMargin
182+
)
183+
))
184+
185+
assert(result.isSuccess, s"Expected build success but got:\n${result.stdout}")
186+
187+
// The healthy file must still produce a semanticdb document.
188+
val docs = result.textDocuments.getDocumentsList.asScala
189+
assert(
190+
docs.exists(_.getUri.contains("Healthy.java")),
191+
s"Expected semanticdb output for Healthy.java, got URIs: ${docs.map(_.getUri).mkString(", ")}"
192+
)
193+
}
194+
}

0 commit comments

Comments
 (0)