Repository: go-critic/go-critic
Pull request #1471 (2025-03-21 15:04:40)

checkers: add dupOption
I first implement it in another copyandpaste linter, i found that the `repeat arg` like `math.Max(a, a)` has already added in go-critic,

so i move the left part `repeat option` here, I have check the open golang github repos, the result can be found here https://github.com/alingse/copyandpaste/issues/7 

of cause, this may report some false-positive case like [docker/compose/blob/main/cmd/compose/compose.go#L399-L407](https://github.com/docker/compose/blob/main/cmd/compose/compose.go#L399-L407)


Patch:
diff --git a/.golangci.yml b/.golangci.yml
index cabad4b..3ad7ac0 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -12,6 +12,7 @@
       "gocritic",
       "gofmt",
       "goimports",
+      "goprintffuncname",
       "gosimple",
       "govet",
       "ineffassign",
@@ -51,6 +52,11 @@
     },
   },
   "issues": {
+    "exclude-case-sensitive": true,
     "exclude-dirs": ["checkers/rules"],
+    "exclude-rules": [
+      # Backward compatibility for CheckerContext.Warn, WarnWithPos, WarnFixable, and WarnFixableWithPos
+      {"linters":["goprintffuncname"], "text":"Warn(WithPos|Fixable)?"},
+    ]
   }
 }
diff --git a/checkers/badRegexp_checker.go b/checkers/badRegexp_checker.go
index 6c68450..8f5cf97 100644
--- a/checkers/badRegexp_checker.go
+++ b/checkers/badRegexp_checker.go
@@ -148,7 +148,7 @@ func (c *badRegexpChecker) walk(e syntax.Expr) {
 
 	case syntax.OpCaret:
 		if !c.isGoodAnchor(e) {
-			c.warn("dangling or redundant ^, maybe \\^ is intended?")
+			c.warnf("dangling or redundant ^, maybe \\^ is intended?")
 		}
 
 	default:
@@ -176,11 +176,11 @@ func (c *badRegexpChecker) updateFlagState(state *regexpFlagState, e syntax.Expr
 
 		if clearing {
 			if !state[ch] {
-				c.warn("clearing unset flag %c in %s", ch, e.Value)
+				c.warnf("clearing unset flag %c in %s", ch, e.Value)
 			}
 		} else {
 			if state[ch] {
-				c.warn("redundant flag %c in %s", ch, e.Value)
+				c.warnf("redundant flag %c in %s", ch, e.Value)
 			}
 		}
 		state[ch] = !clearing
@@ -198,7 +198,7 @@ func (c *badRegexpChecker) checkNestedQuantifier(e syntax.Expr) {
 
 	switch x.Op {
 	case syntax.OpPlus, syntax.OpStar:
-		c.warn("repeated greedy quantifier in %s", e.Value)
+		c.warnf("repeated greedy quantifier in %s", e.Value)
 	}
 }
 
@@ -208,7 +208,7 @@ func (c *badRegexpChecker) checkAltDups(alt syntax.Expr) {
 	set := make(map[string]struct{}, len(alt.Args))
 	for _, a := range alt.Args {
 		if _, ok := set[a.Value]; ok {
-			c.warn("`%s` is duplicated in %s", a.Value, alt.Value)
+			c.warnf("`%s` is duplicated in %s", a.Value, alt.Value)
 		}
 		set[a.Value] = struct{}{}
 	}
@@ -232,7 +232,7 @@ func (c *badRegexpChecker) checkAltAnchor(alt syntax.Expr) {
 			}
 		}
 		if matched {
-			c.warn("^ applied only to `%s` in %s", first.Value[len(`^`):], alt.Value)
+			c.warnf("^ applied only to `%s` in %s", first.Value[len(`^`):], alt.Value)
 		}
 	}
 
@@ -247,7 +247,7 @@ func (c *badRegexpChecker) checkAltAnchor(alt syntax.Expr) {
 			}
 		}
 		if matched {
-			c.warn("$ applied only to `%s` in %s", last.Value[:len(last.Value)-len(`$`)], alt.Value)
+			c.warnf("$ applied only to `%s` in %s", last.Value[:len(last.Value)-len(`$`)], alt.Value)
 		}
 	}
 }
