Skip to content

Commit

Permalink
Merge pull request juju#18814 from jack-w-shaw/add_import_analysis
Browse files Browse the repository at this point in the history
juju#18814

In quite a lot of our source code it turns out we import the same package multiple times under different names/aliases.

This is ugly, and can lead to inconsistancy. One part of a file may use one alias, another may use another. It can become unclear exactly what anything is.

Implement a new static analyser which catches any duplicate imports

## QA steps

static analysis passes
  • Loading branch information
jujubot authored Feb 6, 2025
2 parents 1d75bdf + e7aef35 commit 33d88a0
Show file tree
Hide file tree
Showing 161 changed files with 1,016 additions and 1,137 deletions.
9 changes: 7 additions & 2 deletions .github/golangci-lint.config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ linters-settings:
- loopclosure
- lostcancel
- nilfunc
# TODO(4.0): re-enable me
# - printf
# TODO(4.0): re-enable me
# - printf
- shift
- stdmethods
- structtag
Expand All @@ -30,6 +30,10 @@ linters-settings:
- unreachable
- unsafeptr
- unusedresult
gocritic:
disable-all: true
enabled-checks:
- dupImport
revive:
rules:
- name: unexported-naming
Expand All @@ -54,6 +58,7 @@ linters:
enable:
- gci
- govet
- gocritic
- gofmt
- ineffassign
- misspell
Expand Down
9 changes: 4 additions & 5 deletions agent/agentbootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package agentbootstrap

