Skip to content

Commit

Permalink
Disallow fmt.Errorf usage through code validation (#433)
Browse files Browse the repository at this point in the history
Came across during a code review, new contributors can end up using fmt.Errorf
which does not create stack trace at the error site --and also doesn't preserve
stack trace if used to wrap an error.

Signed-off-by: Ahmet Alp Balkan <[email protected]>
  • Loading branch information
ahmetb authored and k8s-ci-robot committed Dec 11, 2019
1 parent 3b343e8 commit 541b48b
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 2 deletions.
2 changes: 1 addition & 1 deletion cmd/krew/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func preRun(cmd *cobra.Command, _ []string) error {
}
if !isMigrated && cmd.Use != "receipts-upgrade" {
fmt.Fprintln(os.Stderr, "You need to perform a migration to continue using krew.\nPlease run `kubectl krew system receipts-upgrade`")
return fmt.Errorf("krew home outdated")
return errors.New("krew home outdated")
}

if installation.IsWindows() {
Expand Down
2 changes: 1 addition & 1 deletion cmd/validate-krew-manifest/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func validateManifestFile(path string) error {
filename := filepath.Base(path)
manifestExtension := filepath.Ext(filename)
if manifestExtension != constants.ManifestExtension {
return fmt.Errorf("expected manifest extension %q but found %q", constants.ManifestExtension, manifestExtension)
return errors.Errorf("expected manifest extension %q but found %q", constants.ManifestExtension, manifestExtension)
}
pluginNameFromFileName := strings.TrimSuffix(filename, manifestExtension)
klog.V(4).Infof("inferred plugin name as %s", pluginNameFromFileName)
Expand Down
8 changes: 8 additions & 0 deletions hack/verify-code-patterns.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ if [[ -n "$out" ]]; then
exit 1
fi

# Do not use fmt.Errorf as it does not start a stacktrace at error site
out="$(grep --include '*.go' -EIrn 'fmt\.Errorf?' || true)"
if [[ -n "$out" ]]; then
echo >&2 "You used fmt.Errorf; use pkg/errors.Errorf instead to preserve stack traces:"
echo >&2 "$out"
exit 1
fi

# Do not initialize index.{Plugin,Platform} structs in test code.
out="$(grep --include '*_test.go' --exclude-dir 'vendor/' -EIrn '[^]](index\.)(Plugin|Platform){' || true)"
if [[ -n "$out" ]]; then
Expand Down

0 comments on commit 541b48b

Please sign in to comment.