Skip to content

fix: Fix exclude pattern matching bug in errstklint#3

Merged
tomoemon merged 1 commit into
mainfrom
fix-exclude-pattern-matching
Nov 9, 2025
Merged

fix: Fix exclude pattern matching bug in errstklint#3
tomoemon merged 1 commit into
mainfrom
fix-exclude-pattern-matching

Conversation

@tomoemon

@tomoemon tomoemon commented Nov 9, 2025

Copy link
Copy Markdown
Owner

Summary

Fixed a critical bug in matchDoubleStarPattern where exclude patterns like **/*.yo.go and **/*.pb.go incorrectly matched files that didn't have those extensions (e.g., usernotification.go was being excluded when it shouldn't be).

Problem

The linter was not detecting functions missing defer errstk.Wrap(&err) because files were being incorrectly excluded by the pattern matching logic.

Root Cause

In the matchDoubleStarPattern function (analyzer.go:267-314):

  1. When the last pattern part didn't match, the function would fall through the if/else chain and continue to the end of the loop
  2. At the end of the loop, it would unconditionally return true (line 313)
  3. This caused patterns like **/*.yo.go to match any .go file, not just *.yo.go files

Example of the Bug

// Pattern: **/*.yo.go
// File: ./infra/persistence/spanner_dao/usernotification.go

// Expected: false (should NOT be excluded)
// Actual: true (incorrectly excluded!)

This meant that normal .go files were being excluded from linting when they matched the base directory pattern but not the file extension pattern.

Solution

  1. Added explicit return false when the last pattern part doesn't match (line 304)
  2. Changed final return from true to false for the edge case where all parts are empty (line 316)
  3. Added comprehensive unit tests including:
    • Tests for shouldExclude() function
    • Tests for matchDoubleStarPattern() function
    • Regression tests for the specific bug cases

Changes

errstklint/analyzer.go:

  • Line 303-304: Added return false when last pattern part doesn't match
  • Line 315-316: Changed final fallthrough return from true to false

errstklint/analyzer_test.go:

  • Added TestShouldExclude with 14 test cases covering various pattern scenarios
  • Added TestMatchDoubleStarPattern with 6 test cases including regression tests
  • Includes specific regression test: "usernotification.go should not match *.yo.go"

Testing

All tests pass:

$ go test -v ./errstklint
=== RUN   TestAnalyzer
--- PASS: TestAnalyzer (0.14s)
=== RUN   TestShouldExclude
--- PASS: TestShouldExclude (0.00s)
=== RUN   TestMatchDoubleStarPattern  
--- PASS: TestMatchDoubleStarPattern (0.00s)
PASS

Impact

After this fix:

  • ✅ Normal .go files are correctly analyzed
  • ✅ Files matching exclude patterns (like *.yo.go, *.pb.go) are correctly excluded
  • ✅ Complex patterns like **/service/api/**/*.go work correctly

🤖 Generated with Claude Code

Fixed a critical bug in matchDoubleStarPattern where patterns like
**/*.yo.go and **/*.pb.go incorrectly matched files that didn't have
those extensions.

The issue was that when the last pattern part didn't match, the
function would fall through and return true instead of false.

Changes:
- Return false when last pattern part doesn't match
- Return false instead of true at end of loop (empty parts case)
- Add comprehensive unit tests for exclude pattern matching
- Add regression tests for the specific bug case

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tomoemon
tomoemon merged commit d9205cd into main Nov 9, 2025
1 check passed
@tomoemon
tomoemon deleted the fix-exclude-pattern-matching branch November 9, 2025 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant