Skip to content

Commit

Permalink
lint: fix TestForbiddenImports
Browse files Browse the repository at this point in the history
We need to set `Dir` in the `packages.Load()` call or else the whole
thing doesn't work. It would be really nice if it threw an error instead
of simply doing nothing, but it is what it is. To guard against this in
the future I added an error if the list is empty.

Fix the test then all existing violations, except for `raftlogger` which
will need some extra TLC by the owning team (see cockroachdb#132262)

Closes cockroachdb#132258

Epic: none
Release note: None
  • Loading branch information
rickystewart committed Oct 9, 2024
1 parent e813e2f commit f6af2e6
Show file tree
Hide file tree
Showing 22 changed files with 28 additions and 19 deletions.
1 change: 0 additions & 1 deletion pkg/ccl/crosscluster/logical/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ go_library(
"//pkg/util/timeutil",
"//pkg/util/tracing",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_errors//issuelink",
"@com_github_cockroachdb_logtags//:logtags",
"@com_github_cockroachdb_redact//:redact",
"@com_github_lib_pq//oid",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/errors/issuelink"
)

func init() {
Expand Down Expand Up @@ -94,7 +93,7 @@ func createLogicalReplicationStreamPlanHook(
}

if stmt.From.Database != "" {
return errors.UnimplementedErrorf(issuelink.IssueLink{}, "logical replication streams on databases are unsupported")
return errors.UnimplementedErrorf(errors.IssueLink{}, "logical replication streams on databases are unsupported")
}
if len(stmt.From.Tables) != len(stmt.Into.Tables) {
return pgerror.New(pgcode.InvalidParameterValue, "the same number of source and destination tables must be specified")
Expand Down
1 change: 0 additions & 1 deletion pkg/ccl/crosscluster/streamclient/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ go_library(
"@com_github_golang_snappy//:snappy",
"@com_github_jackc_pgconn//:pgconn",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_pkg_errors//:errors",
],
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/crosscluster/streamclient/client_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/golang/snappy"
"github.com/jackc/pgx/v4"
"github.com/pkg/errors"
"github.com/cockroachdb/errors"
)

func subscribeInternal(
Expand Down
1 change: 1 addition & 0 deletions pkg/raft/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ go_library(
"//pkg/raft/raftstoreliveness",
"//pkg/raft/tracker",
"//pkg/util/hlc",
"@com_github_cockroachdb_errors//:errors",
"@org_golang_x_exp//maps",
],
)
Expand Down
3 changes: 1 addition & 2 deletions pkg/raft/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
package raft

import (
"errors"

"github.com/cockroachdb/errors"
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
)

Expand Down
2 changes: 2 additions & 0 deletions pkg/raft/confchange/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ go_library(
"//pkg/raft/quorum",
"//pkg/raft/raftpb",
"//pkg/raft/tracker",
"@com_github_cockroachdb_errors//:errors",
],
)

Expand All @@ -29,5 +30,6 @@ go_test(
"//pkg/raft/raftpb",
"//pkg/raft/tracker",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_cockroachdb_errors//:errors",
],
)
2 changes: 1 addition & 1 deletion pkg/raft/confchange/confchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
package confchange

import (
"errors"
"fmt"
"strings"

"github.com/cockroachdb/cockroach/pkg/raft/quorum"
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/raft/tracker"
"github.com/cockroachdb/errors"
)

// Changer facilitates configuration changes. It exposes methods to handle
Expand Down
1 change: 1 addition & 0 deletions pkg/raft/confchange/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/raft/tracker"
"github.com/cockroachdb/datadriven"
"github.com/cockroachdb/errors"
)

