Skip to content

Commit

Permalink
chore: remove some unused parameters
Browse files Browse the repository at this point in the history
And also enable a linter to catch these in the future. `unparam` seems
to be optimized more heavily to avoid false positives, and is
significantly better at not bringing up weird interface cases that
revive seems to love.

Signed-off-by: Justin Chadwell <me@jedevc.com>
  • Loading branch information
jedevc committed Apr 25, 2024
1 parent dc447ba commit 33e0dae
Show file tree
Hide file tree
Showing 17 changed files with 53 additions and 47 deletions.
19 changes: 16 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,34 @@ linters:
- typecheck
- unconvert
- unused
- unparam
- whitespace

issues:
exclude-rules:
- path: _test\.go
linters:
# tests are very repetitive
- dupl
# tests are allowed to do silly things
- gosec
- path: docs/
linters:
# example dagger module code might use extra ctx/err in signatures for clarity
- unparam
- text: ".* always receives .*"
linters:
# this is sometimes done for clarity
- unparam


linters-settings:
revive:
rules:
# This rule is annoying. Often you want to name the
# parameters for clarity because it conforms to an
# interface.
# This rule is annoying. Often you want to name the parameters for
# clarity because it conforms to an interface. Additionally, unparam
# finds a good number of cases for this anyways (with fewer false
# positives).
- name: unused-parameter
severity: warning
disabled: true
Expand Down
9 changes: 3 additions & 6 deletions cmd/codegen/generator/go/templates/module_objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,7 @@ func (spec *parsedObjectType) marshalJSONMethodCode() (*Statement, error) {
}
concreteFields = append(concreteFields, fieldCode)

getFieldCode, err := spec.setFieldsToMarshalStructCode(field)
if err != nil {
return nil, fmt.Errorf("failed to generate get field code: %w", err)
}
getFieldCode := spec.setFieldsToMarshalStructCode(field)
getFieldCodes = append(getFieldCodes, getFieldCode)
}

Expand Down Expand Up @@ -475,8 +472,8 @@ func (spec *parsedObjectType) setFieldsFromUnmarshalStructCode(field *fieldSpec)
return s, nil
}

