Simplification#14
Merged
Merged
Conversation
remove version from the help header normalize git changed-file display paths
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR removes strict git-branch mode, adds ChangesGit-Branch Scoping, Version Command, and Java Cognitive Complexity
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant parse_command
participant run_command
participant write_version
CLI->>parse_command: ["--version"]
parse_command-->>CLI: CliCommand::Version
CLI->>run_command: dispatch Version
run_command->>write_version: resolve and write version
write_version-->>CLI: version text
sequenceDiagram
participant run
participant changed_branch_scope
participant changed_lines_against_origin
participant filtered_changed_line_duplicate_blocks
participant filtered_changed_line_complex_functions
run->>changed_branch_scope: build branch scope
changed_branch_scope->>changed_lines_against_origin: load changed line ranges
changed_lines_against_origin-->>changed_branch_scope: ChangedFileLines
run->>filtered_changed_line_duplicate_blocks: filter duplicate blocks by ranges
run->>filtered_changed_line_complex_functions: filter complex functions by ranges
filtered_changed_line_duplicate_blocks-->>run: filtered duplicates
filtered_changed_line_complex_functions-->>run: filtered functions
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/report/complexity_detection.rs (1)
249-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the expected Java cognitive score.
> 1.0only proves the function crossed the threshold; it would still pass if the Java scanner under- or over-counted this fixture. Assert the exact score for this sample to make the regression meaningful.Proposed test tightening
- assert!(functions[0].cognitive_complexity > 1.0); + assert_eq!(functions[0].cognitive_complexity, 4.0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/report/complexity_detection.rs` around lines 249 - 254, The test in detect_complex_functions for the Java fixture is too loose because it only checks that cognitive_complexity exceeds the threshold. Tighten the assertion on the returned function from detect_complex_functions by pinning the exact expected cognitive_complexity for the risky sample, while keeping the existing checks on function_name and cyclomatic_complexity so the regression in complexity scoring is meaningful.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/discovery/git.rs`:
- Around line 463-468: The rejects_non_git_directory test in
changed_lines_against_origin still depends on git being available, so it should
use the same git-availability guard as the other tests in git.rs. Update the
test setup to call the existing guard helper before creating TempGitRepo or
invoking changed_lines_against_origin, and keep the non-repo assertion unchanged
so the test skips cleanly when git is missing.
- Around line 334-338: The path normalization in changed_lines_for_path is
incomplete because only current_dir is canonicalized before strip_prefix, which
can cause mismatched repo_root/current_dir spellings to miss relative paths.
Update the logic around absolute and normalized_current_dir so absolute is
canonicalized as well before stripping the prefix, and keep the fallback
behavior intact in case canonicalization fails. Use the existing
changed_lines_for_path flow and its relative path handling to ensure
display-path matching can attach changed-line ranges correctly.
In `@src/report/java_cognitive.rs`:
- Around line 233-236: The boolean sequence complexity logic in
add_boolean_sequence_complexity currently increments for every standalone &&/||
outside a control header, which overcounts contiguous chains like return a && b
&& c. Update this method to reuse the existing sequence-grouping behavior used
elsewhere in java_cognitive so a run of adjacent boolean operators is counted
once, and keep the check tied to is_inside_condition_header() when applying the
score.
- Around line 239-246: Treat nested parentheses in control headers as part of
the header by updating is_inside_condition_header so it recognizes grouped
subexpressions inside an existing condition header, not just the outermost
control-token-adjacent “(”. Adjust the parenthesis-depth check in this method so
boolean operators inside cases like nested groups under if/while headers are
still considered part of the same header and do not get double-counted by
add_condition_complexity. Use the existing is_inside_condition_header and
add_condition_complexity logic as the main locations to update.
- Around line 189-190: The nesting update in the Java cognitive complexity
matcher is decrementing on every closing brace, even for blocks that did not
increase depth. Adjust the `match` handling around `open_block` and the `"}"`
arm in `src/report/java_cognitive.rs` so only blocks that previously contributed
to `nesting` are decremented, preserving enclosing control-flow depth for nested
plain or `try` blocks.
- Around line 177-190: Update the Java control-flow handling in the
token-processing logic so `pending_nesting_blocks` is only applied to the
intended braceless `if`/`for`/`while`/`switch`/`catch` body and is cleared once
that body is consumed, preventing unrelated `{` tokens in the main parser flow
from inheriting stale nesting. Also adjust the `do`/`while` path in the same
match so `do` queues or tracks its body consistently and the trailing `while` is
treated as part of the same construct rather than being scored independently,
keeping the complexity accounting in `java_cognitive.rs` accurate.
---
Nitpick comments:
In `@src/report/complexity_detection.rs`:
- Around line 249-254: The test in detect_complex_functions for the Java fixture
is too loose because it only checks that cognitive_complexity exceeds the
threshold. Tighten the assertion on the returned function from
detect_complex_functions by pinning the exact expected cognitive_complexity for
the risky sample, while keeping the existing checks on function_name and
cyclomatic_complexity so the regression in complexity scoring is meaningful.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a2c4e0d-d735-4955-821f-87b2a1a9a4a2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
Cargo.tomlREADME.mdsrc/cli/args.rssrc/cli/help.rssrc/cli/mod.rssrc/cli/version.rssrc/discovery/git.rssrc/discovery/mod.rssrc/lib.rssrc/report/complexity_detection.rssrc/report/java_cognitive.rssrc/report/mod.rs
📜 Review details
🔇 Additional comments (12)
src/cli/args.rs (1)
14-47: LGTM!Also applies to: 55-80, 111-133, 258-278, 281-521
src/cli/mod.rs (1)
9-17: LGTM!Also applies to: 27-35, 43-56, 63-82
src/cli/version.rs (1)
9-14: LGTM!src/lib.rs (1)
25-38: LGTM!Also applies to: 67-120, 159-225, 335-442, 720-808, 906-1045
src/cli/help.rs (1)
1-16: LGTM!Also applies to: 37-51, 64-81, 87-155
README.md (1)
77-78: LGTM!Also applies to: 114-116, 133-136
Cargo.toml (1)
3-3: 🗄️ Data Integrity & IntegrationCargo.lock already matches the 0.7.7 bump.
src/discovery/git.rs (1)
271-274: LGTM!Also applies to: 490-503
src/discovery/mod.rs (1)
7-7: LGTM!src/report/mod.rs (1)
5-5: LGTM!src/report/complexity_detection.rs (1)
10-10: LGTM!Also applies to: 57-109
src/report/java_cognitive.rs (1)
5-173: LGTM!Also applies to: 195-231, 256-278
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
1 participant
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 by CodeRabbit
--versionoption and updated CLI help output.-git-branchto clarify line-based behavior.0.7.7.