sync upstream

[xushiwei]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces several improvements and bug fixes to the Cobra library. Key additions include a new NoDuplicateArgs positional argument validator and the ability to configure a DefaultShellCompDirective within CompletionOptions, which supports inheritance across command hierarchies. A critical bug where shell completion could inadvertently mutate os.Args was resolved by ensuring argument slices are copied. Additionally, the PR updates the linting configuration, optimizes documentation generation by using fmt.Fprintf, and expands the user guide. Review feedback suggests reverting to the canonical gopkg.in/yaml.v3 import path and using more idiomatic, shorter receiver names in Go methods.

[gemini-code-assist]

The import path for yaml.v3 has been changed to go.yaml.in/yaml/v3. The canonical and standard import path for this library is gopkg.in/yaml.v3. Using a non-canonical mirror path can lead to duplicate packages in the dependency graph if other dependencies use the canonical path, and it deviates from standard Go ecosystem practices.

Suggested change

	go.yaml.in/yaml/v3 v3.0.4
	gopkg.in/yaml.v3 v3.0.4

[gemini-code-assist]

This import should use the canonical path gopkg.in/yaml.v3 instead of the mirror go.yaml.in/yaml/v3 to maintain consistency and avoid potential conflicts with other packages that depend on the standard path.

Suggested change

	"go.yaml.in/yaml/v3"
	"gopkg.in/yaml.v3"

[gemini-code-assist]

The receiver name receiver is not idiomatic in Go. Receiver names should be short (usually one or two letters) and reflect the type name (e.g., o or opt for CompletionOptions).

Suggested change

	func (receiver *CompletionOptions) SetDefaultShellCompDirective(directive ShellCompDirective) {
	receiver.DefaultShellCompDirective = &directive
	}
	func (o *CompletionOptions) SetDefaultShellCompDirective(directive ShellCompDirective) {
	o.DefaultShellCompDirective = &directive
	}

[fennoai]

Code Review Summary

Overall this is a clean upstream sync. The correctness fixes (trimmedArgs mutation, fish escaping, boolean refactors) are sound. A few items below warrant attention before merging.

Findings: 1 medium security, 1 medium docs accuracy, 2 docs clarity, 2 code quality, 1 low security.

[fennoai]

[Security - Medium] Incomplete escaping before eval

The fix correctly escapes $args[2..-1] via string escape, but $args[1] (the binary name) in the same string remains unescaped before being passed to eval on line 59. If an attacker can influence the program name (e.g., a crafted $PATH entry where the binary name contains shell metacharacters), it flows into eval without sanitization.

$lastArg is pre-escaped at assignment time (line 50), so that is safe. But $args[1] should also be wrapped in string escape:

set -l requestComp "%[10]s=0 $(string escape -- $args[1]) %[3]s $(string join ' ' -- (string escape -- $args[2..-1])) $lastArg"

[fennoai]

[Code Quality] Non-idiomatic receiver name

receiver is not a conventional Go receiver name. The rest of the codebase uses short names (c *Command, etc.). Per Go conventions (and the enabled gocritic linter), this should be o or co:

func (o *CompletionOptions) SetDefaultShellCompDirective(directive ShellCompDirective) {

[fennoai]

[Code Quality] Doc comment doesn't explain pointer semantics or scope

The pointer is needed to distinguish nil ("not set, inherit from parent") from 0 (ShellCompDirectiveDefault). This non-obvious design should be documented on the field:

// DefaultShellCompDirective overrides the directive used when no completion
// function is registered. It is a pointer to distinguish "not set" (nil, inherits
// from the nearest ancestor) from "explicitly set to ShellCompDirectiveDefault (0)".
// Applies only to argument/noun completion; flag-name and flag-value completion
// paths use their own fixed directives.
DefaultShellCompDirective *ShellCompDirective

[fennoai]

[Security - Low] Wrong gosec rule ID in nolint annotation

The comment cites G703, which is not a valid gosec rule. The actual rule that fires here is G304 ("Potential file inclusion via variable"), since path is derived from an environment variable. The suppression intent is reasonable, but the incorrect rule ID undermines auditability and may not suppress the actual finding in all gosec versions:

//nolint:gosec // G304: BASH_COMP_DEBUG_FILE is intentionally user-controlled for completion debug logging.

[fennoai]

[Docs - Medium] "Recursively change" is misleading

The text says "you can recursively change the default ShellCompDirective for a command and its subcommands". The actual implementation doesn't push anything down — it walks up the parent chain at completion time and stops at the first ancestor with a non-nil DefaultShellCompDirective. Subcommands inherit by lookup, not by mutation. Suggested rephrasing:

you can set a DefaultShellCompDirective on a command so that it and any subcommands that don't override it will use ShellCompDirectiveNoFileComp:

[fennoai]

[Docs - Medium] Missing cobra. package qualifier in code examples

Both code examples in this new section use unqualified identifiers that require the cobra. prefix for users consuming this library from outside the package:

• Line 324: ShellCompDirectiveNoFileComp → cobra.ShellCompDirectiveNoFileComp
• Line 331: ShellCompDirectiveDefault → cobra.ShellCompDirectiveDefault

Compare with the correctly-qualified examples earlier in the same file (e.g., cobra.NoFileCompletions on line 316).

[fennoai]

[Docs - Low] Note refers to CountVar but section documents CountVarP

The note says "Both CountVar and array flags..." but the code example above uses CountVarP (the variant that accepts a shorthand). Either reference both variants explicitly (CountVar / CountVarP) or use the same name as the example above to avoid confusion.