Skip to content

Commit

Permalink
Switch to go/analysis (#3)
Browse files Browse the repository at this point in the history
Rework linter to use go/analysis - go's static analysis interface
Move test data to for convenient use of analysistest
  • Loading branch information
vovapi authored Oct 6, 2020
1 parent cff715c commit 351e25a
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 223 deletions.
44 changes: 44 additions & 0 deletions errorlint/analysis.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package errorlint

import (
"flag"
"sort"

"golang.org/x/tools/go/analysis"
)

func NewAnalyzer() *analysis.Analyzer {
return &analysis.Analyzer{
Name: "errorlint",
Doc: "Source code linter for Go software that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.",
Run: run,
Flags: flagSet,
}
}

var (
flagSet flag.FlagSet
checkErrorf bool
)

func init() {
flagSet.BoolVar(&checkErrorf, "errorf", false, "Check whether fmt.Errorf uses the %w verb for formatting errors. See the readme for caveats")
}

func run(pass *analysis.Pass) (interface{}, error) {
lints := []Lint{}
if checkErrorf {
l := LintFmtErrorfCalls(pass.Fset, *pass.TypesInfo)
lints = append(lints, l...)
}
l := LintErrorComparisons(pass.Fset, *pass.TypesInfo)
lints = append(lints, l...)
l = LintErrorTypeAssertions(pass.Fset, *pass.TypesInfo)
lints = append(lints, l...)
sort.Sort(ByPosition(lints))

for _, l := range lints {
pass.Report(analysis.Diagnostic{Pos: l.Pos, Message: l.Message})
}
return nil, nil
}
25 changes: 25 additions & 0 deletions errorlint/analysis_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package errorlint

import (
"log"
"testing"

"golang.org/x/tools/go/analysis/analysistest"
)

func TestErrorsAs(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), NewAnalyzer(), "errorsas")
}

func TestErrorsIs(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), NewAnalyzer(), "errorsis")
}

func TestFmtErrorf(t *testing.T) {
analyzer := NewAnalyzer()
err := analyzer.Flags.Set("errorf", "true")
if err != nil {
log.Fatal(err)
}
analysistest.Run(t, analysistest.TestData(), analyzer, "fmterrorf")
}
18 changes: 7 additions & 11 deletions errorlint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

type Lint struct {
Message string
Pos token.Position
Pos token.Pos
}

type ByPosition []Lint
Expand All @@ -20,11 +20,7 @@ func (l ByPosition) Len() int { return len(l) }
func (l ByPosition) Swap(i, j int) { l[i], l[j] = l[j], l[i] }

func (l ByPosition) Less(i, j int) bool {
a, b := l[i].Pos, l[j].Pos
if a.Filename == b.Filename {
return a.Offset < b.Offset
}
return a.Filename < b.Filename
return l[i].Pos < l[j].Pos
}

func LintFmtErrorfCalls(fset *token.FileSet, info types.Info) []Lint {
Expand Down Expand Up @@ -55,7 +51,7 @@ func LintFmtErrorfCalls(fset *token.FileSet, info types.Info) []Lint {
if len(formatVerbs) >= i && formatVerbs[i] != "%w" {
lints = append(lints, Lint{
Message: "non-wrapping format verb for fmt.Errorf. Use `%w` to format errors",
Pos: fset.Position(arg.Pos()),
Pos: arg.Pos(),
})
}
}
Expand Down Expand Up @@ -136,7 +132,7 @@ func LintErrorComparisons(fset *token.FileSet, info types.Info) []Lint {

lints = append(lints, Lint{
Message: fmt.Sprintf("comparing with %s will fail on wrapped errors. Use errors.Is to check for a specific error", binExpr.Op),
Pos: fset.Position(binExpr.Pos()),
Pos: binExpr.Pos(),
})
}

Expand All @@ -157,7 +153,7 @@ func LintErrorComparisons(fset *token.FileSet, info types.Info) []Lint {

lints = append(lints, Lint{
Message: "switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors",
Pos: fset.Position(switchStmt.Pos()),
Pos: switchStmt.Pos(),
})
}

Expand Down Expand Up @@ -197,7 +193,7 @@ func LintErrorTypeAssertions(fset *token.FileSet, info types.Info) []Lint {

lints = append(lints, Lint{
Message: "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors",
Pos: fset.Position(typeAssert.Pos()),
Pos: typeAssert.Pos(),
})
}

Expand All @@ -224,7 +220,7 @@ func LintErrorTypeAssertions(fset *token.FileSet, info types.Info) []Lint {

lints = append(lints, Lint{
Message: "type switch on error will fail on wrapped errors. Use errors.As to check for specific errors",
Pos: fset.Position(typeAssert.Pos()),
Pos: typeAssert.Pos(),
})
}

Expand Down
112 changes: 0 additions & 112 deletions errorlint/lint_test.go

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package testdata
package errorsas

import (
"errors"
Expand All @@ -25,37 +25,37 @@ func TypeCheckGood() {

func TypeAssertion() {
err := doAnotherThing()
_, ok := err.(*MyError) // bad
_, ok := err.(*MyError) // want `type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors`
if ok {
fmt.Println("MyError")
}
}

func TypeSwitch() {
err := doAnotherThing()
switch err.(type) { // bad
switch err.(type) { // want `type switch on error will fail on wrapped errors. Use errors.As to check for specific errors`
case *MyError:
fmt.Println("MyError")
}
}

func TypeSwitchInline() {
switch doAnotherThing().(type) { // bad
switch doAnotherThing().(type) { // want `type switch on error will fail on wrapped errors. Use errors.As to check for specific errors`
case *MyError:
fmt.Println("MyError")
}
}

func TypeSwitchAssign() {
err := doAnotherThing()
switch t := err.(type) { // bad
switch t := err.(type) { // want `type switch on error will fail on wrapped errors. Use errors.As to check for specific errors`
case *MyError:
fmt.Println("MyError", t)
}
}

func TypeSwitchAssignInline() {
switch t := doAnotherThing().(type) { // bad
switch t := doAnotherThing().(type) { // want `type switch on error will fail on wrapped errors. Use errors.As to check for specific errors`
case *MyError:
fmt.Println("MyError", t)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package testdata
package errorsis

import (
"errors"
Expand Down Expand Up @@ -48,42 +48,42 @@ func CompareOperatorNotNilYodaGood() {

func EqualOperator() {
err := doThing()
if err == ErrFoo { // bad
if err == ErrFoo { // want `comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error`
fmt.Println("ErrFoo")
}
}

func NotEqualOperator() {
err := doThing()
if err != ErrFoo { // bad
if err != ErrFoo { // want `comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error`
fmt.Println("not ErrFoo")
}
}

func EqualOperatorYoda() {
err := doThing()
if ErrFoo == err { // bad
if ErrFoo == err { // want `comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error`
fmt.Println("ErrFoo")
}
}

func NotEqualOperatorYoda() {
err := doThing()
if ErrFoo != err { // bad
if ErrFoo != err { // want `comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error`
fmt.Println("not ErrFoo")
}
}

func CompareSwitch() {
err := doThing()
switch err { // bad
switch err { // want `switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors`
case ErrFoo:
fmt.Println("ErrFoo")
}
}

func CompareSwitchInline() {
switch doThing() { // bad
switch doThing() { // want `switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors`
case ErrFoo:
fmt.Println("ErrFoo")
}
Expand Down
Loading

0 comments on commit 351e25a

Please sign in to comment.