Panic when trying to run with a struct that has an internal error field

[rcaetanomeli]

When trying to run my linters, the musttag fails with the following stack trace:

ERRO [runner] Panic: musttag: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference: goroutine 1559 [running]:
runtime/debug.Stack()
        /usr/local/go/src/runtime/debug/stack.go:24 +0x65
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()
        /Users/rcaetano/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:109 +0x285
panic({0x100d21fe0, 0x1018fbdb0})
        /usr/local/go/src/runtime/panic.go:884 +0x213
go/types.(*Package).Path(...)
        /usr/local/go/src/go/types/package.go:31
github.com/junk1tm/musttag.(*checker).parseStructType(0xc001912fb1?, {0x1010e16c8?, 0xc00019bf80?}, 0x4?)
        /Users/rcaetano/go/pkg/mod/github.com/junk1tm/[email protected]/musttag.go:195 +0x86
github.com/junk1tm/musttag.(*checker).checkStructType(0xc00152d7e0, 0xc002825ae0, {0x100e6ac88, 0x4})
        /Users/rcaetano/go/pkg/mod/github.com/junk1tm/[email protected]/musttag.go:238 +0x125
github.com/junk1tm/musttag.run.func1({0x1010e0ca0?, 0xc0012b0e40})
        /Users/rcaetano/go/pkg/mod/github.com/junk1tm/[email protected]/musttag.go:155 +0x2db
golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0xc000792498, {0xc00152d9f8?, 0x1?, 0x10001324e?}, 0xc00152da08)
        /Users/rcaetano/go/pkg/mod/golang.org/x/[email protected]/go/ast/inspector/inspector.go:81 +0x9f
github.com/junk1tm/musttag.run(0xc0005a2870, 0xc00152db28)
        /Users/rcaetano/go/pkg/mod/github.com/junk1tm/[email protected]/musttag.go:108 +0x1a5
github.com/junk1tm/musttag.New.func1(0xc0005a2870?)
        /Users/rcaetano/go/pkg/mod/github.com/junk1tm/[email protected]/musttag.go:54 +0x33c
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc003008d70)
        /Users/rcaetano/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:195 +0xa25
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
        /Users/rcaetano/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:113 +0x1d
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc001600a50, {0x100e769ac, 0x7}, 0xc001291748)
        /Users/rcaetano/go/pkg/mod/github.com/golangci/[email protected]/pkg/timeutils/stopwatch.go:111 +0x4a
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0x2074696e69202f2f?)
        /Users/rcaetano/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:112 +0x85
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc003008d70)
        /Users/rcaetano/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0xb4
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
        /Users/rcaetano/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x208 
WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: musttag: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference 
ERRO Running error: 1 error occurred:
        * can't run linter goanalysis_metalinter: goanalysis_metalinter: musttag: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference

I track down the panic, and got the following example:

package main

import "encoding/json"

type MyError struct {
	Code    string `json:"code"`
	Message string `json:"message"`
	Err     error  `json:"-"`
}

func (e MyError) Error() string {
	return e.Err.Error()
}

func UnmarshalJSON(data []byte) (error, bool) {
	var myErr MyError
	if err := json.Unmarshal(data, &myErr); err != nil {
		return nil, false
	}
	return myErr, true
}

The problem seems to occur when trying to parse the Err field in the parseStructType function. I can solve it by disabling musttag in MyError struct, instead of using - json tag. But it took me a while to track this down, so I think a fix would be appropriated.

[rcaetanomeli]

Actually, even if I disable the lint with //nolint:musttag, if I use json:"-" in the Err the problem will occur. I have to remove the tag from the error, and disable the lint, to not complain about the field without tag.

[tmzane]

Hello @rcaetanomeli, and thank you for the report!

Looks like this is the same problem as in #38 that has been already fixed in #39, at least I can't reproduce the panic using your example on the current main.

I assume you're using the latest version of golangci-lint, which unfortunately does not contain the above fix at the moment. So, let's just way for the next release 😅

Actually, even if I disable the lint with //nolint:musttag, if I use json:"-" in the Err the problem will occur.

The thing is, the //nolint filters apply after a linter's run (if I understand it correctly), that's why musttag still tries to parse the Err field, and thus the panic occurs. Again, the merged fix should be enough to prevent it.

[tmzane]

Closing, as the new golangci-lint version has been released, which includes the mentioned fix 🎉

Let me know if the problem still persists.

[rcaetanomeli]

Great! Thanks @tmzane