func TestConfChangeDataDriven(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import (
"bytes"
"context"
"crypto/rand"
"errors"
"fmt"
"math"
"math/big"
"slices"
"strings"
"sync"

"github.com/cockroachdb/errors"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/raft/confchange"
"github.com/cockroachdb/cockroach/pkg/raft/quorum"
Expand Down
1 change: 1 addition & 0 deletions pkg/raft/rafttest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ go_library(
"//pkg/roachpb",
"//pkg/settings/cluster",
"//pkg/util/hlc",
"//pkg/util/log",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_cockroachdb_errors//:errors",
"@com_github_stretchr_testify//require",
Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/rafttest/interaction_env_handler_add_nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package rafttest

import (
"context"
"errors"
"fmt"
"reflect"
"testing"
Expand All @@ -30,6 +29,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/raft/raftstoreliveness"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/datadriven"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
package rafttest

import (
"errors"
"fmt"
"testing"

"github.com/cockroachdb/cockroach/pkg/raft"
"github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/datadriven"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
package rafttest

import (
"errors"
"testing"

"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/datadriven"
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/rafttest/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package rafttest

import (
"log"
"math/rand"
"sync"
"time"
Expand All @@ -27,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/raft/raftstoreliveness"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/util/log"
)

type node struct {
Expand Down
3 changes: 1 addition & 2 deletions pkg/raft/rawnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
package raft

import (
"errors"

"github.com/cockroachdb/errors"
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/raft/tracker"
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
package raft

import (
"errors"
"sync"

"github.com/cockroachdb/cockroach/pkg/raft/raftlogger"
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/errors"
)

// ErrCompacted is returned by Storage.Entries/Compact when a requested
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/tablemetadatacache/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ go_test(
"//pkg/util/log",
"//pkg/util/timeutil",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_cockroachdb_errors//:errors",
"@com_github_stretchr_testify//require",
],
)
2 changes: 1 addition & 1 deletion pkg/sql/tablemetadatacache/cluster_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
package tablemetadatacache

import (
"errors"
"time"

"github.com/cockroachdb/errors"
"github.com/cockroachdb/cockroach/pkg/settings"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ package tablemetadatacache

import (
"context"
"errors"
"fmt"
"time"

"github.com/cockroachdb/cockroach/pkg/sql/isql"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/errors"
)

const (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package tablemetadatacache

import (
"context"
"errors"
"fmt"
"strings"
"testing"
Expand All @@ -24,6 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
"github.com/stretchr/testify/require"
)

Expand Down
8 changes: 8 additions & 0 deletions pkg/testutils/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1865,12 +1865,16 @@ func TestLint(t *testing.T) {
pkgs, err := packages.Load(
&packages.Config{
Mode: packages.NeedImports | packages.NeedName,
Dir: crdbDir,
},
pkgPath,
)
if err != nil {
return errors.Wrapf(err, "error loading package %s", pkgPath)
}
if len(pkgs) == 0 {
return errors.Newf("could not list packages under %s", pkgPath)
}
for _, pkg := range pkgs {
for _, s := range pkg.Imports {
arg.Out <- pkg.PkgPath + ": " + s.PkgPath
Expand All @@ -1890,6 +1894,7 @@ func TestLint(t *testing.T) {
stream.GrepNot(`cockroach/pkg/util/sysutil: syscall$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/build/bazel/util/tinystringer: errors$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/build/engflow: github\.com/golang/protobuf/proto$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/build/engflow: log$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/util/grpcutil: github\.com/cockroachdb\/errors\/errbase$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/util/future: github\.com/cockroachdb\/errors\/errbase$`),
stream.GrepNot(`cockroach/pkg/roachprod/install: syscall$`), // TODO: switch to sysutil
Expand All @@ -1903,6 +1908,9 @@ func TestLint(t *testing.T) {
stream.GrepNot(`cockroachdb/cockroach/pkg/kv/kvpb/gen: log$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/util/log/gen: log$`),
stream.GrepNot(`cockroach/pkg/util/uuid: github\.com/satori/go\.uuid$`),
// See #132262.
stream.GrepNot(`github.com/cockroachdb/cockroach/pkg/raft/raftlogger: log$`),
stream.GrepNot(`github.com/cockroachdb/cockroach/pkg/workload/debug: log$`),
), func(s string) {
pkgStr := strings.Split(s, ": ")
importingPkg, importedPkg := pkgStr[0], pkgStr[1]
Expand Down

0 comments on commit f6af2e6

Please sign in to comment.