Skip to content

Commit 78ef201

Browse files
committed
handle constant expressions via typechecker
1 parent cb090a4 commit 78ef201

File tree

13 files changed

+818
-53
lines changed

13 files changed

+818
-53
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
name: GolangCI-Lint Compatibility
2+
3+
on:
4+
push:
5+
branches: ["master"]
6+
pull_request:
7+
branches: ["master"]
8+
schedule:
9+
- cron: "0 0 * * 0" # Run weekly on Sundays at midnight
10+
11+
jobs:
12+
test-golangci-compat:
13+
runs-on: ubuntu-latest
14+
steps:
15+
- uses: actions/checkout@v4
16+
17+
- name: Set up Go
18+
uses: actions/setup-go@v5
19+
with:
20+
go-version: "1.21"
21+
check-latest: true
22+
23+
- name: Make script executable
24+
run: chmod +x ./scripts/test-golangci-compat.sh
25+
26+
- name: Run compatibility tests
27+
run: ./scripts/test-golangci-compat.sh

api.go

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package goconst
33
import (
44
"go/ast"
55
"go/token"
6+
"go/types"
67
"sort"
78
"strings"
89
"sync"
@@ -46,8 +47,8 @@ func PutIssueBuffer(issues []Issue) {
4647

4748
// Config contains all configuration options for the goconst analyzer.
4849
type Config struct {
49-
// IgnoreStrings is a regular expression to filter strings
50-
IgnoreStrings string
50+
// IgnoreStrings is a list of regular expressions to filter strings
51+
IgnoreStrings []string
5152
// IgnoreTests indicates whether test files should be excluded
5253
IgnoreTests bool
5354
// MatchWithConstants enables matching strings with existing constants
@@ -68,11 +69,51 @@ type Config struct {
6869
FindDuplicates bool
6970
}
7071

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(
72+
// NewWithIgnorePatterns creates a new instance of the parser with support for multiple ignore patterns.
73+
// This is an alternative constructor that takes a slice of ignore string patterns.
74+
func NewWithIgnorePatterns(
75+
path, ignore string,
76+
ignoreStrings []string,
77+
ignoreTests, matchConstant, numbers, findDuplicates bool,
78+
numberMin, numberMax, minLength, minOccurrences int,
79+
excludeTypes map[Type]bool) *Parser {
80+
81+
// Join multiple patterns with OR for regex
82+
var ignoreStringsPattern string
83+
if len(ignoreStrings) > 0 {
84+
if len(ignoreStrings) > 1 {
85+
// Wrap each pattern in parentheses and join with OR
86+
patterns := make([]string, len(ignoreStrings))
87+
for i, pattern := range ignoreStrings {
88+
patterns[i] = "(" + pattern + ")"
89+
}
90+
ignoreStringsPattern = strings.Join(patterns, "|")
91+
} else {
92+
// Single pattern case
93+
ignoreStringsPattern = ignoreStrings[0]
94+
}
95+
}
96+
97+
return New(
98+
path,
99+
ignore,
100+
ignoreStringsPattern,
101+
ignoreTests,
102+
matchConstant,
103+
numbers,
104+
findDuplicates,
105+
numberMin,
106+
numberMax,
107+
minLength,
108+
minOccurrences,
109+
excludeTypes,
110+
)
111+
}
112+
113+
// RunWithConfig is a convenience function that runs the analysis with a Config object
114+
// directly supporting multiple ignore patterns.
115+
func RunWithConfig(files []*ast.File, fset *token.FileSet, typeInfo *types.Info, cfg *Config) ([]Issue, error) {
116+
p := NewWithIgnorePatterns(
76117
"",
77118
"",
78119
cfg.IgnoreStrings,
@@ -136,9 +177,9 @@ func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
136177
ast.Walk(&treeVisitor{
137178
fileSet: fset,
138179
packageName: emptyStr,
139-
fileName: emptyStr,
140180
p: p,
141181
ignoreRegex: p.ignoreStringsRegex,
182+
typeInfo: typeInfo,
142183
}, f)
143184
}(f)
144185
}
@@ -233,3 +274,10 @@ func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
233274
// Don't return the buffer to pool as the caller now owns it
234275
return issueBuffer, nil
235276
}
277+
278+
// Run analyzes the provided AST files for duplicated strings or numbers
279+
// according to the provided configuration.
280+
// It returns a slice of Issue objects containing the findings.
281+
func Run(files []*ast.File, fset *token.FileSet, typeInfo *types.Info, cfg *Config) ([]Issue, error) {
282+
return RunWithConfig(files, fset, typeInfo, cfg)
283+
}

api_test.go

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"go/ast"
55
"go/parser"
66
"go/token"
7+
"go/types"
78
"testing"
89
)
910

@@ -47,6 +48,19 @@ func example() {
4748
const ConstA = "test"
4849
func example() {
4950
const ConstB = "test"
51+
}`,
52+
config: &Config{
53+
FindDuplicates: true,
54+
},
55+
expectedIssues: 1,
56+
},
57+
{
58+
name: "duplicate computed consts",
59+
code: `package example
60+
const ConstA = "te"
61+
const Test = "test"
62+
func example() {
63+
const ConstB = ConstA + "st"
5064
}`,
5165
config: &Config{
5266
FindDuplicates: true,
@@ -65,7 +79,7 @@ func example() {
6579
config: &Config{
6680
MinStringLength: 3,
6781
MinOccurrences: 2,
68-
IgnoreStrings: "test",
82+
IgnoreStrings: []string{"test"},
6983
},
7084
expectedIssues: 1, // Only "another" should be reported
7185
},
@@ -164,7 +178,10 @@ func example() {
164178
t.Fatalf("Failed to parse test code: %v", err)
165179
}
166180

167-
issues, err := Run([]*ast.File{f}, fset, tt.config)
181+
chkr, info := checker(fset)
182+
_ = chkr.Files([]*ast.File{f})
183+
184+
issues, err := Run([]*ast.File{f}, fset, info, tt.config)
168185
if err != nil {
169186
t.Fatalf("Run() error = %v", err)
170187
}
@@ -201,7 +218,10 @@ func example() {
201218
MatchWithConstants: true,
202219
}
203220

204-
issues, err := Run([]*ast.File{f}, fset, config)
221+
chkr, info := checker(fset)
222+
_ = chkr.Files([]*ast.File{f})
223+
224+
issues, err := Run([]*ast.File{f}, fset, info, config)
205225
if err != nil {
206226
t.Fatalf("Run() error = %v", err)
207227
}
@@ -256,16 +276,16 @@ func example2() {
256276
expectedOccurrenceCount: 3,
257277
},
258278
{
259-
name: "duplicate consts in different packages",
260-
code1: `package package1
279+
name: "duplicate consts in different files",
280+
code1: `package example
261281
const ConstA = "shared"
262282
const ConstB = "shared"
263283
`,
264-
code2: `package package2
284+
code2: `package example
265285
const (
266286
ConstC = "shared"
267287
ConstD = "shared"
268-
ConstE= "unique"
288+
ConstE = "unique"
269289
)`,
270290
config: &Config{
271291
FindDuplicates: true,
@@ -290,7 +310,10 @@ const (
290310
t.Fatalf("Failed to parse test code: %v", err)
291311
}
292312

293-
issues, err := Run([]*ast.File{f1, f2}, fset, tt.config)
313+
chkr, info := checker(fset)
314+
_ = chkr.Files([]*ast.File{f1, f2})
315+
316+
issues, err := Run([]*ast.File{f1, f2}, fset, info, tt.config)
294317
if err != nil {
295318
t.Fatalf("Run() error = %v", err)
296319
}
@@ -348,8 +371,10 @@ func allContexts(param string) string {
348371
MinStringLength: 3,
349372
MinOccurrences: 2,
350373
}
374+
chkr, info := checker(fset)
375+
_ = chkr.Files([]*ast.File{f})
351376

352-
issues, err := Run([]*ast.File{f}, fset, config)
377+
issues, err := Run([]*ast.File{f}, fset, info, config)
353378
if err != nil {
354379
t.Fatalf("Run() error = %v", err)
355380
}
@@ -429,8 +454,10 @@ func multipleContexts() {
429454
MinOccurrences: 2,
430455
ExcludeTypes: tt.excludeTypes,
431456
}
457+
chkr, info := checker(fset)
458+
_ = chkr.Files([]*ast.File{f})
432459

433-
issues, err := Run([]*ast.File{f}, fset, config)
460+
issues, err := Run([]*ast.File{f}, fset, info, config)
434461
if err != nil {
435462
t.Fatalf("Run() error = %v", err)
436463
}
@@ -453,3 +480,13 @@ func multipleContexts() {
453480
})
454481
}
455482
}
483+
484+
func checker(fset *token.FileSet) (*types.Checker, *types.Info) {
485+
cfg := &types.Config{
486+
Error: func(err error) {},
487+
}
488+
info := &types.Info{
489+
Types: make(map[ast.Expr]types.TypeAndValue),
490+
}
491+
return types.NewChecker(cfg, fset, types.NewPackage("", "example"), info), info
492+
}

benchmarks_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ func BenchmarkParseSampleFile(b *testing.B) {
4141
p: p,
4242
fileSet: fset,
4343
packageName: "testdata",
44-
fileName: "testdata/sample.go",
4544
}
4645

4746
ast.Walk(v, f)
@@ -62,9 +61,13 @@ func BenchmarkRun(b *testing.B) {
6261
MatchWithConstants: true,
6362
}
6463

64+
chkr, info := checker(fset)
65+
_ = chkr.Files([]*ast.File{f})
66+
6567
b.ResetTimer()
68+
6669
for i := 0; i < b.N; i++ {
67-
_, err := Run([]*ast.File{f}, fset, config)
70+
_, err := Run([]*ast.File{f}, fset, info, config)
6871
if err != nil {
6972
b.Fatalf("Run() error = %v", err)
7073
}

cmd/goconst/main.go

Lines changed: 54 additions & 3 deletions
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 {

0 commit comments

Comments
 (0)