Skip to content

Commit 544739f

Browse files
committed
Release 0.0.12
- refactor - don’t use nil facts Signed-off-by: Oliver Eikemeier <eikemeier@fillmore-labs.com>
1 parent bd3dc3e commit 544739f

9 files changed

Lines changed: 139 additions & 104 deletions

File tree

pkg/internal/diag/fix.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,18 @@ func (d *Diag) ReplaceWithZeroValue(n ast.Node, t types.Type) []analysis.Suggest
3636
return nil
3737
}
3838

39-
var (
40-
needsImport bool
41-
buf bytes.Buffer
42-
)
39+
q := Qualifier{
40+
Pkg: d.pass.Pkg,
41+
}
42+
43+
if d.CurrentFile != nil {
44+
q.Imports = d.CurrentFile.Imports
45+
}
4346

44-
types.WriteType(&buf, t, d.Qualifier(&needsImport))
47+
var buf bytes.Buffer
48+
types.WriteType(&buf, t, q.Qualifier)
4549

46-
if needsImport {
50+
if q.NeedsImport {
4751
return nil
4852
}
4953

pkg/internal/diag/fix_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,15 @@ package diag_test
1919

2020
import (
2121
"go/ast"
22+
"go/token"
2223
"go/types"
2324
"strings"
2425
"testing"
2526

2627
"golang.org/x/tools/go/analysis"
2728
)
2829

29-
func TestDiag_ReplaceWithZeroValue(t *testing.T) {
30+
func TestDiag_ReplaceWithZeroValue(t *testing.T) { //nolint:funlen
3031
t.Parallel()
3132

3233
tests := []struct {
@@ -60,6 +61,23 @@ func TestDiag_ReplaceWithZeroValue(t *testing.T) {
6061
},
6162
expectedNewText: "MyArray{}",
6263
},
64+
{
65+
name: "type from unimported package",
66+
src: "package testpkg\nvar p int\nvar _ = p",
67+
findNodeAndType: func(f *ast.File, _ *types.Package, _ *types.Info) (ast.Node, types.Type) {
68+
valSpec := f.Decls[1].(*ast.GenDecl).Specs[0].(*ast.ValueSpec) // var _ = p
69+
nodeToReplace := valSpec.Values[0] // p
70+
71+
// Create a type from another package that is not imported.
72+
otherPkg := types.NewPackage("example.com/other", "other")
73+
typeName := types.NewTypeName(token.NoPos, otherPkg, "OtherStruct", nil)
74+
namedType := types.NewNamed(typeName, types.NewStruct(nil, nil), nil)
75+
otherPkg.Scope().Insert(typeName)
76+
77+
return nodeToReplace, namedType
78+
},
79+
expectNilFix: true, // Should be nil because the import is missing.
80+
},
6381
{
6482
name: "unsupported type (pointer) for zero value literal",
6583
src: "package testpkg\ntype MyStruct struct{}\nvar pp **MyStruct\nvar _ = pp",

pkg/internal/diag/qualifier.go

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,27 +22,30 @@ import (
2222
"strconv"
2323
)
2424

25+
// Qualifier holds the current package and imports for [Qualifier.Qualifier].
26+
type Qualifier struct {
27+
Pkg *types.Package
28+
Imports []*ast.ImportSpec
29+
NeedsImport bool
30+
}
31+
2532
// Qualifier returns the package name or alias to use when referring to the given package
26-
// in the context of the currently analyzed file (whose imports are in c.CurrentImports).
27-
func (d *Diag) Qualifier(needsImport *bool) func(pkg *types.Package) string {
28-
return func(pkg *types.Package) string {
29-
if pkg == nil || pkg == d.pass.Pkg {
30-
return ""
31-
}
33+
// in the context of the currently analyzed file (whose imports are in q.Imports).
34+
func (q *Qualifier) Qualifier(pkg *types.Package) string {
35+
if pkg == nil || pkg == q.Pkg {
36+
return ""
37+
}
3238

33-
if d.CurrentFile != nil {
34-
for _, i := range d.CurrentFile.Imports {
35-
if qualifier, ok := importName(i, pkg); ok {
36-
return qualifier
37-
}
38-
}
39+
for _, i := range q.Imports {
40+
if qualifier, ok := importName(i, pkg); ok {
41+
return qualifier
3942
}
43+
}
4044

41-
*needsImport = true
45+
q.NeedsImport = true
4246

43-
// Not imported - default to quoted path, breaking the build in an informative way.
44-
return strconv.Quote(pkg.Path())
45-
}
47+
// Not imported - default to quoted path, breaking the build in an informative way.
48+
return strconv.Quote(pkg.Path())
4649
}
4750

4851
// importName extracts the qualifier for a given package from a single import spec.

pkg/internal/diag/qualifier_test.go

Lines changed: 47 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -25,35 +25,33 @@ import (
2525
. "fillmore-labs.com/zerolint/pkg/internal/diag"
2626
)
2727

28-
func TestDiag_Qualifier(t *testing.T) { //nolint:funlen
28+
func TestQualifier_Qualifier(t *testing.T) { //nolint:funlen
2929
t.Parallel()
3030

3131
tests := []struct {
3232
name string
3333
currentPkgPath string
3434
currentFileSrc string
3535
targetPkgProvider func(currentPkg *types.Package) *types.Package
36-
setupDiag func(r *Diag, f *ast.File) // Optional setup, e.g., for malformed imports
36+
importsProvider func(f *ast.File) []*ast.ImportSpec
3737
needsImport bool
3838
want string
3939
}{
4040
{
41-
name: "target package is nil",
42-
currentPkgPath: "example.com/main",
43-
currentFileSrc: `package main`,
44-
targetPkgProvider: func(_ *types.Package) *types.Package {
45-
return nil
46-
},
47-
want: "",
41+
name: "target package is nil",
42+
currentPkgPath: "example.com/main",
43+
currentFileSrc: `package main`,
44+
targetPkgProvider: func(_ *types.Package) *types.Package { return nil },
45+
importsProvider: func(f *ast.File) []*ast.ImportSpec { return f.Imports },
46+
want: "",
4847
},
4948
{
50-
name: "target package is current package",
51-
currentPkgPath: "example.com/main",
52-
currentFileSrc: `package main`,
53-
targetPkgProvider: func(currentPkg *types.Package) *types.Package {
54-
return currentPkg
55-
},
56-
want: "",
49+
name: "target package is current package",
50+
currentPkgPath: "example.com/main",
51+
currentFileSrc: `package main`,
52+
targetPkgProvider: func(currentPkg *types.Package) *types.Package { return currentPkg },
53+
importsProvider: func(f *ast.File) []*ast.ImportSpec { return f.Imports },
54+
want: "",
5755
},
5856
{
5957
name: "target not imported (no imports in file)",
@@ -62,8 +60,9 @@ func TestDiag_Qualifier(t *testing.T) { //nolint:funlen
6260
targetPkgProvider: func(_ *types.Package) *types.Package {
6361
return types.NewPackage("example.com/other", "other")
6462
},
65-
needsImport: true,
66-
want: "\"example.com/other\"",
63+
importsProvider: func(f *ast.File) []*ast.ImportSpec { return f.Imports },
64+
needsImport: true,
65+
want: "\"example.com/other\"",
6766
},
6867
{
6968
name: "target imported without alias",
@@ -72,7 +71,8 @@ func TestDiag_Qualifier(t *testing.T) { //nolint:funlen
7271
targetPkgProvider: func(_ *types.Package) *types.Package {
7372
return types.NewPackage("example.com/other", "otherpkgname") // Name() should be used
7473
},
75-
want: "otherpkgname",
74+
importsProvider: func(f *ast.File) []*ast.ImportSpec { return f.Imports },
75+
want: "otherpkgname",
7676
},
7777
{
7878
name: "target imported with alias",
@@ -81,7 +81,8 @@ func TestDiag_Qualifier(t *testing.T) { //nolint:funlen
8181
targetPkgProvider: func(_ *types.Package) *types.Package {
8282
return types.NewPackage("example.com/other", "other")
8383
},
84-
want: "custom",
84+
importsProvider: func(f *ast.File) []*ast.ImportSpec { return f.Imports },
85+
want: "custom",
8586
},
8687
{
8788
name: "target imported with dot import",
@@ -90,7 +91,8 @@ func TestDiag_Qualifier(t *testing.T) { //nolint:funlen
9091
targetPkgProvider: func(_ *types.Package) *types.Package {
9192
return types.NewPackage("example.com/other", "other")
9293
},
93-
want: "",
94+
importsProvider: func(f *ast.File) []*ast.ImportSpec { return f.Imports },
95+
want: "",
9496
},
9597
{
9698
name: "target imported with underscore import",
@@ -99,8 +101,9 @@ func TestDiag_Qualifier(t *testing.T) { //nolint:funlen
99101
targetPkgProvider: func(_ *types.Package) *types.Package {
100102
return types.NewPackage("example.com/other", "other")
101103
},
102-
needsImport: true,
103-
want: "\"example.com/other\"",
104+
importsProvider: func(f *ast.File) []*ast.ImportSpec { return f.Imports },
105+
needsImport: true,
106+
want: "\"example.com/other\"",
104107
},
105108
{
106109
name: "target not imported",
@@ -109,8 +112,9 @@ func TestDiag_Qualifier(t *testing.T) { //nolint:funlen
109112
targetPkgProvider: func(_ *types.Package) *types.Package {
110113
return types.NewPackage("example.com/other", "other")
111114
},
112-
needsImport: true,
113-
want: "\"example.com/other\"",
115+
importsProvider: func(f *ast.File) []*ast.ImportSpec { return f.Imports },
116+
needsImport: true,
117+
want: "\"example.com/other\"",
114118
},
115119
{
116120
name: "import path unquote error skips spec",
@@ -119,54 +123,48 @@ func TestDiag_Qualifier(t *testing.T) { //nolint:funlen
119123
targetPkgProvider: func(_ *types.Package) *types.Package {
120124
return types.NewPackage("malformed/path", "skipped")
121125
},
122-
setupDiag: func(_ *Diag, f *ast.File) {
126+
importsProvider: func(f *ast.File) []*ast.ImportSpec {
123127
// Manually add an import spec with a path that strconv.Unquote would fail on if it wasn't already quoted.
124128
// Forcing a direct error from strconv.Unquote is tricky as parser usually creates valid string literals.
125129
// This simulates a scenario where i.Path.Value is not a valid quoted string.
126130
// A real AST from a parser would have `Value: "\"malformed/path\""`.
127131
// If `Value` was just `"malformed/path"` (no quotes), Unquote returns it as is with an error.
128132
f.Imports = append(f.Imports, &ast.ImportSpec{Path: &ast.BasicLit{Kind: token.STRING, Value: "malformed/path"}})
133+
134+
return f.Imports
129135
},
130136
needsImport: true,
131137
want: "\"malformed/path\"", // Falls through to strconv.Quote(pkg.Path()) as the malformed import is skipped
132138
},
133-
{
134-
name: "current file is nil",
135-
currentPkgPath: "example.com/main",
136-
currentFileSrc: `package main`, // Source doesn't matter here
137-
targetPkgProvider: func(_ *types.Package) *types.Package {
138-
return types.NewPackage("example.com/other", "other")
139-
},
140-
setupDiag: func(d *Diag, _ *ast.File) {
141-
d.CurrentFile = nil
142-
},
143-
needsImport: true,
144-
want: "\"example.com/other\"",
145-
},
146139
}
147140

148141
for _, tt := range tests {
149142
t.Run(tt.name, func(t *testing.T) {
150143
t.Parallel()
151144

152-
currentPkg, fset, currentASTFile := parseSourceImports(t, tt.currentPkgPath, tt.currentFileSrc)
153-
info := &types.Info{} // Minimal info, not used by Qualifier method
145+
currentPkg, _, currentASTFile := parseSourceImports(t, tt.currentPkgPath, tt.currentFileSrc)
154146

155-
d := newTestDiag(t, info, currentPkg, fset, currentASTFile)
156-
if tt.setupDiag != nil {
157-
tt.setupDiag(d, currentASTFile)
147+
q := Qualifier{
148+
Pkg: currentPkg,
149+
Imports: tt.importsProvider(currentASTFile),
158150
}
159151

160152
targetPkgToQualify := tt.targetPkgProvider(currentPkg)
161-
needsImport := false
162-
got := d.Qualifier(&needsImport)(targetPkgToQualify)
153+
got := q.Qualifier(targetPkgToQualify)
154+
155+
var targetPkgPath string
156+
if targetPkgToQualify == nil {
157+
targetPkgPath = "<nil>"
158+
} else {
159+
targetPkgPath = targetPkgToQualify.Path()
160+
}
163161

164-
if needsImport != tt.needsImport {
165-
t.Errorf("Qualifier(%s) needsImport %t, want %t", targetPkgToQualify.Path(), needsImport, tt.needsImport)
162+
if q.NeedsImport != tt.needsImport {
163+
t.Errorf("Qualifier(%s) needsImport %t, want %t", targetPkgPath, q.NeedsImport, tt.needsImport)
166164
}
167165

168166
if got != tt.want {
169-
t.Errorf("Qualifier(%s) = %q, want %q", targetPkgToQualify.Path(), got, tt.want)
167+
t.Errorf("Qualifier(%s) = %q, want %q", targetPkgPath, got, tt.want)
170168
}
171169
})
172170
}

pkg/internal/passes/exclusions/analyzer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ var Analyzer = &analysis.Analyzer{ //nolint:gochecknoglobals
2828
Name: "exclusions",
2929
Doc: "determine type exclusions for later passes",
3030
URL: "https://pkg.go.dev/fillmore-labs.com/zerolint/pkg/internal/passes/exclusions",
31-
Run: run,
31+
Run: func(p *analysis.Pass) (any, error) { return (*pass)(p).run() },
3232
RunDespiteErrors: true,
3333

3434
FactTypes: []analysis.Fact{(*excludedFact)(nil)},

0 commit comments

Comments
 (0)