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

bazel: add stylecheck analyzers to nogo #73613

Merged
merged 2 commits into from
Dec 13, 2021

Conversation

rickystewart
Copy link
Collaborator

We skip the checks ST1000, ST1003, ST1016, ST1020, ST1021,
and ST1022 since they're skipped by the default staticcheck config,
and our codebase has many violations of them.

Closes #73568.

Release note: None

@rickystewart rickystewart requested review from rail and a team December 8, 2021 18:33
@rickystewart rickystewart requested a review from a team as a code owner December 8, 2021 18:33
@rickystewart rickystewart requested review from msbutler and removed request for a team December 8, 2021 18:33
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rickystewart
Copy link
Collaborator Author

Only the last commit applies for this review, the other is from #73572.

@rickystewart rickystewart force-pushed the stylecheck branch 2 times, most recently from 0aad1ec to 1e21da7 Compare December 9, 2021 15:32
@rickystewart rickystewart requested a review from a team December 9, 2021 15:32
Copy link
Member

@rail rail left a comment

Choose a reason for hiding this comment

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

Reviewed 18 of 252 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @msbutler and @rail)


build/bazelutil/nogo_config.json, line 108 at r1 (raw file):

    },
    "S1000": {
	"exclude_files": {

Nit: a mix of tabs and space. Worth unifying the style.

Add a new binary `generate-staticcheck`, part of `dev generate bazel`,
that creates a package in `build/bazelutil/staticcheckanalyzers`
capturing a single `staticcheck` check. Then we capture the list of
analyzers in `build/bazelutil/staticcheckanalyzers/def.bzl` and add all
those checks to `nogo`.

However, we can't directly use the `Analyzer`s provided by the upstream
library, because they report *all* violations regardless of
`lint:ignore` directives. In order to address this we add a helper
library at `pkg/testutils/lint/passes/staticcheck` that munges the
`Analyzer`s to additionally pull the `lint:ignore` directives and ignore
any reported diagnostics that correspond to a `lint:ignore`.

This follows the example set by https://github.com/sluongng/staticcheck-codegen,
but we don't directly use that code, since we need to inject our own
stuff and we want to make sure the content here is pinned to the right
version of `staticcheck`.

We run the `staticcheck` and `simple` [checks](https://staticcheck.io/docs/checks).
`stylecheck` is still broken: cockroachdb#73568

Closes cockroachdb#68496.

Release note: None
We skip the checks `ST1000`, `ST1003`, `ST1016`, `ST1020`, `ST1021`,
and `ST1022` since they're skipped by the default `staticcheck` config,
and our codebase has many violations of them.

Closes cockroachdb#73568.

Release note: None
@rickystewart
Copy link
Collaborator Author

bors r=rail

@craig
Copy link
Contributor

craig bot commented Dec 13, 2021

Build succeeded:

@craig craig bot merged commit d411bd0 into cockroachdb:master Dec 13, 2021
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.

bazel: stylecheck checks trigger many failures when run in nogo
3 participants