Repository navigation
Map Any Unexpected Compile Lint Failure to Exit 2 - #267
Conversation
ptr727
added
the
comments
|
A recorded local strict-review pass covers head |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 13 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #267 +/- ##
===========================================
+ Coverage 84.36% 84.61% +0.25%
===========================================
Files 3 3
Lines 614 624 +10
===========================================
+ Hits 518 528 +10
Misses 96 96
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The exit-code fix is sound, but the catch-all comment is inaccurate and the new tests leave the unreadable-file and invalid-JSON paths unverified.
4 open issues
What changed in this PR
This PR closes a gap in the compile lint. An unexpected failure used to escape as a Python exception, which exits 1. Exit 1 means a finding, so a crash looked like a lint result. Now main catches any non-LintError exception, prints its traceback, and exits 2. load_compile_database also turns an unreadable or invalid-JSON database into a LintError, since an interrupted build can leave the file truncated.
Changes:
mainnow delegates to a newrunfunction. ALintErrorprints a message and exits 2. Any other exception prints its traceback and exits 2.load_compile_databasewraps read and parse errors in aLintErrorwith a "cannot read the compile database" message.- New tests check that malformed databases exit 2 and that an unexpected failure keeps its traceback.
| File | Description |
|---|---|
| tools/compile-lint/src/compile_lint/lint.py | Adds the catch-all exit-2 path in main, moves the run body into run, and makes the compile database read raise LintError. |
| tests/compile_lint/test_lint.py | Adds tests for malformed compile databases and for the traceback on an unexpected failure. |
Review effort: Lite
| print(f"compile-lint: {error}", file=sys.stderr) | ||
| return 2 | ||
| except Exception as error: # noqa: BLE001 | ||
| # Any other failure is a bug in the lint, so keep its stack, and never exit 1, a finding. |
| # Invalid JSON is caught where it is read, and the other shapes by main's catch-all. | ||
| @pytest.mark.parametrize("content", ["{not json", '{"directory": "/x"}', '[{"file": "a.cpp"}]']) | ||
| def test_a_malformed_compile_database_is_a_failure_to_run(tmp_path: Path, content: str) -> None: | ||
| root, build = write_build(tmp_path, "") | ||
| (build / "build" / "compile_commands.json").write_text(content) | ||
| assert main(["--config-root", str(root), str(build)]) == 2 |
| except (OSError, ValueError) as error: | ||
| # An interrupted build can leave the file truncated. | ||
| raise LintError(f"cannot read the compile database {path}: {error}") from error |
| except LintError as error: | ||
| print(f"compile-lint: {error}", file=sys.stderr) | ||
| return 2 | ||
| except Exception as error: # noqa: BLE001 |
Fixes a finding the hosted review raised on the develop -> main promotion pull request: an unexpected failure in the compile lint escaped as a Python exception, which exits 1, the code the lint documents for a finding.
mainmaps any exception other than the lint's ownLintErrorto exit 2, and prints its traceback first, since such a failure is a bug in the lint. This closes the class at every site, including a truncated compile database, an unreadable source, and an unbalanced quote in a compile command.load_compile_databasestill turns an unreadable file or invalid JSON into aLintError, since an interrupted build can leave the file truncated.Unit tests, ruff, pyright and the prose gate pass. The new tests fail without the change.
🤖 Generated with Claude Code