@@ -429,7 +429,7 @@ func (c *badRegexpChecker) isGoodAnchor(e syntax.Expr) bool {
 	return false
 }
 
-func (c *badRegexpChecker) warn(format string, args ...interface{}) {
+func (c *badRegexpChecker) warnf(format string, args ...interface{}) {
 	c.ctx.Warn(c.cause, format, args...)
 }
 
diff --git a/checkers/dupOption_checker.go b/checkers/dupOption_checker.go
new file mode 100644
index 0000000..2f45d01
--- /dev/null
+++ b/checkers/dupOption_checker.go
@@ -0,0 +1,113 @@
+package checkers
+
+import (
+	"go/ast"
+	"go/token"
+	"go/types"
+
+	"github.com/go-critic/go-critic/checkers/internal/astwalk"
+	"github.com/go-critic/go-critic/linter"
+	"github.com/go-toolsmith/astcast"
+	"github.com/go-toolsmith/astfmt"
+)
+
+func init() {
+	var info linter.CheckerInfo
+	info.Name = "dupOption"
+	info.Tags = []string{linter.DiagnosticTag, linter.ExperimentalTag}
+	info.Summary = "Detects duplicated option function arguments in variadic function calls"
+	info.Before = `doSomething(name,
+		withWidth(w),
+		withHeight(h),
+		withWidth(w),
+)`
+	info.After = `doSomething(name,
+		withWidth(w),
+		withHeight(h),
+)`
+
+	collection.AddChecker(&info, func(ctx *linter.CheckerContext) (linter.FileWalker, error) {
+		c := &dupOptionChecker{ctx: ctx}
+		return astwalk.WalkerForExpr(c), nil
+	})
+}
+
+type dupOptionChecker struct {
+	astwalk.WalkHandler
+	ctx *linter.CheckerContext
+}
+
+func (c *dupOptionChecker) VisitExpr(expr ast.Expr) {
+	call := astcast.ToCallExpr(expr)
+	variadicArgs, argType := c.getVariadicArgs(call)
+	if len(variadicArgs) == 0 {
+		return
+	}
+
+	if !c.isOptionType(argType) {
+		return
+	}
+
+	dupArgs := c.findDupArgs(variadicArgs)
+	for _, arg := range dupArgs {
+		c.warn(arg)
+	}
+}
+
+func (c *dupOptionChecker) getVariadicArgs(call *ast.CallExpr) ([]ast.Expr, types.Type) {
+	if len(call.Args) == 0 {
+		return nil, nil
+	}
+
+	// skip for someFunc(a, b ...)
+	if call.Ellipsis != token.NoPos {
+		return nil, nil
+	}
+
+	funType := c.ctx.TypeOf(call.Fun)
+	sign, ok := funType.(*types.Signature)
+	if !ok || !sign.Variadic() {
+		return nil, nil
+	}
+
+	last := sign.Params().Len() - 1
+	sliceType, ok := sign.Params().At(last).Type().(*types.Slice)
+	if !ok {
+		return nil, nil
+	}
+	argType := sliceType.Elem()
+	return call.Args[last:], argType
+}
+
+func (c *dupOptionChecker) isOptionType(typeInfo types.Type) bool {
+	typeInfo = typeInfo.Underlying()
+
+	sign, ok := typeInfo.(*types.Signature)
+	if !ok {
+		return false
+	}
+
+	if sign.Params().Len() == 0 {
+		return false
+	}
+
+	return true
+}
+
+func (c *dupOptionChecker) findDupArgs(args []ast.Expr) []ast.Expr {
+	codeMap := make(map[string]bool)
+	dupArgs := make([]ast.Expr, 0)
+	for _, arg := range args {
+		code := astfmt.Sprint(arg)
+		if codeMap[code] {
+			dupArgs = append(dupArgs, arg)
+			continue
+		}
+		codeMap[code] = true
+	}
+	return dupArgs
+}
+
+func (c *dupOptionChecker) warn(arg ast.Node) {
+	c.ctx.Warn(arg, "function argument `%s` is duplicated", arg)
+}


Test patch:
diff --git a/checkers/testdata/dupOption/negative_tests.go b/checkers/testdata/dupOption/negative_tests.go
new file mode 100644
index 0000000..fe9ae4d
--- /dev/null
+++ b/checkers/testdata/dupOption/negative_tests.go
@@ -0,0 +1,12 @@
+package checker_test
+
+func doSome2(w, h int) {
+	_ = newPanel("hello",
+		withWidth(w),
+		withHeight(h),
+	)
+
+	export(nil,
+		withImage(nil, ""),
+	)
+}
diff --git a/checkers/testdata/dupOption/positive_tests.go b/checkers/testdata/dupOption/positive_tests.go
new file mode 100644
index 0000000..d402401
--- /dev/null
+++ b/checkers/testdata/dupOption/positive_tests.go
@@ -0,0 +1,125 @@
+package checker_test
+
+import "context"
+
+type panel struct {
+	name   string
+	width  int
+	height int
+}
+
+func newPanel(name string, opts ...func(c *panel)) *panel {
+	cfg := &panel{
+		name:   name,
+		width:  0,
+		height: 0,
+	}
+	for _, opt := range opts {
+		opt(cfg)
+	}
+
+	return cfg
+}
+
+func withWidth(w int) func(c *panel) {
+	return func(c *panel) {
+		c.width = w
+	}
+}
+
+func withDefaultWidth() func(c *panel) {
+	return withWidth(100)
+}
+
+var defaultWidth = withDefaultWidth()
+
+func withHeight(h int) func(c *panel) {
+	return func(c *panel) {
+		c.height = h
+	}
+}
+
+type exportOptions struct{}
+
+type exportOpt func(context.Context, *exportOptions) error
+
+func withImage(_ any, _ string) exportOpt {
+	return func(_ context.Context, _ *exportOptions) error {
+		return nil
+	}
+}
+
+func export(_ any, _ ...exportOpt) {
+}
+
+type sourceOptions struct{}
+
+type sourceOpt = func(context.Context, *sourceOptions) error
+
+func withGroup(_ string) sourceOpt {
+	return func(_ context.Context, _ *sourceOptions) error {
+		return nil
+	}
+}
+
+func withKey(_ string) sourceOpt {
+	return func(_ context.Context, _ *sourceOptions) error {
+		return nil
+	}
+}
+
+func defaultOpt(_ context.Context, _ *sourceOptions) error {
+	return nil
+}
+
+func newSource(_ any, opts ...sourceOpt) (any, error) {
+	ctx := context.Background()
+	o := &sourceOptions{}
+	for _, opt := range opts {
+		err := opt(ctx, o)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return nil, nil
+}
+
+func doSome1(w, h int) {
+	_ = newPanel("hello",
+		withWidth(w),
+		withHeight(h),
+		/*! function argument `withWidth(w)` is duplicated */
+		withWidth(w),
+	)
+
+	export(nil,
+		withImage(nil, ""),
+		/*! function argument `withImage(nil, "")` is duplicated */
+		withImage(nil, ""),
+	)
+
+	_ = newPanel("case2",
+		withHeight(h),
+		withDefaultWidth(),
+		/*! function argument `withDefaultWidth()` is duplicated */
+		withDefaultWidth(),
+		defaultWidth,
+		/*! function argument `defaultWidth` is duplicated */
+		defaultWidth,
+	)
+
+	_ = newPanel("case3",
+		withHeight(h),
+		defaultWidth,
+		/*! function argument `defaultWidth` is duplicated */
+		defaultWidth,
+	)
+
+	_, _ = newSource("case4",
+		withGroup(""),
+		withKey("key"),
+		defaultOpt,
+		/*! function argument `defaultOpt` is duplicated */
+		defaultOpt,
+	)
+}