Skip to content

Commit

Permalink
Merge pull request #198 from dtcaciuc/analyzer
Browse files Browse the repository at this point in the history
Provide go/analysis analyzer instance
  • Loading branch information
dtcaciuc authored May 28, 2021
2 parents 98b1bd1 + 2cfce26 commit cf5d9bf
Show file tree
Hide file tree
Showing 15 changed files with 438 additions and 130 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ takes no arguments.
The `-blank` flag enables checking for assignments of errors to the
blank identifier. It takes no arguments.

### go/analysis

The package provides `Analyzer` instance that can be used with
[go/analysis](https://pkg.go.dev/golang.org/x/tools/go/analysis) API.

Currently supported flags are `blank`, `assert`, `exclude`, and `excludeonly`.
Just as the API itself, the analyzer is exprimental and may change in the
future.

## Excluding functions

Expand Down
78 changes: 78 additions & 0 deletions errcheck/analyzer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package errcheck

import (
"fmt"
"go/ast"
"go/token"
"reflect"
"regexp"

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

var Analyzer = &analysis.Analyzer{
Name: "errcheck",
Doc: "check for unchecked errors",
Run: runAnalyzer,
ResultType: reflect.TypeOf(Result{}),
}

var (
argBlank bool
argAsserts bool
argExcludeFile string
argExcludeOnly bool
)

func init() {
Analyzer.Flags.BoolVar(&argBlank, "blank", false, "if true, check for errors assigned to blank identifier")
Analyzer.Flags.BoolVar(&argAsserts, "assert", false, "if true, check for ignored type assertion results")
Analyzer.Flags.StringVar(&argExcludeFile, "exclude", "", "Path to a file containing a list of functions to exclude from checking")
Analyzer.Flags.BoolVar(&argExcludeOnly, "excludeonly", false, "Use only excludes from exclude file")
}

func runAnalyzer(pass *analysis.Pass) (interface{}, error) {

exclude := map[string]bool{}
if !argExcludeOnly {
for _, name := range DefaultExcludedSymbols {
exclude[name] = true
}
}
if argExcludeFile != "" {
excludes, err := ReadExcludes(argExcludeFile)
if err != nil {
return nil, fmt.Errorf("Could not read exclude file: %v\n", err)
}
for _, name := range excludes {
exclude[name] = true
}
}

var allErrors []UncheckedError
for _, f := range pass.Files {
v := &visitor{
typesInfo: pass.TypesInfo,
fset: pass.Fset,
blank: argBlank,
asserts: argAsserts,
exclude: exclude,
ignore: map[string]*regexp.Regexp{}, // deprecated & not used
lines: make(map[string][]string),
errors: nil,
}

ast.Walk(v, f)

for _, err := range v.errors {
pass.Report(analysis.Diagnostic{
Pos: token.Pos(int(f.Pos()) + err.Pos.Offset),
Message: "unchecked error",
})
}

allErrors = append(allErrors, v.errors...)
}

return Result{UncheckedErrors: allErrors}, nil
}
92 changes: 27 additions & 65 deletions errcheck/errcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,49 +25,6 @@ func init() {
var (
// ErrNoGoFiles is returned when CheckPackage is run on a package with no Go source files
ErrNoGoFiles = errors.New("package contains no go source files")

// DefaultExcludedSymbols is a list of symbol names that are usually excluded from checks by default.
//
// Note, that they still need to be explicitly copied to Checker.Exclusions.Symbols
DefaultExcludedSymbols = []string{
// bytes
"(*bytes.Buffer).Write",
"(*bytes.Buffer).WriteByte",
"(*bytes.Buffer).WriteRune",
"(*bytes.Buffer).WriteString",

// fmt
"fmt.Errorf",
"fmt.Print",
"fmt.Printf",
"fmt.Println",
"fmt.Fprint(*bytes.Buffer)",
"fmt.Fprintf(*bytes.Buffer)",
"fmt.Fprintln(*bytes.Buffer)",
"fmt.Fprint(*strings.Builder)",
"fmt.Fprintf(*strings.Builder)",
"fmt.Fprintln(*strings.Builder)",
"fmt.Fprint(os.Stderr)",
"fmt.Fprintf(os.Stderr)",
"fmt.Fprintln(os.Stderr)",

// io
"(*io.PipeReader).CloseWithError",
"(*io.PipeWriter).CloseWithError",

// math/rand
"math/rand.Read",
"(*math/rand.Rand).Read",

// strings
"(*strings.Builder).Write",
"(*strings.Builder).WriteByte",
"(*strings.Builder).WriteRune",
"(*strings.Builder).WriteString",

// hash
"(hash.Hash).Write",
}
)

// UncheckedError indicates the position of an unchecked error return.
Expand Down Expand Up @@ -261,16 +218,17 @@ func (c *Checker) CheckPackage(pkg *packages.Package) Result {
}

v := &visitor{
pkg: pkg,
ignore: ignore,
blank: !c.Exclusions.BlankAssignments,
asserts: !c.Exclusions.TypeAssertions,
lines: make(map[string][]string),
exclude: excludedSymbols,
errors: []UncheckedError{},
typesInfo: pkg.TypesInfo,
fset: pkg.Fset,
ignore: ignore,
blank: !c.Exclusions.BlankAssignments,
asserts: !c.Exclusions.TypeAssertions,
lines: make(map[string][]string),
exclude: excludedSymbols,
errors: []UncheckedError{},
}

for _, astFile := range v.pkg.Syntax {
for _, astFile := range pkg.Syntax {
if c.shouldSkipFile(astFile) {
continue
}
Expand All @@ -281,12 +239,13 @@ func (c *Checker) CheckPackage(pkg *packages.Package) Result {

// visitor implements the errcheck algorithm
type visitor struct {
pkg *packages.Package
ignore map[string]*regexp.Regexp
blank bool
asserts bool
lines map[string][]string
exclude map[string]bool
typesInfo *types.Info
fset *token.FileSet
ignore map[string]*regexp.Regexp
blank bool
asserts bool
lines map[string][]string
exclude map[string]bool

errors []UncheckedError
}
Expand All @@ -306,7 +265,7 @@ func (v *visitor) selectorAndFunc(call *ast.CallExpr) (*ast.SelectorExpr, *types
return nil, nil, false
}

fn, ok := v.pkg.TypesInfo.ObjectOf(sel.Sel).(*types.Func)
fn, ok := v.typesInfo.ObjectOf(sel.Sel).(*types.Func)
if !ok {
// Shouldn't happen, but be paranoid
return nil, nil, false
Expand Down Expand Up @@ -393,7 +352,7 @@ func (v *visitor) namesForExcludeCheck(call *ast.CallExpr) []string {

// This will be missing for functions without a receiver (like fmt.Printf),
// so just fall back to the the function's fullName in that case.
selection, ok := v.pkg.TypesInfo.Selections[sel]
selection, ok := v.typesInfo.Selections[sel]
if !ok {
return []string{name}
}
Expand All @@ -420,14 +379,14 @@ func (v *visitor) namesForExcludeCheck(call *ast.CallExpr) []string {
func (v *visitor) argName(expr ast.Expr) string {
// Special-case literal "os.Stdout" and "os.Stderr"
if sel, ok := expr.(*ast.SelectorExpr); ok {
if obj := v.pkg.TypesInfo.ObjectOf(sel.Sel); obj != nil {
if obj := v.typesInfo.ObjectOf(sel.Sel); obj != nil {
vr, ok := obj.(*types.Var)
if ok && vr.Pkg() != nil && vr.Pkg().Name() == "os" && (vr.Name() == "Stderr" || vr.Name() == "Stdout") {
return "os." + vr.Name()
}
}
}
t := v.pkg.TypesInfo.TypeOf(expr)
t := v.typesInfo.TypeOf(expr)
if t == nil {
return ""
}
Expand Down Expand Up @@ -478,7 +437,7 @@ func (v *visitor) ignoreCall(call *ast.CallExpr) bool {
return true
}

if obj := v.pkg.TypesInfo.Uses[id]; obj != nil {
if obj := v.typesInfo.Uses[id]; obj != nil {
if pkg := obj.Pkg(); pkg != nil {
if re, ok := v.ignore[nonVendoredPkgPath(pkg.Path())]; ok {
return re.MatchString(id.Name)
Expand All @@ -504,7 +463,7 @@ func nonVendoredPkgPath(pkgPath string) string {
// len(s) == number of return types of call
// s[i] == true iff return type at position i from left is an error type
func (v *visitor) errorsByArg(call *ast.CallExpr) []bool {
switch t := v.pkg.TypesInfo.Types[call].Type.(type) {
switch t := v.typesInfo.Types[call].Type.(type) {
case *types.Named:
// Single return
return []bool{isErrorType(t)}
Expand Down Expand Up @@ -546,15 +505,18 @@ func (v *visitor) callReturnsError(call *ast.CallExpr) bool {
// isRecover returns true if the given CallExpr is a call to the built-in recover() function.
func (v *visitor) isRecover(call *ast.CallExpr) bool {
if fun, ok := call.Fun.(*ast.Ident); ok {
if _, ok := v.pkg.TypesInfo.Uses[fun].(*types.Builtin); ok {
if _, ok := v.typesInfo.Uses[fun].(*types.Builtin); ok {
return fun.Name == "recover"
}
}
return false
}

// TODO (dtcaciuc) collect token.Pos and then convert them to UncheckedErrors
// after visitor is done running. This will allow to integrate more cleanly
// with analyzer so that we don't have to convert Position back to Pos.
func (v *visitor) addErrorAtPosition(position token.Pos, call *ast.CallExpr) {
pos := v.pkg.Fset.Position(position)
pos := v.fset.Position(position)
lines, ok := v.lines[pos.Filename]
if !ok {
lines = readfile(pos.Filename)
Expand Down
83 changes: 83 additions & 0 deletions errcheck/excludes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package errcheck

import (
"bufio"
"bytes"
"io/ioutil"
"strings"
)

var (
// DefaultExcludedSymbols is a list of symbol names that are usually excluded from checks by default.
//
// Note, that they still need to be explicitly copied to Checker.Exclusions.Symbols
DefaultExcludedSymbols = []string{
// bytes
"(*bytes.Buffer).Write",
"(*bytes.Buffer).WriteByte",
"(*bytes.Buffer).WriteRune",
"(*bytes.Buffer).WriteString",

// fmt
"fmt.Errorf",
"fmt.Print",
"fmt.Printf",
"fmt.Println",
"fmt.Fprint(*bytes.Buffer)",
"fmt.Fprintf(*bytes.Buffer)",
"fmt.Fprintln(*bytes.Buffer)",
"fmt.Fprint(*strings.Builder)",
"fmt.Fprintf(*strings.Builder)",
"fmt.Fprintln(*strings.Builder)",
"fmt.Fprint(os.Stderr)",
"fmt.Fprintf(os.Stderr)",
"fmt.Fprintln(os.Stderr)",

// io
"(*io.PipeReader).CloseWithError",
"(*io.PipeWriter).CloseWithError",

// math/rand
"math/rand.Read",
"(*math/rand.Rand).Read",

// strings
"(*strings.Builder).Write",
"(*strings.Builder).WriteByte",
"(*strings.Builder).WriteRune",
"(*strings.Builder).WriteString",

// hash
"(hash.Hash).Write",
}
)

// ReadExcludes reads an excludes file, a newline delimited file that lists
// patterns for which to allow unchecked errors.
//
// Lines that start with two forward slashes are considered comments and are ignored.
//
func ReadExcludes(path string) ([]string, error) {
var excludes []string

buf, err := ioutil.ReadFile(path)
if err != nil {
return nil, err
}

scanner := bufio.NewScanner(bytes.NewReader(buf))

for scanner.Scan() {
name := scanner.Text()
// Skip comments and empty lines.
if strings.HasPrefix(name, "//") || name == "" {
continue
}
excludes = append(excludes, name)
}
if err := scanner.Err(); err != nil {
return nil, err
}

return excludes, nil
}
39 changes: 39 additions & 0 deletions errcheck/excludes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package errcheck

import (
"reflect"
"testing"
)

func TestReadExcludes(t *testing.T) {
expectedExcludes := []string{
"hello()",
"world()",
}
t.Logf("expectedExcludes: %#v", expectedExcludes)
excludes, err := ReadExcludes("testdata/excludes.txt")
if err != nil {
t.Fatal(err)
}
t.Logf("excludes: %#v", excludes)
if !reflect.DeepEqual(expectedExcludes, excludes) {
t.Fatal("excludes did not match expectedExcludes")
}
}

func TestReadEmptyExcludes(t *testing.T) {
excludes, err := ReadExcludes("testdata/empty_excludes.txt")
if err != nil {
t.Fatal(err)
}
if len(excludes) != 0 {
t.Fatalf("expected empty excludes, got %#v", excludes)
}
}

func TestReadExcludesMissingFile(t *testing.T) {
_, err := ReadExcludes("testdata/missing_file")
if err == nil {
t.Fatal("expected non-nil err, got nil")
}
}
1 change: 1 addition & 0 deletions errcheck/testdata/custom_exclude.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(custom_exclude.ErrorMakerInterfaceWrapper).MakeNilError
File renamed without changes.
File renamed without changes.
Loading

0 comments on commit cf5d9bf

Please sign in to comment.