Skip to content

Commit 0244082

Browse files
jgautheronjgautheron
authored
Multiple ignored strings (#33)
1 parent 37202e1 commit 0244082

File tree

6 files changed

+219
-13
lines changed

6 files changed

+219
-13
lines changed

‎api.go

+54-7
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ func PutIssueBuffer(issues []Issue) {
4646

4747
// Config contains all configuration options for the goconst analyzer.
4848
type Config struct {
49-
// IgnoreStrings is a regular expression to filter strings
50-
IgnoreStrings string
49+
// IgnoreStrings is a list of regular expressions to filter strings
50+
IgnoreStrings []string
5151
// IgnoreTests indicates whether test files should be excluded
5252
IgnoreTests bool
5353
// MatchWithConstants enables matching strings with existing constants
@@ -68,11 +68,51 @@ type Config struct {
6868
FindDuplicates bool
6969
}
7070

71-
// Run analyzes the provided AST files for duplicated strings or numbers
72-
// according to the provided configuration.
73-
// It returns a slice of Issue objects containing the findings.
74-
func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
75-
p := New(
71+
// NewWithIgnorePatterns creates a new instance of the parser with support for multiple ignore patterns.
72+
// This is an alternative constructor that takes a slice of ignore string patterns.
73+
func NewWithIgnorePatterns(
74+
path, ignore string,
75+
ignoreStrings []string,
76+
ignoreTests, matchConstant, numbers, findDuplicates bool,
77+
numberMin, numberMax, minLength, minOccurrences int,
78+
excludeTypes map[Type]bool) *Parser {
79+
80+
// Join multiple patterns with OR for regex
81+
var ignoreStringsPattern string
82+
if len(ignoreStrings) > 0 {
83+
if len(ignoreStrings) > 1 {
84+
// Wrap each pattern in parentheses and join with OR
85+
patterns := make([]string, len(ignoreStrings))
86+
for i, pattern := range ignoreStrings {
87+
patterns[i] = "(" + pattern + ")"
88+
}
89+
ignoreStringsPattern = strings.Join(patterns, "|")
90+
} else {
91+
// Single pattern case
92+
ignoreStringsPattern = ignoreStrings[0]
93+
}
94+
}
95+
96+
return New(
97+
path,
98+
ignore,
99+
ignoreStringsPattern,
100+
ignoreTests,
101+
matchConstant,
102+
numbers,
103+
findDuplicates,
104+
numberMin,
105+
numberMax,
106+
minLength,
107+
minOccurrences,
108+
excludeTypes,
109+
)
110+
}
111+
112+
// RunWithConfig is a convenience function that runs the analysis with a Config object
113+
// directly supporting multiple ignore patterns.
114+
func RunWithConfig(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
115+
p := NewWithIgnorePatterns(
76116
"",
77117
"",
78118
cfg.IgnoreStrings,
@@ -233,3 +273,10 @@ func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
233273
// Don't return the buffer to pool as the caller now owns it
234274
return issueBuffer, nil
235275
}
276+
277+
// Run analyzes the provided AST files for duplicated strings or numbers
278+
// according to the provided configuration.
279+
// It returns a slice of Issue objects containing the findings.
280+
func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
281+
return RunWithConfig(files, fset, cfg)
282+
}

‎api_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func example() {
6565
config: &Config{
6666
MinStringLength: 3,
6767
MinOccurrences: 2,
68-
IgnoreStrings: "test",
68+
IgnoreStrings: []string{"test"},
6969
},
7070
expectedIssues: 1, // Only "another" should be reported
7171
},

‎cmd/goconst/main.go

+54-3
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ Examples:
4545

4646
var (
4747
flagIgnore = flag.String("ignore", "", "ignore files matching the given regular expression")
48-
flagIgnoreStrings = flag.String("ignore-strings", "", "ignore strings matching the given regular expression")
48+
flagIgnoreStrings = flag.String("ignore-strings", "", "ignore strings matching the given regular expressions (comma separated)")
4949
flagIgnoreTests = flag.Bool("ignore-tests", true, "exclude tests from the search")
5050
flagMinOccurrences = flag.Int("min-occurrences", 2, "report from how many occurrences")
5151
flagMinLength = flag.Int("min-length", 3, "only report strings with the minimum given length")
@@ -93,10 +93,17 @@ func main() {
9393
// run analyzes a single path for repeated strings that could be constants.
9494
// It returns true if any issues were found, and an error if the analysis failed.
9595
func run(path string) (bool, error) {
96-
gco := goconst.New(
96+
// Parse ignore strings - handling comma-separated values
97+
var ignoreStrings []string
98+
if *flagIgnoreStrings != "" {
99+
// Split by commas but handle escaping
100+
ignoreStrings = parseCommaSeparatedValues(*flagIgnoreStrings)
101+
}
102+
103+
gco := goconst.NewWithIgnorePatterns(
97104
path,
98105
*flagIgnore,
99-
*flagIgnoreStrings,
106+
ignoreStrings,
100107
*flagIgnoreTests,
101108
*flagMatchConstant,
102109
*flagNumbers,
@@ -115,6 +122,50 @@ func run(path string) (bool, error) {
115122
return printOutput(strs, consts, *flagOutput)
116123
}
117124

125+
// parseCommaSeparatedValues splits a comma-separated string into a slice of strings,
126+
// handling escaping of commas within values.
127+
func parseCommaSeparatedValues(input string) []string {
128+
if input == "" {
129+
return nil
130+
}
131+
132+
// Simple case - no escaping needed
133+
if !strings.Contains(input, "\\,") {
134+
return strings.Split(input, ",")
135+
}
136+
137+
// Handle escaped commas
138+
var result []string
139+
var current strings.Builder
140+
escaped := false
141+
142+
for _, char := range input {
143+
if escaped {
144+
if char == ',' {
145+
current.WriteRune(',')
146+
} else {
147+
current.WriteRune('\\')
148+
current.WriteRune(char)
149+
}
150+
escaped = false
151+
} else if char == '\\' {
152+
escaped = true
153+
} else if char == ',' {
154+
result = append(result, current.String())
155+
current.Reset()
156+
} else {
157+
current.WriteRune(char)
158+
}
159+
}
160+
161+
// Don't forget the last value
162+
if current.Len() > 0 {
163+
result = append(result, current.String())
164+
}
165+
166+
return result
167+
}
168+
118169
// usage prints the usage documentation to the specified writer.
119170
func usage(out io.Writer) {
120171
if _, err := fmt.Fprint(out, usageDoc); err != nil {

‎compat/compat_integration_test.go

+54-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func example() {
4646

4747
// Configure exactly as golangci-lint does
4848
cfg := &goconstAPI.Config{
49-
IgnoreStrings: "test-ignore",
49+
IgnoreStrings: []string{"test-ignore"},
5050
MatchWithConstants: true,
5151
MinStringLength: 3,
5252
MinOccurrences: 2,
@@ -97,4 +97,57 @@ func example() {
9797
issue.Str, issue.MatchingConst, expected.matchingConst)
9898
}
9999
}
100+
}
101+
102+
func TestMultipleIgnorePatternsIntegration(t *testing.T) {
103+
const testCode = `package example
104+
105+
func example() {
106+
// These should be ignored by different patterns
107+
foo1 := "foobar"
108+
foo2 := "foobar"
109+
110+
bar1 := "barbaz"
111+
bar2 := "barbaz"
112+
113+
// These should be detected
114+
test1 := "example"
115+
test2 := "example"
116+
}
117+
`
118+
119+
// Parse the test code
120+
fset := token.NewFileSet()
121+
f, err := parser.ParseFile(fset, "example.go", testCode, 0)
122+
if err != nil {
123+
t.Fatalf("Failed to parse test code: %v", err)
124+
}
125+
126+
// Configure with multiple ignore patterns
127+
cfg := &goconstAPI.Config{
128+
IgnoreStrings: []string{"foo.+", "bar.+"}, // Multiple patterns
129+
MinStringLength: 3,
130+
MinOccurrences: 2,
131+
}
132+
133+
// Run the analysis
134+
issues, err := goconstAPI.Run([]*ast.File{f}, fset, cfg)
135+
if err != nil {
136+
t.Fatalf("Run() error = %v", err)
137+
}
138+
139+
// Verify that "foobar" and "barbaz" are ignored but "example" is found
140+
if len(issues) != 1 {
141+
t.Errorf("Expected 1 issue, got %d", len(issues))
142+
for _, issue := range issues {
143+
t.Logf("Found issue: %q with %d occurrences",
144+
issue.Str, issue.OccurrencesCount)
145+
}
146+
return
147+
}
148+
149+
// The only issue should be "example"
150+
if issues[0].Str != "example" {
151+
t.Errorf("Expected to find 'example', got %q", issues[0].Str)
152+
}
100153
}

‎compat/compat_test.go

+25-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func TestGolangCICompatibility(t *testing.T) {
1414
// See: https://github.com/golangci/golangci-lint/blob/main/pkg/golinters/goconst/goconst.go
1515

1616
cfg := goconstAPI.Config{
17-
IgnoreStrings: "test",
17+
IgnoreStrings: []string{"test"},
1818
MatchWithConstants: true,
1919
MinStringLength: 3,
2020
MinOccurrences: 2,
@@ -51,4 +51,28 @@ func TestGolangCICompatibility(t *testing.T) {
5151
_ = issue.OccurrencesCount
5252
_ = issue.Str
5353
_ = issue.MatchingConst
54+
}
55+
56+
// TestMultipleIgnorePatterns verifies that multiple ignore patterns work correctly
57+
func TestMultipleIgnorePatterns(t *testing.T) {
58+
// Test configuration with multiple ignore patterns
59+
cfg := goconstAPI.Config{
60+
IgnoreStrings: []string{"foo.+", "bar.+", "test"},
61+
MinStringLength: 3,
62+
MinOccurrences: 2,
63+
}
64+
65+
// Create a simple test file
66+
fset := token.NewFileSet()
67+
68+
// We just want to verify that multiple patterns are accepted
69+
_, err := goconstAPI.Run(nil, fset, &cfg)
70+
if err != nil {
71+
// We expect an error since we passed nil files
72+
// but the important part is that multiple patterns are accepted
73+
t.Log("Expected error from nil files:", err)
74+
}
75+
76+
// This tests the construction and acceptance of the config
77+
// Actual pattern matching is tested in integration tests
5478
}

‎scripts/test-golangci-compat.sh

+31
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ func example() {
4848
// This should be ignored due to ignore-strings
4949
skip := "test-ignore"
5050
skip2 := "test-ignore"
51+
52+
// These should be ignored with the multiple pattern test
53+
foo1 := "foo-prefix"
54+
foo2 := "foo-prefix"
55+
56+
bar1 := "bar-prefix"
57+
bar2 := "bar-prefix"
5158
}
5259
EOF
5360

@@ -122,4 +129,28 @@ if ! grep -q '"constants":{"test-const":\[.*"Name":"ExistingConst"' "$TEST_DIR/o
122129
exit 1
123130
fi
124131

132+
# Test 5: Test with multiple ignore patterns (comma-separated)
133+
echo "Test 5: Testing multiple ignore patterns..."
134+
"$GOCONST_BIN" -ignore-strings "test-ignore,foo-prefix,bar-prefix" "$TEST_DIR/testpkg" > "$TEST_DIR/output5.txt"
135+
if grep -q "test-ignore" "$TEST_DIR/output5.txt"; then
136+
echo "Failed: Should NOT detect 'test-ignore' string"
137+
cat "$TEST_DIR/output5.txt"
138+
exit 1
139+
fi
140+
if grep -q "foo-prefix" "$TEST_DIR/output5.txt"; then
141+
echo "Failed: Should NOT detect 'foo-prefix' string"
142+
cat "$TEST_DIR/output5.txt"
143+
exit 1
144+
fi
145+
if grep -q "bar-prefix" "$TEST_DIR/output5.txt"; then
146+
echo "Failed: Should NOT detect 'bar-prefix' string"
147+
cat "$TEST_DIR/output5.txt"
148+
exit 1
149+
fi
150+
if ! grep -q "duplicate" "$TEST_DIR/output5.txt"; then
151+
echo "Failed: Should detect 'duplicate' string"
152+
cat "$TEST_DIR/output5.txt"
153+
exit 1
154+
fi
155+
125156
echo "All compatibility tests PASSED!"

0 commit comments

Comments
 (0)