go/revive
1
0
Fork 0
mirror of https://github.com/mgechev/revive.git synced 2025-02-15 13:53:15 +02:00
Code Issues Releases Activity

Add extra rules and tests

Browse Source
This commit is contained in:
mgechev 2018-01-23 19:13:02 -08:00
parent a955825a28
commit cac7f0e036
17 changed files with 137 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
fixtures/blank-import-lib.go Normal file
View File

@ -0,0 +1,44 @@
// Test that blank imports in library packages are flagged.
// Package foo ...
package foo
// The instructions need to go before the imports below so they will not be
// mistaken for documentation.
import _ "encoding/json"
/* MATCH:9 /a blank import should be only in a main or test package, or have a comment justifying it/ */
import (
"fmt"
_ "os"
/* MATCH:16 /a blank import should be only in a main or test package, or have a comment justifying it/ */
_ "net/http"
/* MATCH:19 /a blank import should be only in a main or test package, or have a comment justifying it/ */
_ "path"
)
import _ "encoding/base64" // Don't gripe about this
import (
// Don't gripe about these next two lines.
_ "compress/zlib"
_ "syscall"
/* MATCH:30 /a blank import should be only in a main or test package, or have a comment justifying it/ */
_ "path/filepath"
)
import (
"go/ast"
_ "go/scanner" // Don't gripe about this or the following line.
_ "go/token"
)
var (
_ fmt.Stringer // for "fmt"
_ ast.Node // for "go/ast"
)

8
fixtures/import-dot.go Normal file
View File

@ -0,0 +1,8 @@
// Test that dot imports are flagged.
// Package pkg ...
package pkg
import . "fmt" // MATCH /should not use dot imports/
var _ Stringer // from "fmt"

3
fixtures/package-doc1.go Normal file
View File

@ -0,0 +1,3 @@
// Test of missing package comment.
package foo // MATCH /should have a package comment, unless it's in another file for this package/

5
fixtures/package-doc2.go Normal file
View File

@ -0,0 +1,5 @@
// Test of package comment in an incorrect form.
// Some random package doc that isn't in the right form.
// MATCH /package comment should be of the form "Package testdata ..."/
package testdata

7
fixtures/package-doc3.go Normal file
View File

@ -0,0 +1,7 @@
// Test of block package comment.
// OK
/*
Package foo is pretty sweet.
*/
package foo

7
fixtures/package-doc4.go Normal file
View File

@ -0,0 +1,7 @@
// Test of block package comment with leading space.
/*
Package foo is pretty sweet.
MATCH /package comment should not have leading space/
*/
package foo

9
fixtures/package-doc5.go Normal file
View File

@ -0,0 +1,9 @@
// Test of detached package comment.
/*
Package foo is pretty sweet.
*/
package foo
// MATCH:6 /package comment is detached; there should be no blank lines between it and the package statement/

5
fixtures/package-main.go Normal file
View File

@ -0,0 +1,5 @@
// Test of package comment for package main.
// OK
// This binary does something awesome.
package main

3
linter/file.go
View File

