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

Conversation

dtcaciuc
Copy link
Collaborator

This PR partially addresses #165, i.e. it provides an Analyzer but does not rework the internals around it yet.

@dtcaciuc
Copy link
Collaborator Author

Existing tests were copied and are largely unchanged. They are adapted to syntax required by the testing utility package https://pkg.go.dev/golang.org/x/tools@v0.1.0/go/analysis/analysistest. Separate test cases were added to test individual enabled flags.

Copy link
Collaborator

@echlebek echlebek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why the package-scoped variable, rather than a way to produce an analyzer with a function?

@dtcaciuc
Copy link
Collaborator Author

dtcaciuc commented Apr 8, 2021 via email

@echlebek
Copy link
Collaborator

Should we merge this as is, and make another issue to migrate the internals?

@dtcaciuc
Copy link
Collaborator Author

Not sure if it constitutes migration, but it should be possible to simplify the code to record raw token.Pos rather than immediately resolve it into file & line with addErrorAtPosition() whenever we find an error as its done now. Then after the visitor is done running, either feed that directly into go/analysis function output or convert to file/line and write to errcheck program output.

I'll take the PR out of draft.

@dtcaciuc dtcaciuc marked this pull request as ready for review April 21, 2021 06:19
@dtcaciuc dtcaciuc merged commit cf5d9bf into kisielk:master May 28, 2021
@dtcaciuc dtcaciuc deleted the analyzer branch May 28, 2021 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants