Skip to content

Commit af8760d

Browse files
Compute and use constant expressions via the type checker. (#35)
* Support for constant expressions * handle constant expressions via typechecker * fix: type check in batches processor * fix: type-checking concurrently is not supported --------- Co-authored-by: Jonathan Gautheron <[email protected]>
1 parent 0244082 commit af8760d

14 files changed

+1172
-213
lines changed

api.go

Lines changed: 11 additions & 6 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"
@@ -64,16 +65,18 @@ type Config struct {
6465
NumberMax int
6566
// ExcludeTypes allows excluding specific types of contexts
6667
ExcludeTypes map[Type]bool
67-
// FindDuplicated constants enables finding constants whose values match existing constants in other packages.
68+
// FindDuplicates enables finding constants whose values match existing constants in other packages.
6869
FindDuplicates bool
70+
// EvalConstExpressions enables evaluation of constant expressions like Prefix + "suffix"
71+
EvalConstExpressions bool
6972
}
7073

7174
// NewWithIgnorePatterns creates a new instance of the parser with support for multiple ignore patterns.
7275
// This is an alternative constructor that takes a slice of ignore string patterns.
7376
func NewWithIgnorePatterns(
7477
path, ignore string,
7578
ignoreStrings []string,
76-
ignoreTests, matchConstant, numbers, findDuplicates bool,
79+
ignoreTests, matchConstant, numbers, findDuplicates, evalConstExpressions bool,
7780
numberMin, numberMax, minLength, minOccurrences int,
7881
excludeTypes map[Type]bool) *Parser {
7982

@@ -101,6 +104,7 @@ func NewWithIgnorePatterns(
101104
matchConstant,
102105
numbers,
103106
findDuplicates,
107+
evalConstExpressions,
104108
numberMin,
105109
numberMax,
106110
minLength,
@@ -111,7 +115,7 @@ func NewWithIgnorePatterns(
111115

112116
// RunWithConfig is a convenience function that runs the analysis with a Config object
113117
// directly supporting multiple ignore patterns.
114-
func RunWithConfig(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) {
118+
func RunWithConfig(files []*ast.File, fset *token.FileSet, typeInfo *types.Info, cfg *Config) ([]Issue, error) {
115119
p := NewWithIgnorePatterns(
116120
"",
117121
"",
@@ -120,6 +124,7 @@ func RunWithConfig(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue
120124
cfg.MatchWithConstants,
121125
cfg.ParseNumbers,
122126
cfg.FindDuplicates,
127+
cfg.EvalConstExpressions,
123128
cfg.NumberMin,
124129
cfg.NumberMax,
125130
cfg.MinStringLength,
@@ -176,9 +181,9 @@ func RunWithConfig(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue
176181
ast.Walk(&treeVisitor{
177182
fileSet: fset,
178183
packageName: emptyStr,
179-
fileName: emptyStr,
180184
p: p,
181185
ignoreRegex: p.ignoreStringsRegex,
186+
typeInfo: typeInfo,
182187
}, f)
183188
}(f)
184189
}
@@ -277,6 +282,6 @@ func RunWithConfig(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue
277282
// Run analyzes the provided AST files for duplicated strings or numbers
278283
// according to the provided configuration.
279284
// 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)
285+
func Run(files []*ast.File, fset *token.FileSet, typeInfo *types.Info, cfg *Config) ([]Issue, error) {
286+
return RunWithConfig(files, fset, typeInfo, cfg)
282287
}

api_test.go

Lines changed: 118 additions & 9 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

@@ -53,6 +54,20 @@ func example() {
5354
},
5455
expectedIssues: 1,
5556
},
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"
64+
}`,
65+
config: &Config{
66+
FindDuplicates: true,
67+
EvalConstExpressions: true,
68+
},
69+
expectedIssues: 1,
70+
},
5671
{
5772
name: "string duplication with ignore",
5873
code: `package example
@@ -164,7 +179,10 @@ func example() {
164179
t.Fatalf("Failed to parse test code: %v", err)
165180
}
166181

167-
issues, err := Run([]*ast.File{f}, fset, tt.config)
182+
chkr, info := checker(fset)
183+
_ = chkr.Files([]*ast.File{f})
184+
185+
issues, err := Run([]*ast.File{f}, fset, info, tt.config)
168186
if err != nil {
169187
t.Fatalf("Run() error = %v", err)
170188
}
@@ -201,7 +219,10 @@ func example() {
201219
MatchWithConstants: true,
202220
}
203221

204-
issues, err := Run([]*ast.File{f}, fset, config)
222+
chkr, info := checker(fset)
223+
_ = chkr.Files([]*ast.File{f})
224+
225+
issues, err := Run([]*ast.File{f}, fset, info, config)
205226
if err != nil {
206227
t.Fatalf("Run() error = %v", err)
207228
}
@@ -256,16 +277,16 @@ func example2() {
256277
expectedOccurrenceCount: 3,
257278
},
258279
{
259-
name: "duplicate consts in different packages",
260-
code1: `package package1
280+
name: "duplicate consts in different files",
281+
code1: `package example
261282
const ConstA = "shared"
262283
const ConstB = "shared"
263284
`,
264-
code2: `package package2
285+
code2: `package example
265286
const (
266287
ConstC = "shared"
267288
ConstD = "shared"
268-
ConstE= "unique"
289+
ConstE = "unique"
269290
)`,
270291
config: &Config{
271292
FindDuplicates: true,
@@ -290,7 +311,10 @@ const (
290311
t.Fatalf("Failed to parse test code: %v", err)
291312
}
292313

293-
issues, err := Run([]*ast.File{f1, f2}, fset, tt.config)
314+
chkr, info := checker(fset)
315+
_ = chkr.Files([]*ast.File{f1, f2})
316+
317+
issues, err := Run([]*ast.File{f1, f2}, fset, info, tt.config)
294318
if err != nil {
295319
t.Fatalf("Run() error = %v", err)
296320
}
@@ -348,8 +372,10 @@ func allContexts(param string) string {
348372
MinStringLength: 3,
349373
MinOccurrences: 2,
350374
}
375+
chkr, info := checker(fset)
376+
_ = chkr.Files([]*ast.File{f})
351377

352-
issues, err := Run([]*ast.File{f}, fset, config)
378+
issues, err := Run([]*ast.File{f}, fset, info, config)
353379
if err != nil {
354380
t.Fatalf("Run() error = %v", err)
355381
}
@@ -429,8 +455,10 @@ func multipleContexts() {
429455
MinOccurrences: 2,
430456
ExcludeTypes: tt.excludeTypes,
431457
}
458+
chkr, info := checker(fset)
459+
_ = chkr.Files([]*ast.File{f})
432460

433-
issues, err := Run([]*ast.File{f}, fset, config)
461+
issues, err := Run([]*ast.File{f}, fset, info, config)
434462
if err != nil {
435463
t.Fatalf("Run() error = %v", err)
436464
}
@@ -453,3 +481,84 @@ func multipleContexts() {
453481
})
454482
}
455483
}
484+
485+
func TestConstExpressions(t *testing.T) {
486+
// Test detecting and matching string constants derived from expressions
487+
code := `package example
488+
489+
const (
490+
Prefix = "example.com/"
491+
Label1 = Prefix + "some_label"
492+
Label2 = Prefix + "another_label"
493+
)
494+
495+
func example() {
496+
// These should match the constants from expressions
497+
a := "example.com/some_label"
498+
b := "example.com/some_label"
499+
500+
// This should also match
501+
web1 := "example.com/another_label"
502+
web2 := "example.com/another_label"
503+
}
504+
`
505+
fset := token.NewFileSet()
506+
f, err := parser.ParseFile(fset, "example.go", code, 0)
507+
if err != nil {
508+
t.Fatalf("Failed to parse test code: %v", err)
509+
}
510+
511+
config := &Config{
512+
MinStringLength: 3,
513+
MinOccurrences: 2,
514+
MatchWithConstants: true,
515+
EvalConstExpressions: true,
516+
}
517+
chkr, info := checker(fset)
518+
_ = chkr.Files([]*ast.File{f})
519+
520+
issues, err := Run([]*ast.File{f}, fset, info, config)
521+
if err != nil {
522+
t.Fatalf("Run() error = %v", err)
523+
}
524+
525+
// We expect issues for both labels
526+
expectedMatches := map[string]string{
527+
"example.com/some_label": "Label1",
528+
"example.com/another_label": "Label2",
529+
}
530+
531+
// Check that we have two issues
532+
if len(issues) != 2 {
533+
t.Errorf("Expected 2 issues, got %d", len(issues))
534+
for _, issue := range issues {
535+
t.Logf("Found issue: %q matches constant %q with %d occurrences",
536+
issue.Str, issue.MatchingConst, issue.OccurrencesCount)
537+
}
538+
return
539+
}
540+
541+
// Check that each string matches the expected constant
542+
for _, issue := range issues {
543+
expectedConst, ok := expectedMatches[issue.Str]
544+
if !ok {
545+
t.Errorf("Unexpected issue for string: %s", issue.Str)
546+
continue
547+
}
548+
549+
if issue.MatchingConst != expectedConst {
550+
t.Errorf("For string %q: got matching const %q, want %q",
551+
issue.Str, issue.MatchingConst, expectedConst)
552+
}
553+
}
554+
}
555+
556+
func checker(fset *token.FileSet) (*types.Checker, *types.Info) {
557+
cfg := &types.Config{
558+
Error: func(err error) {},
559+
}
560+
info := &types.Info{
561+
Types: make(map[ast.Expr]types.TypeAndValue),
562+
}
563+
return types.NewChecker(cfg, fset, types.NewPackage("", "example"), info), info
564+
}

0 commit comments

Comments
 (0)