bazel: stylecheck checks trigger many failures when run in nogo
#73568
Assignees
Labels
A-build-system
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-dev-inf
Comments
rickystewart
added
C-enhancement
rickystewart
changed the title
stylecheck checks trigger many failures when stylecheck checks trigger many failures when run in nogo
|
rickystewart
added a commit
to rickystewart/cockroach
that referenced
this issue
Dec 9, 2021
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
rickystewart
added a commit
to rickystewart/cockroach
that referenced
this issue
Dec 9, 2021
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
added a commit
to rickystewart/cockroach
that referenced
this issue
Dec 13, 2021
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
craig bot
pushed a commit
that referenced
this issue
Dec 13, 2021
72818: sql: use Latest[Non]PrimaryIndexDescriptorVersion constants in more places r=mgartner a=mgartner These constants have been moved from tabledesc to descpb to avoid import cycles. Release note: None 73151: sql: require valid TenantID in start_replication_stream r=rafiss a=stevendanna MakeTenantID panics if someone tries to use 0 as a tenant ID. Here, we return an error immediately in this case rather than crashing. Fixes #71311 Release note: None 73302: sql: add index recommendation to EXPLAIN output r=nehageorge a=nehageorge **indexrec: disable inverted index recommendations** Previously, we would erroneously give index recommendations of non- inverted indexes for indexes that should be inverted. In this PR, we omit index candidates that required inverted indexes. Specifically, this includes JSON columns, array columns, and spatial data columns. Release note: None **sql: add index recommendation to EXPLAIN** Previously, the EXPLAIN output only showed the query plan. Since users often use EXPLAIN to see why a query is poorly performing, it would be useful for them to also see index recommendations to make the query faster. In this commit, index recommendations are added to the bottom of the EXPLAIN output. See the indexrec package under `pkg/sql/opt/indexrec` to understand how index recommendations are generated. Release note (sql change): The output of the EXPLAIN SQL statement has changed. Below the plan, we now output index recommendations for the SQL statement being "EXPLAIN-ed", if there are any. These index recommendations are indexes the user could add or indexes they could replace to make the given query faster. Closes #72761. **indexrec: refactor index recommendation output** Previously, we manually formatted the string for the index recommendation's SQL output. This commit takes advantage of the existing functionality for formatting SQL statements, specifically `CREATE INDEX` and `DROP INDEX`. This will make it easier to add more types of index recommendations in the future. Release note: None 73572: bazel: perform staticcheck checks in `nogo` r=rail a=rickystewart 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: #73568 Closes #68496. Release note: None 73613: bazel: add stylecheck analyzers to nogo r=rail a=rickystewart 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 Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Steven Danna <[email protected]> Co-authored-by: Neha George <[email protected]> Co-authored-by: Ricky Stewart <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I wonder if we don't run these checks on the codebase? Because these checks trigger a lot of errors when run in
nogo.Epic CRDB-8036
The text was updated successfully, but these errors were encountered: