feat(cli): --strict fails a run whose analysis degraded - #242
Merged
Conversation
A run that loses the RTA overlay or the L4 semantic ddg exits 0 and says so only as a WARN on stderr. Interactively that is fine. In a pipeline it is not: `-a 2 --no-build` on an unbuilt project returns a call graph of declared edges only, and nothing in the exit code or the payload distinguishes it from a complete run. `--strict` turns any such degradation into a non-zero exit that names what was lost, and it fails before writing `analysis.json` so a caller cannot pick up a thin payload believing it is whole. Opt-in, and deliberately so. Degrading is a supported mode, not an oversight: `--no-build` on an unbuilt project still yields the tree, the declared call graph, the syntactic CFG/CDG/DDG and, at -a 4, the SDG vertices, param edges and summaries — everything except the WALA-derived overlays. Nine test call sites depend on that, including the container integration test, whose fixture (`mvnw-corrupt-test`) exists precisely to prove the analyzer survives a project whose build is broken. Defaulting to failure would delete a working mode to fix a reporting problem. Detection needs no changes to ScopeUtils, RtaCallGraph or WalaAnalysis: all three degradations are already visible where they are warned about, so the flag only had to collect them and check once, after every overlay pass has had its chance. The flag is an instance field, not another static, for the reason the `--schema` option already records: the pre-existing static options leak between CommandLine instances in one JVM. README gains the flag under a new FAQ entry, and says plainly that degrading is deliberate and exactly which overlays it costs.
Merged
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.
A run that loses the RTA overlay or the L4 semantic ddg exits 0 and reports the loss only as a
WARNon stderr. Interactively that is fine. In a pipeline it is not —-a 2 --no-buildon an unbuilt project returns a call graph of declared edges only, and nothing in the exit code or the payload distinguishes it from a complete run.--strictturns any such degradation into a non-zero exit that names what was lost, and fails before writinganalysis.jsonso a caller cannot pick up a thin payload believing it is whole.Why opt-in, and not a fix to
--no-buildI started by making
--no-buildreject the combinations that need WALA. That was wrong, and the tests said so:CodeAnalyzerV2CliTest:344compiles a fixture, then runs--l3-engine wala --no-buildand expects full overlays. That is the workflow--no-buildexists for — I already built it — and the analyzer cannot know at parse time whether you did.CodeAnalyzerV2CliTest:519pins-a 4 --no-buildat exit 0, because the L4 vertices,param_in/param_outand summaries are engine-free and still run.CodeAnalyzerIntegrationTest:172runs--analysis-level=2 --no-buildagainstmvnw-corrupt-test, a fixture that exists to prove the analyzer survives a project whose build is broken.Nine test call sites pass
--no-build. Degrading is a supported mode, not an oversight: you still get the tree, the declared call graph, the syntactic CFG/CDG/DDG and, at-a 4, the SDG vertices, param edges and summaries. Everything except the WALA-derived overlays. Defaulting to failure would delete a working mode to fix a reporting problem.So the flag is opt-in, default behaviour is byte-identical to today, and all nine sites stay green untouched.
Implementation
No changes to
ScopeUtils,RtaCallGraphorWalaAnalysis. All three degradations are already visible inCodeAnalyzerwhere they are warned about, so the flag only had to collect them and check once — after every overlay pass has had its chance, before anything is written.strictis an instance field, not another static, for the reason--schemaalready records in this class: the pre-existing static options leak betweenCommandLineinstances in one JVM.Verification
CodeAnalyzerIntegrationTest, no Docker daemon here, fails identically on a cleanmain-a 2 --no-buildon an unbuilt project → exit 0, payload written, unchanged--strict→ exit 1, noanalysis.json, message names the lost overlay-a 1 --no-build --strictFour new tests cover exactly those four cases.
Known blemish
The error surfaces wrapped in a
RuntimeExceptionstack trace, becauseCodeAnalyzer.run()wraps every exception fromanalyze(). That is how all existing parameter errors already surface (--l3-engine,--emit neo4j+-a). Unwrapping would change their exit code from 1 to picocli's 2, which is a behaviour change beyond this fix, so I left it.Propagation verdict
Corrected. An earlier draft of this section said python-sdk vendors a 2.4.1 jar and therefore consumes no 3.x release. That was read off
origin/main, which is still the 1.x line (1.5.0) — the wrong branch.The live 2.0 line,
origin/release/2.0at v2.0.0-rc.3, already consumes this analyzer properly:java = ["codeanalyzer-java==3.1.0"]— a real PyPI extra, installed viapip install "cldk[java]"[tool.backend-versions] codeanalyzer-java = "3.1.0"codeanalyzer.py:213-227callscodeanalyzer_java.command(), so the wheel is the single source of both the jar and the JVMSo the 3.1.0 released tonight is what rc.3 consumes. Two real consequences follow:
==3.1.0), so a 3.1.1 will not reach SDK users without a bump. Shipping--strictmeans python-sdk'srelease/2.0needscodeanalyzer-java==3.1.1in both the extra and[tool.backend-versions]. That is the follow-on item, and it is a lockstep bump, not an optional one.jdk4pyJVM (codeanalyzer.py:215), every python-sdk Java user at-a 2or above hits thejavac-absent path documented in docs: the PyPI install needs a real JDK from -a 2 onwards #241 — and silently, which is exactly what--strictexists to surface. That raises this flag's value for the SDK above what it was worth for the CLI alone.Sibling analyzers: no equivalent bug class — neither codeanalyzer-python nor codeanalyzer-typescript has a build-dependent overlay to lose. Docs: README FAQ updated here.