@ -20,6 +20,9 @@ type File struct {
AST *ast.File
}
// IsTest returns if the file contains tests.
func (f *File) IsTest() bool { return strings.HasSuffix(f.Name, "_test.go") }
// NewFile creates a new file
func NewFile(name string, content []byte, pkg *Package) (*File, error) {
f, err := parser.ParseFile(pkg.fset, name, content, parser.ParseComments)

1
linter/rule.go
View File

@ -30,6 +30,7 @@ type Failure struct {
Position FailurePosition
Node ast.Node `json:"-"`
Confidence float64
URL string
// For future use
ReplacementLine string
}

4
rule/blank-imports.go
View File

@ -40,7 +40,7 @@ type lintBlankImports struct {
func (w lintBlankImports) Visit(n ast.Node) ast.Visitor {
// In package main and in tests, we don't complain about blank imports.
if w.fileAst.Name.Name == "main" || isTest(w.file) {
if w.file.Pkg.IsMain() || w.file.IsTest() {
return nil
}
@ -66,6 +66,8 @@ func (w lintBlankImports) Visit(n ast.Node) ast.Visitor {
Node: imp,
Failure: "a blank import should be only in a main or test package, or have a comment justifying it",
Confidence: 1,
Category: "imports",
URL: "",
})
}
}

46
rule/blank-imports_test.go
View File

@ -1,46 +0,0 @@
package rule
import (
"testing"
"github.com/mgechev/revive/rule"
"github.com/mgechev/revive/testutil"
)
func TestBlankImports(t *testing.T) {
t.Parallel()
program := `
package foo
import (
"fmt"
/* MATCH /blank import/ */ [@f1]_ "os"[/@f1]
/* MATCH /blank import/ */ [@f2]_ "net/http"[/@f2]
_ "path"
)
`
testutil.AssertFailures(t, program, &BlankImportsRule{}, rule.Arguments{})
}
func TestBlankImports_ShouldSkipMain(t *testing.T) {
t.Parallel()
program := `
package main
import (
"fmt"
/* MATCH /blank import/ */ _ "os"
/* MATCH /blank import/ */ _ "net/http"
_ "path"
)
`
testutil.AssertSuccess(t, program, &BlankImportsRule{}, rule.Arguments{})
}

18
rule/imports.go → rule/dot-imports.go
View File

@ -6,11 +6,11 @@ import (
"github.com/mgechev/revive/linter"
)
// ImportsRule lints given else constructs.
type ImportsRule struct{}
// DotImportsRule lints given else constructs.
type DotImportsRule struct{}
// Apply applies the rule to given file.
func (r *ImportsRule) Apply(file *linter.File, arguments linter.Arguments) []linter.Failure {
func (r *DotImportsRule) Apply(file *linter.File, arguments linter.Arguments) []linter.Failure {
var failures []linter.Failure
fileAst := file.AST
@ -28,8 +28,8 @@ func (r *ImportsRule) Apply(file *linter.File, arguments linter.Arguments) []lin
}
// Name returns the rule name.
func (r *ImportsRule) Name() string {
return "imports"
func (r *DotImportsRule) Name() string {
return "dot-imports"
}
type lintImports struct {
@ -39,15 +39,17 @@ type lintImports struct {
}
func (w lintImports) Visit(n ast.Node) ast.Visitor {
for _, is := range w.fileAst.Imports {
if is.Name != nil && is.Name.Name == "." && !isTest(w.file) {
for i, is := range w.fileAst.Imports {
_ = i
if is.Name != nil && is.Name.Name == "." && !w.file.IsTest() {
w.onFailure(linter.Failure{
Confidence: 1,
Failure: "should not use dot imports",
Node: is,
Category: "imports",
URL: "#import-dot",
})
}
}
return nil
}

61
rule/imports_test.go
View File

@ -1,61 +0,0 @@
package rule
import (
"testing"
"github.com/mgechev/revive/rule"
"github.com/mgechev/revive/testutil"
)
func TestImports_Failure(t *testing.T) {
t.Parallel()
program := `
package foo
import (
"fmt"
[@f1]. "path"[/@f1]
)
`
testutil.AssertFailures(t, program, &ImportsRule{}, rule.Arguments{})
}
func TestImports(t *testing.T) {
t.Parallel()
program := `
package main
import (
"fmt"
/* MATCH /blank import/ */ _ "os"
/* MATCH /blank import/ */ _ "net/http"
_ "path"
)
`
testutil.AssertSuccess(t, program, &ImportsRule{}, rule.Arguments{})
}
func TestImports_SkipTesting(t *testing.T) {
t.Parallel()
program := `
package main
import (
"fmt"
/* MATCH /blank import/ */ _ "os"
/* MATCH /blank import/ */ _ "net/http"
. "path"
)
`
testutil.AssertSuccessWithName(t, program, "foo_test.go", &ImportsRule{}, rule.Arguments{})
}

33
rule/package-comments.go
View File

@ -46,6 +46,10 @@ type lintPackageComments struct {
}
func (l *lintPackageComments) Visit(n ast.Node) ast.Visitor {
if l.file.IsTest() {
return nil
}
const ref = styleGuideBase + "#package-comments"
prefix := "Package " + l.fileAst.Name.Name + " "
@ -73,9 +77,14 @@ func (l *lintPackageComments) Visit(n ast.Node) ast.Visitor {
Column: 1,
}
l.onFailure(linter.Failure{
Failure: "package comment is detached; there should be no blank lines between it and the package statement",
Category: "comments",
Position: linter.FailurePosition{
Start: pos,
End: pos,
},
Confidence: 0.9,
Position: linter.FailurePosition{Start: pos},
Failure: "package comment is detached; there should be no blank lines between it and the package statement",
URL: ref,
})
return nil
}
@ -83,27 +92,33 @@ func (l *lintPackageComments) Visit(n ast.Node) ast.Visitor {
if l.fileAst.Doc == nil {
l.onFailure(linter.Failure{
Failure: "should have a package comment, unless it's in another file for this package",
Category: "comments",
Node: l.fileAst,
Confidence: 0.2,
Node: l.fileAst.Name,
Failure: "should have a package comment, unless it's in another file for this package",
URL: ref,
})
return nil
}
s := l.fileAst.Doc.Text()
if ts := strings.TrimLeft(s, " \t"); ts != s {
l.onFailure(linter.Failure{
Failure: "package comment should not have leading space",
Confidence: 1,
Category: "comments",
Node: l.fileAst.Doc,
Confidence: 1,
Failure: "package comment should not have leading space",
URL: ref,
})
s = ts
}
// Only non-main packages need to keep to this form.
if l.fileAst.Name.Name != "main" && !strings.HasPrefix(s, prefix) {
if !l.file.Pkg.IsMain() && !strings.HasPrefix(s, prefix) {
l.onFailure(linter.Failure{
Failure: fmt.Sprintf(`package comment should be of the form "%s..."`, prefix),
Confidence: 1,
Category: "comments",
Node: l.fileAst.Doc,
Confidence: 1,
Failure: fmt.Sprintf(`package comment should be of the form "%s..."`, prefix),
URL: ref,
})
}
return nil

39
rule/package-comments_test.go
View File

@ -1,39 +0,0 @@
package rule
import (
"testing"
"github.com/mgechev/revive/rule"
"github.com/mgechev/revive/testutil"
)
func TestPackageCommentsRule(t *testing.T) {
t.Parallel()
program := `
/*
Package foo is pretty sweet.
*/
package [@f]foo[/@f]
func foo(a int, b int, c int) {
return a + b + c;
}
`
testutil.AssertFailures(t, program, &PackageCommentsRule{}, rule.Arguments{})
}
func TestPackageCommentsRule_Success(t *testing.T) {
t.Parallel()
program := `
// Package foo is awesome
package foo
func foo(a int, b int, c int) {
return a + b + c;
}
`
testutil.AssertSuccess(t, program, &PackageCommentsRule{}, rule.Arguments{})
}

9
testutil/lint_test.go
View File

@ -4,6 +4,8 @@
// license that can be found in the LICENSE file or at
// https://developers.google.com/open-source/licenses/bsd.
// The code contains changes from the original source.
package testutil
import (
@ -31,7 +33,12 @@ import (
var lintMatch = flag.String("lint.match", "", "restrict fixtures matches to this pattern")
var rules = []linter.Rule{&rule.VarDeclarationsRule{}}
var rules = []linter.Rule{
&rule.VarDeclarationsRule{},
&rule.PackageCommentsRule{},
&rule.DotImportsRule{},
&rule.BlankImportsRule{},
}
func TestAll(t *testing.T) {
baseDir := "../fixtures/"