Skip to content

Commit ab0f702

Browse files
committed
fix the detection for oneoflint case
improve test cases by using pointer and value receivers, add a verbose mode for linter debugging, make sure integration tests passes
1 parent afbebbd commit ab0f702

File tree

10 files changed

+104
-58
lines changed

10 files changed

+104
-58
lines changed

.github/workflows/go.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,11 @@ jobs:
2222
- name: Build
2323
run: go build -v ./...
2424

25+
- name: sumlint
26+
run: go build github.com/gomoni/sumlint/cmd/sumlint
27+
28+
- name: oneoflint
29+
run: go build github.com/gomoni/sumlint/cmd/oneoflint
30+
2531
- name: Test
2632
run: go test -race -v ./...

cmd/oneoflint/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package main
22

33
import (
4-
"golang.org/x/tools/go/analysis/unitchecker"
5-
64
"github.com/gomoni/sumlint/internal/lint"
5+
6+
"golang.org/x/tools/go/analysis/unitchecker"
77
)
88

99
func main() {

cmd/sumlint/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package main
22

33
import (
4-
"golang.org/x/tools/go/analysis/unitchecker"
5-
64
"github.com/gomoni/sumlint/internal/lint"
5+
6+
"golang.org/x/tools/go/analysis/unitchecker"
77
)
88

99
func main() {

internal/lint/lint.go

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,29 @@
11
package lint
22

33
import (
4+
"flag"
45
"go/ast"
56
"go/token"
67
"go/types"
8+
"log/slog"
9+
"os"
710
"sort"
811
"strings"
912

1013
"golang.org/x/tools/go/analysis"
1114
)
1215

16+
func init() {
17+
var verboseFlag bool
18+
19+
fs1 := flag.NewFlagSet(Sum.Name, flag.ExitOnError)
20+
fs1.BoolVar(&verboseFlag, "verbose", false, "enable verbose output")
21+
Sum.Flags = *fs1
22+
fs2 := flag.NewFlagSet(Oneof.Name, flag.ExitOnError)
23+
fs2.BoolVar(&verboseFlag, "verbose", false, "enable verbose output")
24+
Oneof.Flags = *fs2
25+
}
26+
1327
// InterfaceImplementorsFact stores (packagePath.InterfaceName) -> implementor type names (qualified)
1428
type InterfaceImplementorsFact struct {
1529
Implementors []string // fully qualified (pkgpath.TypeName)
@@ -45,11 +59,28 @@ type analyzer struct {
4559
}
4660

4761
func (a analyzer) run(pass *analysis.Pass) (any, error) {
62+
flagVerbose := getFlag(pass.Analyzer.Flags, "verbose", false)
63+
if flagVerbose {
64+
sumLog := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug}))
65+
sumLog = sumLog.With("analyzer", pass.Analyzer.Name)
66+
slog.SetDefault(sumLog)
67+
}
68+
4869
sumIfaces := a.discoverSumInterfaces(pass)
4970
if len(sumIfaces) > 0 {
71+
slog.Debug("discovered interfaces", "count", len(sumIfaces))
5072
exportImplementorFacts(pass, sumIfaces)
5173
}
5274
ifaceImpls := loadAllInterfaceFacts(pass)
75+
if flagVerbose {
76+
count := 0
77+
for _, impls := range ifaceImpls {
78+
count += len(impls)
79+
}
80+
if count > 0 {
81+
slog.Debug("discovered implementations", "count", count)
82+
}
83+
}
5384

5485
for _, f := range pass.Files {
5586
ast.Inspect(f, func(n ast.Node) bool {
@@ -157,6 +188,7 @@ func exportImplementorFacts(pass *analysis.Pass, sumIfaces map[*types.TypeName]*
157188
if types.Implements(named, ifaceType) || types.Implements(types.NewPointer(named), ifaceType) {
158189
key := qualifiedTypeName(tn)
159190
byIface[ifaceObj][key] = struct{}{}
191+
160192
}
161193
}
162194
}
@@ -167,6 +199,7 @@ func exportImplementorFacts(pass *analysis.Pass, sumIfaces map[*types.TypeName]*
167199
list := make([]string, 0, len(implSet))
168200
for k := range implSet {
169201
list = append(list, k)
202+
slog.Debug("exporting implementation", "interface", ifaceObj.Name(), "name", k)
170203
}
171204
sort.Strings(list) // deterministic ordering for tests
172205
pass.ExportObjectFact(ifaceObj, &InterfaceImplementorsFact{Implementors: list})
@@ -190,6 +223,7 @@ func loadAllInterfaceFacts(pass *analysis.Pass) map[*types.TypeName]map[string]s
190223
if pass.ImportObjectFact(tn, &fact) {
191224
set := make(map[string]struct{}, len(fact.Implementors))
192225
for _, t := range fact.Implementors {
226+
slog.Debug("loadAllInterfaceFacts: scopeNames", "tn", tn, "t", t)
193227
set[t] = struct{}{}
194228
}
195229
res[tn] = set
@@ -211,14 +245,14 @@ func loadAllInterfaceFacts(pass *analysis.Pass) map[*types.TypeName]map[string]s
211245
if pass.ImportObjectFact(tn, &fact) {
212246
set := make(map[string]struct{}, len(fact.Implementors))
213247
for _, t := range fact.Implementors {
248+
slog.Debug("loadAllInterfaceFacts: ImportObjectFact", "tn", tn, "t", t)
214249
set[t] = struct{}{}
215250
}
216251
res[tn] = set
217252
}
218253
}
219254
return res
220255
}
221-
222256
func checkTypeSwitch(pass *analysis.Pass, ts *ast.TypeSwitchStmt, ifaceImpls map[*types.TypeName]map[string]struct{}) {
223257
var asserted types.Type
224258
switch ass := ts.Assign.(type) {
@@ -230,30 +264,58 @@ func checkTypeSwitch(pass *analysis.Pass, ts *ast.TypeSwitchStmt, ifaceImpls map
230264
if !ok || ta.Type != nil {
231265
return
232266
}
233-
asserted = pass.TypesInfo.TypeOf(ta.X)
267+
asserted = pass.TypesInfo.TypeOf(ta.X) // e.g. msg.GetPayload()
234268
case *ast.ExprStmt:
235269
ta, ok := ass.X.(*ast.TypeAssertExpr)
236270
if !ok || ta.Type != nil {
237271
return
238272
}
239-
asserted = pass.TypesInfo.TypeOf(ta.X)
273+
asserted = pass.TypesInfo.TypeOf(ta.X) // e.g. msg.GetPayload()
240274
default:
241275
return
242276
}
243277

244278
if asserted == nil {
245279
return
246280
}
281+
if ptr, ok := asserted.(*types.Pointer); ok {
282+
asserted = ptr.Elem()
283+
}
247284
if _, ok := asserted.Underlying().(*types.Interface); !ok {
248285
return
249286
}
287+
250288
var ifaceObj *types.TypeName
251289
if named, ok := asserted.(*types.Named); ok {
252290
ifaceObj = named.Obj()
253291
} else {
292+
// Try match by underlying interface identity
293+
for tn := range ifaceImpls {
294+
if tn.Type().Underlying() == asserted.Underlying() {
295+
ifaceObj = tn
296+
break
297+
}
298+
}
299+
}
300+
if ifaceObj == nil {
254301
return
255302
}
303+
256304
implSet, hasFact := ifaceImpls[ifaceObj]
305+
// If this package did not discover the interface (e.g. unexported in another package),
306+
// attempt to import its fact now that we have the *types.TypeName from the type switch expression.
307+
if !hasFact || len(implSet) == 0 {
308+
var fact InterfaceImplementorsFact
309+
if pass.ImportObjectFact(ifaceObj, &fact) && len(fact.Implementors) > 0 {
310+
newSet := make(map[string]struct{}, len(fact.Implementors))
311+
for _, implObj := range fact.Implementors {
312+
newSet[implObj] = struct{}{}
313+
}
314+
ifaceImpls[ifaceObj] = newSet
315+
implSet = newSet
316+
hasFact = true
317+
}
318+
}
257319
if !hasFact || len(implSet) == 0 {
258320
return
259321
}
@@ -309,3 +371,14 @@ func qualifiedTypeName(tn *types.TypeName) string {
309371
}
310372
return tn.Pkg().Path() + "." + tn.Name()
311373
}
374+
375+
func getFlag[T any](fs flag.FlagSet, key string, def T) T {
376+
if fl := fs.Lookup(key); fl != nil {
377+
if g, ok := fl.Value.(flag.Getter); ok {
378+
if t, ok := g.Get().(T); ok {
379+
return t
380+
}
381+
}
382+
}
383+
return def
384+
}

list_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,16 @@ func TestLint(t *testing.T) {
6363
scenario: "sumlint",
6464
given: sumlint,
6565
then: []string{`# github.com/gomoni/sumlint/test`,
66-
`./sum.go:23:2: missing default case on SumFoo: code cannot handle nil interface`,
67-
`./sum.go:29:2: non-exhaustive type switch on SumFoo: missing cases for: github.com/gomoni/sumlint/test.B`,
66+
`./sum.go:15:2: missing default case on SumFoo: code cannot handle nil interface`,
67+
`./sum.go:21:2: non-exhaustive type switch on SumFoo: missing cases for: github.com/gomoni/sumlint/test/sum.B`,
6868
},
6969
},
7070
{
7171
scenario: "oneoflint",
7272
given: oneoflint,
7373
then: []string{`# github.com/gomoni/sumlint/test`,
74-
`./sum.go:23:2: missing default case on SumFoo: code cannot handle nil interface`,
75-
`./sum.go:29:2: non-exhaustive type switch on SumFoo: missing cases for: github.com/gomoni/sumlint/test.B`,
74+
`./oneof.go:16:2: missing default case on isMsg_Payload: code cannot handle nil interface`,
75+
`./oneof.go:24:2: non-exhaustive type switch on isMsg_Payload: missing cases for: github.com/gomoni/sumlint/test/one_of.Msg_B`,
7676
},
7777
},
7878
}

test/oneof.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,26 @@ package test
22

33
import "github.com/gomoni/sumlint/test/one_of"
44

5-
type oneof struct{}
5+
type oneofTest struct{}
66

7-
func (o oneof) good(msg *one_of.Msg) {
7+
func (oneofTest) good(msg *one_of.Msg) {
88
switch msg.GetPayload().(type) {
99
case *one_of.Msg_A:
1010
case *one_of.Msg_B:
1111
default:
1212
}
1313
}
1414

15-
func (o oneof) nonDefault(msg *one_of.Msg) {
15+
func (oneofTest) noDefault(msg *one_of.Msg) {
1616
switch msg.GetPayload().(type) {
1717
case *one_of.Msg_A:
1818
case *one_of.Msg_B:
1919
}
2020
}
2121

22-
func (o oneof) noB(msg *one_of.Msg) {
23-
switch msg.GetPayload().(type) {
22+
func (oneofTest) noB(msg *one_of.Msg) {
23+
payload := msg.GetPayload()
24+
switch payload.(type) {
2425
case *one_of.Msg_A:
2526
default:
2627
}

test/sum.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,22 @@ package test
22

33
import "github.com/gomoni/sumlint/test/sum"
44

5-
func good(x sum.SumFoo) {
5+
type sumTest struct{}
6+
7+
func (sumTest) good(x sum.SumFoo) {
68
switch x.(type) {
7-
case sum.A, sum.B:
9+
case sum.A, *sum.B:
810
default:
911
}
1012
}
1113

12-
func noDefault(x sum.SumFoo) {
14+
func (sumTest) noDefault(x sum.SumFoo) {
1315
switch x.(type) {
14-
case sum.A, sum.B:
16+
case sum.A, *sum.B:
1517
}
1618
}
1719

18-
func noB(x sum.SumFoo) {
20+
func (sumTest) noB(x sum.SumFoo) {
1921
switch x.(type) {
2022
case sum.A:
2123
default:

test/sum/sum.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@ func (A) sumFoo() {}
1010

1111
type B struct{}
1212

13-
func (B) sumFoo() {}
13+
func (*B) sumFoo() {}

tests/go.mod

Lines changed: 0 additions & 3 deletions
This file was deleted.

tests/sum.go

Lines changed: 0 additions & 33 deletions
This file was deleted.

0 commit comments

Comments
 (0)