diff --git a/pkg/analysis/commentstart/analyzer.go b/pkg/analysis/commentstart/analyzer.go index 52d4d680..5b461a84 100644 --- a/pkg/analysis/commentstart/analyzer.go +++ b/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 } diff --git a/pkg/analysis/commentstart/testdata/src/a/a.go b/pkg/analysis/commentstart/testdata/src/a/a.go index e68b2351..73f7d7de 100644 --- a/pkg/analysis/commentstart/testdata/src/a/a.go +++ b/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 +} diff --git a/pkg/analysis/commentstart/testdata/src/a/a.go.golden b/pkg/analysis/commentstart/testdata/src/a/a.go.golden index 643ce2dc..92d70dec 100644 --- a/pkg/analysis/commentstart/testdata/src/a/a.go.golden +++ b/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 +} diff --git a/pkg/analysis/helpers/inspector/analyzer.go b/pkg/analysis/helpers/inspector/analyzer.go new file mode 100644 index 00000000..d9a51a60 --- /dev/null +++ b/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 +} diff --git a/pkg/analysis/helpers/inspector/analyzer_test.go b/pkg/analysis/helpers/inspector/analyzer_test.go new file mode 100644 index 00000000..f823c372 --- /dev/null +++ b/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 +} diff --git a/pkg/analysis/helpers/inspector/doc.go b/pkg/analysis/helpers/inspector/doc.go new file mode 100644 index 00000000..c106f604 --- /dev/null +++ b/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 diff --git a/pkg/analysis/helpers/inspector/inspector.go b/pkg/analysis/helpers/inspector/inspector.go new file mode 100644 index 00000000..ce161109 --- /dev/null +++ b/pkg/analysis/helpers/inspector/inspector.go @@ -0,0 +1,87 @@ +package inspector + +import ( + "go/ast" + "go/token" + + "github.com/JoelSpeed/kal/pkg/analysis/helpers/extractjsontags" + "github.com/JoelSpeed/kal/pkg/analysis/helpers/markers" + astinspector "golang.org/x/tools/go/ast/inspector" +) + +// Inspector is an interface that allows for the inspection of fields in structs. +type Inspector interface { + // InspectFields is a function that iterates over fields in structs. + InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers)) +} + +// inspector implements the Inspector interface. +type inspector struct { + inspector *astinspector.Inspector + jsonTags extractjsontags.StructFieldTags + markers markers.Markers +} + +// newInspector creates a new inspector. +func newInspector(astinspector *astinspector.Inspector, jsonTags extractjsontags.StructFieldTags, markers markers.Markers) Inspector { + return &inspector{ + inspector: astinspector, + jsonTags: jsonTags, + markers: markers, + } +} + +// InspectFields iterates over fields in structs, ignoring any struct that is not a type declaration, and any field that is ignored and +// therefore would not be included in the CRD spec. +// For the remaining fields, it calls the provided inspectField function to apply analysis logic. +func (i *inspector) InspectFields(inspectField func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers)) { + // Filter to fields so that we can iterate over fields in a struct. + nodeFilter := []ast.Node{ + (*ast.Field)(nil), + } + + i.inspector.WithStack(nodeFilter, func(n ast.Node, push bool, stack []ast.Node) (proceed bool) { + if !push { + return false + } + + if len(stack) < 3 { + 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 { + // Make sure that we don't inspect structs within a function. + 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 + } + + _, ok = stack[len(stack)-3].(*ast.StructType) + if !ok { + // A field within a struct has a FieldList parent and then a StructType parent. + // If we don't have a StructType parent, then we're not in a struct. + return false + } + + field, ok := n.(*ast.Field) + if !ok { + return true + } + + tagInfo := i.jsonTags.FieldTags(field) + if tagInfo.Ignored { + // Returning false here means we won't inspect the children of an ignored field. + return false + } + + inspectField(field, stack, tagInfo, i.markers) + + return true + }) +} diff --git a/pkg/analysis/helpers/inspector/testdata/src/a/a.go b/pkg/analysis/helpers/inspector/testdata/src/a/a.go new file mode 100644 index 00000000..3ba1bda5 --- /dev/null +++ b/pkg/analysis/helpers/inspector/testdata/src/a/a.go @@ -0,0 +1,59 @@ +package A + +var ( + String string +) + +const ( + Int int = 0 +) + +type A struct { + Field string `json:"field"` // want "field: Field" "json tag: field" + + B `json:"b"` // want "field: B" "json tag: b" + + C `json:",inline"` // want "field: C" + + D `json:"-"` + + E struct { // want "field: E" "json tag: e" + Field string `json:"field"` // want "field: Field" "json tag: field" + } `json:"e"` + + F struct { + Field string `json:"field"` + } `json:"-"` +} + +func (A) DoNothing() {} + +type B struct { + Field string `json:"field"` // want "field: Field" "json tag: field" +} + +type ( + C struct { + Field string `json:"field"` // want "field: Field" "json tag: field" + } + + D struct { + Field string `json:"field"` // want "field: Field" "json tag: field" + } +) + +func Foo() { + type Bar struct { + Field string + } +} + +type Bar interface { + Name() string +} + +var Var = struct { + Field string +}{ + Field: "field", +} diff --git a/pkg/analysis/jsontags/analyzer.go b/pkg/analysis/jsontags/analyzer.go index 4a2b4c79..fc44fa3f 100644 --- a/pkg/analysis/jsontags/analyzer.go +++ b/pkg/analysis/jsontags/analyzer.go @@ -4,15 +4,14 @@ import ( "errors" "fmt" "go/ast" - "go/token" "regexp" "github.com/JoelSpeed/kal/pkg/analysis/helpers/extractjsontags" + "github.com/JoelSpeed/kal/pkg/analysis/helpers/inspector" + "github.com/JoelSpeed/kal/pkg/analysis/helpers/markers" "github.com/JoelSpeed/kal/pkg/config" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" - "golang.org/x/tools/go/ast/inspector" ) const ( @@ -24,7 +23,6 @@ const ( var ( errCouldNotGetInspector = errors.New("could not get inspector") - errCouldNotGetJSONTags = errors.New("could not get json tags") ) type analyzer struct { @@ -48,61 +46,24 @@ func newAnalyzer(cfg config.JSONTagsConfig) (*analysis.Analyzer, error) { Name: name, Doc: "Check that all struct fields in an API are tagged with json tags", Run: a.run, - Requires: []*analysis.Analyzer{inspect.Analyzer, extractjsontags.Analyzer}, + Requires: []*analysis.Analyzer{inspector.Analyzer}, }, nil } func (a *analyzer) 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 fields 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 a.checkField(pass, field, jsonTags) + inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) { + a.checkField(pass, field, jsonTagInfo) }) return nil, nil //nolint:nilnil } -func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, jsonTags extractjsontags.StructFieldTags) (proceed bool) { - tagInfo := jsonTags.FieldTags(field) - +func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.FieldTagInfo) { var prefix string if len(field.Names) > 0 && field.Names[0] != nil { prefix = fmt.Sprintf("field %s", field.Names[0].Name) @@ -112,29 +73,22 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, jsonTags ex if tagInfo.Missing { pass.Reportf(field.Pos(), "%s is missing json tag", prefix) - return true + return } if tagInfo.Inline { - return true - } - - if tagInfo.Ignored { - // Returning false here means we won't inspect the children of an ignored field. - return false + return } if tagInfo.Name == "" { pass.Reportf(field.Pos(), "%s has empty json tag", prefix) - return true + return } matched := a.jsonTagRegex.Match([]byte(tagInfo.Name)) if !matched { pass.Reportf(field.Pos(), "%s json tag does not match pattern %q: %s", prefix, a.jsonTagRegex.String(), tagInfo.Name) } - - return true } func defaultConfig(cfg *config.JSONTagsConfig) { diff --git a/pkg/analysis/jsontags/testdata/src/a/a.go b/pkg/analysis/jsontags/testdata/src/a/a.go index f89614f5..4dc7c49b 100644 --- a/pkg/analysis/jsontags/testdata/src/a/a.go +++ b/pkg/analysis/jsontags/testdata/src/a/a.go @@ -46,3 +46,7 @@ type C struct{} type D struct{} type E struct{} + +type Interface interface { + InaccessibleFunction() string +} diff --git a/pkg/analysis/maxlength/analyzer.go b/pkg/analysis/maxlength/analyzer.go index 00109b06..1c01a316 100644 --- a/pkg/analysis/maxlength/analyzer.go +++ b/pkg/analysis/maxlength/analyzer.go @@ -5,10 +5,10 @@ import ( "fmt" "go/ast" + "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 ( @@ -27,7 +27,6 @@ const ( var ( errCouldNotGetInspector = errors.New("could not get inspector") - errCouldNotGetMarkers = errors.New("could not get markers") ) // Analyzer is the analyzer for the maxlength package. @@ -36,38 +35,17 @@ var Analyzer = &analysis.Analyzer{ Name: name, Doc: "Checks that all strings formatted fields are marked with a maximum length, and that arrays are marked with max items.", Run: run, - Requires: []*analysis.Analyzer{inspect.Analyzer, markers.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 } - markersAccess, ok := pass.ResultOf[markers.Analyzer].(markers.Markers) - if !ok { - return nil, errCouldNotGetMarkers - } - - // Filter to structs so that we can iterate over the fields in a struct. - nodeFilter := []ast.Node{ - (*ast.StructType)(nil), - } - - inspect.Preorder(nodeFilter, func(n ast.Node) { - sTyp, ok := n.(*ast.StructType) - if !ok { - return - } - - if sTyp.Fields == nil { - return - } - - for _, field := range sTyp.Fields.List { - checkField(pass, field, markersAccess) - } + inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) { + checkField(pass, field, markersAccess) }) return nil, nil //nolint:nilnil diff --git a/pkg/analysis/maxlength/testdata/src/a/a.go b/pkg/analysis/maxlength/testdata/src/a/a.go index f19f409e..8ffa51bd 100644 --- a/pkg/analysis/maxlength/testdata/src/a/a.go +++ b/pkg/analysis/maxlength/testdata/src/a/a.go @@ -72,6 +72,13 @@ type MaxLength struct { StringAliasArrayWithMaxItemsAndMaxElementLengthOnAlias []StringAliasWithMaxLength StringAliasArrayWithoutMaxItemsWithMaxElementLengthOnAlias []StringAliasWithMaxLength // want "field StringAliasArrayWithoutMaxItemsWithMaxElementLengthOnAlias must have a maximum items, add kubebuilder:validation:MaxItems" + + Struct struct { + // +kubebuilder:validation:MaxLength:=256 + StringWithMaxLength string + + StringWithoutMaxLength string // want "field StringWithoutMaxLength must have a maximum length, add kubebuilder:validation:MaxLength marker" + } `json:"struct"` } // StringAlias is a string without a MaxLength. diff --git a/pkg/analysis/optionalorrequired/analyzer.go b/pkg/analysis/optionalorrequired/analyzer.go index 74b866eb..b98d7fca 100644 --- a/pkg/analysis/optionalorrequired/analyzer.go +++ b/pkg/analysis/optionalorrequired/analyzer.go @@ -6,11 +6,10 @@ import ( "go/ast" "github.com/JoelSpeed/kal/pkg/analysis/helpers/extractjsontags" + "github.com/JoelSpeed/kal/pkg/analysis/helpers/inspector" "github.com/JoelSpeed/kal/pkg/analysis/helpers/markers" "github.com/JoelSpeed/kal/pkg/config" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" - "golang.org/x/tools/go/ast/inspector" ) const ( @@ -40,8 +39,6 @@ func init() { var ( errCouldNotGetInspector = errors.New("could not get inspector") - errCouldNotGetMarkers = errors.New("could not get markers") - errCouldNotGetJSONTags = errors.New("could not get jsontags") ) type analyzer struct { @@ -80,47 +77,18 @@ func newAnalyzer(cfg config.OptionalOrRequiredConfig) *analysis.Analyzer { Name: name, Doc: "Checks that all struct fields are marked either with the optional or required markers.", Run: a.run, - Requires: []*analysis.Analyzer{inspect.Analyzer, markers.Analyzer, extractjsontags.Analyzer}, + Requires: []*analysis.Analyzer{inspector.Analyzer}, } } func (a *analyzer) 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 } - markersAccess, ok := pass.ResultOf[markers.Analyzer].(markers.Markers) - if !ok { - return nil, errCouldNotGetMarkers - } - - jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags) - if !ok { - return nil, errCouldNotGetJSONTags - } - - // Filter to fields so that we can iterate over fields in a struct. - nodeFilter := []ast.Node{ - (*ast.StructType)(nil), - } - - inspect.Preorder(nodeFilter, func(n ast.Node) { - sTyp, ok := n.(*ast.StructType) - if !ok { - return - } - - if sTyp.Fields == nil { - return - } - - for _, field := range sTyp.Fields.List { - fieldMarkers := markersAccess.FieldMarkers(field) - fieldTagInfo := jsonTags.FieldTags(field) - - a.checkField(pass, field, fieldMarkers, fieldTagInfo) - } + inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) { + a.checkField(pass, field, markersAccess.FieldMarkers(field), jsonTagInfo) }) return nil, nil //nolint:nilnil diff --git a/pkg/analysis/optionalorrequired/testdata/src/a/a.go b/pkg/analysis/optionalorrequired/testdata/src/a/a.go index 5e866731..470c2be1 100644 --- a/pkg/analysis/optionalorrequired/testdata/src/a/a.go +++ b/pkg/analysis/optionalorrequired/testdata/src/a/a.go @@ -57,3 +57,7 @@ func (A) DoNothing() {} type B struct{} type C struct{} + +type Interface interface { + InaccessibleFunction() string +} diff --git a/pkg/analysis/optionalorrequired/testdata/src/a/a.go.golden b/pkg/analysis/optionalorrequired/testdata/src/a/a.go.golden index dbf157d6..5dad146a 100644 --- a/pkg/analysis/optionalorrequired/testdata/src/a/a.go.golden +++ b/pkg/analysis/optionalorrequired/testdata/src/a/a.go.golden @@ -52,3 +52,7 @@ func (A) DoNothing() {} type B struct{} type C struct{} + +type Interface interface { + InaccessibleFunction() string +} diff --git a/pkg/analysis/requiredfields/analyzer.go b/pkg/analysis/requiredfields/analyzer.go index 6aa50c34..04aea3be 100644 --- a/pkg/analysis/requiredfields/analyzer.go +++ b/pkg/analysis/requiredfields/analyzer.go @@ -7,11 +7,10 @@ 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" "github.com/JoelSpeed/kal/pkg/config" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" - "golang.org/x/tools/go/ast/inspector" ) const ( @@ -27,8 +26,6 @@ func init() { var ( errCouldNotGetInspector = errors.New("could not get inspector") - errCouldNotGetMarkers = errors.New("could not get markers") - errCouldNotGetJSONTags = errors.New("could not get json tags") ) type analyzer struct { @@ -47,41 +44,18 @@ func newAnalyzer(cfg config.RequiredFieldsConfig) *analysis.Analyzer { Name: name, Doc: "Checks that all required fields are not pointers, and do not have the omitempty tag.", Run: a.run, - Requires: []*analysis.Analyzer{inspect.Analyzer, markers.Analyzer, extractjsontags.Analyzer}, + Requires: []*analysis.Analyzer{inspector.Analyzer}, } } func (a *analyzer) 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 } - markersAccess, ok := pass.ResultOf[markers.Analyzer].(markers.Markers) - if !ok { - return nil, errCouldNotGetMarkers - } - - jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags) - if !ok { - return nil, errCouldNotGetJSONTags - } - - // Filter to fields so that we can iterate over fields in a struct. - nodeFilter := []ast.Node{ - (*ast.Field)(nil), - } - - inspect.Preorder(nodeFilter, func(n ast.Node) { - field, ok := n.(*ast.Field) - if !ok { - return - } - - fieldMarkers := markersAccess.FieldMarkers(field) - fieldTagInfo := jsonTags.FieldTags(field) - - a.checkField(pass, field, fieldMarkers, fieldTagInfo) + inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) { + a.checkField(pass, field, markersAccess.FieldMarkers(field), jsonTagInfo) }) return nil, nil //nolint:nilnil diff --git a/pkg/analysis/requiredfields/testdata/src/a/a.go b/pkg/analysis/requiredfields/testdata/src/a/a.go index 4244d302..0a38569b 100644 --- a/pkg/analysis/requiredfields/testdata/src/a/a.go +++ b/pkg/analysis/requiredfields/testdata/src/a/a.go @@ -40,3 +40,7 @@ type A struct { // DoNothing is used to check that the analyser doesn't report on methods. func (A) DoNothing() {} + +type Interface interface { + InaccessibleFunction() string +} diff --git a/pkg/analysis/requiredfields/testdata/src/a/a.go.golden b/pkg/analysis/requiredfields/testdata/src/a/a.go.golden index b0fd49db..46db7e81 100644 --- a/pkg/analysis/requiredfields/testdata/src/a/a.go.golden +++ b/pkg/analysis/requiredfields/testdata/src/a/a.go.golden @@ -40,3 +40,7 @@ type A struct { // DoNothing is used to check that the analyser doesn't report on methods. func (A) DoNothing() {} + +type Interface interface { + InaccessibleFunction() string +}