fix: exit status to 1 when no match#2392
Conversation
WalkthroughThis PR changes many CLI functions to return Result instead of Result<()>, threading ExitCode through public entry points, command handlers, workers, and tests so the process can surface explicit success/failure exit codes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2392 +/- ##
==========================================
+ Coverage 87.78% 87.79% +0.01%
==========================================
Files 107 107
Lines 17302 17327 +25
==========================================
+ Hits 15188 15213 +25
Misses 2114 2114 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/cli/src/scan.rs (2)
275-291: Same inverted exit code issue asScanWithConfig.
ScanStdin::consume_itemsalso returnsExitCode::FAILUREwhenerror_count == 0. This should be consistent with the intended behavior defined forScanWithConfig.
163-180: Fix inverted exit code: return SUCCESS when scan completes without errors.The code currently returns
ExitCode::FAILUREwhenerror_count == 0, which is semantically inverted. Tests confirm that a successful scan with no diagnostic errors should exit with code 0 (SUCCESS), not 1 (FAILURE). This matches standard UNIX conventions where 0 indicates success.if error_count > 0 { Err(anyhow::anyhow!(EC::DiagnosticError(error_count))) } else { - Ok(ExitCode::FAILURE) + Ok(ExitCode::SUCCESS) }
🧹 Nitpick comments (1)
crates/cli/src/utils/worker.rs (1)
58-66: Parse errors are silently converted to exit code without reporting.When
parse_stdinreturnsErr, the error is discarded andExitCode::FAILUREis returned. The user won't know why parsing failed.Consider logging or reporting the error before returning:
fn run_std_in<P: Printer>(&self, printer: P) -> Result<ExitCode> { let source = std::io::read_to_string(std::io::stdin())?; let processor = printer.get_processor(); - if let Ok(items) = self.parse_stdin::<P>(source, &processor) { - self.consume_items(Items::once(items)?, printer) - } else { - Ok(ExitCode::FAILURE) - } + match self.parse_stdin::<P>(source, &processor) { + Ok(items) => self.consume_items(Items::once(items)?, printer), + Err(e) => { + eprintln!("ERROR: {e}"); + Ok(ExitCode::FAILURE) + } + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
crates/cli/src/completions.rs(3 hunks)crates/cli/src/lib.rs(4 hunks)crates/cli/src/main.rs(1 hunks)crates/cli/src/new.rs(12 hunks)crates/cli/src/run.rs(6 hunks)crates/cli/src/scan.rs(7 hunks)crates/cli/src/utils/error_context.rs(2 hunks)crates/cli/src/utils/worker.rs(5 hunks)crates/cli/src/verify.rs(4 hunks)crates/cli/tests/run_test.rs(3 hunks)crates/cli/tests/scan_test.rs(3 hunks)crates/cli/tests/verify_test.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Rust code must be formatted with rustfmt (runcargo fmt --all -- --checkbefore committing)
Rust code must pass Clippy with-D clippy::all(cargo clippy --all-targets --all-features --workspace --release --locked -- -D clippy::all)
Files:
crates/cli/src/verify.rscrates/cli/src/utils/error_context.rscrates/cli/src/new.rscrates/cli/src/completions.rscrates/cli/tests/verify_test.rscrates/cli/src/lib.rscrates/cli/src/run.rscrates/cli/tests/scan_test.rscrates/cli/tests/run_test.rscrates/cli/src/scan.rscrates/cli/src/main.rscrates/cli/src/utils/worker.rs
🧠 Learnings (12)
📓 Common learnings
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/worker.rs:92-96
Timestamp: 2024-09-29T22:02:55.187Z
Learning: The behavior related to path handling in issue https://github.com/ast-grep/ast-grep/issues/1343 is not determined yet, so we should keep the existing code as is.
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/worker.rs:92-96
Timestamp: 2024-10-08T18:41:57.037Z
Learning: The behavior related to path handling in issue https://github.com/ast-grep/ast-grep/issues/1343 is not determined yet, so we should keep the existing code as is.
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/lsp.rs:2-2
Timestamp: 2024-10-08T18:41:57.037Z
Learning: `ErrorContext` is exported from `crates/cli/src/utils/mod.rs` using `pub use`.
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/lsp.rs:2-2
Timestamp: 2024-09-29T21:21:57.113Z
Learning: `ErrorContext` is exported from `crates/cli/src/utils/mod.rs` using `pub use`.
📚 Learning: 2024-10-05T19:26:12.834Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1500
File: crates/cli/src/config.rs:60-60
Timestamp: 2024-10-05T19:26:12.834Z
Learning: When `find_rules` returns `(RuleCollection<SgLang>, RuleStats)`, we should handle both elements and add logging of `RuleStats` in the callers, such as in `crates/cli/src/verify.rs` and `crates/cli/src/lsp.rs`.
Applied to files:
crates/cli/src/verify.rs
📚 Learning: 2024-10-08T18:41:57.037Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/lsp.rs:2-2
Timestamp: 2024-10-08T18:41:57.037Z
Learning: `ErrorContext` is exported from `crates/cli/src/utils/mod.rs` using `pub use`.
Applied to files:
crates/cli/src/utils/error_context.rscrates/cli/src/run.rscrates/cli/src/scan.rs
📚 Learning: 2024-10-22T14:04:15.734Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1546
File: crates/cli/src/utils/rule_overwrite.rs:76-91
Timestamp: 2024-10-22T14:04:15.734Z
Learning: In the `ast-grep-cli` application code, avoid unnecessary abstractions; functions don't need to accept iterators instead of `Vec` if not required.
Applied to files:
crates/cli/tests/verify_test.rscrates/cli/src/run.rscrates/cli/tests/scan_test.rscrates/cli/tests/run_test.rscrates/cli/src/scan.rscrates/cli/src/main.rscrates/cli/src/utils/worker.rs
📚 Learning: 2024-09-29T21:59:50.675Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/mod.rs:174-175
Timestamp: 2024-09-29T21:59:50.675Z
Learning: ast-grep requires Rust 1.65 or newer. It's acceptable to use syntax and features introduced in Rust 1.65 or newer in ast-grep code.
Applied to files:
crates/cli/tests/verify_test.rscrates/cli/src/run.rscrates/cli/tests/run_test.rscrates/cli/src/scan.rscrates/cli/src/utils/worker.rs
📚 Learning: 2024-10-28T03:46:27.850Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1560
File: crates/cli/src/scan.rs:405-407
Timestamp: 2024-10-28T03:46:27.850Z
Learning: `ProjectConfig::setup` returns a `Result<Result<ProjectConfig>>`, so wrapping the result in `Ok()` is unnecessary.
Applied to files:
crates/cli/src/lib.rscrates/cli/src/scan.rs
📚 Learning: 2024-10-28T03:45:43.551Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1560
File: crates/cli/src/lib.rs:0-0
Timestamp: 2024-10-28T03:45:43.551Z
Learning: In the `sg` CLI application, the `-l` or `--lang` command argument depends on the configuration and project being set up, so the configuration cannot be parsed using clap's `App` struct at that point.
Applied to files:
crates/cli/src/lib.rs
📚 Learning: 2025-09-03T04:16:39.630Z
Learnt from: CR
Repo: ast-grep/ast-grep PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T04:16:39.630Z
Learning: Applies to **/*.rs : Rust code must pass Clippy with `-D clippy::all` (`cargo clippy --all-targets --all-features --workspace --release --locked -- -D clippy::all`)
Applied to files:
crates/cli/src/run.rscrates/cli/tests/run_test.rs
📚 Learning: 2024-10-08T18:41:57.037Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/worker.rs:69-77
Timestamp: 2024-10-08T18:41:57.037Z
Learning: In `crates/cli/src/utils/worker.rs`, implementing `From<T>` for `Items<T>` is considered over-abstraction and should be avoided.
Applied to files:
crates/cli/src/run.rscrates/cli/src/scan.rscrates/cli/src/utils/worker.rs
📚 Learning: 2024-11-30T01:59:37.083Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1630
File: crates/core/src/node.rs:710-718
Timestamp: 2024-11-30T01:59:37.083Z
Learning: In the `ast-grep` project, when writing tests for position calculations in `crates/core/src/node.rs`, positions should be calculated based on character counts rather than character widths.
Applied to files:
crates/cli/tests/run_test.rs
📚 Learning: 2024-10-08T18:41:57.037Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 831
File: crates/language/src/bash.rs:1-42
Timestamp: 2024-10-08T18:41:57.037Z
Learning: The user `HerringtonDarkholme` has indicated that the test cases for brace expansion pattern matching in `bash.rs` will be fixed in a separate commit.
Applied to files:
crates/cli/tests/run_test.rs
📚 Learning: 2025-09-03T04:16:39.630Z
Learnt from: CR
Repo: ast-grep/ast-grep PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T04:16:39.630Z
Learning: Applies to crates/napi/** : If touching NAPI (Node.js) bindings, run `yarn test` in crates/napi
Applied to files:
crates/cli/tests/run_test.rs
🧬 Code graph analysis (7)
crates/cli/src/completions.rs (1)
crates/cli/src/lib.rs (1)
run_shell_completion(142-142)
crates/cli/src/lib.rs (1)
crates/cli/src/lsp.rs (1)
run_language_server(31-37)
crates/cli/src/run.rs (2)
crates/cli/src/scan.rs (2)
consume_items(163-180)consume_items(275-291)crates/cli/src/utils/worker.rs (1)
consume_items(22-22)
crates/cli/tests/scan_test.rs (2)
crates/cli/tests/verify_test.rs (1)
sg(60-63)crates/cli/src/scan.rs (1)
create_test_files(376-387)
crates/cli/tests/run_test.rs (3)
crates/cli/src/scan.rs (1)
create_test_files(376-387)crates/core/src/tree_sitter/mod.rs (1)
str(346-348)crates/config/src/rule/selector.rs (1)
is_empty(219-221)
crates/cli/src/main.rs (1)
crates/cli/src/lib.rs (1)
execute_main(68-73)
crates/cli/src/utils/worker.rs (2)
crates/cli/src/run.rs (2)
consume_items(187-202)consume_items(283-301)crates/cli/src/scan.rs (2)
consume_items(163-180)consume_items(275-291)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: coverage
🔇 Additional comments (20)
crates/cli/tests/verify_test.rs (1)
3-4: LGTM! Clean type signature update.The addition of
ExitCodeimport and the updatedsghelper signature correctly align with the broader refactoring to propagate exit codes through the CLI.Also applies to: 60-63
crates/cli/tests/run_test.rs (2)
77-87: LGTM! Clear validation of exit-on-no-match behavior.The new test properly validates that
ast-grep runexits with a non-zero status when the pattern finds no matches, with empty stdout as expected.
129-131: Verify that--inspect=summaryshould fail on no match.These tests now expect
.failure()instead of.success()when running with--inspect=summaryand no matches are found. Since--inspectis a diagnostic/tracing flag, please confirm this is the intended behavior—diagnostic modes sometimes use different exit conventions than regular matching modes.Also applies to: 141-142, 147-148
crates/cli/src/main.rs (1)
1-2: LGTM! Straightforward entry point update.The main function signature correctly updated to propagate
ExitCodefromexecute_main.Also applies to: 6-8
crates/cli/tests/scan_test.rs (2)
3-4: LGTM! Type signature update aligns with ExitCode refactoring.The import and
sghelper signature correctly updated to matchmain_with_argsreturn type.Also applies to: 48-51
402-412: LGTM! Correct validation of scan-with-no-match behavior.This test properly validates that
scansucceeds when no rules match (no violations found = success), which is the appropriate behavior for a linting/scanning tool. This intentionally differs from theruncommand, which fails on no matches.crates/cli/src/utils/error_context.rs (1)
7-7: LGTM! Signature correctly updated for consistency.The
exit_with_errorsignature now returnsResult<ExitCode>to align with the CLI's exit code propagation, even though theclap::ErrorandErrorContextbranches terminate viaexit(). The fallbackErr(error)path properly delegates to anyhow's error reporting.Also applies to: 296-310
crates/cli/src/new.rs (1)
12-12: LGTM! Thorough and consistent ExitCode propagation.All creation flow functions properly updated to return
Result<ExitCode>, with success paths returningExitCode::SUCCESSand error paths maintaining proper error context. The changes cascade cleanly through the entire call chain.Also applies to: 148-154, 156-172, 174-182, 184-196, 198-230, 247-275, 288-319, 331-360
crates/cli/src/lib.rs (1)
14-14: LGTM! Core CLI orchestration correctly refactored.The
execute_mainandmain_with_argsfunctions now properly propagateExitCodethrough all command branches. The LSP command correctly maps itsResult<()>toExitCode::SUCCESSon success.Also applies to: 68-73, 126-146
crates/cli/src/verify.rs (1)
19-19: LGTM! Test verification flow correctly updated.The
run_test_rule_implandrun_test_rulefunctions properly updated to returnResult<ExitCode>, with the success branch returningExitCode::SUCCESSand error branches maintaining appropriate error contexts for test failures and snapshot mismatches.Also applies to: 57-112, 200-215
crates/cli/src/scan.rs (2)
2-2: LGTM!Import of
ExitCodeis appropriate for the new return type semantics.
79-101: LGTM!Function signature correctly updated to return
Result<ExitCode>, and all code paths properly propagate the exit code fromrun_scan.crates/cli/src/completions.rs (1)
34-34: LGTM!The
ExitCodeintegration is correct. ReturningSUCCESSwhen shell completions are generated successfully is the expected behavior.Also applies to: 44-46, 48-62
crates/cli/src/utils/worker.rs (2)
8-8: LGTM!Trait signature updates for
ExitCodepropagation are consistent and well-structured.Also applies to: 22-22, 43-48
110-113: LGTM!Signature update for
run_workeris correct, and exit code propagation fromconsume_itemsis properly handled.crates/cli/src/run.rs (5)
2-2: LGTM!Import of
ExitCodealigns with the new return type requirements.
150-169: LGTM!Function signature correctly updated, and all code paths properly propagate exit codes from
run_pattern_with_printer.
171-179: LGTM!Exit codes are properly propagated from all execution paths (stdin, specific lang, inferred lang).
187-202: LGTM!The
has_matchestracking and exit code logic correctly implements grep-like semantics: exit 0 when matches are found, exit 1 when no matches.
283-301: LGTM!Consistent implementation with
RunWithInferredLang. The pattern error check on line 296-297 is a good safeguard—if the pattern has errors and no matches were found, it reports the error rather than silently returning exit code 1.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cli/src/scan.rs (1)
275-291: Inconsistent exit code with ScanWithConfig.
ScanStdin::consume_itemsreturnsExitCode::FAILUREwhenerror_count == 0(line 289), butScanWithConfig::consume_itemsreturnsExitCode::SUCCESSin the same condition (line 178). Based on the past review discussion, thescancommand should exit with SUCCESS when scanning completes without ERROR-severity violations, regardless of whether it's reading from stdin or files.Apply this diff to align with the established semantics:
let error_count = self.error_count.load(Ordering::Acquire); if error_count > 0 { Err(anyhow::anyhow!(EC::DiagnosticError(error_count))) } else { - Ok(ExitCode::FAILURE) + Ok(ExitCode::SUCCESS) }
🧹 Nitpick comments (1)
crates/cli/src/scan.rs (1)
427-475: Consider verifying actual exit codes in tests.The tests currently only check that
run_with_configreturnsOk(...), but don't verify the actualExitCodevalue. Consider enhancing test coverage to assert the expected exit codes (SUCCESS vs FAILURE) to prevent regressions in exit code semantics.Example enhancement:
let exit_code = run_with_config(arg, project_config).unwrap(); assert_eq!(exit_code, ExitCode::SUCCESS);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/cli/src/scan.rs(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Rust code must be formatted with rustfmt (runcargo fmt --all -- --checkbefore committing)
Rust code must pass Clippy with-D clippy::all(cargo clippy --all-targets --all-features --workspace --release --locked -- -D clippy::all)
Files:
crates/cli/src/scan.rs
🧠 Learnings (16)
📓 Common learnings
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/worker.rs:92-96
Timestamp: 2024-09-29T22:02:55.187Z
Learning: The behavior related to path handling in issue https://github.com/ast-grep/ast-grep/issues/1343 is not determined yet, so we should keep the existing code as is.
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/worker.rs:92-96
Timestamp: 2024-10-08T18:41:57.037Z
Learning: The behavior related to path handling in issue https://github.com/ast-grep/ast-grep/issues/1343 is not determined yet, so we should keep the existing code as is.
📚 Learning: 2024-10-21T07:16:14.836Z
Learnt from: philiptaron
Repo: ast-grep/ast-grep PR: 0
File: :0-0
Timestamp: 2024-10-21T07:16:14.836Z
Learning: In the `ast-grep` repository, the existence of the `sg` binary after the build process is guaranteed, making checks for its existence before installing shell completions unnecessary.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-12-08T20:23:06.671Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1652
File: Cargo.toml:35-38
Timestamp: 2024-12-08T20:23:06.671Z
Learning: In the ast-grep project, `tree-sitter-native` is not updated because `tree-sitter-facade-sg` requires a special patch update.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2025-07-27T19:36:20.176Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 2126
File: schemas/css_rule.json:571-572
Timestamp: 2025-07-27T19:36:20.176Z
Learning: Schema files in ast-grep (like schemas/css_rule.json) should not be manually edited since they are machine-generated. Any issues found in these files need to be addressed in the code generation logic, not through direct file modifications.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-10-22T14:04:15.734Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1546
File: crates/cli/src/utils/rule_overwrite.rs:76-91
Timestamp: 2024-10-22T14:04:15.734Z
Learning: In the `ast-grep-cli` application code, avoid unnecessary abstractions; functions don't need to accept iterators instead of `Vec` if not required.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2025-09-03T04:16:39.630Z
Learnt from: CR
Repo: ast-grep/ast-grep PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T04:16:39.630Z
Learning: Applies to **/sgconfig.yml : Use sgconfig.yml as the configuration file name for configuration-based ast-grep scans
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-09-29T22:02:55.187Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/worker.rs:92-96
Timestamp: 2024-09-29T22:02:55.187Z
Learning: The behavior related to path handling in issue https://github.com/ast-grep/ast-grep/issues/1343 is not determined yet, so we should keep the existing code as is.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-10-22T13:56:44.052Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1546
File: crates/cli/src/utils/rule_overwrite.rs:118-121
Timestamp: 2024-10-22T13:56:44.052Z
Learning: The `OverwriteResult` struct in `crates/cli/src/utils/rule_overwrite.rs` will have more override options added in the future, so it should be retained even if it currently only contains an `Option<Severity>`.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2025-07-27T19:35:43.279Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 2126
File: schemas/elixir_rule.json:150-200
Timestamp: 2025-07-27T19:35:43.279Z
Learning: Schema files in the ast-grep project (like schemas/elixir_rule.json) are machine-generated, so duplication and manual maintenance concerns like DRY principles don't apply since humans don't manually edit these files.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-10-05T19:26:12.834Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1500
File: crates/cli/src/config.rs:60-60
Timestamp: 2024-10-05T19:26:12.834Z
Learning: When `find_rules` returns `(RuleCollection<SgLang>, RuleStats)`, we should handle both elements and add logging of `RuleStats` in the callers, such as in `crates/cli/src/verify.rs` and `crates/cli/src/lsp.rs`.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-10-08T18:41:57.037Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1500
File: crates/cli/src/config.rs:60-60
Timestamp: 2024-10-08T18:41:57.037Z
Learning: In the `ast-grep` codebase, it's acceptable for some callers of `find_rules` to intentionally ignore the `RuleStats` returned in its tuple `(RuleCollection<SgLang>, RuleStats)` when the statistics are not needed.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2025-04-27T20:33:36.294Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1957
File: crates/core/src/node.rs:360-365
Timestamp: 2025-04-27T20:33:36.294Z
Learning: In ast-grep, the `field_children` method accepts an `Option<u16>` as its field_id parameter, allowing for direct passing of the result from `field_to_id` without unwrapping.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-09-29T21:21:57.113Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/lsp.rs:2-2
Timestamp: 2024-09-29T21:21:57.113Z
Learning: `ErrorContext` is exported from `crates/cli/src/utils/mod.rs` using `pub use`.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-09-29T21:59:50.675Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/mod.rs:174-175
Timestamp: 2024-09-29T21:59:50.675Z
Learning: ast-grep requires Rust 1.65 or newer. It's acceptable to use syntax and features introduced in Rust 1.65 or newer in ast-grep code.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-10-28T03:46:27.850Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1560
File: crates/cli/src/scan.rs:405-407
Timestamp: 2024-10-28T03:46:27.850Z
Learning: `ProjectConfig::setup` returns a `Result<Result<ProjectConfig>>`, so wrapping the result in `Ok()` is unnecessary.
Applied to files:
crates/cli/src/scan.rs
📚 Learning: 2024-09-29T21:58:00.508Z
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 1497
File: crates/cli/src/utils/worker.rs:69-77
Timestamp: 2024-09-29T21:58:00.508Z
Learning: In `crates/cli/src/utils/worker.rs`, implementing `From<T>` for `Items<T>` is considered over-abstraction and should be avoided.
Applied to files:
crates/cli/src/scan.rs
🧬 Code graph analysis (1)
crates/cli/src/scan.rs (2)
crates/cli/src/utils/worker.rs (1)
consume_items(22-22)crates/cli/src/run.rs (2)
consume_items(187-202)consume_items(283-301)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: coverage
🔇 Additional comments (3)
crates/cli/src/scan.rs (3)
2-2: LGTM: Required import for ExitCode propagation.The import is necessary for the new return type signatures throughout the file.
79-79: LGTM: Function signatures updated to propagate ExitCode.The signature changes for
run_with_configandrun_scancorrectly align with the PR objective to propagate explicit exit codes through the CLI command handlers.Also applies to: 108-108
163-180: LGTM: ScanWithConfig exit code logic is correct.The implementation correctly returns
ExitCode::SUCCESSwhen scanning completes without ERROR-severity rule violations, and propagates errors appropriately. This aligns with the agreed-upon semantics from the previous review discussion.
|
Thank you 🫡 |
close #2388
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.