Skip to content

feat(rules): add SLP223-227, retire noise rules, consolidate error rules - #92

Merged
messagesgoel-blip merged 2 commits into
mainfrom
feat/slp217-slp218-tweaks-slp223-227
Jun 20, 2026
Merged

feat(rules): add SLP223-227, retire noise rules, consolidate error rules#92
messagesgoel-blip merged 2 commits into
mainfrom
feat/slp217-slp218-tweaks-slp223-227

Conversation

@messagesgoel-blip

@messagesgoel-blip messagesgoel-blip commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

User description

Summary

  • SLP217: Narrow to exported/public Go functions; fix module.exports arrow function capture (was silently ignored)
  • SLP218: Extend to detect url.URL{Scheme:"file", Opaque: path} misuse
  • SLP223 (new): Ignored error returns (Close/Remove/MarkRun* etc.) with safe-defer excludes
  • SLP224 (new): HTTP handlers reading r.Body without ContentLength/TransferEncoding/decode-error guards
  • SLP225 (new): Goroutines writing shared state without visible sync guard
  • SLP226 (new): SQL rows/stmt missing defer Close(); BEGIN without matching COMMIT/ROLLBACK
  • SLP227 (new): String literal repeated 3+ times in a hunk (excludes <6-char and HTTP methods)
  • Retire SLP043/SLP050/SLP055 with //go:build ignore_retired build tag after Whimsy benchmark confirmed 95%+ noise
  • Consolidate SLP044/SLP065/SLP114/SLP120 into SLP223 (all were ignored-error variants)
  • Narrow SLP227 to skip strings <6 chars and HTTP method literals

Registry

169 → 162 rules. RETIRED.md added documenting all curation decisions.

Test plan

  • go test ./pkg/rules/... — all pass
  • go vet ./pkg/rules/... — clean
  • Pre-commit AI gate — passed
  • Pre-push full test suite — passed
  • Whimsy 10-PR benchmark: 5.3% overlap on reviewer-only findings (was 1.9%)

Summary by Sourcery

Introduce new safety and quality rules (SLP223-227), refine existing HTTP and path validation rules (SLP217-218, SLP227), and curate the rule registry by retiring noisy rules and documenting these decisions.

New Features:

  • Add SLP223 rule to flag ignored error returns in Go code with safe defer patterns excluded
  • Add SLP224 rule to flag HTTP handlers that read request bodies without size or error handling guards
  • Add SLP225 rule to detect goroutines mutating shared state without visible synchronization
  • Add SLP226 rule to detect SQL rows/statements without Close and transactions without Commit/Rollback
  • Add SLP227 rule to detect repeated non-trivial string literals within a hunk
  • Extend SLP217 to only target exported/public Go and JS/TS functions and to correctly handle module.exports arrow functions
  • Extend SLP218 to also detect misuse of url.URL with file scheme and path-like data in Opaque

Bug Fixes:

  • Fix SLP217 JS/TS parameter validation detection to handle exported functions and module.exports arrows accurately
  • Refine SLP218 to avoid false positives when TransferEncoding is handled or file URLs use Path correctly
  • Narrow SLP227 to ignore short/trivial literals, HTTP methods, tests, generated code, and OpenAPI artifacts

Enhancements:

  • Update rule registry to register new SLP223-227 rules and remove retired/consolidated ones
  • Add RETIRED.md documenting rule retirement and consolidation policy and specific curated rules

Tests:

  • Add unit tests for updated SLP217 and SLP218 behaviors
  • Add dedicated tests covering new rules SLP223, SLP224, SLP225, SLP226, and SLP227
  • Adjust registry tests to enforce the updated rule set and count

CodeAnt-AI Description

Add new safety rules and retire noisy ones

What Changed

  • Added checks for ignored errors, missing HTTP body validation, unsafe shared-state writes in goroutines, unclosed SQL resources, and repeated string literals in new code
  • Tightened existing path validation so it now applies to exported/public functions and catches file URLs built with path data in the wrong field
  • Limited an HTTP request-body rule to cases that actually handle request data, and added coverage for chunked requests and file URL handling
  • Removed several noisy rules from the default set, moved them behind an opt-in build tag, and documented the rule curation decisions

