Provide lint problem categorisation.
diff --git a/lint.go b/lint.go
index 77188e0..5daef38 100644
--- a/lint.go
+++ b/lint.go
@@ -34,6 +34,7 @@
Link string // (optional) the link to the style guide for the problem
Confidence float64 // a value in (0,1] estimating the confidence in this problem's correctness
LineText string // the source line
+ Category string // a short name for the general category of the problem
}
func (p *Problem) String() string {
@@ -93,15 +94,35 @@
return f.problems
}
-func (f *file) errorf(n ast.Node, confidence float64, link, format string, a ...interface{}) {
+type link string
+type category string
+
+// The variadic arguments may start with link and category types,
+// and must end with a format string and any arguments.
+func (f *file) errorf(n ast.Node, confidence float64, args ...interface{}) {
p := f.fset.Position(n.Pos())
- f.problems = append(f.problems, Problem{
+ problem := Problem{
Position: p,
- Text: fmt.Sprintf(format, a...),
- Link: link,
Confidence: confidence,
LineText: srcLine(f.src, p),
- })
+ }
+
+argLoop:
+ for len(args) > 1 { // always leave at least the format string in args
+ switch v := args[0].(type) {
+ case link:
+ problem.Link = string(v)
+ case category:
+ problem.Category = string(v)
+ default:
+ break argLoop
+ }
+ args = args[1:]
+ }
+
+ problem.Text = fmt.Sprintf(args[0].(string), args[1:]...)
+
+ f.problems = append(f.problems, problem)
}
func (f *file) scanSortable() {
@@ -151,20 +172,20 @@
return
}
- const link = styleGuideBase + "#Package_Comments"
+ const ref = styleGuideBase + "#Package_Comments"
if f.f.Doc == nil {
- f.errorf(f.f, 0.2, link, "should have a package comment, unless it's in another file for this package")
+ f.errorf(f.f, 0.2, link(ref), category("comments"), "should have a package comment, unless it's in another file for this package")
return
}
s := f.f.Doc.Text()
prefix := "Package " + f.f.Name.Name + " "
if ts := strings.TrimLeft(s, " \t"); ts != s {
- f.errorf(f.f.Doc, 1, link, "package comment should not have leading space")
+ f.errorf(f.f.Doc, 1, link(ref), category("comments"), "package comment should not have leading space")
s = ts
}
// Only non-main packages need to keep to this form.
if f.f.Name.Name != "main" && !strings.HasPrefix(s, prefix) {
- f.errorf(f.f.Doc, 1, link, `package comment should be of the form "%s..."`, prefix)
+ f.errorf(f.f.Doc, 1, link(ref), category("comments"), `package comment should be of the form "%s..."`, prefix)
}
}
@@ -194,8 +215,8 @@
// This is the first blank import of a group.
if imp.Doc == nil && imp.Comment == nil {
- link := ""
- f.errorf(imp, 1, link, "a blank import should be only in a main or test package, or have a comment justifying it")
+ ref := ""
+ f.errorf(imp, 1, link(ref), category("imports"), "a blank import should be only in a main or test package, or have a comment justifying it")
}
}
}
@@ -206,7 +227,7 @@
for i, is := range f.f.Imports {
_ = i
if is.Name != nil && is.Name.Name == "." && !f.isTest() {
- f.errorf(is, 1, styleGuideBase+"#Import_Dot", "should not use dot imports")
+ f.errorf(is, 1, link(styleGuideBase+"#Import_Dot"), category("imports"), "should not use dot imports")
}
}
@@ -268,7 +289,7 @@
func (f *file) lintNames() {
// Package names need slightly different handling than other names.
if strings.Contains(f.f.Name.Name, "_") && !strings.HasSuffix(f.f.Name.Name, "_test") {
- f.errorf(f.f, 1, "http://golang.org/doc/effective_go.html#package-names", "don't use an underscore in package name")
+ f.errorf(f.f, 1, link("http://golang.org/doc/effective_go.html#package-names"), category("naming"), "don't use an underscore in package name")
}
check := func(id *ast.Ident, thing string) {
@@ -278,12 +299,12 @@
// Handle two common styles from other languages that don't belong in Go.
if len(id.Name) >= 5 && allCapsRE.MatchString(id.Name) && strings.Contains(id.Name, "_") {
- f.errorf(id, 0.8, styleGuideBase+"#Mixed_Caps", "don't use ALL_CAPS in Go names; use CamelCase")
+ f.errorf(id, 0.8, link(styleGuideBase+"#Mixed_Caps"), category("naming"), "don't use ALL_CAPS in Go names; use CamelCase")
return
}
if len(id.Name) > 2 && id.Name[0] == 'k' && id.Name[1] >= 'A' && id.Name[1] <= 'Z' {
should := string(id.Name[1]+'a'-'A') + id.Name[2:]
- f.errorf(id, 0.8, "", "don't use leading k in Go names; %s %s should be %s", thing, id.Name, should)
+ f.errorf(id, 0.8, link("don't use leading k in Go names; %s %s should be %s"), category("naming"), thing, id.Name, should)
}
should := lintName(id.Name)
@@ -291,10 +312,10 @@
return
}
if len(id.Name) > 2 && strings.Contains(id.Name[1:], "_") {
- f.errorf(id, 0.9, "http://golang.org/doc/effective_go.html#mixed-caps", "don't use underscores in Go names; %s %s should be %s", thing, id.Name, should)
+ f.errorf(id, 0.9, link("http://golang.org/doc/effective_go.html#mixed-caps"), category("naming"), "don't use underscores in Go names; %s %s should be %s", thing, id.Name, should)
return
}
- f.errorf(id, 0.8, styleGuideBase+"#Initialisms", "%s %s should be %s", thing, id.Name, should)
+ f.errorf(id, 0.8, link(styleGuideBase+"#Initialisms"), category("naming"), "%s %s should be %s", thing, id.Name, should)
}
checkList := func(fl *ast.FieldList, thing string) {
if fl == nil {
@@ -321,6 +342,7 @@
if f.isTest() && (strings.HasPrefix(v.Name.Name, "Example") || strings.HasPrefix(v.Name.Name, "Test") || strings.HasPrefix(v.Name.Name, "Benchmark")) {
return true
}
+
check(v.Name, "func")
thing := "func"
@@ -487,7 +509,7 @@
return
}
if doc == nil {
- f.errorf(t, 1, docCommentsLink, "exported type %v should have comment or be unexported", t.Name)
+ f.errorf(t, 1, link(docCommentsLink), category("comments"), "exported type %v should have comment or be unexported", t.Name)
return
}
@@ -500,7 +522,7 @@
}
}
if !strings.HasPrefix(s, t.Name.Name+" ") {
- f.errorf(doc, 1, docCommentsLink, `comment on exported type %v should be of the form "%v ..." (with optional leading article)`, t.Name, t.Name)
+ f.errorf(doc, 1, link(docCommentsLink), category("comments"), `comment on exported type %v should be of the form "%v ..." (with optional leading article)`, t.Name, t.Name)
}
}
@@ -542,13 +564,13 @@
name = recv + "." + name
}
if fn.Doc == nil {
- f.errorf(fn, 1, docCommentsLink, "exported %s %s should have comment or be unexported", kind, name)
+ f.errorf(fn, 1, link(docCommentsLink), category("comments"), "exported %s %s should have comment or be unexported", kind, name)
return
}
s := fn.Doc.Text()
prefix := fn.Name.Name + " "
if !strings.HasPrefix(s, prefix) {
- f.errorf(fn.Doc, 1, docCommentsLink, `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
+ f.errorf(fn.Doc, 1, link(docCommentsLink), category("comments"), `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
}
}
@@ -565,7 +587,7 @@
// Check that none are exported except for the first.
for _, n := range vs.Names[1:] {
if ast.IsExported(n.Name) {
- f.errorf(vs, 1, "", "exported %s %s should have its own declaration", kind, n.Name)
+ f.errorf(vs, 1, category("comments"), "exported %s %s should have its own declaration", kind, n.Name)
return
}
}
@@ -583,14 +605,14 @@
if kind == "const" && gd.Lparen.IsValid() {
block = " (or a comment on this block)"
}
- f.errorf(vs, 1, docCommentsLink, "exported %s %s should have comment%s or be unexported", kind, name, block)
+ f.errorf(vs, 1, link(docCommentsLink), category("comments"), "exported %s %s should have comment%s or be unexported", kind, name, block)
genDeclMissingComments[gd] = true
}
return
}
prefix := name + " "
if !strings.HasPrefix(vs.Doc.Text(), prefix) {
- f.errorf(vs.Doc, 1, docCommentsLink, `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
+ f.errorf(vs.Doc, 1, link(docCommentsLink), category("comments"), `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
}
}
@@ -645,7 +667,7 @@
zero = true
}
if zero {
- f.errorf(rhs, 0.9, "", "should drop = %s from declaration of var %s; it is the zero value", f.render(rhs), v.Names[0])
+ f.errorf(rhs, 0.9, category("zero-value"), "should drop = %s from declaration of var %s; it is the zero value", f.render(rhs), v.Names[0])
return false
}
// If the LHS type is an interface, don't warn, since it is probably a
@@ -660,7 +682,7 @@
if defType, ok := isUntypedConst(rhs); ok && !isIdent(v.Type, defType) {
return false
}
- f.errorf(v.Type, 0.8, "", "should omit type %s from declaration of var %s; it will be inferred from the right-hand side", f.render(v.Type), v.Names[0])
+ f.errorf(v.Type, 0.8, category("type-inference"), "should omit type %s from declaration of var %s; it will be inferred from the right-hand side", f.render(v.Type), v.Names[0])
return false
}
return true
@@ -705,7 +727,7 @@
if shortDecl {
extra = " (move short variable declaration to its own line if necessary)"
}
- f.errorf(ifStmt.Else, 1, styleGuideBase+"#Indent_Error_Flow", "if block ends with a return statement, so drop this else and outdent its block"+extra)
+ f.errorf(ifStmt.Else, 1, link(styleGuideBase+"#Indent_Error_Flow"), category("indent"), "if block ends with a return statement, so drop this else and outdent its block"+extra)
}
return true
})
@@ -727,7 +749,7 @@
return true
}
- f.errorf(rs.Value, 1, "", "should omit 2nd value from range; this loop is equivalent to `for %s %s range ...`", f.render(rs.Key), rs.Tok)
+ f.errorf(rs.Value, 1, category("range-loop"), "should omit 2nd value from range; this loop is equivalent to `for %s %s range ...`", f.render(rs.Key), rs.Tok)
return true
})
}
@@ -747,7 +769,7 @@
if !ok || !isPkgDot(ce.Fun, "fmt", "Sprintf") {
return true
}
- f.errorf(node, 1, "", "should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...)")
+ f.errorf(node, 1, category("errors"), "should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...)")
return true
})
}
@@ -778,7 +800,7 @@
prefix = "Err"
}
if !strings.HasPrefix(id.Name, prefix) {
- f.errorf(id, 0.9, "", "error var %s should have name of the form %sFoo", id.Name, prefix)
+ f.errorf(id, 0.9, category("naming"), "error var %s should have name of the form %sFoo", id.Name, prefix)
}
}
}
@@ -837,7 +859,7 @@
if isCap {
conf = 0.6
}
- f.errorf(str, conf, styleGuideBase+"#Error_Strings", msg)
+ f.errorf(str, conf, link(styleGuideBase+"#Error_Strings"), category("errors"), msg)
return true
})
}
@@ -862,18 +884,18 @@
return true
}
name := names[0].Name
- const link = styleGuideBase + "#Receiver_Names"
+ const ref = styleGuideBase + "#Receiver_Names"
if name == "_" {
- f.errorf(n, 1, link, `receiver name should not be an underscore`)
+ f.errorf(n, 1, link(ref), category("naming"), `receiver name should not be an underscore`)
return true
}
if badReceiverNames[name] {
- f.errorf(n, 1, link, `receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"`)
+ f.errorf(n, 1, link(ref), category("naming"), `receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"`)
return true
}
recv := receiverType(fn)
if prev, ok := typeReceiver[recv]; ok && prev != name {
- f.errorf(n, 1, link, "receiver name %s should be consistent with previous receiver name %s for %s", name, prev, recv)
+ f.errorf(n, 1, link(ref), category("naming"), "receiver name %s should be consistent with previous receiver name %s for %s", name, prev, recv)
return true
}
typeReceiver[recv] = name
@@ -904,7 +926,7 @@
default:
return true
}
- f.errorf(as, 0.8, "", "should replace %s with %s%s", f.render(as), f.render(as.Lhs[0]), suffix)
+ f.errorf(as, 0.8, category("unary-op"), "should replace %s with %s%s", f.render(as), f.render(as.Lhs[0]), suffix)
return true
})
}
@@ -933,7 +955,7 @@
if !ok || at.Len != nil {
return true
}
- f.errorf(as, 0.8, "", `can probably use "var %s %s" instead`, f.render(as.Lhs[0]), f.render(at))
+ f.errorf(as, 0.8, category("slice"), `can probably use "var %s %s" instead`, f.render(as.Lhs[0]), f.render(at))
return true
})
}
@@ -954,7 +976,7 @@
// Flag any error parameters found before the last.
for _, r := range ret[:len(ret)-1] {
if isIdent(r.Type, "error") {
- f.errorf(fn, 0.9, "", "error should be the last type when returning multiple items")
+ f.errorf(fn, 0.9, category("arg-order"), "error should be the last type when returning multiple items")
break // only flag one
}
}
