chore: enable unparam linter (#7174)

* chore: fix some doc comments for daqgl to match Signed-off-by: Justin Chadwell <me@jedevc.com> * chore: remove some unused parameters 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> * fixups Signed-off-by: Justin Chadwell <me@jedevc.com> --------- Signed-off-by: Justin Chadwell <me@jedevc.com>
dagger · May 3, 2024 · 2f29d13 · 2f29d13
1 parent 56a039a
commit 2f29d13
Show file tree

Hide file tree

Showing 25 changed files with 76 additions and 76 deletions.
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
@@ -129,9 +129,3 @@ updates:
         applies-to: version-updates
         patterns:
           - "*"
-
-  # ignore all npm dependencies in sdk/rust
-  - package-ecosystem: "npm"
-    directory: "/sdk/rust"
-    ignore:
-      - dependency-name: "*"
diff --git a/.golangci.yml b/.golangci.yml
@@ -26,21 +26,38 @@ 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
+
+  exclude-dirs:
+    # these files are already linted in sdk/go
+    - internal/telemetry
+    - internal/querybuilder
 
 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

diff --git a/ci/docs.go b/ci/docs.go
@@ -63,18 +63,17 @@ func (d Docs) Lint(ctx context.Context) error {
 // Regenerate the API schema and CLI reference docs
 func (d Docs) Generate(ctx context.Context) (*dagger.Directory, error) {
 	eg, ctx := errgroup.WithContext(ctx)
+	_ = ctx
 
 	var sdl *dagger.Directory
 	eg.Go(func() error {
-		var err error
-		sdl, err = d.GenerateSdl(ctx)
-		return err
+		sdl = d.GenerateSdl()
+		return nil
 	})
 	var cli *dagger.Directory
 	eg.Go(func() error {
-		var err error
-		cli, err = d.GenerateCli(ctx)
-		return err
+		cli = d.GenerateCli()
+		return nil
 	})
 
 	if err := eg.Wait(); err != nil {
@@ -85,7 +84,7 @@ func (d Docs) Generate(ctx context.Context) (*dagger.Directory, error) {
 }
 
 // Regenerate the API schema
-func (d Docs) GenerateSdl(ctx context.Context) (*Directory, error) {
+func (d Docs) GenerateSdl() *Directory {
 	introspectionJSON :=
 		util.GoBase(d.Dagger.Source).
 			WithExec([]string{"go", "run", "./cmd/introspect"}, dagger.ContainerWithExecOpts{
@@ -100,14 +99,14 @@ func (d Docs) GenerateSdl(ctx context.Context) (*Directory, error) {
 		WithExec([]string{"graphql-json-to-sdl", "/src/schema.json", "/src/schema.graphql"}).
 		File("/src/schema.graphql")
 
-	return dag.Directory().WithFile(generatedSchemaPath, generated), nil
+	return dag.Directory().WithFile(generatedSchemaPath, generated)
 }
 
 // Regenerate the CLI reference docs
-func (d Docs) GenerateCli(ctx context.Context) (*Directory, error) {
+func (d Docs) GenerateCli() *Directory {
 	// Should we keep `--include-experimental`?
 	generated := util.GoBase(d.Dagger.Source).
 		WithExec([]string{"go", "run", "./cmd/dagger", "gen", "--frontmatter=" + cliZenFrontmatter, "--output=cli.mdx", "--include-experimental"}).
 		File("cli.mdx")
-	return dag.Directory().WithFile(generatedCliZenPath, generated), nil
+	return dag.Directory().WithFile(generatedCliZenPath, generated)
 }
diff --git a/ci/sdk_go.go b/ci/sdk_go.go
@@ -106,7 +106,7 @@ func (t GoSDK) Publish(
 }
 
 // Bump the Go SDK's Engine dependency
-func (t GoSDK) Bump(ctx context.Context, version string) (*Directory, error) {
+func (t GoSDK) Bump(version string) (*Directory, error) {
 	// trim leading v from version
 	version = strings.TrimPrefix(version, "v")
 

diff --git a/ci/sdk_php.go b/ci/sdk_php.go
@@ -83,7 +83,7 @@ func (t PHPSDK) Publish(
 }
 
 // Bump the PHP SDK's Engine dependency
-func (t PHPSDK) Bump(ctx context.Context, version string) (*Directory, error) {
+func (t PHPSDK) Bump(version string) (*Directory, error) {
 	version = strings.TrimPrefix(version, "v")
 	content := fmt.Sprintf(`<?php /* Code generated by dagger. DO NOT EDIT. */ return '%s';
 `, version)

diff --git a/ci/sdk_python.go b/ci/sdk_python.go
@@ -155,7 +155,7 @@ func (t PythonSDK) Publish(
 }
 
 // Bump the Python SDK's Engine dependency
-func (t PythonSDK) Bump(ctx context.Context, version string) (*dagger.Directory, error) {
+func (t PythonSDK) Bump(version string) (*dagger.Directory, error) {
 	// trim leading v from version
 	version = strings.TrimPrefix(version, "v")
 	engineReference := fmt.Sprintf("# Code generated by dagger. DO NOT EDIT.\n\nCLI_VERSION = %q\n", version)

diff --git a/ci/sdk_typescript.go b/ci/sdk_typescript.go
@@ -156,7 +156,7 @@ always-auth=true`, plaintext)
 }
 
 // Bump the Typescript SDK's Engine dependency
-func (t TypescriptSDK) Bump(ctx context.Context, version string) (*Directory, error) {
+func (t TypescriptSDK) Bump(version string) (*Directory, error) {
 	// trim leading v from version
 	version = strings.TrimPrefix(version, "v")
 

diff --git a/cmd/codegen/generator/go/templates/module_objects.go b/cmd/codegen/generator/go/templates/module_objects.go
@@ -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)
 	}
 
@@ -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 {

diff --git a/cmd/dagger/flags.go b/cmd/dagger/flags.go
@@ -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
 

diff --git a/cmd/dagger/functions.go b/cmd/dagger/functions.go
@@ -384,7 +384,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)
@@ -454,7 +454,7 @@ func (fc *FuncCommand) makeSubCmd(dag *dagger.Client, fn *modFunction) *cobra.Co
 		GroupID:               funcGroup.ID,
 		DisableFlagsInUseLine: true,
 		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
 			}
 
@@ -542,15 +542,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
 		}

diff --git a/core/container.go b/core/container.go
@@ -724,7 +724,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
 	}
@@ -747,7 +747,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
 	}
@@ -767,7 +767,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,
@@ -929,7 +928,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
 	}

diff --git a/core/git.go b/core/git.go
@@ -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"
 )
 
@@ -59,8 +58,7 @@ func (*GitRef) TypeDescription() string {
 }
 
 func (ref *GitRef) Tree(ctx context.Context) (*Directory, error) {
-	bk := ref.Query.Buildkit
-	st, err := ref.getState(ctx, bk)
+	st, err := ref.getState(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -69,7 +67,7 @@ func (ref *GitRef) Tree(ctx context.Context) (*Directory, error) {
 
 func (ref *GitRef) Commit(ctx context.Context) (string, error) {
 	bk := ref.Query.Buildkit
-	st, err := ref.getState(ctx, bk)
+	st, err := ref.getState(ctx)
 	if err != nil {
 		return "", err
 	}
@@ -83,7 +81,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, error) {
+func (ref *GitRef) getState(ctx context.Context) (llb.State, error) {
 	opts := []llb.GitOption{}
 
 	if ref.Repo.KeepGitDir {

diff --git a/core/integration/module_test.go b/core/integration/module_test.go
@@ -5815,7 +5815,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()

diff --git a/core/interface.go b/core/interface.go
@@ -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)
 			},
 		})
 	}
@@ -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) {
@@ -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)
 			}

diff --git a/core/modfunc.go b/core/modfunc.go
@@ -16,7 +16,6 @@ import (
 	"github.com/dagger/dagger/analytics"
 	"github.com/dagger/dagger/core/pipeline"
 	"github.com/dagger/dagger/dagql"
-	"github.com/dagger/dagger/dagql/call"
 	"github.com/dagger/dagger/engine/buildkit"
 )
 
@@ -40,7 +39,6 @@ func newModFunction(
 	ctx context.Context,
 	root *Query,
 	mod *Module,
-	modID *call.ID,
 	objDef *ObjectTypeDef,
 	runtime *Container,
 	metadata *Function,

diff --git a/core/module.go b/core/module.go
@@ -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{

diff --git a/core/object.go b/core/object.go
@@ -110,7 +110,6 @@ func (t *ModuleObjectType) GetCallable(ctx context.Context, name string) (Callab
 			ctx,
 			mod.Query,
 			mod,
-			mod.InstanceID,
 			t.typeDef,
 			mod.Runtime,
 			fun,
@@ -230,7 +229,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)
 	}
@@ -323,7 +322,6 @@ func objFun(ctx context.Context, mod *Module, objDef *ObjectTypeDef, fun *Functi
 		ctx,
 		mod.Query,
 		mod,
-		mod.InstanceID,
 		objDef,
 		mod.Runtime,
 		fun,

diff --git a/dagger.json b/dagger.json
@@ -15,4 +15,4 @@
       ]
     }
   ]
-}
+}