Enable additional golangci-lint checks

Refactors code to make them pass.

RELEASE_NOTES=n/a

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>

Author: dominikschulz

.golangci.yml

@@ -13,7 +13,7 @@ linters:
         - asciicheck
         - forcetypeassert
         - gci
-        - gocyclo
+        #- gocyclo # need to refactor internal/create/wizard
         - gofmt
         - goimports
         - gomoddirectives
@@ -26,18 +26,9 @@ linters:
         - nolintlint
         - prealloc
         - predeclared
-    ## These are working but disabled for now
-          ##  - dogsled # two occurences we cannot really correct
-          ##  - dupl # some dups in tests
-          ##  - gochecknoinits # we use a lot of init
-          ##  - goconst # annoying?
-          ##  - godot # annoying?
-          ##  - godox # we have a few todo...
-          ##  - nestif # some work to refactor
-          ##  - paralleltest # lots of work to pass
-          ##  - revive # currently a bit buggy with Go 1.18 but works mostly
-          ##  - tagliatelle # requires some refactoring
-          ##  - testpackage # complains for a lot of packages
+        - godot
+        - nestif
+        # - revive # currently a bit buggy with Go 1.18 but works mostly
 
 ## To enable once they work with Go 1.18:
 ###  - deadcode

internal/action/action.go

@@ -17,7 +17,7 @@ var (
 	stdout io.Writer = os.Stdout
 )
 
-// Action knows everything to run gopass CLI actions
+// Action knows everything to run gopass CLI actions.
 type Action struct {
 	Name    string
 	Store   *root.Store
@@ -26,7 +26,7 @@ type Action struct {
 	rem     *reminder.Store
 }
 
-// New returns a new Action wrapper
+// New returns a new Action wrapper.
 func New(cfg *config.Config, sv semver.Version) (*Action, error) {
 	return newAction(cfg, sv, true)
 }
@@ -49,16 +49,16 @@ func newAction(cfg *config.Config, sv semver.Version, remind bool) (*Action, err
 		if err != nil {
 			debug.Log("failed to init reminder: %s", err)
 		} else {
-			// only populate the reminder variable on success, the implementation
-			// can handle being called on a nil pointer
+			// only populate the reminder variable on success, the implementation.
+			// can handle being called on a nil pointer.
 			act.rem = r
 		}
 	}
 
 	return act, nil
 }
 
-// String implement fmt.Stringer
+// String implement fmt.Stringer.
 func (s *Action) String() string {
 	return s.Store.String()
 }

internal/action/aliases.go

