From b361146df80cedd17ad39ad95bc3f9048337f0e3 Mon Sep 17 00:00:00 2001 From: golangci Date: Sat, 26 May 2018 19:58:17 +0300 Subject: [PATCH] #12: add TypeCheck linter to report compilation errors --- README.md | 1 + pkg/commands/root.go | 10 +-- pkg/commands/run.go | 36 +++++++-- pkg/enabled_linters.go | 29 +++---- pkg/enabled_linters_test.go | 7 -- pkg/golinters/golint.go | 2 +- pkg/golinters/typecheck.go | 76 +++++++++++++++++++ pkg/golinters/typecheck_test.go | 59 ++++++++++++++ pkg/printers/json.go | 2 +- pkg/printers/text.go | 8 +- pkg/printers/utils.go | 2 +- .../processors/max_per_file_from_linter.go | 1 + pkg/runner.go | 17 +---- pkg/testdata/not_compiles/main.go | 3 - pkg/testdata/with_issues/typecheck.go | 4 + .../with_issues/typecheck_many_issues.go | 8 ++ 16 files changed, 206 insertions(+), 59 deletions(-) create mode 100644 pkg/golinters/typecheck.go create mode 100644 pkg/golinters/typecheck_test.go delete mode 100644 pkg/testdata/not_compiles/main.go create mode 100644 pkg/testdata/with_issues/typecheck.go create mode 100644 pkg/testdata/with_issues/typecheck_many_issues.go diff --git a/README.md b/README.md index 3175cb35..98620a0c 100644 --- a/README.md +++ b/README.md @@ -166,6 +166,7 @@ golangci-lint linters - [varcheck](https://github.com/opennota/check): Finds unused global variables and constants - [ineffassign](https://github.com/gordonklaus/ineffassign): Detects when assignments to existing variables are not used - [deadcode](https://github.com/remyoudompheng/go-misc/tree/master/deadcode): Finds unused code +- typecheck: Like the front-end of a Go compiler, parses and type-checks Go code. Similar to [gotype](https://godoc.org/golang.org/x/tools/cmd/gotype). ## Disabled By Default Linters (`-E/--enable`) - [golint](https://github.com/golang/lint): Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 3e7620ff..61273aed 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -17,7 +17,7 @@ func (e *Executor) initRoot() { Long: `Smart, fast linters runner. Run it in cloud for every GitHub pull request on https://golangci.com`, Run: func(cmd *cobra.Command, args []string) { if err := cmd.Help(); err != nil { - log.Fatal(err) + logrus.Fatal(err) } }, PersistentPreRun: func(cmd *cobra.Command, args []string) { @@ -33,10 +33,10 @@ func (e *Executor) initRoot() { if e.cfg.Run.CPUProfilePath != "" { f, err := os.Create(e.cfg.Run.CPUProfilePath) if err != nil { - log.Fatal(err) + logrus.Fatal(err) } if err := pprof.StartCPUProfile(f); err != nil { - log.Fatal(err) + logrus.Fatal(err) } } }, @@ -47,11 +47,11 @@ func (e *Executor) initRoot() { if e.cfg.Run.MemProfilePath != "" { f, err := os.Create(e.cfg.Run.MemProfilePath) if err != nil { - log.Fatal(err) + logrus.Fatal(err) } runtime.GC() // get up-to-date statistics if err := pprof.WriteHeapProfile(f); err != nil { - log.Fatal("could not write memory profile: ", err) + logrus.Fatal("could not write memory profile: ", err) } } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index f51a336b..581d72a8 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -6,6 +6,7 @@ import ( "fmt" "go/build" "go/token" + "io/ioutil" "log" "os" "runtime" @@ -245,7 +246,26 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul return runner.Run(ctx, linters, lintCtx), nil } +func setOutputToDevNull() (savedStdout, savedStderr *os.File) { + savedStdout, savedStderr = os.Stdout, os.Stderr + devNull, err := os.Open(os.DevNull) + if err != nil { + logrus.Warnf("can't open null device %q: %s", os.DevNull, err) + return + } + + os.Stdout, os.Stderr = devNull, devNull + return +} + func (e *Executor) runAndPrint(ctx context.Context, args []string) error { + // Don't allow linters and loader to print anything + log.SetOutput(ioutil.Discard) + savedStdout, savedStderr := setOutputToDevNull() + defer func() { + os.Stdout, os.Stderr = savedStdout, savedStderr + }() + issues, err := e.runAnalysis(ctx, args) if err != nil { return err @@ -288,11 +308,11 @@ func (e *Executor) executeRun(cmd *cobra.Command, args []string) { } if e.cfg.Output.PrintWelcomeMessage { - fmt.Println("Run this tool in cloud on every github pull request in https://golangci.com for free (public repos)") + fmt.Fprintln(printers.StdOut, "Run this tool in cloud on every github pull request in https://golangci.com for free (public repos)") } if err := e.runAndPrint(ctx, args); err != nil { - log.Print(err) + logrus.Warnf("running error: %s", err) if e.exitCode == 0 { e.exitCode = exitCodeIfFailure } @@ -305,11 +325,11 @@ func (e *Executor) parseConfig(cmd *cobra.Command) { if err == pflag.ErrHelp { return } - log.Fatalf("Can't parse args: %s", err) + logrus.Fatalf("Can't parse args: %s", err) } if err := viper.BindPFlags(cmd.Flags()); err != nil { - log.Fatalf("Can't bind cobra's flags to viper: %s", err) + logrus.Fatalf("Can't bind cobra's flags to viper: %s", err) } viper.SetEnvPrefix("GOLANGCI") @@ -318,7 +338,7 @@ func (e *Executor) parseConfig(cmd *cobra.Command) { configFile := e.cfg.Run.Config if e.cfg.Run.NoConfig && configFile != "" { - log.Fatal("can't combine option --config and --no-config") + logrus.Fatal("can't combine option --config and --no-config") } if e.cfg.Run.NoConfig { @@ -342,15 +362,15 @@ func (e *Executor) parseConfigImpl() { if _, ok := err.(viper.ConfigFileNotFoundError); ok { return } - log.Fatalf("Can't read viper config: %s", err) + logrus.Fatalf("Can't read viper config: %s", err) } if err := viper.Unmarshal(&e.cfg); err != nil { - log.Fatalf("Can't unmarshal config by viper: %s", err) + logrus.Fatalf("Can't unmarshal config by viper: %s", err) } if err := e.validateConfig(&commandLineConfig); err != nil { - log.Fatal(err) + logrus.Fatal(err) } } diff --git a/pkg/enabled_linters.go b/pkg/enabled_linters.go index d952b33f..01c59da5 100644 --- a/pkg/enabled_linters.go +++ b/pkg/enabled_linters.go @@ -118,6 +118,7 @@ func GetAllSupportedLinterConfigs() []LinterConfig { newLinterConfig(golinters.Goconst{}).WithPresets(PresetStyle).WithSpeed(9), newLinterConfig(golinters.Deadcode{}).WithFullImport().WithPresets(PresetUnused).WithSpeed(10), newLinterConfig(golinters.Gocyclo{}).WithPresets(PresetComplexity).WithSpeed(8), + newLinterConfig(golinters.TypeCheck{}).WithFullImport().WithPresets(PresetBugs).WithSpeed(10), newLinterConfig(golinters.Gofmt{}).WithPresets(PresetFormatting).WithSpeed(7), newLinterConfig(golinters.Gofmt{UseGoimports: true}).WithPresets(PresetFormatting).WithSpeed(5), @@ -128,9 +129,10 @@ func GetAllSupportedLinterConfigs() []LinterConfig { if os.Getenv("GOLANGCI_COM_RUN") == "1" { disabled := map[string]bool{ - "gocyclo": true, - "dupl": true, - "maligned": true, + golinters.Gocyclo{}.Name(): true, // annoying + golinters.Dupl{}.Name(): true, // annoying + golinters.Maligned{}.Name(): true, // rarely usable + golinters.TypeCheck{}.Name(): true, // annoying because of different building envs } return enableLinterConfigs(lcs, func(lc *LinterConfig) bool { return !disabled[lc.Linter.Name()] @@ -138,16 +140,17 @@ func GetAllSupportedLinterConfigs() []LinterConfig { } enabled := map[string]bool{ - "govet": true, - "errcheck": true, - "staticcheck": true, - "unused": true, - "gosimple": true, - "gas": true, - "structcheck": true, - "varcheck": true, - "ineffassign": true, - "deadcode": true, + golinters.Govet{}.Name(): true, + golinters.Errcheck{}.Name(): true, + golinters.Megacheck{StaticcheckEnabled: true}.Name(): true, + golinters.Megacheck{UnusedEnabled: true}.Name(): true, + golinters.Megacheck{GosimpleEnabled: true}.Name(): true, + golinters.Gas{}.Name(): true, + golinters.Structcheck{}.Name(): true, + golinters.Varcheck{}.Name(): true, + golinters.Ineffassign{}.Name(): true, + golinters.Deadcode{}.Name(): true, + golinters.TypeCheck{}.Name(): true, } return enableLinterConfigs(lcs, func(lc *LinterConfig) bool { return enabled[lc.Linter.Name()] diff --git a/pkg/enabled_linters_test.go b/pkg/enabled_linters_test.go index 52623005..fca42d6d 100644 --- a/pkg/enabled_linters_test.go +++ b/pkg/enabled_linters_test.go @@ -31,7 +31,6 @@ func runGoErrchk(c *exec.Cmd, t *testing.T) { const testdataDir = "testdata" var testdataWithIssuesDir = filepath.Join(testdataDir, "with_issues") -var testdataNotCompilingDir = filepath.Join(testdataDir, "not_compiles") const binName = "golangci-lint" @@ -72,12 +71,6 @@ func testOneSource(t *testing.T, sourcePath string) { runGoErrchk(cmd, t) } -func TestNotCompilingProgram(t *testing.T) { - installBinary(t) - err := exec.Command(binName, "run", "--enable-all", testdataNotCompilingDir).Run() - assert.NoError(t, err) -} - func chdir(b *testing.B, dir string) { if err := os.Chdir(dir); err != nil { b.Fatalf("can't chdir to %s: %s", dir, err) diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index 6b8c758e..a5c22d9e 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -32,7 +32,7 @@ func (g Golint) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, erro issues = append(issues, i...) } if lintErr != nil { - logrus.Warnf("golint: %s", lintErr) + logrus.Infof("golint: %s", lintErr) } return issues, nil diff --git a/pkg/golinters/typecheck.go b/pkg/golinters/typecheck.go new file mode 100644 index 00000000..7954004c --- /dev/null +++ b/pkg/golinters/typecheck.go @@ -0,0 +1,76 @@ +package golinters + +import ( + "context" + "go/token" + "strconv" + "strings" + + "github.com/golangci/golangci-lint/pkg/result" +) + +type TypeCheck struct{} + +func (TypeCheck) Name() string { + return "typecheck" +} + +func (TypeCheck) Desc() string { + return "Like the front-end of a Go compiler, parses and type-checks Go code" +} + +func (lint TypeCheck) parseError(err error) *result.Issue { + // file:line(:colon): message + parts := strings.Split(err.Error(), ":") + if len(parts) < 3 { + return nil + } + + file := parts[0] + line, err := strconv.Atoi(parts[1]) + if err != nil { + return nil + } + + var column int + var message string + if len(parts) == 3 { // no column + message = parts[2] + } else { + column, err = strconv.Atoi(parts[2]) + if err == nil { // column was parsed + message = strings.Join(parts[3:], ":") + } else { + message = strings.Join(parts[2:], ":") + } + } + + message = strings.TrimSpace(message) + if message == "" { + return nil + } + + return &result.Issue{ + Pos: token.Position{ + Filename: file, + Line: line, + Column: column, + }, + Text: message, + FromLinter: lint.Name(), + } +} + +func (lint TypeCheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { + var res []result.Issue + for _, pkg := range lintCtx.Program.InitialPackages() { + for _, err := range pkg.Errors { + i := lint.parseError(err) + if i != nil { + res = append(res, *i) + } + } + } + + return res, nil +} diff --git a/pkg/golinters/typecheck_test.go b/pkg/golinters/typecheck_test.go new file mode 100644 index 00000000..93c39f6c --- /dev/null +++ b/pkg/golinters/typecheck_test.go @@ -0,0 +1,59 @@ +package golinters + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseError(t *testing.T) { + cases := []struct { + in, out string + good bool + }{ + {"f.go:1:2: text", "", true}, + {"f.go:1:2: text: with: colons", "", true}, + + {"f.go:1:2:text wo leading space", "f.go:1:2: text wo leading space", true}, + + {"f.go:1:2:", "", false}, + {"f.go:1:2: ", "", false}, + + {"f.go:1:2", "f.go:1: 2", true}, + {"f.go:1: text no column", "", true}, + {"f.go:1: text no column: but with colon", "", true}, + {"f.go:1:text no column", "f.go:1: text no column", true}, + + {"f.go: no line", "", false}, + {"f.go: 1: text", "", false}, + + {"f.go:", "", false}, + {"f.go", "", false}, + } + + lint := TypeCheck{} + for _, c := range cases { + i := lint.parseError(errors.New(c.in)) + if !c.good { + assert.Nil(t, i) + continue + } + + assert.NotNil(t, i) + + pos := fmt.Sprintf("%s:%d", i.FilePath(), i.Line()) + if i.Pos.Column != 0 { + pos += fmt.Sprintf(":%d", i.Pos.Column) + } + out := fmt.Sprintf("%s: %s", pos, i.Text) + expOut := c.out + if expOut == "" { + expOut = c.in + } + assert.Equal(t, expOut, out) + + assert.Equal(t, "typecheck", i.FromLinter) + } +} diff --git a/pkg/printers/json.go b/pkg/printers/json.go index e98664d2..38987344 100644 --- a/pkg/printers/json.go +++ b/pkg/printers/json.go @@ -22,6 +22,6 @@ func (JSON) Print(issues <-chan result.Issue) (bool, error) { if err != nil { return false, err } - fmt.Fprint(stdOut, string(outputJSON)) + fmt.Fprint(StdOut, string(outputJSON)) return len(allIssues) != 0, nil } diff --git a/pkg/printers/text.go b/pkg/printers/text.go index 84c53cb4..fe94bfe2 100644 --- a/pkg/printers/text.go +++ b/pkg/printers/text.go @@ -88,7 +88,7 @@ func (p *Text) Print(issues <-chan result.Issue) (bool, error) { if issuesN == 0 { outStr := p.SprintfColored(color.FgGreen, "Congrats! No issues were found.") - fmt.Fprintln(stdOut, outStr) + fmt.Fprintln(StdOut, outStr) } else { logrus.Infof("Found %d issues", issuesN) } @@ -105,7 +105,7 @@ func (p Text) printIssue(i *result.Issue) { if i.Pos.Column != 0 { pos += fmt.Sprintf(":%d", i.Pos.Column) } - fmt.Fprintf(stdOut, "%s: %s\n", pos, text) + fmt.Fprintf(StdOut, "%s: %s\n", pos, text) } func (p Text) printIssuedLines(i *result.Issue, lines linesCache) { @@ -123,7 +123,7 @@ func (p Text) printIssuedLines(i *result.Issue, lines linesCache) { } lineStr = string(bytes.Trim(lines[zeroIndexedLine], "\r")) - fmt.Fprintln(stdOut, lineStr) + fmt.Fprintln(StdOut, lineStr) } } @@ -146,5 +146,5 @@ func (p Text) printUnderLinePointer(i *result.Issue, line string) { prefix += strings.Repeat(" ", spacesCount) } - fmt.Fprintf(stdOut, "%s%s\n", prefix, p.SprintfColored(color.FgYellow, "^")) + fmt.Fprintf(StdOut, "%s%s\n", prefix, p.SprintfColored(color.FgYellow, "^")) } diff --git a/pkg/printers/utils.go b/pkg/printers/utils.go index 17aa4157..913f3181 100644 --- a/pkg/printers/utils.go +++ b/pkg/printers/utils.go @@ -5,4 +5,4 @@ import ( "syscall" ) -var stdOut = os.NewFile(uintptr(syscall.Stdout), "/dev/stdout") // was set to /dev/null +var StdOut = os.NewFile(uintptr(syscall.Stdout), "/dev/stdout") // was set to /dev/null diff --git a/pkg/result/processors/max_per_file_from_linter.go b/pkg/result/processors/max_per_file_from_linter.go index cea0cee6..ae8319a5 100644 --- a/pkg/result/processors/max_per_file_from_linter.go +++ b/pkg/result/processors/max_per_file_from_linter.go @@ -27,6 +27,7 @@ func (p MaxPerFileFromLinter) Name() string { var maxPerFileFromLinterConfig = map[string]int{ golinters.Gofmt{}.Name(): 1, golinters.Gofmt{UseGoimports: true}.Name(): 1, + golinters.TypeCheck{}.Name(): 3, } func (p *MaxPerFileFromLinter) Process(issues []result.Issue) ([]result.Issue, error) { diff --git a/pkg/runner.go b/pkg/runner.go index 8a184a4e..952c1ea5 100644 --- a/pkg/runner.go +++ b/pkg/runner.go @@ -3,7 +3,6 @@ package pkg import ( "context" "fmt" - "os" "runtime/debug" "sort" "strings" @@ -107,7 +106,6 @@ func (r *SimpleRunner) runWorkers(ctx context.Context, lintCtx *golinters.Contex lintResultsCh := make(chan lintRes, len(linters)) var wg sync.WaitGroup - savedStdout, savedStderr := setOutputToDevNull() // Don't allow linters to print anything workersFinishTimes := make([]time.Time, lintCtx.Cfg.Run.Concurrency) for i := 0; i < lintCtx.Cfg.Run.Concurrency; i++ { @@ -129,7 +127,6 @@ func (r *SimpleRunner) runWorkers(ctx context.Context, lintCtx *golinters.Contex go func() { wg.Wait() close(lintResultsCh) - os.Stdout, os.Stderr = savedStdout, savedStderr logWorkersStat(workersFinishTimes) }() @@ -190,18 +187,6 @@ func collectIssues(ctx context.Context, resCh <-chan lintRes) <-chan result.Issu return retIssues } -func setOutputToDevNull() (savedStdout, savedStderr *os.File) { - savedStdout, savedStderr = os.Stdout, os.Stderr - devNull, err := os.Open(os.DevNull) - if err != nil { - logrus.Warnf("can't open null device %q: %s", os.DevNull, err) - return - } - - os.Stdout, os.Stderr = devNull, devNull - return -} - func (r SimpleRunner) Run(ctx context.Context, linters []Linter, lintCtx *golinters.Context) <-chan result.Issue { lintResultsCh := r.runWorkers(ctx, lintCtx, linters) processedLintResultsCh := r.processLintResults(ctx, lintResultsCh) @@ -228,7 +213,7 @@ func (r *SimpleRunner) processIssues(ctx context.Context, issues []result.Issue, }) if err != nil { - logrus.Warnf("Can't process result by %s processor: %s", p.Name(), err) + logrus.Infof("Can't process result by %s processor: %s", p.Name(), err) } else { issues = newIssues } diff --git a/pkg/testdata/not_compiles/main.go b/pkg/testdata/not_compiles/main.go deleted file mode 100644 index d7c81e23..00000000 --- a/pkg/testdata/not_compiles/main.go +++ /dev/null @@ -1,3 +0,0 @@ -package p - -func F { diff --git a/pkg/testdata/with_issues/typecheck.go b/pkg/testdata/with_issues/typecheck.go new file mode 100644 index 00000000..a31d5538 --- /dev/null +++ b/pkg/testdata/with_issues/typecheck.go @@ -0,0 +1,4 @@ +package testdata + +fun NotCompiles() { // ERROR "expected declaration, found 'IDENT' fun" +} diff --git a/pkg/testdata/with_issues/typecheck_many_issues.go b/pkg/testdata/with_issues/typecheck_many_issues.go new file mode 100644 index 00000000..9de48deb --- /dev/null +++ b/pkg/testdata/with_issues/typecheck_many_issues.go @@ -0,0 +1,8 @@ +package testdata + +func TypeCheckBadCalls() { + typecheckNotExists1.F1() // ERROR "undeclared name: typecheckNotExists1" + typecheckNotExists2.F2() // ERROR "undeclared name: typecheckNotExists2" + typecheckNotExists3.F3() // ERROR "undeclared name: typecheckNotExists3" + typecheckNotExists4.F4() +}