Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable errcheck everywhere and fix or silent remaining issues #1987

Merged
merged 30 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b63be2d
all
denik Dec 8, 2024
f94b50d
Enable errcheck everywhere
denik Dec 9, 2024
df67c28
silent errcheck
denik Dec 9, 2024
d0af547
silent more
denik Dec 9, 2024
d075b6c
clean up
denik Dec 9, 2024
583f6f2
clean up
denik Dec 9, 2024
9a41b65
refactor: Ignore error on MarkHidden for verbose flag
denik Dec 9, 2024
1c8db2f
refactor: Ignore errors from MarkHidden flag calls
denik Dec 9, 2024
8cd405e
fmt
denik Dec 9, 2024
b34a1e8
ignore error from MarkDeprecated
denik Dec 9, 2024
5f12b47
ignore errors from RegisterCompletionFunc
denik Dec 9, 2024
1fbe51d
do not panic on write errosr
denik Dec 9, 2024
3af2a09
clean up
denik Dec 9, 2024
8e77be3
ignore err where it was intended
denik Dec 9, 2024
207c5fd
rm risky panics
denik Dec 9, 2024
65b4f78
rm risky panics
denik Dec 9, 2024
3063835
clean up
denik Dec 9, 2024
0c5fa24
clean ups
denik Dec 9, 2024
a05f0c0
fix assignment mismatch
denik Dec 9, 2024
e1c607b
fixes
denik Dec 9, 2024
cc7a46d
keep ignoring errors that were ignored before
denik Dec 10, 2024
3b99cdf
refactor: Remove error handling for Set method calls
denik Dec 10, 2024
16d412a
Keep libs/dyn unchanged; annotate with //nolint:errcheck
denik Dec 10, 2024
d59dd39
annotate with nolint:errcheck
denik Dec 10, 2024
e4352e8
fix: Correct flag marking for verbose flag in deploy command
denik Dec 10, 2024
010ce1e
refactor: Remove unnecessary error assignments from MarkHidden calls
denik Dec 10, 2024
0832ccd
refactor: Simplify flag deprecation and completion registration
denik Dec 10, 2024
ee67bb3
refactor: Remove unnecessary error discards from RegisterFlagCompleti…
denik Dec 10, 2024
09a2a40
exclude cobra functions from errcheck
denik Dec 10, 2024
883a1c3
Add MarkFlagRequired to exceptions
denik Dec 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ linters-settings:
replacement: 'a[b:]'
- pattern: 'interface{}'
replacement: 'any'
errcheck:
exclude-functions:
- (*github.com/spf13/cobra.Command).RegisterFlagCompletionFunc
- (*github.com/spf13/cobra.Command).MarkFlagRequired
- (*github.com/spf13/pflag.FlagSet).MarkDeprecated
- (*github.com/spf13/pflag.FlagSet).MarkHidden
issues:
exclude-dirs-use-default: false # recommended by docs https://golangci-lint.run/usage/false-positives/
exclude-rules:
- path-except: (_test\.go|internal)
linters:
- errcheck
5 changes: 4 additions & 1 deletion bundle/config/mutator/initialize_urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
}
orgId := strconv.FormatInt(workspaceId, 10)
host := b.WorkspaceClient().Config.CanonicalHostName()
initializeForWorkspace(b, orgId, host)
err = initializeForWorkspace(b, orgId, host)
if err != nil {
return diag.FromErr(err)
}
return nil
}

Expand Down
3 changes: 1 addition & 2 deletions bundle/config/mutator/resolve_resource_references.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ func (m *resolveResourceReferences) Apply(ctx context.Context, b *bundle.Bundle)
return fmt.Errorf("failed to resolve %s, err: %w", v.Lookup, err)
}

v.Set(id)
return nil
return v.Set(id)
})
}

Expand Down
10 changes: 8 additions & 2 deletions bundle/deploy/state_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,14 @@ func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic
}

