Refactor struct field selection to Inspector helper

pkg/analysis/commentstart/analyzer.go

@@ -8,16 +8,15 @@ import (
 	"strings"
 
 	"github.com/JoelSpeed/kal/pkg/analysis/helpers/extractjsontags"
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/inspector"
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/markers"
 	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/go/analysis/passes/inspect"
-	"golang.org/x/tools/go/ast/inspector"
 )
 
 const name = "commentstart"
 
 var (
 	errCouldNotGetInspector = errors.New("could not get inspector")
-	errCouldNotGetJSONTags  = errors.New("could not get json tags")
 )
 
 // Analyzer is the analyzer for the commentstart package.
@@ -26,79 +25,37 @@ var Analyzer = &analysis.Analyzer{
 	Name:     name,
 	Doc:      "Check that all struct fields in an API have a godoc, and that the godoc starts with the serialised field name",
 	Run:      run,
-	Requires: []*analysis.Analyzer{inspect.Analyzer, extractjsontags.Analyzer},
+	Requires: []*analysis.Analyzer{inspector.Analyzer},
 }
 
 func run(pass *analysis.Pass) (interface{}, error) {
-	inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
 	if !ok {
 		return nil, errCouldNotGetInspector
 	}
 
-	jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
-	if !ok {
-		return nil, errCouldNotGetJSONTags
-	}
-
-	// Filter to structs so that we can iterate over fields in a struct.
-	nodeFilter := []ast.Node{
-		(*ast.Field)(nil),
-	}
-
-	inspect.WithStack(nodeFilter, func(n ast.Node, push bool, stack []ast.Node) (proceed bool) {
-		if !push {
-			return false
-		}
-
-		if len(stack) < 2 {
-			return true
-		}
-
-		// The 0th node in the stack is the *ast.File.
-		// The 1st node in the stack is the *ast.GenDecl.
-		decl, ok := stack[1].(*ast.GenDecl)
-		if !ok {
-			return false
-		}
-
-		if decl.Tok != token.TYPE {
-			// Returning false here means we won't inspect non-type declarations (e.g. var, const, import).
-			return false
-		}
-
-		field, ok := n.(*ast.Field)
-		if !ok {
-			return true
-		}
-
-		return checkField(pass, field, jsonTags)
+	inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
+		checkField(pass, field, jsonTagInfo)
 	})
 
 	return nil, nil //nolint:nilnil
 }
 
-func checkField(pass *analysis.Pass, field *ast.Field, jsonTags extractjsontags.StructFieldTags) (proceed bool) {
-	if field == nil || len(field.Names) == 0 {
-		// Returning false here means we don't inspect inline fields.
-		// Types of inline fields will be inspected on its declaration.
-		return false
-	}
-
-	tagInfo := jsonTags.FieldTags(field)
-	if tagInfo.Ignored {
-		// Returning false here means we won't inspect the children of an ignored field.
-		return false
-	}
-
+func checkField(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.FieldTagInfo) {
 	if tagInfo.Name == "" {
-		return true
+		return
 	}
 
-	fieldName := field.Names[0].Name
+	var fieldName string
+	if len(field.Names) > 0 {
+		fieldName = field.Names[0].Name
+	} else if ident, ok := field.Type.(*ast.Ident); ok {
+		fieldName = ident.Name
+	}
 
 	if field.Doc == nil {
 		pass.Reportf(field.Pos(), "field %s is missing godoc comment", fieldName)
-		return true
+		return
 	}
 
 	firstLine := field.Doc.List[0]
@@ -125,6 +82,4 @@ func checkField(pass *analysis.Pass, field *ast.Field, jsonTags extractjsontags.
 			pass.Reportf(field.Doc.List[0].Pos(), "godoc for field %s should start with '%s ...'", fieldName, tagInfo.Name)
 		}
 	}
-
-	return true
 }

pkg/analysis/commentstart/testdata/src/a/a.go

@@ -22,6 +22,8 @@ type CommentStartTestStruct struct {
 
 	StructForInlineField `json:",inline"`
 
+	A `json:"a"` // want "field A is missing godoc comment"
+
 	// IncorrectStartComment is a field with an incorrect start to the comment. // want "godoc for field IncorrectStartComment should start with 'incorrectStartComment ...'"
 	IncorrectStartComment string `json:"incorrectStartComment"`
 
@@ -53,6 +55,10 @@ type StructForInlineField struct {
 	NoComment string `json:"noComment"` // want "field NoComment is missing godoc comment"
 }
 
+type A struct {
+	NoComment string `json:"noComment"` // want "field NoComment is missing godoc comment"
+}
+
 type unexportedStruct struct {
 	NoComment string `json:"noComment"` // want "field NoComment is missing godoc comment"
 }
@@ -71,3 +77,7 @@ func FunctionWithStructs() {
 		NoComment string `json:"noComment"`
 	}
 }
+
+type Interface interface {
+	InaccessibleFunction() string
+}

pkg/analysis/commentstart/testdata/src/a/a.go.golden

