Improve overall performance, better test coverage

Author: jgautheron

.github/workflows/test.yml

@@ -0,0 +1,74 @@
+name: Go Tests
+
+on:
+  push:
+    branches: [master]
+  pull_request:
+    branches: [master]
+
+jobs:
+  test:
+    name: Test
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        go-version: [1.13.x, 1.14.x, 1.15.x, 1.16.x, 1.17.x]
+
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: ${{ matrix.go-version }}
+
+      - name: Check out code
+        uses: actions/checkout@v2
+
+      - name: Get dependencies
+        run: go get -v -t -d ./...
+
+      - name: Build
+        run: go build -v ./...
+
+      - name: Run tests with race detector
+        run: go test -race -v ./...
+
+      - name: Run benchmarks
+        run: go test -bench=. -run=^$ ./...
+
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.17.x
+
+      - name: Check out code
+        uses: actions/checkout@v2
+
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v2
+        with:
+          version: latest
+
+  coverage:
+    name: Code coverage
+    runs-on: ubuntu-latest
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.17.x
+
+      - name: Check out code
+        uses: actions/checkout@v2
+
+      - name: Run coverage
+        run: go test -coverprofile=coverage.txt -covermode=atomic ./...
+
+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@v2
+        with:
+          file: ./coverage.txt
+          fail_ci_if_error: false

README.md

@@ -23,7 +23,7 @@ Usage:
 Flags:
 
   -ignore            exclude files matching the given regular expression
-  -ignore-strings    exclude strings matching the given regular expression  
+  -ignore-strings    exclude strings matching the given regular expression
   -ignore-tests      exclude tests from the search (default: true)
   -min-occurrences   report from how many occurrences (default: 2)
   -min-length        only report strings with the minimum given length (default: 3)
