1
0
mirror of https://github.com/mgechev/revive.git synced 2024-11-28 08:49:11 +02:00

Add extra rule

This commit is contained in:
mgechev 2018-01-23 23:01:49 -08:00
parent cac7f0e036
commit cc249b6cfa
9 changed files with 228 additions and 24 deletions

36
fixtures/const-block.go Normal file
View File

@ -0,0 +1,36 @@
// Test for docs in const blocks
// Package foo ...
package foo
const (
// Prefix for something.
// MATCH /comment on exported const InlineWhatever should be of the form "InlineWhatever ..."/
InlineWhatever = "blah"
Whatsit = "missing_comment" // MATCH /exported const Whatsit should have comment (or a comment on this block) or be unexported/
// We should only warn once per block for missing comments,
// but always complain about malformed comments.
WhosYourDaddy = "another_missing_one"
// Something
// MATCH /comment on exported const WhatDoesHeDo should be of the form "WhatDoesHeDo ..."/
WhatDoesHeDo = "it's not a tumor!"
)
// These shouldn't need doc comments.
const (
Alpha = "a"
Beta = "b"
Gamma = "g"
)
// The comment on the previous const block shouldn't flow through to here.
const UndocAgain = 6 // MATCH /exported const UndocAgain should have comment or be unexported/
const (
SomeUndocumented = 7 // MATCH /exported const SomeUndocumented should have comment (or a comment on this block) or be unexported/
)

17
fixtures/docs_test.go Normal file
View File

@ -0,0 +1,17 @@
// This file ends in _test.go, so we should not warn about doc comments.
// OK
package pkg
import "testing"
type H int
func TestSomething(t *testing.T) {
}
func TestSomething_suffix(t *testing.T) {
}
func ExampleBuffer_reader() {
}

20
fixtures/sort.go Normal file
View File

@ -0,0 +1,20 @@
// Test that we don't ask for comments on sort.Interface methods.
// Package pkg ...
package pkg
// T is ...
type T []int
// Len by itself should get documented.
func (t T) Len() int { return len(t) } // MATCH /exported method T.Len should have comment or be unexported/
// U is ...
type U []int
func (u U) Len() int { return len(u) }
func (u U) Less(i, j int) bool { return u[i] < u[j] }
func (u U) Swap(i, j int) { u[i], u[j] = u[j], u[i] }
func (u U) Other() {} // MATCH /exported method U.Other should have comment or be unexported/

25
fixtures/stutter.go Normal file
View File

@ -0,0 +1,25 @@
// Test of stuttery names.
// Package donut ...
package donut
// DonutMaker makes donuts.
type DonutMaker struct{} // MATCH /type name will be used as donut.DonutMaker by other packages, and that stutters; consider calling this Maker/
// DonutRank computes the ranking of a donut.
func DonutRank(d Donut) int { // MATCH /func name will be used as donut.DonutRank by other packages, and that stutters; consider calling this Rank/
return 0
}
// Donut is a delicious treat.
type Donut struct{} // ok because it is the whole name
// Donuts are great, aren't they?
type Donuts []Donut // ok because it didn't start a new word
type donutGlaze int // ok because it is unexported
// DonutMass reports the mass of a donut.
func (d *Donut) DonutMass() (grams int) { // okay because it is a method
return 38
}

38
fixtures/value-spec.go Normal file
View File

@ -0,0 +1,38 @@
// Test that exported names have correct comments.
// Package pkg does something.
package pkg
import "time"
type T int // MATCH /exported type T should have comment or be unexported/
func (T) F() {} // MATCH /exported method T.F should have comment or be unexported/
// this is a nice type.
// MATCH /comment on exported type U should be of the form "U ..." (with optional leading article)/
type U string
// this is a neat function.
// MATCH /comment on exported method U.G should be of the form "G ..."/
func (U) G() {}
// A V is a string.
type V string
// V.H has a pointer receiver
func (*V) H() {} // MATCH /exported method V.H should have comment or be unexported/
var W = "foo" // MATCH /exported var W should have comment or be unexported/
const X = "bar" // MATCH /exported const X should have comment or be unexported/
var Y, Z int // MATCH /exported var Z should have its own declaration/
// Location should be okay, since the other var name is an underscore.
var Location, _ = time.LoadLocation("Europe/Istanbul") // not Constantinople
// this is improperly documented
// MATCH /comment on exported const Thing should be of the form "Thing ..."/
const Thing = "wonderful"

