From 833ccb0d098d58df5484e220b5e7dfe96e4defe7 Mon Sep 17 00:00:00 2001 From: 0x002A Date: Wed, 6 Mar 2019 15:39:17 +0100 Subject: [PATCH 1/3] Implemented the ability to forbid the usage of certain expressions --- checkstyle.go | 57 +++++++++++++++++++++++++++++++------------- checkstyle_test.go | 17 +++++++------ testdata/defer.go | 9 +++++++ testdata/no_defer.go | 8 +++++++ 4 files changed, 67 insertions(+), 24 deletions(-) create mode 100644 testdata/defer.go create mode 100644 testdata/no_defer.go diff --git a/checkstyle.go b/checkstyle.go index 2adf788..eea851c 100644 --- a/checkstyle.go +++ b/checkstyle.go @@ -7,6 +7,7 @@ import ( "go/format" "go/parser" "go/token" + "reflect" "strconv" "strings" ) @@ -14,13 +15,14 @@ import ( type ProblemType string const ( - FileLine ProblemType = "file_line" - FunctionLine ProblemType = "func_line" - ParamsNum ProblemType = "params_num" - ResultsNum ProblemType = "results_num" - Formated ProblemType = "formated" - PackageName ProblemType = "pkg_name" - CamelName ProblemType = "camel_name" + FileLine ProblemType = "file_line" + FunctionLine ProblemType = "func_line" + ParamsNum ProblemType = "params_num" + ResultsNum ProblemType = "results_num" + Formated ProblemType = "formated" + PackageName ProblemType = "pkg_name" + CamelName ProblemType = "camel_name" + ForbiddenExpr ProblemType = "forbidden_expr" ) type Problem struct { @@ -36,16 +38,17 @@ type Checker interface { } type checker struct { - FunctionComment bool `json:"func_comment"` - FileLine int `json:"file_line"` - FunctionLine int `json:"func_line"` - MaxIndent int `json:"max_indent"` - Formated bool `json:"formated"` - Fatal []string `json:"fatal"` - ParamsNum int `json:"params_num"` - ResultsNum int `json:"results_num"` - PackageName bool `json:"pkg_name"` - CamelName bool `json:"camel_name"` + FunctionComment bool `json:"func_comment"` + FileLine int `json:"file_line"` + FunctionLine int `json:"func_line"` + MaxIndent int `json:"max_indent"` + Formated bool `json:"formated"` + Fatal []string `json:"fatal"` + ParamsNum int `json:"params_num"` + ResultsNum int `json:"results_num"` + PackageName bool `json:"pkg_name"` + CamelName bool `json:"camel_name"` + ForbiddenExpr []reflect.Type `json:"forbidden_expr"` } func New(config []byte) (Checker, error) { @@ -159,6 +162,11 @@ func genFuncBodyProblem(name string, start token.Position) Problem { return Problem{Description: desc, Position: &start, Type: ResultsNum} } +func genForbExprProblem(name string, start token.Position) Problem { + desc := "expr " + name + " is forbidden" + return Problem{Description: desc, Position: &start, Type: ResultsNum} +} + func (f *file) checkPkgName(pkg *ast.Ident) { //ref "http://golang.org/doc/effective_go.html#package-names" pkgName := pkg.Name @@ -353,6 +361,20 @@ func (f *file) checkAssign(assign *ast.AssignStmt) { } } +func (f *file) checkForbiddenExpr(n *ast.Node, t reflect.Type) { + if t != nil { + for _, fbExpr := range f.config.ForbiddenExpr { + t = t.Elem() + if t == fbExpr { + pos := f.fset.Position((*n).Pos()) + desc := "expr " + t.String() + " is forbidden" + problem := Problem{Description: desc, Position: &pos, Type: ForbiddenExpr} + f.problems = append(f.problems, problem) + } + } + } +} + func (f *file) checkFileContent() { if f.isTest() { return @@ -380,6 +402,7 @@ func (f *file) checkFileContent() { case *ast.StructType: f.checkStruct(decl2) } + f.checkForbiddenExpr(&node, reflect.TypeOf(node)) return true }) case *ast.GenDecl: diff --git a/checkstyle_test.go b/checkstyle_test.go index 33e3a84..2d51bc8 100644 --- a/checkstyle_test.go +++ b/checkstyle_test.go @@ -1,6 +1,7 @@ package checkstyle import ( + "go/ast" "go/parser" "go/token" "io/ioutil" @@ -203,10 +204,10 @@ func TestPackageName(t *testing.T) { } } -func TestCamelName(t *testing.T) { - fileName := "underscore_name.go" +func TestDefer(t *testing.T) { + fileName := "defer.go" file := readFile(fileName) - _checker := checker{CamelName: false} + _checker := checker{ForbiddenExpr: []reflect.Type{}} ps, err := _checker.Check(fileName, file) if err != nil { t.Fatal(err) @@ -214,15 +215,17 @@ func TestCamelName(t *testing.T) { if len(ps) != 0 { t.Fatal("expect no error") } - _checkerFail := checker{CamelName: true} + fbExpr := []reflect.Type{reflect.TypeOf(ast.DeferStmt{})} + _checkerFail := checker{ForbiddenExpr: fbExpr} ps, err = _checkerFail.Check(fileName, file) if err != nil { t.Fatal(err) } - if len(ps) != 30 { - t.Fatal("expect 30 error but ", len(ps)) + if len(ps) != 1 { + t.Fatal("expect 1 error but ", len(ps)) } - fileName = "camel_name.go" + + fileName = "no_defer.go" file = readFile(fileName) ps, err = _checkerFail.Check(fileName, file) if err != nil { diff --git a/testdata/defer.go b/testdata/defer.go new file mode 100644 index 0000000..cf5703d --- /dev/null +++ b/testdata/defer.go @@ -0,0 +1,9 @@ +package testdata + +import "fmt" + +func HelloWorld() { + defer fmt.Println("world") + + fmt.Println("hello") +} diff --git a/testdata/no_defer.go b/testdata/no_defer.go new file mode 100644 index 0000000..6d554b8 --- /dev/null +++ b/testdata/no_defer.go @@ -0,0 +1,8 @@ +package testdata + +import "fmt" + +func HelloWorld() { + fmt.Println("hello") + fmt.Println("world") +} From 8c7e9921ea6f1d864aaa0f34782d1a54dd83ab27 Mon Sep 17 00:00:00 2001 From: 0x002A Date: Wed, 6 Mar 2019 16:20:49 +0100 Subject: [PATCH 2/3] Use function to generate problem --- checkstyle.go | 5 ++--- checkstyle_test.go | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/checkstyle.go b/checkstyle.go index eea851c..f03fabb 100644 --- a/checkstyle.go +++ b/checkstyle.go @@ -164,7 +164,7 @@ func genFuncBodyProblem(name string, start token.Position) Problem { func genForbExprProblem(name string, start token.Position) Problem { desc := "expr " + name + " is forbidden" - return Problem{Description: desc, Position: &start, Type: ResultsNum} + return Problem{Description: desc, Position: &start, Type: ForbiddenExpr} } func (f *file) checkPkgName(pkg *ast.Ident) { @@ -367,8 +367,7 @@ func (f *file) checkForbiddenExpr(n *ast.Node, t reflect.Type) { t = t.Elem() if t == fbExpr { pos := f.fset.Position((*n).Pos()) - desc := "expr " + t.String() + " is forbidden" - problem := Problem{Description: desc, Position: &pos, Type: ForbiddenExpr} + problem := genForbExprProblem(t.String(), pos) f.problems = append(f.problems, problem) } } diff --git a/checkstyle_test.go b/checkstyle_test.go index 2d51bc8..d92670b 100644 --- a/checkstyle_test.go +++ b/checkstyle_test.go @@ -224,7 +224,6 @@ func TestDefer(t *testing.T) { if len(ps) != 1 { t.Fatal("expect 1 error but ", len(ps)) } - fileName = "no_defer.go" file = readFile(fileName) ps, err = _checkerFail.Check(fileName, file) From cf2c98965e37d408d3a1bdb58c3fe1869298b5b6 Mon Sep 17 00:00:00 2001 From: 0x002A Date: Wed, 6 Mar 2019 20:27:37 +0100 Subject: [PATCH 3/3] Re-added test which I unintentionally removed --- checkstyle_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/checkstyle_test.go b/checkstyle_test.go index d92670b..e085b59 100644 --- a/checkstyle_test.go +++ b/checkstyle_test.go @@ -204,6 +204,36 @@ func TestPackageName(t *testing.T) { } } +func TestCamelName(t *testing.T) { + fileName := "underscore_name.go" + file := readFile(fileName) + _checker := checker{CamelName: false} + ps, err := _checker.Check(fileName, file) + if err != nil { + t.Fatal(err) + } + if len(ps) != 0 { + t.Fatal("expect no error") + } + _checkerFail := checker{CamelName: true} + ps, err = _checkerFail.Check(fileName, file) + if err != nil { + t.Fatal(err) + } + if len(ps) != 30 { + t.Fatal("expect 30 error but ", len(ps)) + } + fileName = "camel_name.go" + file = readFile(fileName) + ps, err = _checkerFail.Check(fileName, file) + if err != nil { + t.Fatal(err) + } + if len(ps) != 0 { + t.Fatal("expect no error") + } +} + func TestDefer(t *testing.T) { fileName := "defer.go" file := readFile(fileName)