fix(semanticdb-javac): report analysis exceptions as WARNING instead of ERROR#862
Open
rpalcolea wants to merge 1 commit intosourcegraph:mainfrom
Open
fix(semanticdb-javac): report analysis exceptions as WARNING instead of ERROR#862rpalcolea wants to merge 1 commit intosourcegraph:mainfrom
rpalcolea wants to merge 1 commit intosourcegraph:mainfrom
Conversation
…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 sourcegraph#861 Signed-off-by: Roberto Perez Alcolea <rperezalcolea@netflix.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 updatesSemanticdbTaskListener.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
CompletionFailurefor anonymous inner classes like DataType was breaking compileJava entirely.Fixes #861
Test plan
What we're testing
SemanticdbTaskListener.reportException()previously emittedDiagnostic.Kind.ERROR,causing javac to exit non-zero and fail the build when semanticdb-javac hit an internal
exception. After the fix it emits
Kind.WARNING, so the build succeeds with partialsemanticdb output.
New unit test
PartialClasspathSuite.scalaintests/unit/src/test/scala/tests/.How it triggers
reportException()The test pre-creates the semanticdb output path (
META-INF/semanticdb/example) as aregular file instead of a directory. When
SemanticdbTaskListener.writeSemanticdb()calls
Files.createDirectories(output.getParent()), it throwsFileAlreadyExistsException(a subtype of
IOException). The existingcatch (IOException e)block catches it andcalls
reportException()→reporter.warning()→trees.printMessage(Kind.WARNING, ...).This approach is JDK-version-agnostic. An earlier design attempted to trigger
CompletionFailureby deleting an inner class file from the classpath, but Java 21+handles missing inner class files silently (omitting them rather than throwing), so
that approach was not reliable across JDK versions.
Test 1 — compilation succeeds with a warning
UsesA.javaresult.isSuccess == trueresult.stdout.contains("warning:")!result.stdout.contains("\nerror:")Test 2 — semanticdb still produced for healthy files
UsesA.java(triggers exception) alongsideHealthy.java(clean)result.isSuccess == trueHealthy.javahas a semanticdb document in the outputExisting tests that continue to pass
Full unit suite was run on the branch — all tests pass:
Key suites that exercise compilation behaviour:
OverridesSuite— type resolution across filesJavacClassesDirectorySuite— targetroot outputSnapshotCommandSuite— end-to-end symbol output