// Truncating the file before writing
local.Truncate(0)
local.Seek(0, 0)
err = local.Truncate(0)
if err != nil {
return diag.FromErr(err)
}
_, err = local.Seek(0, 0)
if err != nil {
return diag.FromErr(err)
}

// Write file to disk.
log.Infof(ctx, "Writing remote deployment state file to local cache directory")
Expand Down
10 changes: 8 additions & 2 deletions bundle/render/render_text_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,16 @@ func RenderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics,
if err != nil {
return fmt.Errorf("failed to render summary: %w", err)
}
io.WriteString(out, "\n")
_, err = io.WriteString(out, "\n")
if err != nil {
return err
}
}
trailer := buildTrailer(diags)
io.WriteString(out, trailer)
_, err = io.WriteString(out, trailer)
if err != nil {
return err
}
}

return nil
Expand Down
5 changes: 4 additions & 1 deletion cmd/auth/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@ func render(ctx context.Context, cmd *cobra.Command, status *authStatus, templat
if err != nil {
return err
}
cmd.OutOrStdout().Write(buf)
_, err = cmd.OutOrStdout().Write(buf)
denik marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
default:
return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/auth/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func newEnvCommand() *cobra.Command {
if err != nil {
return err
}
cmd.OutOrStdout().Write(raw)
_, _ = cmd.OutOrStdout().Write(raw)
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/auth/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func newTokenCommand(persistentAuth *auth.PersistentAuth) *cobra.Command {
if err != nil {
return err
}
cmd.OutOrStdout().Write(raw)
_, _ = cmd.OutOrStdout().Write(raw)
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/bundle/debug/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ For more information about filesystem mirrors, see the Terraform documentation:
}
switch root.OutputType(cmd) {
case flags.OutputText:
cmdio.Render(cmd.Context(), dependencies.Terraform)
_ = cmdio.Render(cmd.Context(), dependencies.Terraform)
case flags.OutputJSON:
buf, err := json.MarshalIndent(dependencies, "", " ")
if err != nil {
return err
}
cmd.OutOrStdout().Write(buf)
_, _ = cmd.OutOrStdout().Write(buf)
default:
return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
}
Expand Down
10 changes: 8 additions & 2 deletions cmd/bundle/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,19 @@ task or a Python wheel task, the second example applies.
if err != nil {
return err
}
cmd.OutOrStdout().Write([]byte(resultString))
_, err = cmd.OutOrStdout().Write([]byte(resultString))
if err != nil {
return err
}
case flags.OutputJSON:
b, err := json.MarshalIndent(output, "", " ")
if err != nil {
return err
}
cmd.OutOrStdout().Write(b)
_, err = cmd.OutOrStdout().Write(b)
if err != nil {
return err
}
default:
return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/bundle/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func newSummaryCommand() *cobra.Command {
if err != nil {
return err
}
cmd.OutOrStdout().Write(buf)
_, _ = cmd.OutOrStdout().Write(buf)
default:
return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/bundle/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func renderJsonOutput(cmd *cobra.Command, b *bundle.Bundle, diags diag.Diagnosti
if err != nil {
return err
}
cmd.OutOrStdout().Write(buf)
_, _ = cmd.OutOrStdout().Write(buf)
return diags.Error()
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/root/io.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func initOutputFlag(cmd *cobra.Command) *outputFlag {
// Configure defaults from environment, if applicable.
// If the provided value is invalid it is ignored.
if v, ok := env.Lookup(cmd.Context(), envOutputFormat); ok {
f.output.Set(v)
f.output.Set(v) //nolint:errcheck
}

cmd.PersistentFlags().VarP(&f.output, "output", "o", "output type: text or json")
Expand Down
11 changes: 7 additions & 4 deletions cmd/root/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ func (f *logFlags) makeLogHandler(opts slog.HandlerOptions) (slog.Handler, error

func (f *logFlags) initializeContext(ctx context.Context) (context.Context, error) {
if f.debug {
f.level.Set("debug")
err := f.level.Set("debug")
if err != nil {
return nil, err
}
}

opts := slog.HandlerOptions{}
Expand Down Expand Up @@ -81,13 +84,13 @@ func initLogFlags(cmd *cobra.Command) *logFlags {
// Configure defaults from environment, if applicable.
// If the provided value is invalid it is ignored.
if v, ok := env.Lookup(cmd.Context(), envLogFile); ok {
f.file.Set(v)
f.file.Set(v) //nolint:errcheck
}
if v, ok := env.Lookup(cmd.Context(), envLogLevel); ok {
f.level.Set(v)
f.level.Set(v) //nolint:errcheck
}
if v, ok := env.Lookup(cmd.Context(), envLogFormat); ok {
f.output.Set(v)
f.output.Set(v) //nolint:errcheck
}

flags := cmd.PersistentFlags()
Expand Down
2 changes: 1 addition & 1 deletion cmd/root/progress_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func initProgressLoggerFlag(cmd *cobra.Command, logFlags *logFlags) *progressLog
// Configure defaults from environment, if applicable.
// If the provided value is invalid it is ignored.
if v, ok := env.Lookup(cmd.Context(), envProgressFormat); ok {
f.Set(v)
_ = f.Set(v)
}

flags := cmd.PersistentFlags()
Expand Down
4 changes: 3 additions & 1 deletion libs/auth/callback.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ func newCallback(ctx context.Context, a *PersistentAuth) (*callbackServer, error
a: a,
}
cb.srv.Handler = cb
go cb.srv.Serve(cb.ln)
go func() {
_ = cb.srv.Serve(cb.ln)
}()
return cb, nil
}

Expand Down
18 changes: 9 additions & 9 deletions libs/cmdio/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,29 +188,29 @@ func (l *Logger) writeJson(event Event) {
// we panic because there we cannot catch this in jobs.RunNowAndWait
panic(err)
}
l.Writer.Write([]byte(b))
l.Writer.Write([]byte("\n"))
_, _ = l.Writer.Write([]byte(b))
_, _ = l.Writer.Write([]byte("\n"))
}

func (l *Logger) writeAppend(event Event) {
l.Writer.Write([]byte(event.String()))
l.Writer.Write([]byte("\n"))
_, _ = l.Writer.Write([]byte(event.String()))
_, _ = l.Writer.Write([]byte("\n"))
}

func (l *Logger) writeInplace(event Event) {
if l.isFirstEvent {
// save cursor location
l.Writer.Write([]byte("\033[s"))
_, _ = l.Writer.Write([]byte("\033[s"))
}

// move cursor to saved location
l.Writer.Write([]byte("\033[u"))
_, _ = l.Writer.Write([]byte("\033[u"))

// clear from cursor to end of screen
l.Writer.Write([]byte("\033[0J"))
_, _ = l.Writer.Write([]byte("\033[0J"))

l.Writer.Write([]byte(event.String()))
l.Writer.Write([]byte("\n"))
_, _ = l.Writer.Write([]byte(event.String()))
_, _ = l.Writer.Write([]byte("\n"))
l.isFirstEvent = false
}

Expand Down
4 changes: 3 additions & 1 deletion libs/cmdio/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,9 @@ func renderUsingTemplate(ctx context.Context, r templateRenderer, w io.Writer, h
if err != nil {
return err
}
tw.Write([]byte("\n"))
if _, err := tw.Write([]byte("\n")); err != nil {
return err
}
// Do not flush here. Instead, allow the first 100 resources to determine the initial spacing of the header columns.
}
t, err := base.Parse(tmpl)
Expand Down
4 changes: 2 additions & 2 deletions libs/dyn/convert/from_typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptio

// Either if the key was set in the reference or the field is not zero-valued, we include it.
if ok || nv.Kind() != dyn.KindNil {
out.Set(refk, nv)
out.Set(refk, nv) // nolint:errcheck
}
}

Expand Down Expand Up @@ -184,7 +184,7 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) {

// Every entry is represented, even if it is a nil.
// Otherwise, a map with zero-valued structs would yield a nil as well.
out.Set(refk, nv)
out.Set(refk, nv) //nolint:errcheck
}

return dyn.V(out), nil
Expand Down
6 changes: 3 additions & 3 deletions libs/dyn/convert/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen
}
}

out.Set(pk, nv)
out.Set(pk, nv) //nolint:errcheck
}

// Return the normalized value if missing fields are not included.
Expand Down Expand Up @@ -162,7 +162,7 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen
continue
}
if v.IsValid() {
out.Set(dyn.V(k), v)
out.Set(dyn.V(k), v) // nolint:errcheck
}
}

