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

Improve compilation errors output #1859

Closed
wants to merge 2 commits into from

Conversation

ldez
Copy link
Member

@ldez ldez commented Mar 19, 2021

Fixes #1002
Related to #1820 and #886

I think this also fixes partially #1792 (#1792 (comment))

This PR will improve in the report when the analyzed code contains compilation errors.

It's a bit complex to explain but I will try to explain it.

typecheck is like the front-end of a Go compiler, parses and type-checks Go code.
This linter is able to manage compilation error, that's his role.
So this linter must be the first linter.

To put typecheck as the first linter, I improved the sort of the linters inside enabled_set.go and I changed the load mode to goanalysis.LoadModeWholeProgram (because the linters with goanalysis.LoadModeWholeProgram are not put inside the metalinter and they are executed before the metalinter)

This change will not improve the output when only one linter is launched, it's another topic.
But I think that the output will be better in 90% of the use-cases: when several linters (including typecheck) are run

Before my PR
$ golangci-lint run
WARN [runner] Can't run linter goanalysis_metalinter: bodyclose: failed prerequisites: [buildssa@github.com/golangci/golangci-lint/pkg/lint: analysis skipped: errors in package: [/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/lint/runner.go:18:2: could not import github.com/golangci/golangci-lint/pkg/lint/lintersdb (/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/lint/lintersdb/manager.go:13:2: could not import github.com/golangci/golangci-lint/pkg/golinters (/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:21:9: undeclared name: linterName)) /home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/lint/runner.go:22:2: could not import github.com/golangci/golangci-lint/pkg/result/processors (/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/result/processors/nolint.go:11:2: could not import github.com/golangci/golangci-lint/pkg/golinters (/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:21:9: undeclared name: linterName))]] 
WARN [runner] Can't run linter unused: buildir: failed to load package golinters: could not load export data: no export data for "github.com/golangci/golangci-lint/pkg/golinters" 
ERRO Running error: buildir: failed to load package golinters: could not load export data: no export data for "github.com/golangci/golangci-lint/pkg/golinters"
With my PR
$ ./golangci-lint run
pkg/lint/runner.go:18:2: could not import github.com/golangci/golangci-lint/pkg/lint/lintersdb (pkg/lint/lintersdb/manager.go:13:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName)) (typecheck)
        "github.com/golangci/golangci-lint/pkg/lint/lintersdb"
        ^
pkg/lint/runner.go:22:2: could not import github.com/golangci/golangci-lint/pkg/result/processors (pkg/result/processors/nolint.go:11:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName)) (typecheck)
        "github.com/golangci/golangci-lint/pkg/result/processors"
        ^
pkg/commands/executor.go:26:2: could not import github.com/golangci/golangci-lint/pkg/lint (pkg/lint/runner.go:18:2: could not import github.com/golangci/golangci-lint/pkg/lint/lintersdb (pkg/lint/lintersdb/manager.go:13:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName))) (typecheck)
        "github.com/golangci/golangci-lint/pkg/lint"
        ^
pkg/commands/executor.go:27:2: could not import github.com/golangci/golangci-lint/pkg/lint/lintersdb (pkg/lint/lintersdb/manager.go:13:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName)) (typecheck)
        "github.com/golangci/golangci-lint/pkg/lint/lintersdb"
        ^
pkg/commands/run.go:26:2: could not import github.com/golangci/golangci-lint/pkg/result/processors (pkg/result/processors/nolint.go:11:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName)) (typecheck)
        "github.com/golangci/golangci-lint/pkg/result/processors"
        ^
cmd/golangci-lint/main.go:7:2: could not import github.com/golangci/golangci-lint/pkg/commands (pkg/commands/executor.go:26:2: could not import github.com/golangci/golangci-lint/pkg/lint (pkg/lint/runner.go:18:2: could not import github.com/golangci/golangci-lint/pkg/lint/lintersdb (pkg/lint/lintersdb/manager.go:13:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName)))) (typecheck)
        "github.com/golangci/golangci-lint/pkg/commands"
        ^
