Skip to content

Commit

Permalink
Add function exclusion config (#6)
Browse files Browse the repository at this point in the history
  • Loading branch information
rawen17 authored Oct 21, 2022
1 parent 0c4ac0d commit 269b821
Show file tree
Hide file tree
Showing 8 changed files with 373 additions and 65 deletions.
101 changes: 78 additions & 23 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,22 @@

---

`defer` is a golang analyzer that finds defer functions which return anything.
`nakedefer` is a golang analyzer that finds defer functions which return anything.

### Installation

```shell
go get -u github.com/GaijinEntertainment/go-defer/cmd/defer
go get -u github.com/GaijinEntertainment/go-nakedefer/cmd/nakedefer
```

### Usage

```
defer ./...
nakedefer [-flag] [package]
Flags:
-e value
Regular expression to exclude function names
```


Expand All @@ -30,38 +33,90 @@ func funcNotReturnAnyType() {
}

func funcReturnErr() error {
return errors.New("some error")
return errors.New("some error")
}

// valid
func someFuncWithValidDefer1() {
defer func() {
}()
func funcReturnFuncAndErr() (func(), error) {
return func() {
}, nil
}

// valid
func someFuncWithValidDefer2() {
defer funcNotReturnAnyType()
func ignoreFunc() error {
return errors.New("some error")
}

// invalid, deferred call should not return any type
func someFuncWithInvalidDefer1() {
defer func() error {
func testCaseValid1() {
defer funcNotReturnAnyType() // valid

defer func() { // valid
funcNotReturnAnyType()
}()

defer func() { // valid
_ = funcReturnErr()
}()
}

func testCaseInvalid1() {
defer funcReturnErr() // invalid

defer funcReturnFuncAndErr() // invalid

defer func() error { // invalid
return nil
}()

defer func() func() { // invalid
return func() {}
}()
}

// invalid, deferred call should not return any type
func someFuncWithInvalidDefer2() {
defer funcReturnErr()
func testCase1() {
defer fmt.Errorf("some text") // invalid

r := new(bytes.Buffer)
defer io.LimitReader(r, 1) // invalid

srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("DONE"))
}))
defer srv.Close() // invalid
defer srv.CloseClientConnections() // invalid
defer srv.Certificate() // invalid
}

// invalid, deferred call should not return any type
func someFuncWithInvalidDefer3() {
defer func() func() {
return func() {
func testCase2() {
s := datatest.SomeStruct{}
defer s.RetNothing() // valid
defer s.RetErr() // invalid
defer s.RetInAndErr() // invalid
}

func testCaseExclude1() {
// exclude ignoreFunc
defer ignoreFunc() // valid - excluded
}

func testCaseExclude2() {
// exclude os\.(Create|WriteFile|Chmod)
defer os.Create("file_test1") // valid - excluded
defer os.WriteFile("file_test2", []byte("data"), os.ModeAppend) // valid - excluded
defer os.Chmod("file_test3", os.ModeAppend) // valid - excluded
defer os.FindProcess(100500) // invalid
}

func testCaseExclude3() {
// exclude fmt\.Print.*
defer fmt.Println("e1") // valid - excluded
defer fmt.Print("e1") // valid - excluded
defer fmt.Printf("e1") // valid - excluded
defer fmt.Sprintf("some text") // invalid
}

}
}()
func testCaseExclude4() {
// exclude io\.Close
rc, _ := zlib.NewReader(bytes.NewReader([]byte("111")))
defer rc.Close() // valid - excluded
}
```
4 changes: 2 additions & 2 deletions cmd/defer/main.go → cmd/nakedefer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package main
import (
"golang.org/x/tools/go/analysis/singlechecker"

"github.com/GaijinEntertainment/go-defer/pkg/analyzer"
"github.com/GaijinEntertainment/go-nakedefer/pkg/analyzer"
)

func main() {
a, err := analyzer.NewAnalyzer()
a, err := analyzer.NewAnalyzer([]string{})
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module github.com/GaijinEntertainment/go-defer
module github.com/GaijinEntertainment/go-nakedefer

go 1.19

Expand Down
158 changes: 134 additions & 24 deletions pkg/analyzer/analyzer.go
Original file line number Diff line number Diff line change
@@ -1,36 +1,76 @@
package analyzer

import (
"bytes"
"errors"
"flag"
"go/ast"
"go/printer"
"go/token"
"go/types"
"strings"

"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
)

var (
ErrEmptyExcludePattern = errors.New("pattern for excluding function can't be empty")
)

type analyzer struct {
typesInfo *types.Info
exclude PatternsList
}

// NewAnalyzer returns a go/analysis-compatible analyzer.
func NewAnalyzer() (*analysis.Analyzer, error) {
func NewAnalyzer(exclude []string) (*analysis.Analyzer, error) {
a := analyzer{} //nolint:exhaustruct

var err error

a.exclude, err = newPatternsList(exclude)
if err != nil {
return nil, err
}

return &analysis.Analyzer{ //nolint:exhaustruct
Name: "defer",
Name: "nakedefer",
Doc: "Checks that deferred call does not return anything.",
Run: run,
Run: a.run,
Requires: []*analysis.Analyzer{inspect.Analyzer},
Flags: a.newFlagSet(),
}, nil
}

func run(pass *analysis.Pass) (interface{}, error) {
func (a *analyzer) newFlagSet() flag.FlagSet {
fs := flag.NewFlagSet("nakedefer flags", flag.PanicOnError)

fs.Var(
&reListVar{values: &a.exclude},
"e",
"Regular expression to exclude function names",
)

return *fs
}

func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) {
insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) //nolint:forcetypeassert

nodeFilter := []ast.Node{
(*ast.DeferStmt)(nil),
}

insp.Preorder(nodeFilter, newVisitor(pass))
a.typesInfo = pass.TypesInfo

insp.Preorder(nodeFilter, a.newVisitor(pass))

return nil, nil //nolint:nilnil
}

func newVisitor(pass *analysis.Pass) func(node ast.Node) {
func (a *analyzer) newVisitor(pass *analysis.Pass) func(node ast.Node) {
return func(node ast.Node) {
deferStmt, ok := node.(*ast.DeferStmt)
if !ok {
Expand All @@ -41,46 +81,116 @@ func newVisitor(pass *analysis.Pass) func(node ast.Node) {
return
}

var outgoingFieldList *ast.FieldList
funcName := a.funcName(deferStmt.Call)
if funcName != "" && a.exclude.MatchesAny(funcName) {
return
}

var hasReturn bool
switch v := deferStmt.Call.Fun.(type) {
case *ast.Ident: // function is named
outgoingFieldList = getFuncDeclResults(v)
case *ast.FuncLit: // function is anonymous
outgoingFieldList = getFuncLitResults(v)
hasReturn = a.isFuncLitReturnValues(v)
case *ast.Ident:
hasReturn = a.isIdentReturnValues(v)
case *ast.SelectorExpr:
hasReturn = a.isSelExprReturnValues(v)
default:
return
}

if outgoingFieldList == nil || outgoingFieldList.List == nil {
return
}

if len(outgoingFieldList.List) == 0 {
if !hasReturn {
return
}

pass.Reportf(node.Pos(), "deferred call should not return anything")
}
}

func getFuncDeclResults(ident *ast.Ident) *ast.FieldList {
if ident.Obj == nil {
return nil
func (a *analyzer) isIdentReturnValues(ident *ast.Ident) bool {
if ident == nil || ident.Obj == nil {
return false
}

funcDecl, ok := ident.Obj.Decl.(*ast.FuncDecl)
if !ok {
return nil
return false
}

if funcDecl.Type == nil || funcDecl.Type.Results == nil {
return false
}

if len(funcDecl.Type.Results.List) == 0 {
return false
}

return true
}

func (a *analyzer) isFuncLitReturnValues(funcLit *ast.FuncLit) bool {
if funcLit == nil || funcLit.Type == nil {
return false
}

if funcLit.Type == nil || funcLit.Type.Results == nil {
return false
}

if len(funcLit.Type.Results.List) == 0 {
return false
}

return true
}

func (a *analyzer) isSelExprReturnValues(selExpr *ast.SelectorExpr) bool {
if selExpr == nil {
return false
}

t, ok := a.typesInfo.Types[selExpr].Type.(*types.Signature)
if !ok {
return false
}

if t.Results() == nil || t.Results().Len() == 0 {
return false
}

return true
}

func (a *analyzer) funcName(call *ast.CallExpr) string {
fn, ok := a.getFunc(call)
if !ok {
return gofmt(call.Fun)
}

return funcDecl.Type.Results
name := fn.FullName()
name = strings.ReplaceAll(name, "(", "")
name = strings.ReplaceAll(name, ")", "")

return name
}

func getFuncLitResults(funcLit *ast.FuncLit) *ast.FieldList {
if funcLit.Type == nil {
return nil
func (a *analyzer) getFunc(call *ast.CallExpr) (*types.Func, bool) {
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
return nil, false
}

return funcLit.Type.Results
fn, ok := a.typesInfo.ObjectOf(sel.Sel).(*types.Func)
if !ok {
return nil, false
}

return fn, true
}

func gofmt(x interface{}) string {
buf := bytes.Buffer{}
fs := token.NewFileSet()
printer.Fprint(&buf, fs, x)

return buf.String()
}
6 changes: 4 additions & 2 deletions pkg/analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

"golang.org/x/tools/go/analysis/analysistest"

"github.com/GaijinEntertainment/go-defer/pkg/analyzer"
"github.com/GaijinEntertainment/go-nakedefer/pkg/analyzer"
)

func TestAll(t *testing.T) {
Expand All @@ -20,7 +20,9 @@ func TestAll(t *testing.T) {

testdata := filepath.Join(filepath.Dir(filepath.Dir(wd)), "testdata")

a, err := analyzer.NewAnalyzer()
a, err := analyzer.NewAnalyzer(
[]string{"ignoreFunc", "os\\.(Create|WriteFile|Chmod)", "fmt\\.Print.*", "io\\.Close"},
)
if err != nil {
t.Error(err)
}
Expand Down
Loading

0 comments on commit 269b821

Please sign in to comment.