func (spec *parsedObjectType) setFieldsToMarshalStructCode(field *fieldSpec) (*Statement, error) {
return Empty().Id("concrete").Dot(field.goName).Op("=").Id("r").Dot(field.goName), nil
func (spec *parsedObjectType) setFieldsToMarshalStructCode(field *fieldSpec) *Statement {
return Empty().Id("concrete").Dot(field.goName).Op("=").Id("r").Dot(field.goName)
}

type fieldSpec struct {
Expand Down
2 changes: 1 addition & 1 deletion cmd/dagger/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ func (v *moduleSourceValue) Get(ctx context.Context, dag *dagger.Client, _ *dagg

// AddFlag adds a flag appropriate for the argument type. Should return a
// pointer to the value.
func (r *modFunctionArg) AddFlag(flags *pflag.FlagSet, dag *dagger.Client) (any, error) {
func (r *modFunctionArg) AddFlag(flags *pflag.FlagSet) (any, error) {
name := r.FlagName()
usage := r.Description

Expand Down
8 changes: 4 additions & 4 deletions cmd/dagger/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ func (fc *FuncCommand) load(c *cobra.Command, a []string) (cmd *cobra.Command, _

if obj.Constructor != nil {
// add constructor args as top-level flags
if err := fc.addArgsForFunction(c, a, obj.Constructor, dag); err != nil {
if err := fc.addArgsForFunction(c, a, obj.Constructor); err != nil {
return nil, nil, err
}
fc.selectFunc(obj.Name, obj.Constructor, c, dag)
Expand Down Expand Up @@ -446,7 +446,7 @@ func (fc *FuncCommand) makeSubCmd(dag *dagger.Client, fn *modFunction) *cobra.Co
Long: fn.Description,
GroupID: funcGroup.ID,
PreRunE: func(cmd *cobra.Command, args []string) (err error) {
if err := fc.addArgsForFunction(cmd, args, fn, dag); err != nil {
if err := fc.addArgsForFunction(cmd, args, fn); err != nil {
return err
}

Expand Down Expand Up @@ -534,15 +534,15 @@ func (fc *FuncCommand) makeSubCmd(dag *dagger.Client, fn *modFunction) *cobra.Co
return newCmd
}

func (fc *FuncCommand) addArgsForFunction(cmd *cobra.Command, cmdArgs []string, fn *modFunction, dag *dagger.Client) error {
func (fc *FuncCommand) addArgsForFunction(cmd *cobra.Command, cmdArgs []string, fn *modFunction) error {
fc.mod.LoadTypeDef(fn.ReturnType)

for _, arg := range fn.Args {
fc.mod.LoadTypeDef(arg.TypeDef)
}

for _, arg := range fn.Args {
_, err := arg.AddFlag(cmd.Flags(), dag)
_, err := arg.AddFlag(cmd.Flags())
if err != nil {
return err
}
Expand Down
10 changes: 5 additions & 5 deletions cmd/engine/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ func (w *runcExecutor) run(
})
return err
}
err = exitError(ctx, w.callWithIO(ctx, id, bundle, process, startedCallback, killer, runcCall))
err = exitError(ctx, w.callWithIO(ctx, process, startedCallback, killer, runcCall))
if err != nil {
w.runc.Delete(context.TODO(), id, &runc.DeleteOpts{})
return err
Expand Down Expand Up @@ -589,18 +589,18 @@ func (w *runcExecutor) Exec(ctx context.Context, id string, process executor.Pro
spec.Process.Env = process.Meta.Env
}

err = w.exec(ctx, id, state.Bundle, spec.Process, process, nil)
err = w.exec(ctx, id, spec.Process, process, nil)
return exitError(ctx, err)
}

func (w *runcExecutor) exec(ctx context.Context, id, bundle string, specsProcess *specs.Process, process executor.ProcessInfo, started func()) error {
func (w *runcExecutor) exec(ctx context.Context, id string, specsProcess *specs.Process, process executor.ProcessInfo, started func()) error {
killer, err := newExecProcKiller(w.runc, id)
if err != nil {
return fmt.Errorf("failed to initialize process killer: %w", err)
}
defer killer.Cleanup()

return w.callWithIO(ctx, id, bundle, process, started, killer, func(ctx context.Context, started chan<- int, io runc.IO, pidfile string) error {
return w.callWithIO(ctx, process, started, killer, func(ctx context.Context, started chan<- int, io runc.IO, pidfile string) error {
return w.runc.Exec(ctx, id, *specsProcess, &runc.ExecOpts{
Started: started,
IO: io,
Expand Down Expand Up @@ -930,7 +930,7 @@ func handleSignals(ctx context.Context, runcProcess *procHandle, signals <-chan

type runcCall func(ctx context.Context, started chan<- int, io runc.IO, pidfile string) error

func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, process executor.ProcessInfo, started func(), killer procKiller, call runcCall) error {
func (w *runcExecutor) callWithIO(ctx context.Context, process executor.ProcessInfo, started func(), killer procKiller, call runcCall) error {
runcProcess, ctx := runcProcessHandle(ctx, killer)
defer runcProcess.Release()

Expand Down
7 changes: 3 additions & 4 deletions core/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ func (container *Container) WithSecretVariable(ctx context.Context, name string,
}

func (container *Container) Directory(ctx context.Context, dirPath string) (*Directory, error) {
dir, _, err := locatePath(ctx, container, dirPath, NewDirectory)
dir, _, err := locatePath(container, dirPath, NewDirectory)
if err != nil {
return nil, err
}
Expand All @@ -749,7 +749,7 @@ func (container *Container) Directory(ctx context.Context, dirPath string) (*Dir
}

func (container *Container) File(ctx context.Context, filePath string) (*File, error) {
file, _, err := locatePath(ctx, container, filePath, NewFile)
file, _, err := locatePath(container, filePath, NewFile)
if err != nil {
return nil, err
}
Expand All @@ -769,7 +769,6 @@ func (container *Container) File(ctx context.Context, filePath string) (*File, e
}

func locatePath[T *File | *Directory](
ctx context.Context,
container *Container,
containerPath string,
init func(*Query, *pb.Definition, string, Platform, ServiceBindings) T,
Expand Down Expand Up @@ -931,7 +930,7 @@ func (container *Container) chown(
}

func (container *Container) writeToPath(ctx context.Context, subdir string, fn func(dir *Directory) (*Directory, error)) (*Container, error) {
dir, mount, err := locatePath(ctx, container, subdir, NewDirectory)
dir, mount, err := locatePath(container, subdir, NewDirectory)
if err != nil {
return nil, err
}
Expand Down
8 changes: 3 additions & 5 deletions core/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/vektah/gqlparser/v2/ast"

"github.com/dagger/dagger/engine"
"github.com/dagger/dagger/engine/buildkit"
"github.com/dagger/dagger/engine/sources/gitdns"
)

Expand Down Expand Up @@ -59,14 +58,13 @@ func (*GitRef) TypeDescription() string {
}

func (ref *GitRef) Tree(ctx context.Context) (*Directory, error) {
bk := ref.Query.Buildkit
st := ref.getState(ctx, bk)
st := ref.getState(ctx)
return NewDirectorySt(ctx, ref.Query, *st, "", ref.Repo.Platform, ref.Repo.Services)
}

func (ref *GitRef) Commit(ctx context.Context) (string, error) {
st := ref.getState(ctx)
bk := ref.Query.Buildkit
st := ref.getState(ctx, bk)
p, err := resolveProvenance(ctx, bk, *st)
if err != nil {
return "", err
Expand All @@ -77,7 +75,7 @@ func (ref *GitRef) Commit(ctx context.Context) (string, error) {
return p.Sources.Git[0].Commit, nil
}

func (ref *GitRef) getState(ctx context.Context, bk *buildkit.Client) *llb.State {
func (ref *GitRef) getState(ctx context.Context) *llb.State {
opts := []llb.GitOption{}

if ref.Repo.KeepGitDir {
Expand Down
2 changes: 1 addition & 1 deletion core/integration/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5705,7 +5705,7 @@ func hostDaggerCommand(ctx context.Context, t testing.TB, workdir string, args .
}

// runs a dagger cli command directly on the host, rather than in an exec
func hostDaggerExec(ctx context.Context, t testing.TB, workdir string, args ...string) ([]byte, error) {
func hostDaggerExec(ctx context.Context, t testing.TB, workdir string, args ...string) ([]byte, error) { //nolint: unparam
t.Helper()
cmd := hostDaggerCommand(ctx, t, workdir, args...)
output, err := cmd.CombinedOutput()
Expand Down
6 changes: 3 additions & 3 deletions core/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func (iface *InterfaceType) Install(ctx context.Context, dag *dagql.Server) erro
if err != nil {
return nil, fmt.Errorf("failed to get object return type for %s.%s: %w", ifaceName, fieldDef.Name, err)
}
return wrapIface(ctx, dag, ifaceReturnType, objReturnType, res)
return wrapIface(ifaceReturnType, objReturnType, res)
},
})
}
Expand Down Expand Up @@ -287,7 +287,7 @@ func (iface *InterfaceType) Install(ctx context.Context, dag *dagql.Server) erro
return nil
}

func wrapIface(ctx context.Context, dag *dagql.Server, ifaceType *InterfaceType, underlyingType ModType, res dagql.Typed) (dagql.Typed, error) {
func wrapIface(ifaceType *InterfaceType, underlyingType ModType, res dagql.Typed) (dagql.Typed, error) {
switch underlyingType := underlyingType.(type) {
case *InterfaceType, *ModuleObjectType:
switch res := res.(type) {
Expand Down Expand Up @@ -323,7 +323,7 @@ func wrapIface(ctx context.Context, dag *dagql.Server, ifaceType *InterfaceType,
if ret.Elem == nil { // set the return type
ret.Elem = item
}
val, err := wrapIface(ctx, dag, ifaceType, underlyingType.Underlying, item)
val, err := wrapIface(ifaceType, underlyingType.Underlying, item)
if err != nil {
return nil, fmt.Errorf("failed to wrap item %d: %w", i, err)
}
Expand Down
1 change: 0 additions & 1 deletion core/modfunc.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ func newModFunction(
ctx context.Context,
root *Query,
mod *Module,
modID *call.ID,
objDef *ObjectTypeDef,
runtime *Container,
metadata *Function,
Expand Down
1 change: 0 additions & 1 deletion core/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ func (mod *Module) Initialize(ctx context.Context, oldSelf dagql.Instance[*Modul
ctx,
mod.Query,
oldSelf.Self,
oldSelf.ID(),
nil,
mod.Runtime,
NewFunction("", &TypeDef{
Expand Down
4 changes: 1 addition & 3 deletions core/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ func (t *ModuleObjectType) GetCallable(ctx context.Context, name string) (Callab
ctx,
mod.Query,
mod,
mod.InstanceID,
t.typeDef,
mod.Runtime,
fun,
Expand Down Expand Up @@ -231,7 +230,7 @@ func (obj *ModuleObject) installConstructor(ctx context.Context, dag *dagql.Serv
return fmt.Errorf("constructor function for object %s must return that object", objDef.OriginalName)
}

fn, err := newModFunction(ctx, mod.Query, mod, mod.InstanceID, objDef, mod.Runtime, fnTypeDef)
fn, err := newModFunction(ctx, mod.Query, mod, objDef, mod.Runtime, fnTypeDef)
if err != nil {
return fmt.Errorf("failed to create function: %w", err)
}
Expand Down Expand Up @@ -324,7 +323,6 @@ func objFun(ctx context.Context, mod *Module, objDef *ObjectTypeDef, fun *Functi
ctx,
mod.Query,
mod,
mod.InstanceID,
objDef,
mod.Runtime,
fun,
Expand Down
9 changes: 6 additions & 3 deletions engine/sources/blob/blobsource.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,11 @@ func IdentifierFromPB(op *pb.SourceOp) (*SourceIdentifier, error) {
if !ok {
return nil, fmt.Errorf("invalid blob source identifier %q", op.Identifier)
}
if scheme != BlobScheme {
return nil, fmt.Errorf("invalid blob source identifier %q", op.Identifier)
}
bs := &blobSource{}
return bs.identifier(scheme, ref, op.GetAttrs(), nil)
return bs.identifier(ref, op.GetAttrs(), nil)
}

func NewSource(opt Opt) (source.Source, error) {
Expand All @@ -91,10 +94,10 @@ func (bs *blobSource) Schemes() []string {
}

func (bs *blobSource) Identifier(scheme, ref string, sourceAttrs map[string]string, p *pb.Platform) (source.Identifier, error) {
return bs.identifier(scheme, ref, sourceAttrs, p)
return bs.identifier(ref, sourceAttrs, p)
}

func (bs *blobSource) identifier(scheme, ref string, sourceAttrs map[string]string, _ *pb.Platform) (*SourceIdentifier, error) {
func (bs *blobSource) identifier(ref string, sourceAttrs map[string]string, _ *pb.Platform) (*SourceIdentifier, error) {
desc := ocispecs.Descriptor{
Digest: digest.Digest(ref),
Annotations: map[string]string{},
Expand Down
4 changes: 2 additions & 2 deletions sdk/go/telemetry/batch_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ func (bsp *batchSpanProcessor) enqueue(sd trace.ReadOnlySpan) {
if bsp.o.BlockOnQueueFull {
bsp.enqueueBlockOnQueueFull(ctx, sd)
} else {
bsp.enqueueDrop(ctx, sd)
bsp.enqueueDrop(sd)
}
}

Expand All @@ -427,7 +427,7 @@ func (bsp *batchSpanProcessor) enqueueBlockOnQueueFull(ctx context.Context, sd t
}
}

func (bsp *batchSpanProcessor) enqueueDrop(ctx context.Context, sd trace.ReadOnlySpan) bool {
func (bsp *batchSpanProcessor) enqueueDrop(sd trace.ReadOnlySpan) bool {
if !sd.SpanContext().IsSampled() {
return false
}
Expand Down
4 changes: 2 additions & 2 deletions telemetry/inflight/batch_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func (bsp *batchSpanProcessor) enqueue(sd trace.ReadOnlySpan) {
if bsp.o.BlockOnQueueFull {
bsp.enqueueBlockOnQueueFull(ctx, sd)
} else {
bsp.enqueueDrop(ctx, sd)
bsp.enqueueDrop(sd)
}
}

Expand All @@ -429,7 +429,7 @@ func (bsp *batchSpanProcessor) enqueueBlockOnQueueFull(ctx context.Context, sd t
}
}

func (bsp *batchSpanProcessor) enqueueDrop(ctx context.Context, sd trace.ReadOnlySpan) bool {
func (bsp *batchSpanProcessor) enqueueDrop(sd trace.ReadOnlySpan) bool {
if !sd.SpanContext().IsSampled() {
return false
}
Expand Down
2 changes: 1 addition & 1 deletion telemetry/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func TestLoadCircleCILabels(t *testing.T) {
}
}

func run(t *testing.T, exe string, args ...string) string { //nolint: unparam
func run(t *testing.T, exe string, args ...string) string {
t.Helper()
cmd := exec.Command(exe, args...)
cmd.Stderr = os.Stderr
Expand Down
4 changes: 2 additions & 2 deletions telemetry/sdklog/batch_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func (bsp *batchLogProcessor) enqueue(log *LogData) {
if bsp.o.BlockOnQueueFull {
bsp.enqueueBlockOnQueueFull(ctx, log)
} else {
bsp.enqueueDrop(ctx, log)
bsp.enqueueDrop(log)
}
}

Expand All @@ -310,7 +310,7 @@ func (bsp *batchLogProcessor) enqueueBlockOnQueueFull(ctx context.Context, log *
}
}

func (bsp *batchLogProcessor) enqueueDrop(ctx context.Context, log *LogData) bool {
func (bsp *batchLogProcessor) enqueueDrop(log *LogData) bool {
select {
case bsp.queue <- log:
return true
Expand Down

0 comments on commit 33e0dae

Please sign in to comment.