Expand Down Expand Up @@ -201,7 +201,7 @@ func (n normalizeOptions) normalizeMap(typ reflect.Type, src dyn.Value, seen []r
}
}

out.Set(pk, nv)
out.Set(pk, nv) //nolint:errcheck
}

return dyn.NewValue(out, src.Locations()), diags
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/jsonloader/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func decodeValue(decoder *json.Decoder, o *Offset) (dyn.Value, error) {
return invalidValueWithLocation(decoder, o), err
}

obj.Set(keyVal, val)
obj.Set(keyVal, val) //nolint:errcheck
}
// Consume the closing '}'
if _, err := decoder.Token(); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions libs/dyn/mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func newMappingWithSize(size int) Mapping {
func newMappingFromGoMap(vin map[string]Value) Mapping {
m := newMappingWithSize(len(vin))
for k, v := range vin {
m.Set(V(k), v)
m.Set(V(k), v) //nolint:errcheck
}
return m
}
Expand Down Expand Up @@ -144,6 +144,6 @@ func (m Mapping) Clone() Mapping {
// Merge merges the key-value pairs from another Mapping into the current Mapping.
func (m *Mapping) Merge(n Mapping) {
for _, p := range n.pairs {
m.Set(p.Key, p.Value)
m.Set(p.Key, p.Value) //nolint:errcheck
}
}
4 changes: 2 additions & 2 deletions libs/dyn/merge/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ func mergeMap(a, b dyn.Value) (dyn.Value, error) {
if err != nil {
return dyn.InvalidValue, err
}
out.Set(pk, merged)
out.Set(pk, merged) //nolint:errcheck
} else {
// Otherwise, just set the value.
out.Set(pk, pv)
out.Set(pk, pv) //nolint:errcheck
}
}

Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/pattern.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (c anyKeyComponent) visit(v Value, prefix Path, suffix Pattern, opts visitO
return InvalidValue, err
}

m.Set(pk, nv)
m.Set(pk, nv) //nolint:errcheck
}

return NewValue(m, v.Locations()), nil
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/visit.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts

// Return an updated map value.
m = m.Clone()
m.Set(V(component.key), nv)
m.Set(V(component.key), nv) //nolint:errcheck
return Value{
v: m,
k: KindMap,
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/visit_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func Foreach(fn MapFunc) MapFunc {
if err != nil {
return InvalidValue, err
}
m.Set(pk, nv)
m.Set(pk, nv) //nolint:errcheck
}
return NewValue(m, v.Locations()), nil
case KindSequence:
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/visit_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func SetByPath(v Value, p Path, nv Value) (Value, error) {

// Return an updated map value.
m = m.Clone()
m.Set(V(component.key), nv)
m.Set(V(component.key), nv) //nolint:errcheck
return Value{
v: m,
k: KindMap,
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro
if err != nil {
return InvalidValue, err
}
out.Set(pk, nv)
out.Set(pk, nv) //nolint:errcheck
}
v.v = out
case KindSequence:
Expand Down
Loading
Loading