View File

@ -50,8 +50,8 @@ func (p *Package) IsMain() bool {
return false
}
// TypeCheck performs type checking for given package.
func (p *Package) TypeCheck() error {
// typeCheck performs type checking for given package.
func (p *Package) typeCheck() error {
config := &types.Config{
// By setting a no-op error reporter, the type checker does as much work as possible.
Error: func(error) {},
@ -85,8 +85,63 @@ func (p *Package) TypeOf(expr ast.Expr) types.Type {
return p.TypesInfo.TypeOf(expr)
}
type walker struct {
nmap map[string]int
has map[string]int
}
func (w *walker) Visit(n ast.Node) ast.Visitor {
fn, ok := n.(*ast.FuncDecl)
if !ok || fn.Recv == nil || len(fn.Recv.List) == 0 {
return w
}
// TODO(dsymonds): We could check the signature to be more precise.
recv := receiverType(fn)
if i, ok := w.nmap[fn.Name.Name]; ok {
w.has[recv] |= i
}
return w
}
func (p *Package) scanSortable() {
p.Sortable = make(map[string]bool)
// bitfield for which methods exist on each type.
const (
Len = 1 << iota
Less
Swap
)
nmap := map[string]int{"Len": Len, "Less": Less, "Swap": Swap}
has := make(map[string]int)
for _, f := range p.files {
ast.Walk(&walker{nmap, has}, f.AST)
}
for typ, ms := range has {
if ms == Len|Less|Swap {
p.Sortable[typ] = true
}
}
}
// receiverType returns the named type of the method receiver, sans "*",
// or "invalid-type" if fn.Recv is ill formed.
func receiverType(fn *ast.FuncDecl) string {
switch e := fn.Recv.List[0].Type.(type) {
case *ast.Ident:
return e.Name
case *ast.StarExpr:
if id, ok := e.X.(*ast.Ident); ok {
return id.Name
}
}
// The parser accepts much more than just the legal forms.
return "invalid-type"
}
func (p *Package) lint(rules []Rule, config RulesConfig, failures chan Failure) {
p.TypeCheck()
p.typeCheck()
p.scanSortable()
var wg sync.WaitGroup
for _, file := range p.files {
wg.Add(1)

View File

@ -40,7 +40,7 @@ func main() {
panic(err)
}
var formatter formatter.JSONFormatter
var formatter formatter.CLIFormatter
output, err := formatter.Format(failures)
if err != nil {
panic(err)

View File

@ -68,20 +68,21 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) {
if commonMethods[name] {
return
}
// TODO: RE-ENABLE
// switch name {
// case "Len", "Less", "Swap":
// if f.pkg.sortable[recv] {
// return
// }
// }
switch name {
case "Len", "Less", "Swap":
if w.file.Pkg.Sortable[recv] {
return
}
}
name = recv + "." + name
}
if fn.Doc == nil {
w.onFailure(linter.Failure{
Node: fn,
Failure: fmt.Sprintf("exported %s %s should have comment or be unexported", kind, name),
Confidence: 1,
URL: "#doc-comments",
Category: "comments",
Failure: fmt.Sprintf("exported %s %s should have comment or be unexported", kind, name),
})
return
}
@ -90,8 +91,10 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) {
if !strings.HasPrefix(s, prefix) {
w.onFailure(linter.Failure{
Node: fn.Doc,
Confidence: 0.8,
URL: "#doc-comments",
Category: "comments",
Failure: fmt.Sprintf(`comment on exported %s %s should be of the form "%s..."`, kind, name, prefix),
Confidence: 1,
})
}
}
@ -119,8 +122,10 @@ func (w *lintExported) checkStutter(id *ast.Ident, thing string) {
if next, _ := utf8.DecodeRuneInString(rem); next == '_' || unicode.IsUpper(next) {
w.onFailure(linter.Failure{
Node: id,
Failure: fmt.Sprintf("%s name will be used as %s.%s by other packages, and that stutters; consider calling this %s", thing, pkg, name, rem),
Confidence: 0.8,
URL: "#package-names",
Category: "naming",
Failure: fmt.Sprintf("%s name will be used as %s.%s by other packages, and that stutters; consider calling this %s", thing, pkg, name, rem),
})
}
}
@ -132,8 +137,10 @@ func (w *lintExported) lintTypeDoc(t *ast.TypeSpec, doc *ast.CommentGroup) {
if doc == nil {
w.onFailure(linter.Failure{
Node: t,
Failure: fmt.Sprintf("exported type %v should have comment or be unexported", t.Name),
Confidence: 1,
URL: "#doc-comments",
Category: "comments",
Failure: fmt.Sprintf("exported type %v should have comment or be unexported", t.Name),
})
return
}
@ -149,8 +156,10 @@ func (w *lintExported) lintTypeDoc(t *ast.TypeSpec, doc *ast.CommentGroup) {
if !strings.HasPrefix(s, t.Name.Name+" ") {
w.onFailure(linter.Failure{
Node: doc,
Failure: fmt.Sprintf(`comment on exported type %v should be of the form "%v ..." (with optional leading article)`, t.Name, t.Name),
Confidence: 1,
URL: "#doc-comments",
Category: "comments",
Failure: fmt.Sprintf(`comment on exported type %v should be of the form "%v ..." (with optional leading article)`, t.Name, t.Name),
})
}
}
@ -166,9 +175,10 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD
for _, n := range vs.Names[1:] {
if ast.IsExported(n.Name) {
w.onFailure(linter.Failure{
Node: vs,
Failure: fmt.Sprintf("exported %s %s should have its own declaration", kind, n.Name),
Category: "comments",
Confidence: 1,
Failure: fmt.Sprintf("exported %s %s should have its own declaration", kind, n.Name),
Node: vs,
})
return
}
@ -190,9 +200,11 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD
block = " (or a comment on this block)"
}
w.onFailure(linter.Failure{
Node: vs,
Failure: fmt.Sprintf("exported %s %s should have comment%s or be unexported", kind, name, block),
Confidence: 1,
Node: vs,
URL: "#doc-comments",
Category: "comments",
Failure: fmt.Sprintf("exported %s %s should have comment%s or be unexported", kind, name, block),
})
genDeclMissingComments[gd] = true
return
@ -210,15 +222,16 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD
prefix := name + " "
if !strings.HasPrefix(doc.Text(), prefix) {
w.onFailure(linter.Failure{
Node: doc,
Failure: fmt.Sprintf(`comment on exported %s %s should be of the form "%s..."`, kind, name, prefix),
Confidence: 1,
Node: doc,
URL: "#doc-comments",
Category: "comments",
Failure: fmt.Sprintf(`comment on exported %s %s should be of the form "%s..."`, kind, name, prefix),
})
}
}
func (w *lintExported) Visit(n ast.Node) ast.Visitor {
switch v := n.(type) {
case *ast.GenDecl:
if v.Tok == token.IMPORT {
@ -250,6 +263,5 @@ func (w *lintExported) Visit(n ast.Node) ast.Visitor {
w.lintValueSpecDoc(v, w.lastGen, w.genDeclMissingComments)
return nil
}
return w
}

View File

@ -38,6 +38,7 @@ var rules = []linter.Rule{
&rule.PackageCommentsRule{},
&rule.DotImportsRule{},
&rule.BlankImportsRule{},
&rule.ExportedRule{},
}
func TestAll(t *testing.T) {