scripts/expand_website_templates/main.go:21:2: could not import github.com/golangci/golangci-lint/pkg/lint/lintersdb (pkg/lint/lintersdb/manager.go:13:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName)) (typecheck)
        "github.com/golangci/golangci-lint/pkg/lint/lintersdb"
        ^
pkg/golinters/deadcode.go:21:9: undeclared name: `linterName` (typecheck)
                Name: linterName,
                      ^
pkg/lint/lintersdb/manager.go:13:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName) (typecheck)
        "github.com/golangci/golangci-lint/pkg/golinters"
        ^
pkg/lint/lintersdb/manager.go:501:2: isLocalRun declared but not used (typecheck)
        isLocalRun := os.Getenv("GOLANGCI_COM_RUN") == ""
        ^
pkg/result/processors/nolint.go:11:2: could not import github.com/golangci/golangci-lint/pkg/golinters (pkg/golinters/deadcode.go:21:9: undeclared name: linterName) (typecheck)
        "github.com/golangci/golangci-lint/pkg/golinters"
        ^

@ldez ldez added the bug Something isn't working label Mar 19, 2021
@ldez ldez marked this pull request as draft March 19, 2021 18:49
@ldez ldez force-pushed the fix/improve-compile-errors-output branch from ce5b910 to 6fe0d05 Compare March 19, 2021 18:53
@bombsimon
Copy link
Member

Wow, awesome job! Does typecheck list all compilation errors by being run first? If so, this pretty much solves all issues asking for this!

@ldez
Copy link
Member Author

ldez commented Mar 20, 2021

FYI typecheck is not a real linter it's just a way to parse some error.
The code in pkg/golinters/typecheck.go is just a kind of placeholder.

Run: func(pass *analysis.Pass) (interface{}, error) {
return nil, nil
},

The real code is here:

func parseError(srcErr packages.Error) (*result.Issue, error) {
pos, err := libpackages.ParseErrorPosition(srcErr.Pos)
if err != nil {
return nil, err
}
return &result.Issue{
Pos: *pos,
Text: srcErr.Msg,
FromLinter: "typecheck",
}, nil
}
func buildIssuesFromErrorsForTypecheckMode(errs []error, lintCtx *linter.Context) ([]result.Issue, error) {
var issues []result.Issue
uniqReportedIssues := map[string]bool{}
for _, err := range errs {
itErr, ok := errors.Cause(err).(*IllTypedError)
if !ok {
return nil, err
}
for _, err := range libpackages.ExtractErrors(itErr.Pkg) {
i, perr := parseError(err)
if perr != nil { // failed to parse
if uniqReportedIssues[err.Msg] {
continue
}
uniqReportedIssues[err.Msg] = true
lintCtx.Log.Errorf("typechecking error: %s", err.Msg)
} else {
i.Pkg = itErr.Pkg // to save to cache later
issues = append(issues, *i)
}
}
}
return issues, nil
}

Currently, I try to handle a side effect of using LoadModeWholeProgram on typecheck.
So I'm trying to change the core system to handle errors, if I succeed all the issues related to the compilation errors message will be fixed.

@ldez
Copy link
Member Author

ldez commented Mar 20, 2021

I think that I found the "ultimate" fix 🎉

But for that, I will reorganize the code without any changes, and after I will fix the core problem.
This will produce a lot of changes because I will just move the code.

My plan:

  • drop all my commits
  • split the pkg/golinters/goanalysis/runner.go file into multiple files (because it's a pain to debug a 1500 lines file with recursive calls, with multiple struct and multiple embedded and anonymous functions)
  • apply the "ultimate fix"
  • fix the sorting of the linters to fix the randomness of the result of the analysis

Maybe I will create another PR 🤔

@ldez
Copy link
Member Author

ldez commented Mar 20, 2021

Superseded by #1861

@ldez ldez closed this Mar 20, 2021
@ldez ldez deleted the fix/improve-compile-errors-output branch March 20, 2021 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--output-format no longer reporting typecheck errors
2 participants