Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve #69: Support struct literal comment directive #70

47 changes: 45 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,35 @@ Flags:
4ex:
github.com/GaijinEntertainment/go-exhaustruct/v3/analyzer\.<anonymous>
github.com/GaijinEntertainment/go-exhaustruct/v3/analyzer\.TypeInfo

-use-directives value
Boolean that determines whether to scan for comment directives in the form of
'//exhaustruct:enforce' and '//exhaustruct:ignore' for enforcement decisions. Comment
directives on struct literals have higher precedence than command-level inclusion and
exclusion flags.

Default: false
```

### Example

Invocation

```shell
exhaustruct -e 'mymodule/excluded.*' -i 'mymodule/.*' -use-directives 'true'
```

Code

```go
// Package excluded.go
package excluded

type Point struct {
X int
Y int
}

// Package a.go
package a

Expand All @@ -68,10 +92,18 @@ var b Shape = Shape{
Width: 3,
}

// valid due to ignore directive in spite of missing `volume` (when -use-directives=true)
//exhaustruct:ignore
var c Shape = Shape{
Length: 5,
Width: 3,
}

// Package b.go
package b

import "a"
import "excluded"

// valid
var b Shape = a.Shape{
Expand All @@ -80,7 +112,18 @@ var b Shape = a.Shape{
}

// invalid, `Width` is missing
var b Shape = a.Shape{
var c Shape = a.Shape{
Length: 5,
}
```

// valid due to exclusion on this type
var d = excluded.Point{
X: 1,
}

// invalid, `Y` is missing, due to enforce directive (when -use-directives=true)
//exhaustruct:enforce
var e = excluded.Point{
X: 1,
}
```
149 changes: 126 additions & 23 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"go/ast"
"go/token"
"go/types"
"strings"
"sync"

"golang.org/x/tools/go/analysis"
Expand All @@ -17,20 +18,26 @@ import (
)

type analyzer struct {
include pattern.List `exhaustruct:"optional"`
exclude pattern.List `exhaustruct:"optional"`
include pattern.List `exhaustruct:"optional"`
exclude pattern.List `exhaustruct:"optional"`
useDirectives bool

fieldsCache map[types.Type]fields.StructFields
fieldsCacheMu sync.RWMutex `exhaustruct:"optional"`

typeProcessingNeed map[string]bool
typeProcessingNeedMu sync.RWMutex `exhaustruct:"optional"`

commentMapCache map[*ast.File]ast.CommentMap `exhaustruct:"optional"`
commentMapCacheMu sync.RWMutex `exhaustruct:"optional"`
}

func NewAnalyzer(include, exclude []string) (*analysis.Analyzer, error) {
func NewAnalyzer(include, exclude []string, useDirectives bool) (*analysis.Analyzer, error) {
a := analyzer{
fieldsCache: make(map[types.Type]fields.StructFields),
typeProcessingNeed: make(map[string]bool),
commentMapCache: make(map[*ast.File]ast.CommentMap),
useDirectives: useDirectives,
}

var err error
Expand Down Expand Up @@ -67,6 +74,9 @@ Anonymous structs can be matched by '<anonymous>' alias.
4ex:
github.com/GaijinEntertainment/go-exhaustruct/v3/analyzer\.<anonymous>
github.com/GaijinEntertainment/go-exhaustruct/v3/analyzer\.TypeInfo`)
fs.BoolVar(&a.useDirectives, "use-directives", a.useDirectives,
`Use directives to enforce or ignore analysis on a per struct literal basis, overriding
any include/exclude patterns. Default: false.`)

return *fs
}
Expand Down Expand Up @@ -115,7 +125,12 @@ func (a *analyzer) newVisitor(pass *analysis.Pass) func(n ast.Node, push bool, s
}
}

pos, msg := a.processStruct(pass, lit, structTyp, typeInfo)
var enforcement EnforcementDirective
if a.useDirectives {
enforcement = a.decideEnforcementDirective(pass, lit, stack)
}

pos, msg := a.processStruct(pass, lit, structTyp, typeInfo, enforcement)
if pos != nil {
pass.Reportf(*pos, msg)
}
Expand Down Expand Up @@ -179,8 +194,9 @@ func (a *analyzer) processStruct(
lit *ast.CompositeLit,
structTyp *types.Struct,
info *TypeInfo,
enforcement EnforcementDirective,
) (*token.Pos, string) {
if !a.shouldProcessType(info) {
if !a.shouldProcessLit(info, enforcement) {
return nil, ""
}

Expand All @@ -201,13 +217,92 @@ func (a *analyzer) processStruct(
return nil, ""
}

// shouldProcessType returns true if type should be processed basing off include
// and exclude patterns, defined though constructor and\or flags.
func (a *analyzer) shouldProcessType(info *TypeInfo) bool {
// shouldProcessLit returns true if type should be processed basing off include
// and exclude patterns, defined though constructor and\or flags, as well as off
// comment directives.
func (a *analyzer) shouldProcessLit(
info *TypeInfo, enforcement EnforcementDirective,
) bool {
// enforcement directives always have highest precedence if present
switch enforcement {
case Enforce:
return true

case Ignore:
return false

case EnforcementUnspecified:
}

if len(a.include) == 0 && len(a.exclude) == 0 {
return true
}

return a.isTypeProcessingNeeded(info)
}

//revive:disable-next-line:unused-receiver
func (a *analyzer) litSkippedFields(
lit *ast.CompositeLit,
typ *types.Struct,
onlyExported bool,
) fields.StructFields {
a.fieldsCacheMu.RLock()
f, ok := a.fieldsCache[typ]
a.fieldsCacheMu.RUnlock()

if !ok {
a.fieldsCacheMu.Lock()
f = fields.NewStructFields(typ)
a.fieldsCache[typ] = f
a.fieldsCacheMu.Unlock()
}

return f.SkippedFields(lit, onlyExported)
}

func (a *analyzer) decideEnforcementDirective(
pass *analysis.Pass, lit *ast.CompositeLit, stack []ast.Node,
) EnforcementDirective {
if !a.useDirectives {
return EnforcementUnspecified
}

//revive:disable-next-line:unchecked-type-assertion
file, _ := stack[0].(*ast.File)
commentMap := a.getFileCommentMap(pass.Fset, file)

if enforcement := parseEnforcement(commentMap[lit]); enforcement != EnforcementUnspecified {
return enforcement
}

parent := stack[len(stack)-2]
// allow directives to appear in parent nodes except other composite literals
if _, parentIsCompLit := parent.(*ast.CompositeLit); parentIsCompLit {
return EnforcementUnspecified
}

return parseEnforcement(commentMap[parent])
}

func (a *analyzer) getFileCommentMap(fileSet *token.FileSet, file *ast.File) ast.CommentMap {
a.commentMapCacheMu.RLock()
commentMap, exists := a.commentMapCache[file]
a.commentMapCacheMu.RUnlock()

if !exists {
// TODO: consider avoiding risk of double-computation by using per-file mutex
commentMap = ast.NewCommentMap(fileSet, file, file.Comments)

a.commentMapCacheMu.Lock()
a.commentMapCache[file] = commentMap
a.commentMapCacheMu.Unlock()
}

return commentMap
}

func (a *analyzer) isTypeProcessingNeeded(info *TypeInfo) bool {
name := info.String()

a.typeProcessingNeedMu.RLock()
Expand All @@ -233,26 +328,34 @@ func (a *analyzer) shouldProcessType(info *TypeInfo) bool {
return res
}

//revive:disable-next-line:unused-receiver
func (a *analyzer) litSkippedFields(
lit *ast.CompositeLit,
typ *types.Struct,
onlyExported bool,
) fields.StructFields {
a.fieldsCacheMu.RLock()
f, ok := a.fieldsCache[typ]
a.fieldsCacheMu.RUnlock()
func parseEnforcement(commentGroups []*ast.CommentGroup) EnforcementDirective {
// go from the end to the beginning
for i := len(commentGroups) - 1; i >= 0; i-- {
for j := len(commentGroups[i].List) - 1; j >= 0; j-- {
c := commentGroups[i].List[j]

if !ok {
a.fieldsCacheMu.Lock()
f = fields.NewStructFields(typ)
a.fieldsCache[typ] = f
a.fieldsCacheMu.Unlock()
normalized := strings.TrimSpace(c.Text)
switch normalized {
case "//exhaustruct:enforce":
return Enforce

case "//exhaustruct:ignore":
return Ignore
}
}
}

return f.SkippedFields(lit, onlyExported)
return EnforcementUnspecified
}

type EnforcementDirective int

const (
EnforcementUnspecified EnforcementDirective = iota
Enforce
Ignore
)

type TypeInfo struct {
Name string
PackageName string
Expand Down
17 changes: 17 additions & 0 deletions analyzer/analyzer_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@ func BenchmarkAnalyzer(b *testing.B) {
a, err := analyzer.NewAnalyzer(
[]string{`.*[Tt]est.*`, `.*External`, `.*Embedded`, `.*\.<anonymous>`},
[]string{`.*Excluded$`, `e\.<anonymous>`},
false,
)
require.NoError(b, err)

b.ResetTimer()
b.ReportAllocs()

for i := 0; i < b.N; i++ {
_ = analysistest.Run(b, testdataPath, a, "i")
}
}

func BenchmarkAnalyzer_WithDirectives(b *testing.B) {
a, err := analyzer.NewAnalyzer(
[]string{`.*[Tt]est.*`, `.*External`, `.*Embedded`, `.*\.<anonymous>`},
[]string{`.*Excluded$`, `e\.<anonymous>`},
true,
)
require.NoError(b, err)

Expand Down
18 changes: 14 additions & 4 deletions analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,37 @@ var testdataPath, _ = filepath.Abs("./testdata/") //nolint:gochecknoglobals
func TestAnalyzer(t *testing.T) {
t.Parallel()

a, err := analyzer.NewAnalyzer([]string{""}, nil)
a, err := analyzer.NewAnalyzer([]string{""}, nil, true)
assert.Nil(t, a)
assert.Error(t, err)

a, err = analyzer.NewAnalyzer([]string{"["}, nil)
a, err = analyzer.NewAnalyzer([]string{"["}, nil, true)
assert.Nil(t, a)
assert.Error(t, err)

a, err = analyzer.NewAnalyzer(nil, []string{""})
a, err = analyzer.NewAnalyzer(nil, []string{""}, true)
assert.Nil(t, a)
assert.Error(t, err)

a, err = analyzer.NewAnalyzer(nil, []string{"["})
a, err = analyzer.NewAnalyzer(nil, []string{"["}, true)
assert.Nil(t, a)
assert.Error(t, err)

a, err = analyzer.NewAnalyzer(
[]string{`.*[Tt]est.*`, `.*External`, `.*Embedded`, `.*\.<anonymous>`},
[]string{`.*Excluded$`, `e\.<anonymous>`},
false,
)
require.NoError(t, err)

analysistest.Run(t, testdataPath, a, "i", "e")

a, err = analyzer.NewAnalyzer(
[]string{`.*[Tt]est.*`, `.*External`, `.*Embedded`, `.*\.<anonymous>`},
[]string{`.*Excluded$`, `e\.<anonymous>`},
true,
)
require.NoError(t, err)

analysistest.Run(t, testdataPath, a, "i", "e", "directives")
}
Loading
Loading