@@ -10,7 +10,7 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-// AliasesPrint prints all cofigured aliases
+// AliasesPrint prints all cofigured aliases.
 func (s *Action) AliasesPrint(c *cli.Context) error {
 	out.Printf(c.Context, "Configured aliases:")
 	aliases := pwrules.AllAliases()
@@ -25,7 +25,7 @@ func (s *Action) AliasesPrint(c *cli.Context) error {
 	return nil
 }
 
-// AliasesAdd adds a single alias to a domain
+// AliasesAdd adds a single alias to a domain.
 func (s *Action) AliasesAdd(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	domain := c.Args().First()
@@ -43,7 +43,7 @@ func (s *Action) AliasesAdd(c *cli.Context) error {
 	return nil
 }
 
-// AliasesRemove removes a single alias from a domain
+// AliasesRemove removes a single alias from a domain.
 func (s *Action) AliasesRemove(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	domain := c.Args().First()
@@ -61,7 +61,7 @@ func (s *Action) AliasesRemove(c *cli.Context) error {
 	return nil
 }
 
-// AliasesDelete remove an alias mapping for a domain
+// AliasesDelete remove an alias mapping for a domain.
 func (s *Action) AliasesDelete(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	domain := c.Args().First()

internal/action/audit.go

@@ -9,7 +9,7 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-// Audit validates passwords against common flaws
+// Audit validates passwords against common flaws.
 func (s *Action) Audit(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 

internal/action/binary.go

@@ -23,21 +23,21 @@ var (
 	binstdin = os.Stdin
 )
 
-// Cat prints to or reads from STDIN/STDOUT
+// Cat prints to or reads from STDIN/STDOUT.
 func (s *Action) Cat(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	name := c.Args().First()
 	if name == "" {
 		return ExitError(ExitNoName, nil, "Usage: %s cat <NAME>", c.App.Name)
 	}
 
-	// handle pipe to stdin
+	// handle pipe to stdin.
 	info, err := binstdin.Stat()
 	if err != nil {
 		return ExitError(ExitIO, err, "failed to stat stdin: %s", err)
 	}
 
-	// if content is piped to stdin, read and save it
+	// if content is piped to stdin, read and save it.
 	if info.Mode()&os.ModeCharDevice == 0 {
 		debug.Log("Reading from STDIN ...")
 		content := &bytes.Buffer{}
@@ -78,14 +78,14 @@ func secFromBytes(dst, src string, in []byte) gopass.Secret {
 	return sec
 }
 
-// BinaryCopy copies either from the filesystem to the store or from the store
-// to the filesystem
+// BinaryCopy copies either from the filesystem to the store or from the store.
+// to the filesystem.
 func (s *Action) BinaryCopy(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	from := c.Args().Get(0)
 	to := c.Args().Get(1)
 
-	// argument checking is in s.binaryCopy
+	// argument checking is in s.binaryCopy.
 	if err := s.binaryCopy(ctx, c, from, to, false); err != nil {
 		return ExitError(ExitUnknown, err, "%s", err)
 	}
@@ -94,25 +94,25 @@ func (s *Action) BinaryCopy(c *cli.Context) error {
 
 // BinaryMove works like Copy but will remove (shred/wipe) the source
 // after a successful copy. Mostly useful for securely moving secrets into
-// the store if they are no longer needed / wanted on disk afterwards
+// the store if they are no longer needed / wanted on disk afterwards.
 func (s *Action) BinaryMove(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	from := c.Args().Get(0)
 	to := c.Args().Get(1)
 
-	// argument checking is in s.binaryCopy
+	// argument checking is in s.binaryCopy.
 	if err := s.binaryCopy(ctx, c, from, to, true); err != nil {
 		return ExitError(ExitUnknown, err, "%s", err)
 	}
 	return nil
 }
 
 // binaryCopy implements the control flow for copy and move. We support two
-// workflows:
-// 1. From the filesystem to the store
-// 2. From the store to the filesystem
+// workflows:.
+// 1. From the filesystem to the store.
+// 2. From the store to the filesystem.
 //
-// Copying secrets in the store must be done through the regular copy command
+// Copying secrets in the store must be done through the regular copy command.
 func (s *Action) binaryCopy(ctx context.Context, c *cli.Context, from, to string, deleteSource bool) error {
 	if from == "" || to == "" {
 		op := "copy"
@@ -124,10 +124,10 @@ func (s *Action) binaryCopy(ctx context.Context, c *cli.Context, from, to string
 
 	switch {
 	case fsutil.IsFile(from) && fsutil.IsFile(to):
-		// copying from on file to another file is not supported
+		// copying from on file to another file is not supported.
 		return fmt.Errorf("ambiguity detected. Only from or to can be a file")
 	case s.Store.Exists(ctx, from) && s.Store.Exists(ctx, to):
-		// copying from one secret to another secret is not supported
+		// copying from one secret to another secret is not supported.
 		return fmt.Errorf("ambiguity detected. Either from or to must be a file")
 	case fsutil.IsFile(from) && !fsutil.IsFile(to):
 		return s.binaryCopyFromFileToStore(ctx, from, to, deleteSource)
@@ -139,11 +139,11 @@ func (s *Action) binaryCopy(ctx context.Context, c *cli.Context, from, to string
 }
 
 func (s *Action) binaryCopyFromFileToStore(ctx context.Context, from, to string, deleteSource bool) error {
-	// if the source is a file the destination must no to avoid ambiguities
+	// if the source is a file the destination must not to avoid ambiguities.
 	// if necessary this can be resolved by using a absolute path for the file
-	// and a relative one for the secret
+	// and a relative one for the secret.
 
-	// copy from FS to store
+	// copy from FS to store.
 	buf, err := os.ReadFile(from)
 	if err != nil {
 		return fmt.Errorf("failed to read file from %q: %w", from, err)
@@ -159,7 +159,7 @@ func (s *Action) binaryCopyFromFileToStore(ctx context.Context, from, to string,
 	}
 
 	// it's important that we return if the validation fails, because
-	// in that case we don't want to shred our (only) copy of this data!
+	// in that case we don't want to shred our (only) copy of this data!.
 	if err := s.binaryValidate(ctx, buf, to); err != nil {
 		return fmt.Errorf("failed to validate written data: %w", err)
 	}
@@ -171,9 +171,9 @@ func (s *Action) binaryCopyFromFileToStore(ctx context.Context, from, to string,
 
 func (s *Action) binaryCopyFromStoreToFile(ctx context.Context, from, to string, deleteSource bool) error {
 	// if the source is no file we assume it's a secret and to is a filename
-	// (which may already exist or not)
+	// (which may already exist or not).
 
-	// copy from store to FS
+	// copy from store to FS.
 	buf, err := s.binaryGet(ctx, from)
 	if err != nil {
 		return fmt.Errorf("failed to read data from %q: %w", from, err)
@@ -187,7 +187,7 @@ func (s *Action) binaryCopyFromStoreToFile(ctx context.Context, from, to string,
 	}
 
 	// as before: if validation of the written data fails, we MUST NOT
-	// delete the (only) source
+	// delete the (only) source.
 	if err := s.binaryValidate(ctx, buf, from); err != nil {
 		return fmt.Errorf("failed to validate the written data: %w", err)
 	}
@@ -238,7 +238,7 @@ func (s *Action) binaryGet(ctx context.Context, name string) ([]byte, error) {
 	return buf, nil
 }
 
-// Sum decodes binary content and computes the SHA256 checksum
+// Sum decodes binary content and computes the SHA256 checksum.
 func (s *Action) Sum(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	name := c.Args().First()

internal/action/clone.go

@@ -16,7 +16,7 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-// Clone will fetch and mount a new password store from a git repo
+// Clone will fetch and mount a new password store from a git repo.
 func (s *Action) Clone(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	if c.IsSet("crypto") {
@@ -28,7 +28,7 @@ func (s *Action) Clone(c *cli.Context) error {
 		return ExitError(ExitUsage, nil, "Usage: %s clone repo [mount]", s.Name)
 	}
 
-	// gopass clone [--crypto=foo] [--path=/some/store] git://foo/bar team0
+	// gopass clone [--crypto=foo] [--path=/some/store] git://foo/bar team0.
 	repo := c.Args().Get(0)
 	mount := ""
 	if c.Args().Len() > 1 {
@@ -63,40 +63,40 @@ func (s *Action) clone(ctx context.Context, repo, mount, path string) error {
 		return ExitError(ExitAlreadyInitialized, nil, "Can not clone %s to the root store, as this store is already initialized. Please try cloning to a submount: `%s clone %s sub`", repo, s.Name, repo)
 	}
 
-	// make sure the parent directory exists
+	// make sure the parent directory exists.
 	if parentPath := filepath.Dir(path); !fsutil.IsDir(parentPath) {
 		if err := os.MkdirAll(parentPath, 0700); err != nil {
 			return ExitError(ExitUnknown, err, "Failed to create parent directory for clone: %s", err)
 		}
 	}
 
-	// clone repo
+	// clone repo.
 	out.Noticef(ctx, "Cloning git repository %q to %q ...", repo, path)
 	if _, err := backend.Clone(ctx, storageBackendOrDefault(ctx), repo, path); err != nil {
 		return ExitError(ExitGit, err, "failed to clone repo %q to %q: %s", repo, path, err)
 	}
 
-	// add mount
+	// add mount.
 	debug.Log("Mounting cloned repo %q at %q", path, mount)
 	if err := s.cloneAddMount(ctx, mount, path); err != nil {
 		return err
 	}
 
-	// save new mount in config file
+	// save new mount in config file.
 	if err := s.cfg.Save(); err != nil {
 		return ExitError(ExitIO, err, "Failed to update config: %s", err)
 	}
 
-	// try to init git config
+	// try to init git config.
 	out.Notice(ctx, "Configuring git repository ...")
 
-	// ask for git config values
+	// ask for git config values.
 	username, email, err := s.cloneGetGitConfig(ctx, mount)
 	if err != nil {
 		return err
 	}
 
-	// initialize git config
+	// initialize git config.
 	if err := s.Store.RCSInitConfig(ctx, mount, username, email); err != nil {
 		debug.Log("Stacktrace: %+v\n", err)
 		out.Errorf(ctx, "Failed to configure git: %s", err)
@@ -131,8 +131,8 @@ func (s *Action) cloneAddMount(ctx context.Context, mount, path string) error {
 
 func (s *Action) cloneGetGitConfig(ctx context.Context, name string) (string, string, error) {
 	out.Printf(ctx, "🎩 Gathering information for the git repository ...")
-	// for convenience, set defaults to user-selected values from available private keys
-	// NB: discarding returned error since this is merely a best-effort look-up for convenience
+	// for convenience, set defaults to user-selected values from available private keys.
+	// NB: discarding returned error since this is merely a best-effort look-up for convenience.
 	username, email, _ := cui.AskForGitConfigUser(ctx, s.Store.Crypto(ctx, name))
 	if username == "" {
 		username = termio.DetectName(ctx, nil)

internal/action/clone_test.go

@@ -19,7 +19,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// aGitRepo creates and initializes a small git repo
+// aGitRepo creates and initializes a small git repo.
 func aGitRepo(ctx context.Context, u *gptest.Unit, t *testing.T, name string) string {
 	gd := filepath.Join(u.Dir, name)
 	assert.NoError(t, os.MkdirAll(gd, 0700))

internal/action/commands.go

@@ -52,7 +52,7 @@ func ShowFlags() []cli.Flag {
 	}
 }
 
-// GetCommands returns the cli commands exported by this module
+// GetCommands returns the cli commands exported by this module.
 func (s *Action) GetCommands() []*cli.Command {
 	return []*cli.Command{
 		{