Skip to content

Commit d8efeb9

Browse files
authored
Add support for finding constants with duplicated values. (#30)
* feat: add find-duplicates flag * fix: update readme
1 parent 6e4966a commit d8efeb9

11 files changed

+251
-71
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ Flags:
2828
-min-occurrences report from how many occurrences (default: 2)
2929
-min-length only report strings with the minimum given length (default: 3)
3030
-match-constant look for existing constants matching the values
31+
-find-duplicates look for constants with identical values
3132
-numbers search also for duplicated numbers
3233
-min minimum value, only works with -numbers
3334
-max maximum value, only works with -numbers

api.go

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,21 @@ package goconst
33
import (
44
"go/ast"
55
"go/token"
6+
"sort"
67
"strings"
78
"sync"
89
)
910

10-
// Issue represents a finding of duplicated strings or numbers.
11+
// Issue represents a finding of duplicated strings, numbers, or constants.
1112
// Each Issue includes the position where it was found, how many times it occurs,
1213
// the string itself, and any matching constant name.
1314
type Issue struct {
1415
Pos token.Position
1516
OccurrencesCount int
1617
Str string
1718
MatchingConst string
19+
DuplicateConst string
20+
DuplicatePos token.Position
1821
}
1922

2023
// IssuePool provides a pool of Issue slices to reduce allocations
@@ -61,6 +64,8 @@ type Config struct {
6164
NumberMax int
6265
// ExcludeTypes allows excluding specific types of contexts
6366
ExcludeTypes map[Type]bool
67+
// FindDuplicated constants enables finding constants whose values match existing constants in other packages.
68+
FindDuplicates bool
6469
}
6570

6671
// Run analyzes the provided AST files for duplicated strings or numbers
@@ -74,6 +79,7 @@ func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
7479
cfg.IgnoreTests,
7580
cfg.MatchWithConstants,
7681
cfg.ParseNumbers,
82+
cfg.FindDuplicates,
7783
cfg.NumberMin,
7884
cfg.NumberMax,
7985
cfg.MinStringLength,
@@ -155,6 +161,8 @@ func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
155161
}
156162
}
157163

