Allow use of _xxx with unused-parameter rule

[Gusted]

Is your feature request related to a problem? Please describe.
We're enabling the unused-parameter rule on our repo to detect if there are possible bugs. A lot of parameters can be changed to _, but in certain cases it will make the code more unreadable

Describe the solution you'd like
Add a option(or by default) that certain use-case are allowed, this can be limited to just the _ prefix or a generic regex option. Using the _ prefix is common in JS to still have the variable name but show that the variable is unused.

Describe alternatives you've considered
Adding the //revive:disable... comment, but that's better to avoided for each function that needs this.

Additional context
None

[chavacava]

Hi @Gusted thanks for filling the issue.

Using the _ prefix is common in JS to still have the variable name but show that the variable is unused.

It is common in JS but not in GO, to the point that golint (thus revive) will warn you with a:
don't use underscores in Go names

[Gusted]

I see, fair point.

Thank you for your response.

[silverwind]

Is it out of question to have a rule configuration option for this? It could allow the user to define a regex pattern. The default pattern could be ^_$ and users would be free to define ^_ for example.

[chavacava]

Is it out of question to have a rule configuration option for this?

Not at all. My intention with the previous comment was to highlight that prefixing identifiers with _ is not idiomatic in GO

It could allow the user to define a regex pattern

Yes, it is something that could be studied

[twmb]

Prefixing with _ is common for fields in structs when you want to define a struct but not have it exported. This is commonly used for forcing users of the struct to specify all fields, and to specifically pad the struct. This can be seen within the Go stdlib itself:

$ rg '^\s+_[a-zA-Z0-9]+\s+[a-zA-Z0-9]'
syscall/syscall_freebsd.go
144:		_p0          unsafe.Pointer

runtime/defs_plan9_amd64.go
31:	_type uint64

runtime/stack.go
1373:	_ptrdata int32 // ptrdata, or -ptrdata is GC prog is used

runtime/defs_linux_ppc64.go
184:	_pad0       int32

runtime/defs1_netbsd_amd64.go
97:	_signo  int32
98:	_code   int32
99:	_errno  int32
100:	_pad    int32
...

(I have to disable structcheck and unused for this in golangci-lint)

[chavacava]

@silverwind thanks for joining to the discussion

could you provide more details about the use of _ in structs?

Prefixing with _ is common for fields in structs when you want to define a struct but not have it exported

Do you mean _ disables the struct or the field to be exported?
What is the difference between

type something struct {
  signo int32
}

and

type something struct {
  _signo int32
}

This is commonly used for forcing users of the struct to specify all fields,

I know that adding a blank field is a hack to force you to use field names when instantiating a struct (https://go.dev/play/p/hJeg6a0D_x9)
But prefixing fields with _ does not have any effect (https://go.dev/play/p/xPuOwJ8CY9m)
What I'm missing?

[...] and to specifically pad the struct.

What do you mean by pad the struct and how _ helps in that?

[twmb]

I know that adding a blank field is a hack to force you to use field names when instantiating a struct
TIL that a field in a struct can literally be named _! I've always used _somename. My concern is largely moot now.

[silverwind]

Can't really comment much on golang practices, but I'm using the ^_ pattern in JavaScript via eslint which allows to define a regex pattern and I thought it would be great to have this flexibilty too in golang.

Eslint has distinct pattern configuration for various types of variables (primarly arguments and regular variables), maybe this is something to consider to split into multiple options too.

[comdiv]

I've not see allwedRegex argumnet in unused-parameter implementation, so i think it's not too hard add it there with ^_$ by default.

[comdiv]

Think same is for unused-receiver

[comdiv]

Provided MR for this feature

[chavacava]

Closed by #858

[ccoVeille]

The discussion was quite long, the fix is great. But I think it would be interesting if the use case could be explained in more details.

I mean the _ prefix is pretty strange. After reading the discussion, I'm still unsure if _ prefix has a consequence on exportability in Go

@silverwind talked about it to replicate a pattern used in JavaScript

I'm still unsure what was the whole purpose of the feature/issue it try to solve

[chavacava]

Salut @ccoVeille,

The _ prefix is not a Go idiom at all. Using it has no effect in the semantic of identifiers.
In other languages like Python or JavaScript it is somewhat a convention to prefix private/unused identifiers with _.
My understanding is that this thread was initiated because @Gusted found that refactoring

func (b *footnoteBlockParser) Continue(node ast.Node, reader text.Reader, pc parser.Context)

into

func (b *footnoteBlockParser) Continue(_ ast.Node, reader text.Reader, _ parser.Context)

leads to less understandable code than

func (b *footnoteBlockParser) Continue(_node ast.Node, reader text.Reader, _pc parser.Context)

IMO (check my comments in this thread) using the prefix is not necessary/useful.
I (as the vast majority of the Go community) keep using just _ to name unused parameters.

[ccoVeille]

Thanks @chavacava it's way clearer now. And I agree with you. _node would be a nonsense to me.

Also, I would expect Go to use memory when passing such argument while it won't with _

But I understand the logic of saying that an interface passing two strings as argument that is not used look a bit odd

func (c *Whatever) Format (_ string, _ string) string {
    return "whatever"
}

But I would prefer to keep this syntax than having

func (c *Whatever) Format (_fmt string, _arg string) string {
    return "whatever"
}

[silverwind]

While I still agree that _foo syntax should be allowed optionally, I see that type aliases would be another good option in such cases where the type like string does not give enough info:

type Name = string
type Arg = string

func Format (_ Name, _ Arg) string {
    return "whatever"
}

[ccoVeille]

I see that type aliases would be another good option in such cases where the type like string does not give enough info:\n\ntype Name = string\ntype Arg = string\n\nfunc Format (_ Name, _ Arg) string {\n return "whatever"\n}

That's right

[silverwind]

There is also https://github.com/mvdan/unparam which by default ignores _xxx, so it seems it's a superior version to revive's unused-parameter.