import (
"context"
stdcontext "context"
"fmt"

"github.com/juju/clock"
Expand Down Expand Up @@ -55,7 +54,7 @@ import (
// DqliteInitializerFunc is a function that initializes the dqlite database
// for the controller.
type DqliteInitializerFunc func(
ctx stdcontext.Context,
ctx context.Context,
mgr database.BootstrapNodeManager,
modelUUID model.UUID,
logger logger.Logger,
Expand Down Expand Up @@ -177,7 +176,7 @@ func NewAgentBootstrap(args AgentBootstrapArgs) (*AgentBootstrap, error) {
// Initialize returns the newly initialized state and bootstrap machine.
// If it fails, the state may well be irredeemably compromised.
// TODO (stickupkid): Split this function into testable smaller functions.
func (b *AgentBootstrap) Initialize(ctx stdcontext.Context) (_ *state.Controller, resultErr error) {
func (b *AgentBootstrap) Initialize(ctx context.Context) (_ *state.Controller, resultErr error) {
agentConfig := b.agentConfig
if agentConfig.Tag().Id() != agent.BootstrapControllerId || !coreagent.IsAllowedControllerTag(agentConfig.Tag().Kind()) {
return nil, errors.Errorf("InitializeState not called with bootstrap controller's configuration")
Expand Down Expand Up @@ -418,7 +417,7 @@ func (b *AgentBootstrap) getCloudCredential() (cloud.Credential, names.CloudCred
}

func (b *AgentBootstrap) getEnviron(
ctx stdcontext.Context,
ctx context.Context,
controllerUUID string,
cloudSpec environscloudspec.CloudSpec,
modelConfig *config.Config,
Expand Down Expand Up @@ -499,7 +498,7 @@ func (b *AgentBootstrap) initBootstrapMachine(

// initControllerCloudService creates cloud service for controller service.
func (b *AgentBootstrap) initControllerCloudService(
ctx stdcontext.Context,
ctx context.Context,
cloudSpec environscloudspec.CloudSpec,
provider environs.EnvironProvider,
st *state.State,
Expand Down
2 changes: 1 addition & 1 deletion api/client/charms/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package charms_test

import (
"archive/zip"
context "context"
"context"
"os"
"path/filepath"

Expand Down
3 changes: 1 addition & 2 deletions api/client/charms/localcharmclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@ import (
jujuversion "github.com/juju/juju/core/version"
"github.com/juju/juju/internal/charm"
"github.com/juju/juju/internal/testing"
coretesting "github.com/juju/juju/internal/testing"
"github.com/juju/juju/rpc/params"
"github.com/juju/juju/testcharms"
)

type addCharmSuite struct {
coretesting.BaseSuite
testing.BaseSuite
}

var _ = gc.Suite(&addCharmSuite{})
Expand Down
15 changes: 7 additions & 8 deletions api/clientcredentialsloginprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,30 @@ import (
apiservererrors "github.com/juju/juju/apiserver/errors"
apiservertesting "github.com/juju/juju/apiserver/testing"
jujuhttp "github.com/juju/juju/internal/http"
coretesting "github.com/juju/juju/internal/testing"
jtesting "github.com/juju/juju/internal/testing"
"github.com/juju/juju/internal/testing"
"github.com/juju/juju/rpc/params"
)

type clientCredentialsLoginProviderProviderSuite struct {
coretesting.BaseSuite
testing.BaseSuite
}

var _ = gc.Suite(&clientCredentialsLoginProviderProviderSuite{})

func (s *clientCredentialsLoginProviderProviderSuite) APIInfo() *api.Info {
srv := apiservertesting.NewAPIServer(func(modelUUID string) (interface{}, error) {
var err error
if modelUUID != "" && modelUUID != jtesting.ModelTag.Id() {
if modelUUID != "" && modelUUID != testing.ModelTag.Id() {
err = fmt.Errorf("%w: %q", apiservererrors.UnknownModelError, modelUUID)
}
return &testRootAPI{}, err
})
s.AddCleanup(func(_ *gc.C) { srv.Close() })
info := &api.Info{
Addrs: srv.Addrs,
CACert: jtesting.CACert,
ControllerUUID: jtesting.ControllerTag.Id(),
ModelTag: jtesting.ModelTag,
CACert: testing.CACert,
ControllerUUID: testing.ControllerTag.Id(),
ModelTag: testing.ModelTag,
}
return info
}
Expand Down Expand Up @@ -105,7 +104,7 @@ func (s *clientCredentialsLoginProviderProviderSuite) TestClientCredentialsLogin

// A separate suite for tests that don't need to communicate with a Juju controller.
type clientCredentialsLoginProviderBasicSuite struct {
coretesting.BaseSuite
testing.BaseSuite
}

var _ = gc.Suite(&clientCredentialsLoginProviderBasicSuite{})
Expand Down
19 changes: 9 additions & 10 deletions api/legacyloginprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,13 @@ import (
apiservertesting "github.com/juju/juju/apiserver/testing"
jujuversion "github.com/juju/juju/core/version"
jujuhttp "github.com/juju/juju/internal/http"
coretesting "github.com/juju/juju/internal/testing"
jtesting "github.com/juju/juju/internal/testing"
"github.com/juju/juju/internal/testing"
jujutesting "github.com/juju/juju/juju/testing"
"github.com/juju/juju/rpc/params"
)

type legacyLoginProviderSuite struct {
coretesting.BaseSuite
testing.BaseSuite

mockRootAPI *MockRootAPI
mockAdminAPI *MockAdminAPI
Expand Down Expand Up @@ -60,17 +59,17 @@ func (s *legacyLoginProviderSuite) setupMocks(c *gc.C) *gomock.Controller {
func (s *legacyLoginProviderSuite) APIInfo() *api.Info {
srv := apiservertesting.NewAPIServer(func(modelUUID string) (interface{}, error) {
var err error
if modelUUID != "" && modelUUID != jtesting.ModelTag.Id() {
if modelUUID != "" && modelUUID != testing.ModelTag.Id() {
err = fmt.Errorf("%w: %q", apiservererrors.UnknownModelError, modelUUID)
}
return s.mockRootAPI, err
})
s.AddCleanup(func(_ *gc.C) { srv.Close() })
info := &api.Info{
Addrs: srv.Addrs,
CACert: jtesting.CACert,
ControllerUUID: jtesting.ControllerTag.Id(),
ModelTag: jtesting.ModelTag,
CACert: testing.CACert,
ControllerUUID: testing.ControllerTag.Id(),
ModelTag: testing.ModelTag,
}
return info
}
Expand All @@ -91,8 +90,8 @@ func (s *legacyLoginProviderSuite) TestLegacyProviderLogin(c *gc.C) {
ClientVersion: jujuversion.Current.String(),
})
return params.LoginResult{
ControllerTag: jtesting.ControllerTag.String(),
ModelTag: jtesting.ModelTag.String(),
ControllerTag: testing.ControllerTag.String(),
ModelTag: testing.ModelTag.String(),
Servers: [][]params.HostPort{},
ServerVersion: jujuversion.Current.String(),
PublicDNSName: "somewhere.example.com",
Expand Down Expand Up @@ -151,7 +150,7 @@ func (s *legacyLoginProviderSuite) TestLegacyProviderWithNilTag(c *gc.C) {

// A separate suite for tests that don't need to connect to a controller.
type legacyLoginProviderBasicSuite struct {
coretesting.BaseSuite
testing.BaseSuite
}

var _ = gc.Suite(&legacyLoginProviderBasicSuite{})
Expand Down
15 changes: 7 additions & 8 deletions api/sessiontokenloginprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,31 +20,30 @@ import (
apiservererrors "github.com/juju/juju/apiserver/errors"
apiservertesting "github.com/juju/juju/apiserver/testing"
jujuhttp "github.com/juju/juju/internal/http"
coretesting "github.com/juju/juju/internal/testing"
jtesting "github.com/juju/juju/internal/testing"
"github.com/juju/juju/internal/testing"
"github.com/juju/juju/rpc/params"
)

type sessionTokenLoginProviderProviderSuite struct {
jtesting.BaseSuite
testing.BaseSuite
}

var _ = gc.Suite(&sessionTokenLoginProviderProviderSuite{})

func (s *sessionTokenLoginProviderProviderSuite) APIInfo() *api.Info {
srv := apiservertesting.NewAPIServer(func(modelUUID string) (interface{}, error) {
var err error
if modelUUID != "" && modelUUID != jtesting.ModelTag.Id() {
if modelUUID != "" && modelUUID != testing.ModelTag.Id() {
err = fmt.Errorf("%w: %q", apiservererrors.UnknownModelError, modelUUID)
}
return &testRootAPI{}, err
})
s.AddCleanup(func(_ *gc.C) { srv.Close() })
info := &api.Info{
Addrs: srv.Addrs,
CACert: jtesting.CACert,
ControllerUUID: jtesting.ControllerTag.Id(),
ModelTag: jtesting.ModelTag,
CACert: testing.CACert,
ControllerUUID: testing.ControllerTag.Id(),
ModelTag: testing.ModelTag,
}
return info
}
Expand Down Expand Up @@ -179,7 +178,7 @@ func (s *sessionTokenLoginProviderProviderSuite) TestInvalidSessionTokenLogin(c

// A separate suite for tests that don't need to communicate with a controller.
type sessionTokenLoginProviderBasicSuite struct {
coretesting.BaseSuite
testing.BaseSuite
}

var _ = gc.Suite(&sessionTokenLoginProviderBasicSuite{})
Expand Down
2 changes: 1 addition & 1 deletion apiserver/common/service_mock_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions apiserver/facades/agent/agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ import (
"github.com/juju/juju/core/watcher/watchertest"
ttesting "github.com/juju/juju/internal/testing"
"github.com/juju/juju/juju/testing"
jujutesting "github.com/juju/juju/juju/testing"
"github.com/juju/juju/rpc/params"
"github.com/juju/juju/state"
)

type agentSuite struct {
jujutesting.ApiServerSuite
testing.ApiServerSuite

resources *common.Resources
authorizer apiservertesting.FakeAuthorizer
Expand Down
9 changes: 4 additions & 5 deletions apiserver/facades/agent/caasapplication/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/juju/juju/caas"
_ "github.com/juju/juju/caas/kubernetes/provider"
"github.com/juju/juju/controller"
jujucontroller "github.com/juju/juju/controller"
"github.com/juju/juju/core/network"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/internal/charm"
Expand All @@ -30,7 +29,7 @@ type mockState struct {
app mockApplication
model mockModel
units map[string]*mockUnit
controllerConfig jujucontroller.Config
controllerConfig controller.Config
}

func newMockState() *mockState {
Expand All @@ -39,8 +38,8 @@ func newMockState() *mockState {
controllerTag: names.NewControllerTag("ffffffff-ffff-ffff-ffff-ffffffffffff"),
tag: names.NewModelTag("ffffffff-ffff-ffff-ffff-ffffffffffff"),
},
controllerConfig: jujucontroller.Config{
jujucontroller.CACertKey: jtesting.CACert,
controllerConfig: controller.Config{
controller.CACertKey: jtesting.CACert,
},
}
return st
Expand Down Expand Up @@ -74,7 +73,7 @@ func (st *mockState) Unit(name string) (caasapplication.Unit, error) {
return unit, nil
}

func (st *mockState) ControllerConfig() (jujucontroller.Config, error) {
func (st *mockState) ControllerConfig() (controller.Config, error) {
st.MethodCall(st, "ControllerConfig")
if err := st.NextErr(); err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions apiserver/facades/agent/uniter/goal-state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package uniter_test

import (
stdcontext "context"
"context"
"time"

"github.com/juju/errors"
Expand Down Expand Up @@ -482,7 +482,7 @@ func (s *uniterGoalStateSuite) assertInScope(c *gc.C, relUnit *state.RelationUni
// goalStates call uniter.GoalStates API and compares the output with the
// expected result.
func testGoalStates(c *gc.C, thisUniter *uniter.UniterAPI, args params.Entities, expected params.GoalStateResults) {
result, err := thisUniter.GoalStates(stdcontext.Background(), args)
result, err := thisUniter.GoalStates(context.Background(), args)
c.Assert(err, jc.ErrorIsNil)
for i := range result.Results {
if result.Results[i].Error != nil {
Expand Down
3 changes: 1 addition & 2 deletions apiserver/facades/agent/uniter/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
coreunit "github.com/juju/juju/core/unit"
"github.com/juju/juju/core/watcher"
"github.com/juju/juju/domain/application/charm"
applicationcharm "github.com/juju/juju/domain/application/charm"
"github.com/juju/juju/domain/unitstate"
"github.com/juju/juju/environs/config"
internalcharm "github.com/juju/juju/internal/charm"
Expand Down Expand Up @@ -100,7 +99,7 @@ type ApplicationService interface {
// GetAvailableCharmArchiveSHA256 returns the SHA256 hash of the charm archive
// for the given charm name, source and revision. If the charm is not available,
// [applicationerrors.CharmNotResolved] is returned.
GetAvailableCharmArchiveSHA256(ctx context.Context, locator applicationcharm.CharmLocator) (string, error)
GetAvailableCharmArchiveSHA256(ctx context.Context, locator charm.CharmLocator) (string, error)

// GetCharmLXDProfile returns the LXD profile along with the revision of the
// charm using the charm name, source and revision.
Expand Down
5 changes: 2 additions & 3 deletions apiserver/facades/agent/uniter/uniter.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/juju/juju/core/status"
coreunit "github.com/juju/juju/core/unit"
"github.com/juju/juju/core/watcher"
corewatcher "github.com/juju/juju/core/watcher"
applicationerrors "github.com/juju/juju/domain/application/errors"
machineerrors "github.com/juju/juju/domain/machine/errors"
"github.com/juju/juju/domain/unitstate"
Expand Down Expand Up @@ -2951,7 +2950,7 @@ func (u *UniterAPIv20) Watch(ctx context.Context, args params.Entities) (params.
continue
}

var watcher corewatcher.NotifyWatcher
var watcher watcher.NotifyWatcher
switch tag.(type) {
case names.ApplicationTag:
watcher, err = u.applicationService.WatchApplication(ctx, tag.Id())
Expand All @@ -2971,7 +2970,7 @@ func (u *UniterAPIv20) Watch(ctx context.Context, args params.Entities) (params.
}

// watchUnit returns a state notify watcher for the given unit.
func (u *UniterAPI) watchUnit(tag names.Tag) (corewatcher.NotifyWatcher, error) {
func (u *UniterAPI) watchUnit(tag names.Tag) (watcher.NotifyWatcher, error) {
entity0, err := u.st.FindEntity(tag)
if err != nil {
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions apiserver/facades/client/applicationoffers/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package applicationoffers_test

import (
stdcontext "context"
"context"
"time"

"github.com/go-macaroon-bakery/macaroon-bakery/v3/bakery"
Expand Down Expand Up @@ -340,7 +340,7 @@ type mockBakeryService struct {
caveats map[string][]checkers.Caveat
}

func (s *mockBakeryService) NewMacaroon(ctx stdcontext.Context, version bakery.Version, caveats []checkers.Caveat, ops ...bakery.Op) (*bakery.Macaroon, error) {
func (s *mockBakeryService) NewMacaroon(ctx context.Context, version bakery.Version, caveats []checkers.Caveat, ops ...bakery.Op) (*bakery.Macaroon, error) {
s.MethodCall(s, "NewMacaroon", caveats)
mac, err := macaroon.New(nil, []byte("id"), "", macaroon.LatestVersion)
if err != nil {
Expand All @@ -360,6 +360,6 @@ func (s *mockBakeryService) ExpireStorageAfter(when time.Duration) (authenticati
return s, nil
}

func getFakeControllerInfo(ctx stdcontext.Context) ([]string, string, error) {
func getFakeControllerInfo(ctx context.Context) ([]string, string, error) {
return []string{"192.168.1.1:17070"}, testing.CACert, nil
}
Loading

0 comments on commit 33d88a0

Please sign in to comment.