Skip to content

Commit

Permalink
feat: add support for ignore and enforce comment directives (#88)
Browse files Browse the repository at this point in the history
  • Loading branch information
xobotyi authored Jan 4, 2024
1 parent 91ba6cd commit a2d55a9
Show file tree
Hide file tree
Showing 7 changed files with 243 additions and 9 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
name: "Test"
strategy:
matrix:
go-version: [ "1.21" ]
go-version: [ "1.20", "1.21" ]
os: [ "ubuntu-latest", "windows-latest", "macos-latest" ]
runs-on: ${{ matrix.os }}
steps:
Expand Down
2 changes: 2 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ linters-settings:
disabled: false
arguments: [ 6 ]
# disabled rules
- name: unchecked-type-assertion # forcetypeassert is already used
disabled: true
- name: max-public-structs # quite annoying rule
disabled: true
- name: banned-characters # we don't have banned chars
Expand Down
54 changes: 46 additions & 8 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"

"github.com/GaijinEntertainment/go-exhaustruct/v3/internal/comment"
"github.com/GaijinEntertainment/go-exhaustruct/v3/internal/fields"
"github.com/GaijinEntertainment/go-exhaustruct/v3/internal/pattern"
)
Expand All @@ -23,6 +24,8 @@ type analyzer struct {
fieldsCache map[types.Type]fields.StructFields
fieldsCacheMu sync.RWMutex `exhaustruct:"optional"`

comments comment.Cache

typeProcessingNeed map[string]bool
typeProcessingNeedMu sync.RWMutex `exhaustruct:"optional"`
}
Expand All @@ -31,6 +34,7 @@ func NewAnalyzer(include, exclude []string) (*analysis.Analyzer, error) {
a := analyzer{
fieldsCache: make(map[types.Type]fields.StructFields),
typeProcessingNeed: make(map[string]bool),
comments: comment.Cache{},
}

var err error
Expand Down Expand Up @@ -74,12 +78,7 @@ Anonymous structs can be matched by '<anonymous>' alias.
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) //nolint:forcetypeassert

insp.WithStack(
[]ast.Node{
(*ast.CompositeLit)(nil),
},
a.newVisitor(pass),
)
insp.WithStack([]ast.Node{(*ast.CompositeLit)(nil)}, a.newVisitor(pass))

return nil, nil //nolint:nilnil
}
Expand Down Expand Up @@ -115,7 +114,10 @@ func (a *analyzer) newVisitor(pass *analysis.Pass) func(n ast.Node, push bool, s
}
}

pos, msg := a.processStruct(pass, lit, structTyp, typeInfo)
file := a.comments.Get(pass.Fset, stack[0].(*ast.File)) //nolint:forcetypeassert
rc := getCompositeLitRelatedComments(stack, file)
pos, msg := a.processStruct(pass, lit, structTyp, typeInfo, rc)

if pos != nil {
pass.Reportf(*pos, msg)
}
Expand All @@ -124,6 +126,35 @@ func (a *analyzer) newVisitor(pass *analysis.Pass) func(n ast.Node, push bool, s
}
}

// getCompositeLitRelatedComments returns all comments that are related to checked node. We
// have to traverse the stack manually as ast do not associate comments with
// [ast.CompositeLit].
func getCompositeLitRelatedComments(stack []ast.Node, cm ast.CommentMap) []*ast.CommentGroup {
comments := make([]*ast.CommentGroup, 0)

for i := len(stack) - 1; i >= 0; i-- {
node := stack[i]

switch node.(type) {
case *ast.CompositeLit, // stack[len(stack)-1]
*ast.ReturnStmt, // return ...
*ast.IndexExpr, // map[enum]...{...}[key]
*ast.CallExpr, // myfunc(map...)
*ast.UnaryExpr, // &map...
*ast.AssignStmt, // variable assignment (without var keyword)
*ast.DeclStmt, // var declaration, parent of *ast.GenDecl
*ast.GenDecl, // var declaration, parent of *ast.ValueSpec
*ast.ValueSpec: // var declaration
comments = append(comments, cm[node]...)

default:
return comments
}
}

return comments
}

