Repository navigation
fix: exit session-construction failures by diagnostic class - #1031
Merged
Merged
Conversation
The check, lookup, and serve wrappers hardcoded exit 2 for every CompilerSession::new / ToolService::new failure, so the process exit disagreed with Diagnostic.class whenever a construction diagnostic carried anything but Usage. CompilerSession::new now classifies lint-rule-error by failure kind: a missing or unreadable --rule-file path (RuleRegistryError::Io) is Usage, while invalid rule content stays SourceError. activate_pinned declares each package failure with the LintPackageError exit class that `lint package` already reports, so a bad pin keeps exit 2 at the session boundary. check and lookup accept --rule-file like lint and serve, and the failure tables pin lint-rule-error on both classes. A CLI test drives missing and invalid rule files through check, lookup, and serve. Fixes #1028
Remove the explanatory headers and local comments that restate the class-to-exit mapping. Code and assertions are unchanged.
Collaborator
Author
|
Round-1 finding fixed in f951bb5: the newly added narrating comments are removed; code and assertions are unchanged. fmt, clippy -D warnings, and the three affected test targets pass. Ready for follow-up review. |
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.
Summary
Fixes #1028 — session-construction diagnostics now drive CLI exits by their declared
Diagnostic.classinstead of a hardcoded usage exit.session/registry.rs:lint-rule-errorsplits byRuleRegistryErrorkind —Io(missing/unreadable--rule-file) isUsage→ exit 2;Rule/Catalog/DuplicateId(invalid content) staysSourceError→ exit 1, per the owner decision's first option since the registry distinguishes the two.lint_package.rs:activate_pinneddeclares eachLintPackageErrorwith its existingexit_class()— the taxonomylint packagealready reports. Without this, honoringDiagnostic.classwould have regressed bad package pins (lint-package-source, a usage error) from exit 2 to exit 1 throughlint/serve.session.rs(new):session::refusereports a construction diagnostic and returnsdiagnostic.class.exit_code();run_configuredandrun_lookupuse it, andserve::runexits by the class directly. No wrapper pins a fixed exit anymore.cli.rs:checkandlookupaccept repeatable--rule-filewired intoSessionConfig.lint_rule_paths, matchinglint/serve— required so all three surfaces reach the construction boundary the same way.lint-rule-errorrows restored and pinned incheck_failures.rs(missing →Usage, invalid →SourceError) andlookup_failures.rs; newtests/session_exits.rsdrives missing and invalid rule files throughcheck,lookup, andserveand asserts exits 2 and 1.main.rs/tests/cli.rswere already over the repository size-guard limit, so the shared refusal path lives insrc/session.rsand the new test intests/session_exits.rs;main.rsnets −6 lines vs HEAD.Verification
cargo fmt --all -- --check— cleancargo clippy --workspace --all-targets --all-features -- -D warnings— cleancargo test --workspace --all-targets --all-features </dev/null— 74 test binaries pass, 0 failures (12mock_providertests ignored; they require theLPP_MOCK_PROVIDERpin)git diff --check— cleancargo test -p wright-cli --test session_exits,-p wright-driver --test check_failures,--test lookup_failures— all passrun_configured's construction exit toexit::USAGE→session_construction_exits_follow_the_diagnostic_classfailed as required; reverted.Unverified
run_configuredonly;run_lookupandserve::runuse the same class-derived path and are covered by the same test, but were not individually mutated.