@@ -43,10 +43,43 @@ Examples:
   goconst -min-occurrences 5 $(go list -m -f '{{.Dir}}')
 ```
 
+### Development
+
+#### Running Tests
+
+The project includes a comprehensive test suite. To run the tests:
+
+```bash
+# Run all tests
+go test ./...
+
+# Run tests with verbose output
+go test -v ./...
+
+# Run tests with race detector
+go test -race ./...
+
+# Run benchmarks
+go test -bench=. ./...
+
+# Check test coverage
+go test -cover ./...
+```
+
+#### Contributing
+
+Contributions are welcome! Before submitting a PR:
+
+1. Make sure all tests pass
+2. Add tests for new functionality
+3. Ensure your code passes linting checks
+4. Update documentation as needed
+
 ### Other static analysis tools
 
 - [gogetimports](https://github.com/jgautheron/gogetimports): Get a JSON-formatted list of imports.
 - [usedexports](https://github.com/jgautheron/usedexports): Find exported variables that could be unexported.
 
 ### License
+
 MIT

api.go

@@ -4,27 +4,66 @@ import (
 	"go/ast"
 	"go/token"
 	"strings"
+	"sync"
 )
 
+// Issue represents a finding of duplicated strings or numbers.
+// Each Issue includes the position where it was found, how many times it occurs,
+// the string itself, and any matching constant name.
 type Issue struct {
 	Pos              token.Position
 	OccurrencesCount int
 	Str              string
 	MatchingConst    string
 }
 
+// IssuePool provides a pool of Issue slices to reduce allocations
+var IssuePool = sync.Pool{
+	New: func() interface{} {
+		return make([]Issue, 0, 100)
+	},
+}
+
+// GetIssueBuffer retrieves an Issue slice from the pool
+func GetIssueBuffer() []Issue {
+	return IssuePool.Get().([]Issue)[:0] // Reset length but keep capacity
+}
+
+// PutIssueBuffer returns an Issue slice to the pool
+func PutIssueBuffer(issues []Issue) {
+	// Make sure to clear references before returning to pool
+	for i := range issues {
+		issues[i].MatchingConst = ""
+		issues[i].Str = ""
+	}
+	IssuePool.Put(issues[:0]) // Reset length but keep capacity
+}
+
+// Config contains all configuration options for the goconst analyzer.
 type Config struct {
-	IgnoreStrings      string
-	IgnoreTests        bool
+	// IgnoreStrings is a regular expression to filter strings
+	IgnoreStrings string
+	// IgnoreTests indicates whether test files should be excluded
+	IgnoreTests bool
+	// MatchWithConstants enables matching strings with existing constants
 	MatchWithConstants bool
-	MinStringLength    int
-	MinOccurrences     int
-	ParseNumbers       bool
-	NumberMin          int
-	NumberMax          int
-	ExcludeTypes       map[Type]bool
+	// MinStringLength is the minimum length a string must have to be reported
+	MinStringLength int
+	// MinOccurrences is the minimum number of occurrences required to report a string
+	MinOccurrences int
+	// ParseNumbers enables detection of duplicated numbers
+	ParseNumbers bool
+	// NumberMin sets the minimum value for reported number matches
+	NumberMin int
+	// NumberMax sets the maximum value for reported number matches
+	NumberMax int
+	// ExcludeTypes allows excluding specific types of contexts
+	ExcludeTypes map[Type]bool
 }
 
+// Run analyzes the provided AST files for duplicated strings or numbers
+// according to the provided configuration.
+// It returns a slice of Issue objects containing the findings.
 func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
 	p := New(
 		"",
@@ -39,38 +78,117 @@ func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
 		cfg.MinOccurrences,
 		cfg.ExcludeTypes,
 	)
-	var issues []Issue
+
+	// Pre-allocate slice based on estimated result size
+	expectedIssues := len(files) * 5 // Assuming average of 5 issues per file
+	if expectedIssues > 1000 {
+		expectedIssues = 1000 // Cap at reasonable maximum
+	}
+
+	// Get issue buffer from pool instead of allocating
+	issueBuffer := GetIssueBuffer()
+	if cap(issueBuffer) < expectedIssues {
+		// Only allocate new buffer if existing one is too small
+		PutIssueBuffer(issueBuffer)
+		issueBuffer = make([]Issue, 0, expectedIssues)
+	}
+
+	// Process files concurrently
+	var wg sync.WaitGroup
+	sem := make(chan struct{}, p.maxConcurrency)
+
+	// Create a filtered files slice with capacity hint
+	filteredFiles := make([]*ast.File, 0, len(files))
+
+	// Filter test files first if needed
 	for _, f := range files {
 		if p.ignoreTests {
-			if filename := fset.Position(f.Pos()).Filename; strings.HasSuffix(filename, testSuffix) {
+			if filename := fset.Position(f.Pos()).Filename; strings.HasSuffix(filename, "_test.go") {
 				continue
 			}
 		}
-		ast.Walk(&treeVisitor{
-			fileSet:     fset,
-			packageName: "",
-			fileName:    "",
-			p:           p,
-		}, f)
+		filteredFiles = append(filteredFiles, f)
+	}
+
+	// Process each file in parallel
+	for _, f := range filteredFiles {
+		wg.Add(1)
+		sem <- struct{}{} // acquire semaphore
+
+		go func(f *ast.File) {
+			defer func() {
+				<-sem // release semaphore
+				wg.Done()
+			}()
+
+			// Use empty interned strings for package/file names
+			// The visitor logic will set these appropriately
+			emptyStr := InternString("")
+
+			ast.Walk(&treeVisitor{
+				fileSet:     fset,
+				packageName: emptyStr,
+				fileName:    emptyStr,
+				p:           p,
+				ignoreRegex: p.ignoreStringsRegex,
+			}, f)
+		}(f)
 	}
+
+	wg.Wait()
+
 	p.ProcessResults()
 
-	for str, item := range p.strs {
-		fi := item[0]
-		i := Issue{
+	// Process each string that passed the filters
+	p.stringMutex.RLock()
+	p.stringCountMutex.RLock()
+
+	// Get a string buffer from pool instead of allocating
+	stringKeys := GetStringBuffer()
+
+	// Create an array of strings to sort for stable output
+	for str := range p.strs {
+		if count := p.stringCount[str]; count >= p.minOccurrences {
+			stringKeys = append(stringKeys, str)
+		}
+	}
+
+	// Process strings in a predictable order for stable output
+	for _, str := range stringKeys {
+		positions := p.strs[str]
+		if len(positions) == 0 {
+			continue
+		}
+
+		// Use the first position as representative
+		fi := positions[0]
+
+		// Create issue using the counted value to avoid recounting
+		issue := Issue{
 			Pos:              fi.Position,
-			OccurrencesCount: len(item),
+			OccurrencesCount: p.stringCount[str],
 			Str:              str,
 		}
 
-		if len(p.consts) != 0 {
+		// Check for matching constants
+		if len(p.consts) > 0 {
+			p.constMutex.RLock()
 			if cst, ok := p.consts[str]; ok {
 				// const should be in the same package and exported
-				i.MatchingConst = cst.Name
+				issue.MatchingConst = cst.Name
 			}
+			p.constMutex.RUnlock()
 		}
-		issues = append(issues, i)
+
+		issueBuffer = append(issueBuffer, issue)
 	}
 
-	return issues, nil
+	p.stringCountMutex.RUnlock()
+	p.stringMutex.RUnlock()
+
+	// Return string buffer to pool
+	PutStringBuffer(stringKeys)
+
+	// Don't return the buffer to pool as the caller now owns it
+	return issueBuffer, nil
 }