From c7c1011a5c6eb37694a3907b252f80d8f18b77a9 Mon Sep 17 00:00:00 2001 From: "Ian H. Pittwood" Date: Wed, 5 Feb 2020 16:26:19 -0600 Subject: [PATCH] Fix various code style issues Fixes possible name collisions between variable names and package names Remove redundant import naming Use nil slices for slice declarations instead of empty slices Use make for slices of fixed lengths Remove redundant parentheses Replace deprecated `SetOutput` method with `SetOut` Fix swapped actual/expected arguments on assertEqualGolden Change-Id: Ia39ef44372c3e44948e5440575125bdb470898df --- cmd/bootstrap/bootstrap_remotedirect.go | 2 +- cmd/completion/completion.go | 2 +- cmd/config/config.go | 6 +-- cmd/config/get_authinfo.go | 6 +-- cmd/config/get_cluster.go | 2 +- cmd/config/get_context.go | 2 +- cmd/config/get_context_test.go | 4 +- cmd/config/init.go | 2 +- cmd/config/set_authinfo_test.go | 2 +- cmd/config/set_cluster.go | 4 +- cmd/config/set_cluster_test.go | 2 +- cmd/config/set_context.go | 4 +- cmd/config/set_context_test.go | 2 +- cmd/root.go | 2 +- pkg/config/config.go | 62 ++++++++++++------------- pkg/config/test_utils.go | 4 +- pkg/document/bundle.go | 14 +++--- pkg/document/document_test.go | 2 +- pkg/remote/redfish/redfish.go | 4 +- pkg/remote/redfish/utils.go | 4 +- pkg/remote/redfish/utils_test.go | 2 +- pkg/remote/remote_direct.go | 2 +- pkg/remote/remote_direct_test.go | 2 +- testutil/utilities.go | 4 +- 24 files changed, 71 insertions(+), 71 deletions(-) diff --git a/cmd/bootstrap/bootstrap_remotedirect.go b/cmd/bootstrap/bootstrap_remotedirect.go index 67fb28fc9..690643ed0 100644 --- a/cmd/bootstrap/bootstrap_remotedirect.go +++ b/cmd/bootstrap/bootstrap_remotedirect.go @@ -5,7 +5,7 @@ import ( "opendev.org/airship/airshipctl/pkg/environment" alog "opendev.org/airship/airshipctl/pkg/log" - remote "opendev.org/airship/airshipctl/pkg/remote" + "opendev.org/airship/airshipctl/pkg/remote" ) // RemoteDirect settings for remotedirect command diff --git a/cmd/completion/completion.go b/cmd/completion/completion.go index 601d6b7a8..bff609942 100644 --- a/cmd/completion/completion.go +++ b/cmd/completion/completion.go @@ -27,7 +27,7 @@ var ( ) func NewCompletionCommand() *cobra.Command { - shells := []string{} + shells := make([]string, 0, len(completionShells)) for s := range completionShells { shells = append(shells, s) } diff --git a/cmd/config/config.go b/cmd/config/config.go index b7d136692..6f6b4cbd0 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -11,9 +11,9 @@ func NewConfigCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Comma configRootCmd := &cobra.Command{ Use: "config", DisableFlagsInUseLine: true, - Short: ("Modify airshipctl config files"), - Long: (`Modify airshipctl config files using subcommands -like "airshipctl config set-context --current-context my-context" `), + Short: "Modify airshipctl config files", + Long: `Modify airshipctl config files using subcommands +like "airshipctl config set-context --current-context my-context" `, } configRootCmd.AddCommand(NewCmdConfigSetCluster(rootSettings)) configRootCmd.AddCommand(NewCmdConfigGetCluster(rootSettings)) diff --git a/cmd/config/get_authinfo.go b/cmd/config/get_authinfo.go index 4948feb68..826b07cd9 100644 --- a/cmd/config/get_authinfo.go +++ b/cmd/config/get_authinfo.go @@ -27,13 +27,13 @@ import ( ) var ( - getAuthInfoLong = (`Display a specific user information, or all defined users if no name is provided`) + getAuthInfoLong = `Display a specific user information, or all defined users if no name is provided` - getAuthInfoExample = (`# List all the users airshipctl knows about + getAuthInfoExample = `# List all the users airshipctl knows about airshipctl config get-credential # Display a specific user information -airshipctl config get-credential e2e`) +airshipctl config get-credential e2e` ) // An AuthInfo refers to a particular user for a cluster diff --git a/cmd/config/get_cluster.go b/cmd/config/get_cluster.go index 5e00d6443..4a153fba2 100644 --- a/cmd/config/get_cluster.go +++ b/cmd/config/get_cluster.go @@ -27,7 +27,7 @@ import ( ) var ( - getClusterLong = (`Display a specific cluster or all defined clusters if no name is provided`) + getClusterLong = "Display a specific cluster or all defined clusters if no name is provided" getClusterExample = fmt.Sprintf(` # List all the clusters airshipctl knows about diff --git a/cmd/config/get_context.go b/cmd/config/get_context.go index cb902e363..6c5318700 100644 --- a/cmd/config/get_context.go +++ b/cmd/config/get_context.go @@ -27,7 +27,7 @@ import ( ) var ( - getContextLong = (`Display a specific context, the current-context or all defined contexts if no name is provided`) + getContextLong = "Display a specific context, the current-context or all defined contexts if no name is provided" getContextExample = fmt.Sprintf(`# List all the contexts airshipctl knows about airshipctl config get-context diff --git a/cmd/config/get_context_test.go b/cmd/config/get_context_test.go index 7db2d1230..0ba951c78 100644 --- a/cmd/config/get_context_test.go +++ b/cmd/config/get_context_test.go @@ -72,8 +72,8 @@ func TestGetContextCmd(t *testing.T) { Name: "missing", CmdLine: fmt.Sprintf("%s", missingContext), Cmd: cmd.NewCmdConfigGetContext(settings), - Error: fmt.Errorf("Context %s information was not "+ - "found in the configuration.", missingContext), + Error: fmt.Errorf(`Context %s information was not +found in the configuration.`, missingContext), }, { Name: "get-current-context", diff --git a/cmd/config/init.go b/cmd/config/init.go index 9cd437343..f381c3579 100644 --- a/cmd/config/init.go +++ b/cmd/config/init.go @@ -24,7 +24,7 @@ import ( ) var ( - configInitLong = (`Generate initial configuration files for airshipctl`) + configInitLong = "Generate initial configuration files for airshipctl" ) // NewCmdConfigInit returns a Command instance for 'config init' sub command diff --git a/cmd/config/set_authinfo_test.go b/cmd/config/set_authinfo_test.go index 457832dba..99d2a1748 100644 --- a/cmd/config/set_authinfo_test.go +++ b/cmd/config/set_authinfo_test.go @@ -125,7 +125,7 @@ func (test setAuthInfoTest) run(t *testing.T) { buf := bytes.NewBuffer([]byte{}) cmd := NewCmdConfigSetAuthInfo(settings) - cmd.SetOutput(buf) + cmd.SetOut(buf) cmd.SetArgs(test.args) err := cmd.Flags().Parse(test.flags) require.NoErrorf(t, err, "unexpected error flags args to command: %v, flags: %v", err, test.flags) diff --git a/cmd/config/set_cluster.go b/cmd/config/set_cluster.go index cd5fa91cd..35efc874e 100644 --- a/cmd/config/set_cluster.go +++ b/cmd/config/set_cluster.go @@ -28,9 +28,9 @@ import ( ) var ( - setClusterLong = (` + setClusterLong = ` Sets a cluster entry in arshipctl config. -Specifying a name that already exists will merge new fields on top of existing values for those fields.`) +Specifying a name that already exists will merge new fields on top of existing values for those fields.` setClusterExample = fmt.Sprintf(` # Set only the server field on the e2e cluster entry without touching other values. diff --git a/cmd/config/set_cluster_test.go b/cmd/config/set_cluster_test.go index 62efb3035..0d53240f4 100644 --- a/cmd/config/set_cluster_test.go +++ b/cmd/config/set_cluster_test.go @@ -194,7 +194,7 @@ func (test setClusterTest) run(t *testing.T) { buf := bytes.NewBuffer([]byte{}) cmd := NewCmdConfigSetCluster(settings) - cmd.SetOutput(buf) + cmd.SetOut(buf) cmd.SetArgs(test.args) err := cmd.Flags().Parse(test.flags) require.NoErrorf(t, err, "unexpected error flags args to command: %v, flags: %v", err, test.flags) diff --git a/cmd/config/set_context.go b/cmd/config/set_context.go index bab38808c..ddca0aa5b 100644 --- a/cmd/config/set_context.go +++ b/cmd/config/set_context.go @@ -27,9 +27,9 @@ import ( ) var ( - setContextLong = (` + setContextLong = ` Sets a context entry in arshipctl config. -Specifying a name that already exists will merge new fields on top of existing values for those fields.`) +Specifying a name that already exists will merge new fields on top of existing values for those fields.` setContextExample = fmt.Sprintf(` # Create a completely new e2e context entry diff --git a/cmd/config/set_context_test.go b/cmd/config/set_context_test.go index 012e314d3..d7ba27e36 100644 --- a/cmd/config/set_context_test.go +++ b/cmd/config/set_context_test.go @@ -154,7 +154,7 @@ func (test setContextTest) run(t *testing.T) { buf := bytes.NewBuffer([]byte{}) cmd := NewCmdConfigSetContext(settings) - cmd.SetOutput(buf) + cmd.SetOut(buf) cmd.SetArgs(test.args) err := cmd.Flags().Parse(test.flags) require.NoErrorf(t, err, "unexpected error flags args to command: %v, flags: %v", err, test.flags) diff --git a/cmd/root.go b/cmd/root.go index 4e0d0f137..83b55a0e2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -43,7 +43,7 @@ func NewRootCmd(out io.Writer) (*cobra.Command, *environment.AirshipCTLSettings, settings.InitConfig() }, } - rootCmd.SetOutput(out) + rootCmd.SetOut(out) rootCmd.AddCommand(NewVersionCommand()) settings.InitFlags(rootCmd) diff --git a/pkg/config/config.go b/pkg/config/config.go index f3eb315a2..6396647f4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -367,12 +367,12 @@ func (c *Config) PersistConfig() error { } func (c *Config) String() string { - yaml, err := c.ToYaml() + yamlData, err := c.ToYaml() // This is hiding the error perhaps if err != nil { return "" } - return string(yaml) + return string(yamlData) } func (c *Config) ToYaml() ([]byte, error) { @@ -398,18 +398,18 @@ func (c *Config) KubeConfig() *kubeconfig.Config { } func (c *Config) ClusterNames() []string { - names := []string{} - for k := range c.Clusters { - names = append(names, k) + names := make([]string, 0, len(c.Clusters)) + for c := range c.Clusters { + names = append(names, c) } sort.Strings(names) return names } func (c *Config) ContextNames() []string { - names := []string{} - for k := range c.Contexts { - names = append(names, k) + names := make([]string, 0, len(c.Contexts)) + for c := range c.Contexts { + names = append(names, c) } sort.Strings(names) return names @@ -498,7 +498,7 @@ func (c *Config) ModifyCluster(cluster *Cluster, theCluster *ClusterOptions) (*C } func (c *Config) GetClusters() ([]*Cluster, error) { - clusters := []*Cluster{} + clusters := make([]*Cluster, 0, len(c.ClusterNames())) for _, cName := range c.ClusterNames() { for _, ctName := range AllClusterTypes { cluster, err := c.GetCluster(cName, ctName) @@ -524,7 +524,7 @@ func (c *Config) GetContext(cName string) (*Context, error) { } func (c *Config) GetContexts() ([]*Context, error) { - contexts := []*Context{} + contexts := make([]*Context, 0, len(c.ContextNames())) // Given that we change the testing metholdogy // The ordered names are no longer required for _, cName := range c.ContextNames() { @@ -630,7 +630,7 @@ func (c *Config) GetAuthInfo(aiName string) (*AuthInfo, error) { } func (c *Config) GetAuthInfos() ([]*AuthInfo, error) { - authinfos := []*AuthInfo{} + authinfos := make([]*AuthInfo, 0, len(c.AuthInfos)) for cName := range c.AuthInfos { authinfo, err := c.GetAuthInfo(cName) if err == nil { @@ -825,11 +825,11 @@ func (m *Manifest) Equal(n *Manifest) bool { } func (m *Manifest) String() string { - yaml, err := yaml.Marshal(&m) + yamlData, err := yaml.Marshal(&m) if err != nil { return "" } - return string(yaml) + return string(yamlData) } // Repository functions @@ -839,21 +839,21 @@ func (r *Repository) Equal(s *Repository) bool { } var urlMatches bool if r.Url != nil && s.Url != nil { - urlMatches = (r.Url.String() == s.Url.String()) + urlMatches = r.Url.String() == s.Url.String() } else { // this catches cases where one or both are nil - urlMatches = (r.Url == s.Url) + urlMatches = r.Url == s.Url } return urlMatches && r.Username == s.Username && r.TargetPath == s.TargetPath } func (r *Repository) String() string { - yaml, err := yaml.Marshal(&r) + yamlData, err := yaml.Marshal(&r) if err != nil { return "" } - return string(yaml) + return string(yamlData) } // Modules functions @@ -864,11 +864,11 @@ func (m *Modules) Equal(n *Modules) bool { return reflect.DeepEqual(m.BootstrapInfo, n.BootstrapInfo) } func (m *Modules) String() string { - yaml, err := yaml.Marshal(&m) + yamlData, err := yaml.Marshal(&m) if err != nil { return "" } - return string(yaml) + return string(yamlData) } // Bootstrap functions @@ -882,11 +882,11 @@ func (b *Bootstrap) Equal(c *Bootstrap) bool { } func (b *Bootstrap) String() string { - yaml, err := yaml.Marshal(&b) + yamlData, err := yaml.Marshal(&b) if err != nil { return "" } - return string(yaml) + return string(yamlData) } // Container functions @@ -900,11 +900,11 @@ func (c *Container) Equal(d *Container) bool { } func (c *Container) String() string { - yaml, err := yaml.Marshal(&c) + yamlData, err := yaml.Marshal(&c) if err != nil { return "" } - return string(yaml) + return string(yamlData) } // Builder functions @@ -918,11 +918,11 @@ func (b *Builder) Equal(c *Builder) bool { } func (b *Builder) String() string { - yaml, err := yaml.Marshal(&b) + yamlData, err := yaml.Marshal(&b) if err != nil { return "" } - return string(yaml) + return string(yamlData) } // ClusterComplexName functions @@ -980,26 +980,26 @@ HAS SOMETHING LIKE THIS */ func KClusterString(kCluster *kubeconfig.Cluster) string { - yaml, err := yaml.Marshal(&kCluster) + yamlData, err := yaml.Marshal(&kCluster) if err != nil { return "" } - return string(yaml) + return string(yamlData) } func KContextString(kContext *kubeconfig.Context) string { - yaml, err := yaml.Marshal(&kContext) + yamlData, err := yaml.Marshal(&kContext) if err != nil { return "" } - return string(yaml) + return string(yamlData) } func KAuthInfoString(kAuthInfo *kubeconfig.AuthInfo) string { - yaml, err := yaml.Marshal(&kAuthInfo) + yamlData, err := yaml.Marshal(&kAuthInfo) if err != nil { return "" } - return string(yaml) + return string(yamlData) } diff --git a/pkg/config/test_utils.go b/pkg/config/test_utils.go index 3b1463d6c..a285d94e8 100644 --- a/pkg/config/test_utils.go +++ b/pkg/config/test_utils.go @@ -95,9 +95,9 @@ func DummyManifest() *Manifest { func DummyRepository() *Repository { // TODO(howell): handle this error //nolint: errcheck - url, _ := url.Parse("http://dummy.url.com") + parsedUrl, _ := url.Parse("http://dummy.url.com") return &Repository{ - Url: url, + Url: parsedUrl, Username: "dummy_user", TargetPath: "dummy_targetpath", } diff --git a/pkg/document/bundle.go b/pkg/document/bundle.go index a6aa4557a..011c15d3a 100644 --- a/pkg/document/bundle.go +++ b/pkg/document/bundle.go @@ -155,14 +155,14 @@ func (b *BundleFactory) GetFileSystem() fs.FileSystem { // GetAllDocuments returns all documents in this bundle func (b *BundleFactory) GetAllDocuments() ([]Document, error) { - docSet := []Document{} - for _, res := range b.ResMap.Resources() { + docSet := make([]Document, len(b.ResMap.Resources())) + for i, res := range b.ResMap.Resources() { // Construct Bundle document for each resource returned doc, err := NewDocument(res) if err != nil { return docSet, err } - docSet = append(docSet, doc) + docSet[i] = doc } return docSet, nil } @@ -170,7 +170,7 @@ func (b *BundleFactory) GetAllDocuments() ([]Document, error) { // GetByName finds a document by name, error if more than one document found // or if no documents found func (b *BundleFactory) GetByName(name string) (Document, error) { - resSet := []*resource.Resource{} + resSet := make([]*resource.Resource, 0, len(b.ResMap.Resources())) for _, res := range b.ResMap.Resources() { if res.GetName() == name { resSet = append(resSet, res) @@ -198,14 +198,14 @@ func (b *BundleFactory) Select(selector types.Selector) ([]Document, error) { } // Construct Bundle document for each resource returned - docSet := []Document{} - for _, res := range resources { + docSet := make([]Document, len(resources)) + for i, res := range resources { var doc Document doc, err = NewDocument(res) if err != nil { return docSet, err } - docSet = append(docSet, doc) + docSet[i] = doc } return docSet, err } diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 83960cb25..aaf242be5 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -30,7 +30,7 @@ func TestDocument(t *testing.T) { docs, err := bundle.GetAllDocuments() require.NoError(err, "Unexpected error trying to GetAllDocuments") - nameList := []string{} + nameList := make([]string, 0, len(docs)) for _, doc := range docs { nameList = append(nameList, doc.GetName()) diff --git a/pkg/remote/redfish/redfish.go b/pkg/remote/redfish/redfish.go index cb2609cc6..31357a979 100644 --- a/pkg/remote/redfish/redfish.go +++ b/pkg/remote/redfish/redfish.go @@ -105,14 +105,14 @@ func NewRedfishRemoteDirectClient(ctx context.Context, var api redfishApi.RedfishAPI = redfishClient.NewAPIClient(cfg).DefaultApi - url, err := url.Parse(remoteURL) + parsedUrl, err := url.Parse(remoteURL) if err != nil { return RedfishRemoteDirect{}, NewRedfishConfigErrorf("Invalid URL format: %v", err) } client := RedfishRemoteDirect{ Context: ctx, - RemoteURL: *url, + RemoteURL: *parsedUrl, EphemeralNodeId: ephNodeID, IsoPath: isoPath, Api: api, diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index b3375c1c9..967cdecc7 100644 --- a/pkg/remote/redfish/utils.go +++ b/pkg/remote/redfish/utils.go @@ -25,8 +25,8 @@ func GetResourceIDFromURL(refURL string) string { log.Fatal(err) } - url := strings.TrimSuffix(u.Path, "/") - elems := strings.Split(url, "/") + trimmedUrl := strings.TrimSuffix(u.Path, "/") + elems := strings.Split(trimmedUrl, "/") id := elems[len(elems)-1] return id diff --git a/pkg/remote/redfish/utils_test.go b/pkg/remote/redfish/utils_test.go index 10d817513..c204e8caa 100644 --- a/pkg/remote/redfish/utils_test.go +++ b/pkg/remote/redfish/utils_test.go @@ -85,7 +85,7 @@ func TestRedfishUtilIsIdInList(t *testing.T) { {OdataId: "/path/to/id/3"}, {OdataId: "/path/to/id/4"}, } - emptyList := []redfishClient.IdRef{} + var emptyList []redfishClient.IdRef res := IsIDInList(idList, "1") assert.True(t, res) diff --git a/pkg/remote/remote_direct.go b/pkg/remote/remote_direct.go index cbf155ecb..876b7226e 100644 --- a/pkg/remote/remote_direct.go +++ b/pkg/remote/remote_direct.go @@ -4,7 +4,7 @@ import ( "context" alog "opendev.org/airship/airshipctl/pkg/log" - redfish "opendev.org/airship/airshipctl/pkg/remote/redfish" + "opendev.org/airship/airshipctl/pkg/remote/redfish" ) const ( diff --git a/pkg/remote/remote_direct_test.go b/pkg/remote/remote_direct_test.go index 3e5ea137d..0a47c2849 100644 --- a/pkg/remote/remote_direct_test.go +++ b/pkg/remote/remote_direct_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" - redfish "opendev.org/airship/airshipctl/pkg/remote/redfish" + "opendev.org/airship/airshipctl/pkg/remote/redfish" ) func TestUnknownRemoteType(t *testing.T) { diff --git a/testutil/utilities.go b/testutil/utilities.go index a66192964..1bf5a60b7 100644 --- a/testutil/utilities.go +++ b/testutil/utilities.go @@ -48,7 +48,7 @@ func RunTest(t *testing.T, test *CmdTest) { cmd := test.Cmd actual := &bytes.Buffer{} - cmd.SetOutput(actual) + cmd.SetOut(actual) args := strings.Fields(test.CmdLine) cmd.SetArgs(args) @@ -95,7 +95,7 @@ func assertEqualGolden(t *testing.T, test *CmdTest, actual []byte) { goldenFilePath := filepath.Join(goldenDir, test.Name+goldenFileSuffix) golden, err := ioutil.ReadFile(goldenFilePath) require.NoErrorf(t, err, "Failed while reading golden file at %s", goldenFilePath) - assert.Equal(t, string(actual), string(golden)) + assert.Equal(t, string(golden), string(actual)) } func checkError(t *testing.T, actual, expected error) {