Impact

✅ Fewer missed error handling bugs
✅ Clearer HTTP request validation
✅ Fewer noisy rule matches in default scans

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

…arrow SLP217/218/227

- Narrow SLP217 to exported/public functions (Go + JS module.exports arrow)
- Extend SLP218 to catch url.URL{Scheme:"file", Opaque: path} misuse
- Add SLP223 (ignored error returns with safe-defer excludes)
- Add SLP224 (HTTP handlers missing body/param validation)
- Add SLP225 (goroutines mutating shared state without sync guard)
- Add SLP226 (SQL rows/stmt missing defer Close, BEGIN without COMMIT)
- Add SLP227 (string literal repeated 3+ times in a hunk)
- Retire SLP043/SLP050/SLP055 with //go:build ignore_retired tag (high noise)
- Consolidate SLP044/SLP065/SLP114/SLP120 into SLP223 (delete source files)
- Narrow SLP227 to exclude strings <6 chars and HTTP method literals
- Add RETIRED.md curation log documenting decisions
- Update registry: 169 → 162 rules
- Fix SLP217 module.exports arrow capture group being silently ignored
@cr-gpt

cr-gpt Bot commented Jun 20, 2026

Copy link
Copy Markdown

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@codeant-ai

codeant-ai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@sourcery-ai

sourcery-ai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds four new reviewer-gap rules (SLP223–SLP227), tightens existing rules SLP217/SLP218, and curates the registry by retiring high-noise rules and consolidating older ignored-error rules into the new SLP223, with tests and documentation updated accordingly.

File-Level Changes

Change Details Files
Tighten SLP217/SLP218 heuristics for exported functions and HTTP/file URL handling, with expanded tests.
  • Restrict SLP217 to exported/public Go and JS/TS functions by updating Go and JS regexes and only collecting parameter lists for exported names.
  • Improve SLP217 parameter-validation detection to correctly recognize JavaScript truthy checks like if (!sourceRoot) and module.exports arrow functions.
  • Extend SLP218 to also detect url.URL{Scheme:"file", Opaque: path} misuse via new regexes for file URLs and path-like opaque values.
  • Refine SLP218 ContentLength/TransferEncoding logic to only flag true body-present gates and ignore non-gating comparisons (<=0, ==-1, ==0).
  • Rewrite SLP217 and SLP218 tests to cover new exported/unexported cases, JS module exports, ContentLength/TransferEncoding guards, and file URL path-vs-opaque behavior.
pkg/rules/slp217.go
pkg/rules/slp217_test.go
pkg/rules/slp218.go
pkg/rules/slp218_test.go
Introduce SLP223 to flag ignored error returns with safe defer-aware exclusions.
  • Implement SLP223.Check to scan added Go lines for _ = <call>(...) patterns using regexes, skipping tests/docs and known safe encoder calls in deferred functions.
  • Add helper functions and regexes to detect ignored error calls, safe calls (e.g., json.NewEncoder(...).Encode), and whether a line is inside a deferred anonymous function.
  • Create focused tests for SLP223 covering ignored Close/Mkdir/rows.Close patterns, safe deferred encoders, and correctly handled errors.
pkg/rules/slp223.go
pkg/rules/slp223_test.go
Introduce SLP224 to flag HTTP handlers that read request bodies without size or error guards.
  • Implement SLP224.Check to identify Go HTTP handlers (functions taking *http.Request) and body reads/decodes within their hunks.
  • Use regex-based detection of ContentLength/TransferEncoding guards and explicit decode-error handling; emit findings only when body reads lack both.
  • Add tests that verify SLP224 flags unguarded json.NewDecoder(r.Body).Decode and allows handlers with ContentLength/TransferEncoding or explicit error handling, while ignoring non-http.Request helpers.
