Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide go/analysis analyzer instance #198

Merged
merged 6 commits into from
May 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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