From 2fed25a5aa46122d22fd2f9fb46eb077e02376c8 Mon Sep 17 00:00:00 2001 From: Corey Schuman Date: Mon, 5 Jan 2026 21:16:30 -0500 Subject: [PATCH] feat: add GitHub Actions CI and perf:ignore comment support CI/CD: - Add comprehensive GitHub Actions workflow with: - Multi-version Go testing (1.21, 1.22, 1.23) - golangci-lint integration - Self-audit (dogfooding goperf on itself) - Cross-platform binary builds Ignore comments: - Add // perf:ignore for line-level suppression - Add // perf:ignore rule-name for rule-specific suppression - Add // perf:ignore-start / // perf:ignore-end for block suppression - Full test coverage for ignore parsing --- .github/workflows/ci.yml | 92 ++++++++++++++++++++++ README.md | 41 ++++++++++ rules/analyzer.go | 11 ++- rules/ignore_test.go | 143 ++++++++++++++++++++++++++++++++++ rules/types.go | 160 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 446 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/ci.yml create mode 100644 rules/ignore_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..bff2d3f --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,92 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + go-version: ['1.21', '1.22', '1.23'] + + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go-version }} + + - name: Build + run: go build -v ./... + + - name: Test + run: go test -v -race -coverprofile=coverage.out ./... + + - name: Upload coverage + if: matrix.go-version == '1.23' + uses: codecov/codecov-action@v4 + with: + files: ./coverage.out + fail_ci_if_error: false + + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.23' + + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: latest + + self-audit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.23' + + - name: Build goperf + run: go build -o goperf . + + - name: Self-audit (dogfooding) + run: ./goperf --fail-on=critical ./... + + release: + needs: [test, lint, self-audit] + runs-on: ubuntu-latest + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.23' + + - name: Build binaries + run: | + GOOS=linux GOARCH=amd64 go build -o goperf-linux-amd64 . + GOOS=linux GOARCH=arm64 go build -o goperf-linux-arm64 . + GOOS=darwin GOARCH=amd64 go build -o goperf-darwin-amd64 . + GOOS=darwin GOARCH=arm64 go build -o goperf-darwin-arm64 . + GOOS=windows GOARCH=amd64 go build -o goperf-windows-amd64.exe . + + - name: Upload artifacts + uses: actions/upload-artifact@v4 + with: + name: binaries + path: goperf-* diff --git a/README.md b/README.md index 6815ce6..a2f1ca3 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,47 @@ goperf --fail-on=critical ./... | **MEDIUM** | Moderate impact | Should fix | | **LOW** | Minor optimization | Nice to have | +## Ignoring Issues + +Sometimes you need to suppress a warning - perhaps it's a false positive, or you've verified the code is intentional. Use `// perf:ignore` comments: + +### Line-level Ignore + +```go +// perf:ignore +for _, item := range items { + db.Exec(query, item) // This line is ignored +} +``` + +Or on the same line: +```go +db.Exec(query, item) // perf:ignore +``` + +### Ignore Specific Rule + +```go +// perf:ignore sql-in-loop +for _, item := range items { + db.Exec(query, item) // Only sql-in-loop is ignored + result = append(result, item) // Still flagged for append-in-loop +} +``` + +### Block Ignore + +```go +// perf:ignore-start +for _, item := range items { + db.Exec(query, item) +} +for _, other := range others { + db.Query(q, other) +} +// perf:ignore-end +``` + ## Contributing Contributions welcome! Areas we'd love help with: diff --git a/rules/analyzer.go b/rules/analyzer.go index 10a5746..bc636a8 100644 --- a/rules/analyzer.go +++ b/rules/analyzer.go @@ -59,18 +59,27 @@ func (a *Analyzer) Analyze(files []string) []Issue { continue } + // Parse ignore comments + ignoreSet := NewIgnoreSet(src) + // Run all rules for _, rule := range a.rules { issues := rule.Check(file, fset, src) for i := range issues { issues[i].File = filename + + // Skip ignored issues + if ignoreSet.ShouldIgnore(issues[i].Line, issues[i].Rule) { + continue + } + if a.config.Context > 0 { pos := fset.Position(token.Pos(issues[i].Line)) pos.Line = issues[i].Line issues[i].Context = ExtractContext(src, pos, a.config.Context) } + allIssues = append(allIssues, issues[i]) } - allIssues = append(allIssues, issues...) } } diff --git a/rules/ignore_test.go b/rules/ignore_test.go new file mode 100644 index 0000000..0db4297 --- /dev/null +++ b/rules/ignore_test.go @@ -0,0 +1,143 @@ +package rules + +import ( + "testing" +) + +func TestIgnoreSet_LineIgnore(t *testing.T) { + src := []byte(`package main + +func foo() { + // perf:ignore + for _, item := range items { + db.Exec(query, item) // This should be ignored + } +} +`) + is := NewIgnoreSet(src) + + // Line 4 is the comment, line 5 is the for loop - both should be ignored + if !is.ShouldIgnore(4, "sql-in-loop") { + t.Error("Line 4 should be ignored") + } + if !is.ShouldIgnore(5, "sql-in-loop") { + t.Error("Line 5 should be ignored") + } + // Line 6 should NOT be ignored (only next line after comment) + if is.ShouldIgnore(7, "sql-in-loop") { + t.Error("Line 7 should NOT be ignored") + } +} + +func TestIgnoreSet_SameLineIgnore(t *testing.T) { + src := []byte(`package main + +func foo() { + db.Exec(query, item) // perf:ignore +} +`) + is := NewIgnoreSet(src) + + // Line 4 has the ignore comment on the same line + if !is.ShouldIgnore(4, "sql-in-loop") { + t.Error("Line 4 should be ignored (same-line comment)") + } +} + +func TestIgnoreSet_BlockIgnore(t *testing.T) { + src := []byte(`package main + +func foo() { + // perf:ignore-start + for _, item := range items { + db.Exec(query, item) + } + for _, other := range others { + db.Query(q, other) + } + // perf:ignore-end + + // This should NOT be ignored + db.Exec(query, x) +} +`) + is := NewIgnoreSet(src) + + // Lines 4-11 should be ignored (inside block) + for line := 4; line <= 11; line++ { + if !is.ShouldIgnore(line, "sql-in-loop") { + t.Errorf("Line %d should be ignored (inside block)", line) + } + } + + // Line 14 should NOT be ignored (after block) + if is.ShouldIgnore(14, "sql-in-loop") { + t.Error("Line 14 should NOT be ignored (after block)") + } +} + +func TestIgnoreSet_SpecificRule(t *testing.T) { + src := []byte(`package main + +func foo() { + // perf:ignore sql-in-loop + for _, item := range items { + db.Exec(query, item) // ignored + result = append(result, item) // NOT ignored - different rule + } +} +`) + is := NewIgnoreSet(src) + + // sql-in-loop should be ignored + if !is.ShouldIgnore(5, "sql-in-loop") { + t.Error("sql-in-loop should be ignored on line 5") + } + + // append-in-loop should NOT be ignored (different rule) + if is.ShouldIgnore(5, "append-in-loop") { + t.Error("append-in-loop should NOT be ignored on line 5") + } +} + +func TestIgnoreSet_NoIgnore(t *testing.T) { + src := []byte(`package main + +func foo() { + // This is a normal comment + db.Exec(query, item) +} +`) + is := NewIgnoreSet(src) + + if is.ShouldIgnore(5, "sql-in-loop") { + t.Error("Line 5 should NOT be ignored") + } +} + +func TestParseIgnoreComment(t *testing.T) { + tests := []struct { + line string + wantRule string + wantFound bool + }{ + {"// perf:ignore", "", true}, + {"// perf:ignore sql-in-loop", "sql-in-loop", true}, + {" // perf:ignore", "", true}, + {"code() // perf:ignore", "", true}, + {"// perf:ignore-start", "", false}, // Should not match block markers + {"// perf:ignore-end", "", false}, + {"// normal comment", "", false}, + {"no comment", "", false}, + } + + for _, tt := range tests { + rule, found := parseIgnoreComment(tt.line) + if found != tt.wantFound { + t.Errorf("parseIgnoreComment(%q): found = %v, want %v", tt.line, found, tt.wantFound) + } + if rule != tt.wantRule { + t.Errorf("parseIgnoreComment(%q): rule = %q, want %q", tt.line, rule, tt.wantRule) + } + } +} diff --git a/rules/types.go b/rules/types.go index 1dc4807..cb5a755 100644 --- a/rules/types.go +++ b/rules/types.go @@ -103,3 +103,163 @@ func splitLines(src []byte) []string { } return lines } + +// IgnoreSet tracks which lines should be ignored based on perf:ignore comments +type IgnoreSet struct { + lines map[int]bool // Line-level ignores + ranges [][2]int // Start/end ranges for block ignores + rules map[int]string // Optional: specific rules to ignore per line +} + +// NewIgnoreSet parses source code for perf:ignore comments +// Supports: +// - // perf:ignore - ignore the next line or same line +// - // perf:ignore rule-name - ignore specific rule +// - // perf:ignore-start / // perf:ignore-end - block ignore +func NewIgnoreSet(src []byte) *IgnoreSet { + is := &IgnoreSet{ + lines: make(map[int]bool), + rules: make(map[int]string), + } + + lines := splitLines(src) + var blockStart int + inBlock := false + + for i, line := range lines { + lineNum := i + 1 // 1-indexed + + // Check for block markers + if containsIgnoreStart(line) { + inBlock = true + blockStart = lineNum + continue + } + if containsIgnoreEnd(line) && inBlock { + is.ranges = append(is.ranges, [2]int{blockStart, lineNum}) + inBlock = false + continue + } + + // Check for line-level ignore + if rule, hasIgnore := parseIgnoreComment(line); hasIgnore { + // Ignore the current line and the next line + is.lines[lineNum] = true + is.lines[lineNum+1] = true + if rule != "" { + is.rules[lineNum] = rule + is.rules[lineNum+1] = rule + } + } + } + + return is +} + +// ShouldIgnore returns true if the given line should be ignored +func (is *IgnoreSet) ShouldIgnore(line int, rule string) bool { + // Check block ranges + for _, r := range is.ranges { + if line >= r[0] && line <= r[1] { + return true + } + } + + // Check line-level ignores + if is.lines[line] { + // If a specific rule is set, only ignore that rule + if specificRule, ok := is.rules[line]; ok && specificRule != "" { + return rule == specificRule + } + return true + } + + return false +} + +func containsIgnoreStart(line string) bool { + return containsComment(line, "perf:ignore-start") +} + +func containsIgnoreEnd(line string) bool { + return containsComment(line, "perf:ignore-end") +} + +func containsComment(line, marker string) bool { + idx := -1 + for i := 0; i < len(line)-1; i++ { + if line[i] == '/' && line[i+1] == '/' { + idx = i + break + } + } + if idx == -1 { + return false + } + comment := line[idx:] + for i := 0; i < len(comment)-len(marker)+1; i++ { + if comment[i:i+len(marker)] == marker { + return true + } + } + return false +} + +// parseIgnoreComment checks if line has a perf:ignore comment and returns +// the optional rule name to ignore +func parseIgnoreComment(line string) (rule string, hasIgnore bool) { + marker := "perf:ignore" + + idx := -1 + for i := 0; i < len(line)-1; i++ { + if line[i] == '/' && line[i+1] == '/' { + idx = i + break + } + } + if idx == -1 { + return "", false + } + + comment := line[idx+2:] // Skip // + + // Find perf:ignore in comment + markerIdx := -1 + for i := 0; i < len(comment)-len(marker)+1; i++ { + if comment[i:i+len(marker)] == marker { + markerIdx = i + break + } + } + if markerIdx == -1 { + return "", false + } + + // Don't match perf:ignore-start or perf:ignore-end + afterMarker := comment[markerIdx+len(marker):] + if len(afterMarker) > 0 && afterMarker[0] == '-' { + return "", false + } + + // Extract optional rule name + afterMarker = trimLeftSpace(afterMarker) + if len(afterMarker) > 0 { + // Get the rule name (first word after perf:ignore) + end := 0 + for end < len(afterMarker) && afterMarker[end] != ' ' && afterMarker[end] != '\t' && afterMarker[end] != '\n' { + end++ + } + rule = afterMarker[:end] + } + + return rule, true +} + +func trimLeftSpace(s string) string { + for i := 0; i < len(s); i++ { + if s[i] != ' ' && s[i] != '\t' { + return s[i:] + } + } + return "" +}