pkg/rules/slp224.go
pkg/rules/slp224_test.go
Introduce SLP225 to detect goroutines mutating shared state without synchronization.
  • Implement SLP225.Check to scan added Go hunks for anonymous goroutines and detect map/field/package-level writes inside them.
  • Add regexes to detect goroutine starts, map/index assignments, field writes, package/global variable writes, and synchronization constructs (mutexes, atomics, channels, WaitGroups).
  • Implement naive brace-based goroutine block parsing (goroutineEnd) and helper to collect added lines per hunk.
  • Add tests covering unsafe goroutine writes to maps and struct fields, and safe cases guarded by mutexes, atomics, or no writes.
pkg/rules/slp225.go
pkg/rules/slp225_test.go
Introduce SLP226 to detect SQL rows/stmt resources and transactions without matching Close/Commit/Rollback.
  • Implement SLP226.Check to scan added Go hunks for sql-like variable assignments (rows, stmt, tx) and ensure matching defer Close or Commit/Rollback within the hunk.
  • Add regexes to capture SQL variable declarations (Query/Prepare/Begin/BeginTx), and helpers to collect variable names, format Close/txn-close patterns, and distinguish transactions from rows/stmts.
  • Create tests that exercise missing defer rows.Close, missing stmt.Close, unbalanced transactions, and correctly balanced tx with defer-rollback/commit closures.
pkg/rules/slp226.go
pkg/rules/slp226_test.go
Introduce SLP227 to flag repeated non-trivial string literals in a hunk, with aggressive narrowing to avoid noise.
  • Implement SLP227.Check to scan added lines for repeated string literals using regex, counting per-literal occurrences and emitting findings when a literal appears 3+ times.
  • Define filtering logic (trivial and trivialLiteralRe) to skip trivial literals, short strings (<6 chars), numeric/punctuation-only strings, HTTP method names, tests/docs/generated/OpenAPI files, and comments.
  • Add tests to verify detection of repeated meaningful strings, and that trivial strings, format verbs, OpenAPI/test/generated files, and two-copy cases are not flagged.
pkg/rules/slp227.go
pkg/rules/slp227_test.go
Curate rule registry by retiring high-noise rules and registering new SLP223–SLP227, with updated tests and documentation.
  • Remove SLP043, SLP050, SLP055, SLP065, SLP114, and SLP120 from the default registry and associated registry tests, treating SLP043/050/055 as retired via build tags and deleting consolidated rules.
  • Register new rules SLP223–SLP227 in registry.Default and extend registry tests to assert their presence and to update total rule counts from 164 to 162.
  • Add RETIRED.md documenting retirement/consolidation policy and specific curation decisions, including benchmarks, noise rationale, and how to re-enable retired rules with build tags.
pkg/rules/registry.go
pkg/rules/registry_test.go
pkg/rules/slp043.go
pkg/rules/slp050.go
pkg/rules/slp050_test.go
pkg/rules/slp055.go
pkg/rules/slp055_test.go
pkg/rules/slp044.go
pkg/rules/slp065.go
pkg/rules/slp065_test.go
pkg/rules/slp114.go
pkg/rules/slp114_test.go
pkg/rules/slp120.go
pkg/rules/slp120_test.go
pkg/rules/RETIRED.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@messagesgoel-blip, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 minute and 38 seconds. Learn how PR review limits work.

To continue reviewing without waiting, enable usage-based billing in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9475d666-68b0-476d-bafb-8e5043166b42

📥 Commits

Reviewing files that changed from the base of the PR and between 3f291c3 and 9fe28b7.

📒 Files selected for processing (29)
  • pkg/rules/RETIRED.md
  • pkg/rules/registry.go
  • pkg/rules/registry_test.go
  • pkg/rules/slp043.go
  • pkg/rules/slp044.go
  • pkg/rules/slp050.go
  • pkg/rules/slp050_test.go
  • pkg/rules/slp055.go
  • pkg/rules/slp055_test.go
  • pkg/rules/slp065.go
  • pkg/rules/slp065_test.go
  • pkg/rules/slp114.go
  • pkg/rules/slp114_test.go
  • pkg/rules/slp120.go
  • pkg/rules/slp120_test.go
  • pkg/rules/slp217.go
  • pkg/rules/slp217_test.go
  • pkg/rules/slp218.go
  • pkg/rules/slp218_test.go
  • pkg/rules/slp223.go
  • pkg/rules/slp223_test.go
  • pkg/rules/slp224.go
  • pkg/rules/slp224_test.go
  • pkg/rules/slp225.go
  • pkg/rules/slp225_test.go
  • pkg/rules/slp226.go
  • pkg/rules/slp226_test.go
  • pkg/rules/slp227.go
  • pkg/rules/slp227_test.go

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Jun 20, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • SLP224's handlerSigRe only matches top-level functions and misses common method receivers like func (s *Server) Handler(w http.ResponseWriter, r *http.Request), so handlers implemented as methods won't be checked; consider extending the regex to allow an optional receiver before the function name.
  • SLP225's goroutineStartRe only covers go func() with no parameters and will miss idiomatic patterns like go func(ctx context.Context) { ... }(ctx); broadening the matcher to allow parameter lists and immediate calls would catch more real-world goroutines.
  • In SLP226, findings always use added[0].NewLineNo and the first added line as the snippet, which can point far from the actual rows/stmt/tx declaration; consider tracking the specific line where each variable is introduced (e.g., via the regex match position) to produce more precise file/line/snippet information.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- SLP224's `handlerSigRe` only matches top-level functions and misses common method receivers like `func (s *Server) Handler(w http.ResponseWriter, r *http.Request)`, so handlers implemented as methods won't be checked; consider extending the regex to allow an optional receiver before the function name.
- SLP225's `goroutineStartRe` only covers `go func()` with no parameters and will miss idiomatic patterns like `go func(ctx context.Context) { ... }(ctx)`; broadening the matcher to allow parameter lists and immediate calls would catch more real-world goroutines.
- In SLP226, findings always use `added[0].NewLineNo` and the first added line as the snippet, which can point far from the actual `rows/stmt/tx` declaration; consider tracking the specific line where each variable is introduced (e.g., via the regex match position) to produce more precise file/line/snippet information.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request retires and consolidates several rules (such as SLP044, SLP065, SLP114, and SLP120) into a new documentation file RETIRED.md and introduces five new rules (SLP223 through SLP227) targeting ignored errors, unvalidated HTTP handlers, unsynchronized goroutines, SQL resource leaks, and repeated string literals. It also refactors SLP217 and SLP218. Feedback on the changes highlights a correctness bug in the backward brace counting of inDeferredFunc (SLP223), a bug in goroutineEnd (SLP225) that fails to track simple goroutines, false positives in SLP226 due to flagging unclosable QueryRow results, and a performance issue in SLP217 caused by compiling a regular expression inside a loop.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread pkg/rules/slp223.go
Comment thread pkg/rules/slp225.go
Comment thread pkg/rules/slp226.go Outdated
Comment thread pkg/rules/slp217.go
Comment thread pkg/rules/slp225.go Outdated
Comment thread pkg/rules/slp225.go Outdated
Comment thread pkg/rules/slp226.go Outdated
Comment thread pkg/rules/slp226.go Outdated
Comment thread pkg/rules/slp218.go Outdated
Comment thread pkg/rules/slp224.go Outdated
Comment thread pkg/rules/slp224.go
Comment thread pkg/rules/slp224.go
- slp223: rewrite inDeferredFunc with forward-depth stack
- slp225: rewrite goroutineEnd; per-goroutine sync guard; handle single-line goroutines
- slp226: remove QueryRow from sqlVarRe; scan only added lines
- slp224: add receiver group to handlerSigRe; scan full hunk for guards
- slp217: compile validation regex once per call, not per-line
- slp218: replace string.Contains with word-boundary regex for non-gating checks
@cr-gpt

cr-gpt Bot commented Jun 20, 2026

Copy link
Copy Markdown

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

@messagesgoel-blip
messagesgoel-blip merged commit 4837722 into main Jun 20, 2026
2 checks passed
messagesgoel-blip pushed a commit that referenced this pull request Jun 20, 2026
- README: 164 → 162 rules, SLP210-222 extended to SLP210-227
- CHANGELOG: v0.0.26 entry (rule curation + PR #92 bug fixes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant