Skip to content

refactor and don't print congrats if timeouted #53

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 2, 2018
Merged

refactor and don't print congrats if timeouted #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@ test:
golangci-lint run --fast --no-config -v
golangci-lint run --fast --no-config -v
golangci-lint run --no-config -v
golangci-lint run --fast --no-config -v ./pkg/testdata/with_issues/typecheck.go
golangci-lint run --fast --no-config -v ./test/testdata/typecheck.go
go test -v -race ./...

.PHONY: test
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ This issue is important because often you'd like to set concurrency to CPUs coun
3. It will take more time because of different usages and need of tracking of versions of `n` linters.

# Performance
Benchmarks were executed on MacBook Pro (Retina, 13-inch, Late 2013), 2,4 GHz Intel Core i5, 8 GB 1600 MHz DDR3. It has 4 cores and concurrency for linters was default: number of cores. Benchmark runs and measures timings automatically, it's code is [here](https://github.com/golangci/golangci-lint/blob/master/pkg/enabled_linters_test.go) (`BenchmarkWithGometalinter`).
Benchmarks were executed on MacBook Pro (Retina, 13-inch, Late 2013), 2,4 GHz Intel Core i5, 8 GB 1600 MHz DDR3. It has 4 cores and concurrency for linters was default: number of cores. Benchmark runs and measures timings automatically, it's code is [here](https://github.com/golangci/golangci-lint/blob/master/test/bench.go) (`BenchmarkWithGometalinter`).

We measure peak memory usage (RSS) by tracking of processes RSS every 5 ms.

Expand Down
6 changes: 3 additions & 3 deletions pkg/commands/linters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ func (e Executor) executeLinters(cmd *cobra.Command, args []string) {

color.Green("\nLinters presets:")
for _, p := range pkg.AllPresets() {
linters := pkg.GetAllLintersForPreset(p)
linters := pkg.GetAllLinterConfigsForPreset(p)
linterNames := []string{}
for _, linter := range linters {
linterNames = append(linterNames, linter.Name())
for _, lc := range linters {
linterNames = append(linterNames, lc.Linter.Name())
}
fmt.Fprintf(printers.StdOut, "%s: %s\n", color.YellowString(p), strings.Join(linterNames, ", "))
}
Expand Down
189 changes: 13 additions & 176 deletions pkg/commands/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,24 @@ import (
"context"
"errors"
"fmt"
"go/build"
"go/token"
"io/ioutil"
"log"
"os"
"os/exec"
"runtime"
"strings"
"time"

"github.com/golangci/go-tools/ssa"
"github.com/golangci/go-tools/ssa/ssautil"
"github.com/golangci/golangci-lint/pkg"
"github.com/golangci/golangci-lint/pkg/astcache"
"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/golinters"
"github.com/golangci/golangci-lint/pkg/lint"
"github.com/golangci/golangci-lint/pkg/printers"
"github.com/golangci/golangci-lint/pkg/result"
"github.com/golangci/golangci-lint/pkg/result/processors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/spf13/viper"
"golang.org/x/tools/go/loader"
)

const (
Expand Down Expand Up @@ -126,170 +119,6 @@ func (e *Executor) initRun() {
e.parseConfig(runCmd)
}

func isFullImportNeeded(linters []pkg.Linter) bool {
for _, linter := range linters {
lc := pkg.GetLinterConfig(linter.Name())
if lc.DoesFullImport {
return true
}
}

return false
}

func isSSAReprNeeded(linters []pkg.Linter) bool {
for _, linter := range linters {
lc := pkg.GetLinterConfig(linter.Name())
if lc.NeedsSSARepr {
return true
}
}

return false
}

func loadWholeAppIfNeeded(ctx context.Context, linters []pkg.Linter, cfg *config.Run, paths *fsutils.ProjectPaths) (*loader.Program, *loader.Config, error) {
if !isFullImportNeeded(linters) {
return nil, nil, nil
}

startedAt := time.Now()
defer func() {
logrus.Infof("Program loading took %s", time.Since(startedAt))
}()

bctx := build.Default
bctx.BuildTags = append(bctx.BuildTags, cfg.BuildTags...)
loadcfg := &loader.Config{
Build: &bctx,
AllowErrors: true, // Try to analyze event partially
}
rest, err := loadcfg.FromArgs(paths.MixedPaths(), cfg.AnalyzeTests)
if err != nil {
return nil, nil, fmt.Errorf("can't parepare load config with paths: %s", err)
}
if len(rest) > 0 {
return nil, nil, fmt.Errorf("unhandled loading paths: %v", rest)
}

prog, err := loadcfg.Load()
if err != nil {
return nil, nil, fmt.Errorf("can't load paths: %s", err)
}

return prog, loadcfg, nil
}

func buildSSAProgram(ctx context.Context, lprog *loader.Program) *ssa.Program {
startedAt := time.Now()
defer func() {
logrus.Infof("SSA repr building took %s", time.Since(startedAt))
}()

ssaProg := ssautil.CreateProgram(lprog, ssa.GlobalDebug)
ssaProg.Build()
return ssaProg
}

func discoverGoRoot() (string, error) {
goroot := os.Getenv("GOROOT")
if goroot != "" {
return goroot, nil
}

output, err := exec.Command("go", "env", "GOROOT").Output()
if err != nil {
return "", fmt.Errorf("can't execute go env GOROOT: %s", err)
}

return strings.TrimSpace(string(output)), nil
}

// separateNotCompilingPackages moves not compiling packages into separate slices:
// a lot of linters crash on such packages. Leave them only for those linters
// which can work with them.
func separateNotCompilingPackages(lintCtx *golinters.Context) {
prog := lintCtx.Program

if prog.Created != nil {
compilingCreated := make([]*loader.PackageInfo, 0, len(prog.Created))
for _, info := range prog.Created {
if len(info.Errors) != 0 {
lintCtx.NotCompilingPackages = append(lintCtx.NotCompilingPackages, info)
} else {
compilingCreated = append(compilingCreated, info)
}
}
prog.Created = compilingCreated
}

if prog.Imported != nil {
for k, info := range prog.Imported {
if len(info.Errors) != 0 {
lintCtx.NotCompilingPackages = append(lintCtx.NotCompilingPackages, info)
delete(prog.Imported, k)
}
}
}
}

func buildLintCtx(ctx context.Context, linters []pkg.Linter, cfg *config.Config) (*golinters.Context, error) {
// Set GOROOT to have working cross-compilation: cross-compiled binaries
// have invalid GOROOT. XXX: can't use runtime.GOROOT().
goroot, err := discoverGoRoot()
if err != nil {
return nil, fmt.Errorf("can't discover GOROOT: %s", err)
}
os.Setenv("GOROOT", goroot)
build.Default.GOROOT = goroot
logrus.Infof("set GOROOT=%q", goroot)

args := cfg.Run.Args
if len(args) == 0 {
args = []string{"./..."}
}

paths, err := fsutils.GetPathsForAnalysis(ctx, args, cfg.Run.AnalyzeTests)
if err != nil {
return nil, err
}

prog, loaderConfig, err := loadWholeAppIfNeeded(ctx, linters, &cfg.Run, paths)
if err != nil {
return nil, err
}

var ssaProg *ssa.Program
if prog != nil && isSSAReprNeeded(linters) {
ssaProg = buildSSAProgram(ctx, prog)
}

var astCache *astcache.Cache
if prog != nil {
astCache, err = astcache.LoadFromProgram(prog)
if err != nil {
return nil, err
}
} else {
astCache = astcache.LoadFromFiles(paths.Files)
}

ret := &golinters.Context{
Paths: paths,
Cfg: cfg,
Program: prog,
SSAProgram: ssaProg,
LoaderConfig: loaderConfig,
ASTCache: astCache,
}

if prog != nil {
separateNotCompilingPackages(ret)
}

return ret, nil
}

func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan result.Issue, error) {
e.cfg.Run.Args = args

Expand All @@ -298,7 +127,11 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul
return nil, err
}

lintCtx, err := buildLintCtx(ctx, linters, e.cfg)
ctxLinters := make([]lint.LinterConfig, 0, len(linters))
for _, lc := range linters {
ctxLinters = append(ctxLinters, lint.LinterConfig(lc))
}
lintCtx, err := lint.BuildContext(ctx, ctxLinters, e.cfg)
if err != nil {
return nil, err
}
Expand All @@ -315,7 +148,7 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul
if lintCtx.Program != nil {
fset = lintCtx.Program.Fset
}
runner := pkg.SimpleRunner{
runner := lint.SimpleRunner{
Processors: []processors.Processor{
processors.NewPathPrettifier(), // must be before diff processor at least
processors.NewExclude(excludeTotalPattern),
Expand All @@ -329,7 +162,11 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul
},
}

return runner.Run(ctx, linters, lintCtx), nil
runLinters := make([]lint.RunnerLinterConfig, 0, len(linters))
for _, lc := range linters {
runLinters = append(runLinters, lint.RunnerLinterConfig(lc))
}
return runner.Run(ctx, runLinters, lintCtx), nil
}

func setOutputToDevNull() (savedStdout, savedStderr *os.File) {
Expand Down Expand Up @@ -364,7 +201,7 @@ func (e *Executor) runAndPrint(ctx context.Context, args []string) error {
p = printers.NewText(e.cfg.Output.PrintIssuedLine,
e.cfg.Output.Format == config.OutFormatColoredLineNumber, e.cfg.Output.PrintLinterName)
}
gotAnyIssues, err := p.Print(issues)
gotAnyIssues, err := p.Print(ctx, issues)
if err != nil {
return fmt.Errorf("can't print %d issues: %s", len(issues), err)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var DefaultExcludePatterns = []string{
// golint
"should have comment",
"comment on exported method",
"func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this",

// gas
"G103:", // Use of unsafe calls should be audited
Expand Down
Loading