From 9bb943735e075b302ac6504689bb2f0c36a6fd3f Mon Sep 17 00:00:00 2001 From: dshemin Date: Thu, 28 Nov 2019 10:14:21 +0700 Subject: [PATCH] Fix review comments --- .gitignore | 1 - rule/file-header.go | 84 ++++++++++++----------------------- test/file-header_test.go | 6 --- test/import-blacklist_test.go | 9 ++-- test/unused-param_test.go | 5 ++- 5 files changed, 39 insertions(+), 66 deletions(-) diff --git a/.gitignore b/.gitignore index 77068c9..9b8cdca 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ -.idea golinter revive vendor diff --git a/rule/file-header.go b/rule/file-header.go index 145f914..570dccf 100644 --- a/rule/file-header.go +++ b/rule/file-header.go @@ -1,7 +1,6 @@ package rule import ( - "go/ast" "regexp" "github.com/mgechev/revive/lint" @@ -10,68 +9,33 @@ import ( // FileHeaderRule lints given else constructs. type FileHeaderRule struct{} -// Apply applies the rule to given file. -func (r *FileHeaderRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { - var failures []lint.Failure - - header, ok := arguments[0].(string) - if !ok { - panic("Invalid argument to the FileHeaderRule") - } - - regex, err := regexp.Compile(header) - if err != nil { - panic(err.Error()) - } - - fileAst := file.AST - walker := lintFileHeader{ - file: file, - fileAst: fileAst, - regex: regex, - onFailure: func(failure lint.Failure) { - failures = append(failures, failure) - }, - } - - ast.Walk(walker, fileAst) - - return failures -} - -// Name returns the rule name. -func (r *FileHeaderRule) Name() string { - return "file-header" -} - -type lintFileHeader struct { - file *lint.File - fileAst *ast.File - regex *regexp.Regexp - onFailure func(lint.Failure) -} - var ( multiRegexp = regexp.MustCompile("^/\\*") singleRegexp = regexp.MustCompile("^//") ) -func (w lintFileHeader) Visit(_ ast.Node) ast.Visitor { - failure := lint.Failure{ - Node: w.fileAst, - Confidence: 1, - Failure: "the file doesn't have an appropriate header", +// Apply applies the rule to given file. +func (r *FileHeaderRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + header, ok := arguments[0].(string) + if !ok { + panic("Invalid argument to the FileHeaderRule") } - if len(w.fileAst.Comments) == 0 { - w.onFailure(failure) - return nil + failure := []lint.Failure{ + { + Node: file.AST, + Confidence: 1, + Failure: "the file doesn't have an appropriate header", + }, } - g := w.fileAst.Comments[0] + if len(file.AST.Comments) == 0 { + return failure + } + + g := file.AST.Comments[0] if g == nil { - w.onFailure(failure) - return nil + return failure } comment := "" for _, c := range g.List { @@ -84,8 +48,18 @@ func (w lintFileHeader) Visit(_ ast.Node) ast.Visitor { comment += text } - if !w.regex.Match([]byte(comment)) { - w.onFailure(failure) + regex, err := regexp.Compile(header) + if err != nil { + panic(err.Error()) + } + + if !regex.Match([]byte(comment)) { + return failure } return nil } + +// Name returns the rule name. +func (r *FileHeaderRule) Name() string { + return "file-header" +} diff --git a/test/file-header_test.go b/test/file-header_test.go index eae1350..773b294 100644 --- a/test/file-header_test.go +++ b/test/file-header_test.go @@ -37,10 +37,4 @@ func BenchmarkLintFileHeader(b *testing.B) { benchRule(b, "lint-file-header1", &rule.FileHeaderRule{}, &lint.RuleConfig{ Arguments: []interface{}{"foobar"}, }) - //var t *testing.T - //for i := 0; i <= b.N; i++ { - // testRule(t, "lint-file-header1", &rule.FileHeaderRule{}, &lint.RuleConfig{ - // Arguments: []interface{}{"foobar"}, - // }) - //} } diff --git a/test/import-blacklist_test.go b/test/import-blacklist_test.go index 4df739b..d427f0b 100644 --- a/test/import-blacklist_test.go +++ b/test/import-blacklist_test.go @@ -17,7 +17,10 @@ func TestImportsBlacklist(t *testing.T) { func BenchmarkImportsBlacklist(b *testing.B) { args := []interface{}{"crypto/md5", "crypto/sha1"} - benchRule(b, "imports-blacklist", &rule.ImportsBlacklistRule{}, &lint.RuleConfig{ - Arguments: args, - }) + var t *testing.T + for i := 0; i <= b.N; i++ { + testRule(t, "imports-blacklist", &rule.ImportsBlacklistRule{}, &lint.RuleConfig{ + Arguments: args, + }) + } } diff --git a/test/unused-param_test.go b/test/unused-param_test.go index 4c0e24f..7b6472c 100644 --- a/test/unused-param_test.go +++ b/test/unused-param_test.go @@ -11,5 +11,8 @@ func TestUnusedParam(t *testing.T) { } func BenchmarkUnusedParam(b *testing.B) { - benchRule(b, "unused-param", &rule.UnusedParamRule{}) + var t *testing.T + for i := 0; i <= b.N; i++ { + testRule(t, "unused-param", &rule.UnusedParamRule{}) + } }