164+
sort.Strings(stringKeys)
165+
158166
// Process strings in a predictable order for stable output
159167
for _, str := range stringKeys {
160168
positions := p.strs[str]
@@ -175,9 +183,9 @@ func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
175183
// Check for matching constants
176184
if len(p.consts) > 0 {
177185
p.constMutex.RLock()
178-
if cst, ok := p.consts[str]; ok {
186+
if csts, ok := p.consts[str]; ok && len(csts) > 0 {
179187
// const should be in the same package and exported
180-
issue.MatchingConst = cst.Name
188+
issue.MatchingConst = csts[0].Name
181189
}
182190
p.constMutex.RUnlock()
183191
}
@@ -188,6 +196,37 @@ func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
188196
p.stringCountMutex.RUnlock()
189197
p.stringMutex.RUnlock()
190198

199+
// process duplicate constants
200+
p.constMutex.RLock()
201+
202+
// reuse string buffer for const keys
203+
stringKeys = stringKeys[:0]
204+
205+
// Create an array of strings and sort for stable output
206+
for str := range p.consts {
207+
if len(p.consts[str]) > 1 {
208+
stringKeys = append(stringKeys, str)
209+
}
210+
}
211+
212+
sort.Strings(stringKeys)
213+
214+
// report an issue for every duplicated const
215+
for _, str := range stringKeys {
216+
positions := p.consts[str]
217+
218+
for i := 1; i < len(positions); i++ {
219+
issueBuffer = append(issueBuffer, Issue{
220+
Pos: positions[i].Position,
221+
Str: str,
222+
DuplicateConst: positions[0].Name,
223+
DuplicatePos: positions[0].Position,
224+
})
225+
}
226+
}
227+
228+
p.constMutex.RUnlock()
229+
191230
// Return string buffer to pool
192231
PutStringBuffer(stringKeys)
193232

api_test.go

Lines changed: 85 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,18 @@ func example() {
4141
},
4242
expectedIssues: 1,
4343
},
44+
{
45+
name: "duplicate consts",
46+
code: `package example
47+
const ConstA = "test"
48+
func example() {
49+
const ConstB = "test"
50+
}`,
51+
config: &Config{
52+
FindDuplicates: true,
53+
},
54+
expectedIssues: 1,
55+
},
4456
{
4557
name: "string duplication with ignore",
4658
code: `package example
@@ -212,50 +224,93 @@ func example() {
212224

213225
func TestMultipleFilesAnalysis(t *testing.T) {
214226
// Test analyzing multiple files at once
215-
code1 := `package example
227+
tests := []struct {
228+
name string
229+
code1 string
230+
code2 string
231+
config *Config
232+
expectedIssues int
233+
expectedStr string
234+
expectedOccurrenceCount int
235+
}{
236+
{
237+
name: "duplicate strings",
238+
code1: `package example
216239
func example1() {
217240
a := "shared"
218241
b := "shared"
219242
}
220-
`
221-
code2 := `package example
243+
`,
244+
code2: `package example
222245
func example2() {
223246
c := "shared"
224247
d := "unique"
225248
}
226-
`
227-
fset := token.NewFileSet()
228-
f1, err := parser.ParseFile(fset, "file1.go", code1, 0)
229-
if err != nil {
230-
t.Fatalf("Failed to parse test code: %v", err)
249+
`,
250+
config: &Config{
251+
MinStringLength: 3,
252+
MinOccurrences: 2,
253+
},
254+
expectedIssues: 1,
255+
expectedStr: "shared",
256+
expectedOccurrenceCount: 3,
257+
},
258+
{
259+
name: "duplicate consts in different packages",
260+
code1: `package package1
261+
const ConstA = "shared"
262+
const ConstB = "shared"
263+
`,
264+
code2: `package package2
265+
const (
266+
ConstC = "shared"
267+
ConstD = "shared"
268+
ConstE= "unique"
269+
)`,
270+
config: &Config{
271+
FindDuplicates: true,
272+
},
273+
expectedIssues: 3,
274+
expectedStr: "shared",
275+
expectedOccurrenceCount: 0,
276+
},
231277
}
232278

233-
f2, err := parser.ParseFile(fset, "file2.go", code2, 0)
234-
if err != nil {
235-
t.Fatalf("Failed to parse test code: %v", err)
236-
}
279+
for _, tt := range tests {
280+
t.Run(tt.name, func(t *testing.T) {
237281

238-
config := &Config{
239-
MinStringLength: 3,
240-
MinOccurrences: 2,
241-
}
282+
fset := token.NewFileSet()
283+
f1, err := parser.ParseFile(fset, "file1.go", tt.code1, 0)
284+
if err != nil {
285+
t.Fatalf("Failed to parse test code: %v", err)
286+
}
242287

243-
issues, err := Run([]*ast.File{f1, f2}, fset, config)
244-
if err != nil {
245-
t.Fatalf("Run() error = %v", err)
246-
}
288+
f2, err := parser.ParseFile(fset, "file2.go", tt.code2, 0)
289+
if err != nil {
290+
t.Fatalf("Failed to parse test code: %v", err)
291+
}
247292

248-
// Should find "shared" appearing 3 times across both files
249-
if len(issues) != 1 {
250-
t.Fatalf("Expected 1 issue, got %d", len(issues))
251-
}
293+
issues, err := Run([]*ast.File{f1, f2}, fset, tt.config)
294+
if err != nil {
295+
t.Fatalf("Run() error = %v", err)
296+
}
252297

253-
issue := issues[0]
254-
if issue.Str != "shared" {
255-
t.Errorf("Issue.Str = %v, want %v", issue.Str, "shared")
256-
}
257-
if issue.OccurrencesCount != 3 {
258-
t.Errorf("Issue.OccurrencesCount = %v, want 3", issue.OccurrencesCount)
298+
// Should find "shared" appearing 3 times across both files
299+
if len(issues) != tt.expectedIssues {
300+
t.Fatalf("Expected %d issue, got %d", tt.expectedIssues, len(issues))
301+
}
302+
303+
if len(issues) > 0 {
304+
issue := issues[0]
305+
if issue.Str != tt.expectedStr {
306+
t.Errorf("Issue.Str = %v, want %v", issue.Str, tt.expectedStr)
307+
}
308+
309+
if issue.OccurrencesCount != tt.expectedOccurrenceCount {
310+
t.Errorf("Issue.OccurrencesCount = %v, want %d", issue.OccurrencesCount, tt.expectedOccurrenceCount)
311+
}
312+
}
313+
})
259314
}
260315
}
261316

benchmarks_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ func BenchmarkParseTree(b *testing.B) {
8181
false, // ignoreTests
8282
false, // matchConstant
8383
false, // numbers
84+
true, // findDuplicates
8485
0, // numberMin
8586
0, // numberMax
8687
3, // minLength
@@ -104,6 +105,7 @@ func BenchmarkParseTree(b *testing.B) {
104105
false, // ignoreTests
105106
false, // matchConstant
106107
true, // numbers
108+
true, // findDuplicates
107109
0, // numberMin
108110
0, // numberMax
109111
3, // minLength
@@ -127,6 +129,7 @@ func BenchmarkParseTree(b *testing.B) {
127129
false, // ignoreTests
128130
true, // matchConstant
129131
false, // numbers
132+
true, // findDuplicates
130133
0, // numberMin
131134
0, // numberMax
132135
3, // minLength
@@ -157,6 +160,7 @@ func BenchmarkParallelProcessing(b *testing.B) {
157160
false,
158161
false,
159162
true,
163+
true,
160164
0,
161165
0,
162166
3,
@@ -304,6 +308,7 @@ func helperFunction%d() string {
304308
false,
305309
false,
306310
true,
311+
true,
307312
0,
308313
0,
309314
3,
@@ -332,6 +337,7 @@ func helperFunction%d() string {
332337
false,
333338
false,
334339
true,
340+
true,
335341
0,
336342
0,
337343
3,
@@ -377,7 +383,7 @@ func BenchmarkFileReadingPerformance(b *testing.B) {
377383

378384
// Benchmark the optimized file reading
379385
b.Run(fmt.Sprintf("OptimizedIO_%d", size), func(b *testing.B) {
380-
parser := New("", "", "", false, false, false, 0, 0, 3, 2, make(map[Type]bool))
386+
parser := New("", "", "", false, false, false, true, 0, 0, 3, 2, make(map[Type]bool))
381387
b.ResetTimer()
382388

383389
for i := 0; i < b.N; i++ {

cmd/goconst/main.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Flags:
2626
-min-occurrences report from how many occurrences (default: 2)
2727
-min-length only report strings with the minimum given length (default: 3)
2828
-match-constant look for existing constants matching the strings
29+
-find-duplicates look for constants with identical values
2930
-numbers search also for duplicated numbers
3031
-min minimum value, only works with -numbers
3132
-max maximum value, only works with -numbers
@@ -49,6 +50,7 @@ var (
4950
flagMinOccurrences = flag.Int("min-occurrences", 2, "report from how many occurrences")
5051
flagMinLength = flag.Int("min-length", 3, "only report strings with the minimum given length")
5152
flagMatchConstant = flag.Bool("match-constant", false, "look for existing constants matching the strings")
53+
flagFindDuplicates = flag.Bool("find-duplicates", false, "look for constants with duplicated values")
5254
flagNumbers = flag.Bool("numbers", false, "search also for duplicated numbers")
5355
flagMin = flag.Int("min", 0, "minimum value, only works with -numbers")
5456
flagMax = flag.Int("max", 0, "maximum value, only works with -numbers")
@@ -98,6 +100,7 @@ func run(path string) (bool, error) {
98100
*flagIgnoreTests,
99101
*flagMatchConstant,
100102
*flagNumbers,
103+
*flagFindDuplicates,
101104
*flagMin,
102105
*flagMax,
103106
*flagMinLength,
@@ -139,7 +142,7 @@ func printOutput(strs goconst.Strings, consts goconst.Constants, output string)
139142
for str, item := range strs {
140143
for _, xpos := range item {
141144
fmt.Printf(
142-
`%s:%d:%d:%d other occurrence(s) of "%s" found in: %s`,
145+
`%s:%d:%d:%d other occurrence(s) of %q found in: %s`,
143146
xpos.Filename,
144147
xpos.Line,
145148
xpos.Column,
@@ -157,10 +160,19 @@ func printOutput(strs goconst.Strings, consts goconst.Constants, output string)
157160
if len(consts) == 0 {
158161
continue
159162
}
160-
if cst, ok := consts[str]; ok {
163+
if csts, ok := consts[str]; ok && len(csts) > 0 {
161164
// const should be in the same package and exported
162-
fmt.Printf(`A matching constant has been found for "%s": %s`, str, cst.Name)
163-
fmt.Printf("\n\t%s\n", cst.String())
165+
fmt.Printf(`A matching constant has been found for %q: %s`, str, csts[0].Name)
166+
fmt.Printf("\n\t%s\n", csts[0].String())
167+
}
168+
}
169+
for val, csts := range consts {
170+
if len(csts) > 1 {
171+
fmt.Printf("Duplicate constant(s) with value %q have been found:\n", val)
172+
173+
for i := 0; i < len(csts); i++ {
174+
fmt.Printf("\t%s: %s\n", csts[i].String(), csts[i].Name)
175+
}
164176
}
165177
}
166178
default:

integration_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ func TestIntegrationWithTestdata(t *testing.T) {
1414
numberMin int
1515
numberMax int
1616
minLength int
17+
findDuplicates bool
1718
minOccurrences int
1819
expectedStrings int
1920
}{
@@ -80,6 +81,7 @@ func TestIntegrationWithTestdata(t *testing.T) {
8081
tt.ignoreTests,
8182
tt.matchConstant,
8283
tt.numbers,
84+
tt.findDuplicates,
8385
tt.numberMin,
8486
tt.numberMax,
8587
tt.minLength,
@@ -140,6 +142,7 @@ func TestIntegrationExcludeTypes(t *testing.T) {
140142
false, // ignoreTests
141143
false, // matchConstant
142144
false, // numbers
145+
false, // findDuplicates
143146
0, // numberMin
144147
0, // numberMax
145148
3, // minLength

0 commit comments

Comments
 (0)