feat: better report message (#27)

musttag.go

@@ -6,6 +6,8 @@ import (
 	"go/ast"
 	"go/token"
 	"go/types"
+	"path"
+	"path/filepath"
 	"reflect"
 	"strconv"
 	"strings"
@@ -23,6 +25,11 @@ type Func struct {
 	ArgPos int    // ArgPos is the position of the argument to check.
 }
 
+func (fn Func) shortName() string {
+	name := strings.NewReplacer("*", "", "(", "", ")", "").Replace(fn.Name)
+	return path.Base(name)
+}
+
 // New creates a new musttag analyzer.
 // To report a custom function provide its description via Func,
 // it will be added to the builtin ones.
@@ -77,26 +84,25 @@ var (
 	// should the same struct be reported only once for the same tag?
 	reportOnce = true
 
-	// reportf is a wrapper for pass.Reportf (as a variable, so it could be mocked in tests).
-	reportf = func(pass *analysis.Pass, pos token.Pos, fn Func) {
-		// TODO(junk1tm): print the name of the struct type as well?
-		pass.Reportf(pos, "exported fields should be annotated with the %q tag", fn.Tag)
+	reportf = func(pass *analysis.Pass, st *structType, fn Func, fnPos token.Position) {
+		const format = "`%s` should be annotated with the `%s` tag as it is passed to `%s` at %s"
+		pass.Reportf(st.Pos, format, st.Name, fn.Tag, fn.shortName(), fnPos)
 	}
 
 	// HACK(junk1tm): mainModulePackages() does not return packages from `testdata`,
 	// because it is ignored by the go tool, and thus, by the `go list` command.
-	// For tests to pass we need to add the package with tests to the main module manually.
-	testPackage string
+	// For tests to pass we need to add the packages with tests to the main module manually.
+	testPackages []string
 )
 
 // run starts the analysis.
 func run(pass *analysis.Pass, funcs map[string]Func) (any, error) {
-	mainModule, err := mainModulePackages()
+	moduleDir, modulePackages, err := mainModule()
 	if err != nil {
 		return nil, err
 	}
-	if testPackage != "" {
-		mainModule[testPackage] = struct{}{}
+	for _, pkg := range testPackages {
+		modulePackages[pkg] = struct{}{}
 	}
 
 	// store previous reports to prevent reporting
@@ -147,7 +153,7 @@ func run(pass *analysis.Pass, funcs map[string]Func) (any, error) {
 		}
 
 		checker := checker{
-			mainModule: mainModule,
+			mainModule: modulePackages,
 			seenTypes:  make(map[string]struct{}),
 		}
 
@@ -167,7 +173,9 @@ func run(pass *analysis.Pass, funcs map[string]Func) (any, error) {
 			return // already reported.
 		}
 
-		reportf(pass, result.Pos, fn)
+		p := pass.Fset.Position(call.Pos())
+		p.Filename, _ = filepath.Rel(moduleDir, p.Filename)
+		reportf(pass, result, fn, p)
 		reports[r] = struct{}{}
 	})
 
@@ -219,7 +227,7 @@ func (c *checker) parseStructType(t types.Type, pos token.Pos) (*structType, boo
 		return &structType{
 			Struct: t,
 			Pos:    pos,
-			Name:   "{anonymous struct}",
+			Name:   "anonymous struct",
 		}, true
 	}
 

musttag_test.go

@@ -5,7 +5,6 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"golang.org/x/tools/go/analysis"
@@ -18,34 +17,29 @@ func TestAnalyzer(t *testing.T) {
 	// So, to be able to run tests with external dependencies,
 	// we first need to write a GOPATH-like tree of stubs.
 	prepareTestFiles(t)
-	testPackage = "tests"
+	testPackages = []string{"tests", "builtins"}
 
 	analyzer := New(
 		Func{Name: "example.com/custom.Marshal", Tag: "custom", ArgPos: 0},
 		Func{Name: "example.com/custom.Unmarshal", Tag: "custom", ArgPos: 1},
 	)
 
 	t.Run("builtins", func(t *testing.T) {
+		original := reportf
+		reportf = func(pass *analysis.Pass, st *structType, fn Func, fnPos token.Position) {
+			fnPos.Line = 0 // it's annoying to fix line numbers expectations when a new line is added.
+			original(pass, st, fn, fnPos)
+		}
 		testdata := analysistest.TestData()
 		analysistest.Run(t, testdata, analyzer, "builtins")
 	})
 
 	t.Run("tests", func(t *testing.T) {
-		original := struct {
-			reportOnce bool
-			reportf    func(pass *analysis.Pass, pos token.Pos, fn Func)
-		}{
-			reportOnce: reportOnce,
-			reportf:    reportf,
-		}
-		defer func() { reportOnce, reportf = original.reportOnce, original.reportf }()
-
 		// for the tests we want to record reports from all functions.
 		reportOnce = false
-		reportf = func(pass *analysis.Pass, pos token.Pos, fn Func) {
-			pass.Reportf(pos, shortName(fn.Name))
+		reportf = func(pass *analysis.Pass, st *structType, fn Func, fnPos token.Position) {
+			pass.Reportf(st.Pos, fn.shortName())
 		}
-
 		testdata := analysistest.TestData()
 		analysistest.Run(t, testdata, analyzer, "tests")
 	})
@@ -79,13 +73,6 @@ func TestFlags(t *testing.T) {
 	})
 }
 
-func shortName(name string) string {
-	name = strings.ReplaceAll(name, "*", "")
-	name = strings.ReplaceAll(name, "(", "")
-	name = strings.ReplaceAll(name, ")", "")
-	return filepath.Base(name)
-}
-
 func prepareTestFiles(t *testing.T) {
 	testdata := analysistest.TestData()
 

testdata/builtins.go

@@ -1,4 +1,4 @@
-package tests
+package builtins
 
 import (
 	"encoding/json"
@@ -13,11 +13,20 @@ import (
 
 // TODO(junk1tm): drop `reportOnce` and test each builtin function.
 
+type User struct { /* want
+	"`User` should be annotated with the `json` tag as it is passed to `json.Marshal` at testdata/src/builtins/builtins.go"
+	"`User` should be annotated with the `xml` tag as it is passed to `xml.Marshal` at testdata/src/builtins/builtins.go"
+	"`User` should be annotated with the `yaml` tag as it is passed to `yaml.v3.Marshal` at testdata/src/builtins/builtins.go"
+	"`User` should be annotated with the `toml` tag as it is passed to `toml.Unmarshal` at testdata/src/builtins/builtins.go"
+	"`User` should be annotated with the `mapstructure` tag as it is passed to `mapstructure.Decode` at testdata/src/builtins/builtins.go"
+	"`User` should be annotated with the `custom` tag as it is passed to `custom.Marshal` at testdata/src/builtins/builtins.go"
+	*/
+	Name  string
+	Email string `json:"email" xml:"email" yaml:"email" toml:"email" mapstructure:"email" custom:"email"`
+}
+
 func testJSON() {
-	var user struct { // want `exported fields should be annotated with the "json" tag`
-		Name  string
-		Email string `json:"email"`
-	}
+	var user User
 	json.Marshal(user)
 	json.MarshalIndent(user, "", "")
 	json.Unmarshal(nil, &user)
@@ -26,10 +35,7 @@ func testJSON() {
 }
 
 func testXML() {
-	var user struct { // want `exported fields should be annotated with the "xml" tag`
-		Name  string
-		Email string `xml:"email"`
-	}
+	var user User
 	xml.Marshal(user)
 	xml.MarshalIndent(user, "", "")
 	xml.Unmarshal(nil, &user)
@@ -40,21 +46,15 @@ func testXML() {
 }
 
 func testYAML() {
-	var user struct { // want `exported fields should be annotated with the "yaml" tag`
-		Name  string
-		Email string `yaml:"email"`
-	}
+	var user User
 	yaml.Marshal(user)
 	yaml.Unmarshal(nil, &user)
 	yaml.NewEncoder(nil).Encode(user)
 	yaml.NewDecoder(nil).Decode(&user)
 }
 
 func testTOML() {
-	var user struct { // want `exported fields should be annotated with the "toml" tag`
-		Name  string
-		Email string `toml:"email"`
-	}
+	var user User
 	toml.Unmarshal(nil, &user)
 	toml.Decode("", &user)
 	toml.DecodeFS(nil, "", &user)
@@ -64,21 +64,15 @@ func testTOML() {
 }
 
 func testMapstructure() {
-	var user struct { // want `exported fields should be annotated with the "mapstructure" tag`
-		Name  string
-		Email string `mapstructure:"email"`
-	}
+	var user User
 	mapstructure.Decode(nil, &user)
 	mapstructure.DecodeMetadata(nil, &user, nil)
 	mapstructure.WeakDecode(nil, &user)
 	mapstructure.WeakDecodeMetadata(nil, &user, nil)
 }
 
 func testCustom() {
-	var user struct { // want `exported fields should be annotated with the "custom" tag`
-		Name  string
-		Email string `custom:"email"`
-	}
+	var user User
 	custom.Marshal(user)
 	custom.Unmarshal(nil, &user)
 }

utils.go

@@ -6,8 +6,8 @@ import (
 	"strings"
 )
 
-// mainModulePackages returns a set of packages that belong to the main module.
-func mainModulePackages() (map[string]struct{}, error) {
+// mainModule returns the directory and the set of packages of the main module.
+func mainModule() (dir string, packages map[string]struct{}, _ error) {
 	// https://pkg.go.dev/cmd/go#hdr-Package_lists_and_patterns
 	// > When using modules, "all" expands to all packages in the main module
 	// > and their dependencies, including dependencies needed by tests of any of those.
@@ -20,15 +20,21 @@ func mainModulePackages() (map[string]struct{}, error) {
 
 	out, err := exec.Command(cmd[0], cmd[1:]...).Output()
 	if err != nil {
-		return nil, fmt.Errorf("running go list: %w", err)
+		return "", nil, fmt.Errorf("running `go list all`: %w", err)
 	}
 
 	list := strings.TrimSpace(string(out))
-	m := make(map[string]struct{}, len(list))
+	packages = make(map[string]struct{}, len(list))
 	for _, pkg := range strings.Split(list, "\n") {
-		m[pkg] = struct{}{}
-		m[pkg+"_test"] = struct{}{} // `*_test` packages belong to the main module, see issue #24.
+		packages[pkg] = struct{}{}
+		packages[pkg+"_test"] = struct{}{} // `*_test` packages belong to the main module, see issue #24.
 	}
 
-	return m, nil
+	out, err = exec.Command("go", "list", "-m", "-f={{.Dir}}").Output()
+	if err != nil {
+		return "", nil, fmt.Errorf("running `go list -m`: %w", err)
+	}
+
+	dir = strings.TrimSpace(string(out))
+	return dir, packages, nil
 }