Config, Outsider finder, Scope parser#5
Conversation
|
Warning Review limit reached
More reviews will be available in 27 minutes and 7 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR refactors the validator to load scope-validation configuration from ChangesConfiguration-driven scope validation
Sequence DiagramsequenceDiagram
participant CLI as CLI Command
participant LoadCfg as LoadConfig()
participant Parser as DefaultScopeParser
participant Finder as DefaultOutsiderFinder
participant Validator
CLI->>LoadCfg: Call LoadConfig()
LoadCfg-->>CLI: Return Config{ScopeRegex, Patterns}
CLI->>Validator: NewValidator(Config, Options)
Validator->>Parser: NewDefaultScopeParser(config.ScopeRegex)
Validator->>Finder: NewDefaultOutsiderFinder(config.Patterns)
CLI->>Validator: Validate(commit)
Validator->>Parser: Parse(message)
Parser-->>Validator: scope string
Validator->>Finder: Find(scope, files)
Finder-->>Validator: []Outsider
Validator-->>CLI: Violation{Scopes, Outsiders}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@pkg/validator/config.go`:
- Around line 42-44: The regexp decode hook (regexDecode) currently panics on a
type mismatch; change it to return a typed error instead so v.Unmarshal(&cfg,
regexDecode) can propagate it (avoid crashing the CLI). Locate the decode hook
implementation in pkg/validator/config.go (the function referenced as
regexDecode used by v.Unmarshal) and replace the panic(fmt.Sprintf("expected
string but got %T", data)) with returning an error (e.g., fmt.Errorf("regex
decode: expected string but got %T", data)) following the decode hook's error
return convention so callers receive a normal error value rather than a panic.
In `@pkg/validator/outsider_finder.go`:
- Line 21: Replace uses of glob.MustCompile in outsider_finder.go with the
non-panicking glob.Compile and propagate/handle errors so invalid glob patterns
produce a validation error instead of panicking; specifically, in the code that
builds the globs slice (the line using glob.MustCompile(p, '/')) and the other
MustCompile occurrence referenced in the comment, call glob.Compile(p, '/') (or
glob.Compile(scope+"/**", '/')) and if Compile returns an error convert it into
the appropriate validation error return path used by this package (same style as
other validation errors in outsider_finder.go), rather than allowing a panic to
occur.
In `@pkg/validator/scope_parser_test.go`:
- Around line 21-22: Remove the duplicated table-driven test entry named "match
with scope" in scope_parser_test.go so only one case with regex:
compile(`^[a-z]+(?:\((?P<scope>[^)]+)\))?!?:\s`), message: "feat(api): add
endpoint", want: "api" remains; locate the test cases slice in the Test...
function (the table containing entries with name/regex/message/want) and delete
the redundant second occurrence to avoid running the same subtest twice.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5beeea5d-7ee1-485a-96a1-94a817b56824
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
cmd/root.gogo.modpkg/validator/config.gopkg/validator/git.gopkg/validator/mocks_test.gopkg/validator/outsider_finder.gopkg/validator/outsider_finder_test.gopkg/validator/scope_parser.gopkg/validator/scope_parser_test.gopkg/validator/validator.gopkg/validator/validator_test.go
| for scope, pats := range patterns { | ||
| globs := make([]glob.Glob, 0, len(pats)) | ||
| for _, p := range pats { | ||
| globs = append(globs, glob.MustCompile(p, '/')) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the outsider finder implementation and nearby code
FILE="pkg/validator/outsider_finder.go"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,200p'
echo "== End =="
# Find all MustCompile usages in the repo for context
echo
echo "== Searching for glob.MustCompile usage =="
rg -n "glob\.MustCompile" -S . || true
# Identify the glob package import in the file
echo
echo "== Imports in outsider_finder.go =="
rg -n "import|github\.com|glob" "$FILE" || trueRepository: thumbrise/commitlint-scope
Length of output: 177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="pkg/validator/outsider_finder.go"
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,220p'
echo
echo "== Search for glob.MustCompile usage =="
rg -n "glob\.MustCompile" -S .
echo
echo "== Search for glob\.Compile usage =="
rg -n "glob\.Compile" -S . || trueRepository: thumbrise/commitlint-scope
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="pkg/validator/outsider_finder.go"
nl -ba "$FILE" | sed -n '1,220p'Repository: thumbrise/commitlint-scope
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="pkg/validator/outsider_finder.go"
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,260p'
echo
echo "== Search for glob.MustCompile usage =="
rg -n "glob\.MustCompile" -S . || true
echo
echo "== Search for glob\.Compile usage =="
rg -n "glob\.Compile" -S . || true
echo
echo "== Imports in $FILE =="
sed -n '1,80p' "$FILE"Repository: thumbrise/commitlint-scope
Length of output: 176
Fix panic risk from glob.MustCompile in outsider finder.
glob.MustCompile is used to compile both configured glob patterns and a fallback pattern (scope + "/**"). Since MustCompile panics on invalid glob syntax, malformed config input or an unexpected scope value can crash validation instead of returning a validation error.
Also apply the same non-panicking glob.Compile/error-path handling to the other MustCompile occurrence at the referenced second location.
🤖 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 `@pkg/validator/outsider_finder.go` at line 21, Replace uses of
glob.MustCompile in outsider_finder.go with the non-panicking glob.Compile and
propagate/handle errors so invalid glob patterns produce a validation error
instead of panicking; specifically, in the code that builds the globs slice (the
line using glob.MustCompile(p, '/')) and the other MustCompile occurrence
referenced in the comment, call glob.Compile(p, '/') (or
glob.Compile(scope+"/**", '/')) and if Compile returns an error convert it into
the appropriate validation error return path used by this package (same style as
other validation errors in outsider_finder.go), rather than allowing a panic to
occur.
|
🎉 This PR is included in version 0.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary by CodeRabbit
New Features
.commitlint-scopefile to customize scope validation patterns and regex.Bug Fixes
Refactor