@@ -22,6 +22,8 @@ type CommentStartTestStruct struct {
 
 	StructForInlineField `json:",inline"`
 
+	A `json:"a"` // want "field A is missing godoc comment"
+
 	// incorrectStartComment is a field with an incorrect start to the comment. // want "godoc for field IncorrectStartComment should start with 'incorrectStartComment ...'"
 	IncorrectStartComment string `json:"incorrectStartComment"`
 
@@ -53,6 +55,10 @@ type StructForInlineField struct {
 	NoComment string `json:"noComment"` // want "field NoComment is missing godoc comment"
 }
 
+type A struct {
+	NoComment string `json:"noComment"` // want "field NoComment is missing godoc comment"
+}
+
 type unexportedStruct struct {
 	NoComment string `json:"noComment"` // want "field NoComment is missing godoc comment"
 }
@@ -71,3 +77,7 @@ func FunctionWithStructs() {
 		NoComment string `json:"noComment"`
 	}
 }
+
+type Interface interface {
+	InaccessibleFunction() string
+}

pkg/analysis/helpers/inspector/analyzer.go

@@ -0,0 +1,51 @@
+package inspector
+
+import (
+	"errors"
+	"reflect"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	astinspector "golang.org/x/tools/go/ast/inspector"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/extractjsontags"
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/markers"
+)
+
+const name = "inspector"
+
+var (
+	errCouldNotGetInspector = errors.New("could not get inspector")
+	errCouldNotGetJSONTags  = errors.New("could not get json tags")
+	errCouldNotGetMarkers   = errors.New("could not get markers")
+)
+
+// Analyzer is the analyzer for the inspector package.
+// It provides common functionality for analyzers that need to inspect fields and struct.
+// Abstracting away filtering of fields that the analyzers should and shouldn't be worrying about.
+var Analyzer = &analysis.Analyzer{
+	Name:       name,
+	Doc:        "Provides common functionality for analyzers that need to inspect fields and struct",
+	Run:        run,
+	Requires:   []*analysis.Analyzer{inspect.Analyzer, extractjsontags.Analyzer, markers.Analyzer},
+	ResultType: reflect.TypeOf(newInspector(nil, nil, nil)),
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	astinspector, ok := pass.ResultOf[inspect.Analyzer].(*astinspector.Inspector)
+	if !ok {
+		return nil, errCouldNotGetInspector
+	}
+
+	jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
+	if !ok {
+		return nil, errCouldNotGetJSONTags
+	}
+
+	markersAccess, ok := pass.ResultOf[markers.Analyzer].(markers.Markers)
+	if !ok {
+		return nil, errCouldNotGetMarkers
+	}
+
+	return newInspector(astinspector, jsonTags, markersAccess), nil
+}

pkg/analysis/helpers/inspector/analyzer_test.go

@@ -0,0 +1,52 @@
+package inspector_test
+
+import (
+	"errors"
+	"go/ast"
+	"testing"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/extractjsontags"
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/inspector"
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/markers"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestInspector(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	analysistest.Run(t, testdata, testAnalyzer, "a")
+}
+
+var errCouldNotGetInspector = errors.New("could not get inspector")
+
+var testAnalyzer = &analysis.Analyzer{
+	Name:     "test",
+	Doc:      "tests the inspector analyzer",
+	Run:      run,
+	Requires: []*analysis.Analyzer{inspector.Analyzer},
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
+	if !ok {
+		return nil, errCouldNotGetInspector
+	}
+
+	inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
+		var fieldName string
+		if len(field.Names) > 0 {
+			fieldName = field.Names[0].Name
+		} else if ident, ok := field.Type.(*ast.Ident); ok {
+			fieldName = ident.Name
+		}
+
+		pass.Reportf(field.Pos(), "field: %v", fieldName)
+
+		if jsonTagInfo.Name != "" {
+			pass.Reportf(field.Pos(), "json tag: %v", jsonTagInfo.Name)
+		}
+	})
+
+	return nil, nil //nolint:nilnil
+}

pkg/analysis/helpers/inspector/doc.go

@@ -0,0 +1,34 @@
+/*
+inspector is a helper package that iterates over fields in structs, calling an inspection function on fields
+that should be considered for analysis.
+
+The inspector extracts common logic of iterating and filtering through struct fields, so that analyzers
+need not re-implement the same filtering over and over.
+
+For example, the inspector filters out struct definitions that are not type declarations, and fields that are ignored.
+
+Example:
+
+	type A struct {
+		// This field is included in the analysis.
+		Field string `json:"field"`
+
+		// This field, and the fields within are ignored due to the json tag.
+		F struct {
+			Field string `json:"field"`
+		} `json:"-"`
+	}
+
+	// Any struct defined within a function is ignored.
+	func Foo() {
+		type Bar struct {
+			Field string
+		}
+	}
+
+	// All fields within interface declarations are ignored.
+	type Bar interface {
+		Name() string
+	}
+*/
+package inspector