func getStructType(pass *analysis.Pass, lit *ast.CompositeLit) (*types.Struct, *TypeInfo, bool) {
switch typ := pass.TypesInfo.TypeOf(lit).(type) {
case *types.Named: // named type
Expand Down Expand Up @@ -179,8 +210,15 @@ func (a *analyzer) processStruct(
lit *ast.CompositeLit,
structTyp *types.Struct,
info *TypeInfo,
comments []*ast.CommentGroup,
) (*token.Pos, string) {
if !a.shouldProcessType(info) {
shouldProcess := a.shouldProcessType(info)

if shouldProcess && comment.HasDirective(comments, comment.DirectiveIgnore) {
return nil, ""
}

if !shouldProcess && !comment.HasDirective(comments, comment.DirectiveEnforce) {
return nil, ""
}

Expand Down
45 changes: 45 additions & 0 deletions analyzer/testdata/src/i/directives.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package i

func excludedConsumer(e TestExcluded) string {
return e.A
}

func shouldNotFailOnIgnoreDirective() (Test, error) {
// directive on previous line
//exhaustruct:ignore
_ = Test2{}

// directive at the end of the line
_ = Test{} //exhaustruct:ignore

// some style weirdness
_ =
//exhaustruct:ignore
Test3{
B: 0,
}

// directive after the literal
_ = Test{
B: 0,
} //exhaustruct:ignore

//exhaustruct:ignore
return Test{}, nil
}

func shouldFailOnExcludedButEnforced() {
// directive on previous line associated with different ast leaf
//exhaustruct:enforce
_ = excludedConsumer(TestExcluded{B: 0}) // want "i.TestExcluded is missing field A"

// initially excluded, but enforced
//exhaustruct:enforce
_ = TestExcluded{} // want "i.TestExcluded is missing fields A, B"
}

func shouldFailOnMisappliedDirectives() {
// wrong directive name
//exhaustive:enforce
_ = TestExcluded{B: 0}
}
35 changes: 35 additions & 0 deletions internal/comment/cache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package comment

import (
"go/ast"
"go/token"
"sync"
)

type Cache struct {
comments map[*ast.File]ast.CommentMap
mu sync.RWMutex
}

// Get returns a comment map for a given file. In case if a comment map is not
// found, it creates a new one.
func (c *Cache) Get(fset *token.FileSet, f *ast.File) ast.CommentMap {
c.mu.RLock()
if cm, ok := c.comments[f]; ok {
c.mu.RUnlock()
return cm
}
c.mu.RUnlock()

c.mu.Lock()
defer c.mu.Unlock()

if c.comments == nil {
c.comments = make(map[*ast.File]ast.CommentMap)
}

cm := ast.NewCommentMap(fset, f, f.Comments)
c.comments[f] = cm

return cm
}
28 changes: 28 additions & 0 deletions internal/comment/directive.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package comment

import (
"go/ast"
"strings"
)

type Directive string

const (
prefix = `//exhaustruct:`
DirectiveIgnore Directive = prefix + `ignore`
DirectiveEnforce Directive = prefix + `enforce`
)

// HasDirective parses a directive from a given list of comments.
// If no directive is found, the second return value is `false`.
func HasDirective(comments []*ast.CommentGroup, expected Directive) bool {
for _, cg := range comments {
for _, commentLine := range cg.List {
if strings.HasPrefix(commentLine.Text, string(expected)) {
return true
}
}
}

return false
}
86 changes: 86 additions & 0 deletions internal/comment/directive_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package comment_test

import (
"go/ast"
"testing"

"github.com/stretchr/testify/assert"

"github.com/GaijinEntertainment/go-exhaustruct/v3/internal/comment"
)

func TestParseDirective(t *testing.T) {
t.Parallel()

tests := []struct {
name string
comments []*ast.CommentGroup
directive comment.Directive
found bool
}{
{
name: "no directive",
comments: []*ast.CommentGroup{
{
List: []*ast.Comment{
{
Text: "// some comment",
},
},
},
},
directive: comment.DirectiveIgnore,
found: false,
},
{
name: "directive found",
comments: []*ast.CommentGroup{
{
List: []*ast.Comment{
{
Text: "//exhaustruct:ignore",
},
{
Text: "// some comment",
},
{
Text: "//exhaustruct:enforce",
},
},
},
},
directive: comment.DirectiveIgnore,
found: true,
},
{
name: "directive found (partial line match)",
comments: []*ast.CommentGroup{
{
List: []*ast.Comment{
{
Text: "//exhaustruct:ignore",
},
{
Text: "// some comment",
},
{
Text: "//exhaustruct:enforce beacuse of some reason",
},
},
},
},
directive: comment.DirectiveEnforce,
found: true,
},
}

for _, tt := range tests {
tt := tt

t.Run(tt.name, func(t *testing.T) {
t.Parallel()

assert.Equal(t, tt.found, comment.HasDirective(tt.comments, tt.directive))
})
}
}

0 comments on commit a2d55a9

Please sign in to comment.