Best way to mark unused parameters/error values reported by linter

[Sergei Dyshel]

Hello all,

I'm incorporating golangci-lint linter aggregator in my codebase and facing a dilemma  on how to mark false positives for these 2  linters:

• errcheck - reports unchecked errors.
• unparam - reports unused function parameters.

The "official" way is to use special comment directives that make linter ignore the line:

func foo(

name string,

//nolint:unparam

age int,

) {

//nolint:errcheck

file, _ := os.Open(name)

....

Another, may be more portable way, would be using special functions which do nothing:

// defined in some library utilities package

func IgnoreErr(err error) {}

func Unused(a any) {}

func foo(

name string,

age int,

) {

Unused(age)

file, err := os.Open(name)

IgnoreErr(err)

...

What do you think the proper/idiomatic/better way among these two?

[Steven Hartland]

I would use the following if I truly wanted to ignore an error, but I would question if ignoring the error from os.Open was ever a good idea. If you want to handle a file not found error then do so, don't just ignore the error.

file, _ := os.Open(name) //nolint: errcheck

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/6fc8a933-b68c-4e07-a4ee-98c4fe01ba8dn%40googlegroups.com.

[Marcello H]

What I do (and is a good practice), is adding behind the comment more info.

//nolint:errcheck // no need to test it, because it was tested before with stat

Op maandag 6 maart 2023 om 23:05:42 UTC+1 schreef Steven Hartland:

[Duncan Harris]

What is the reason for including the unused parameter in the function signature?

If it was a method and the reason was to comply with an interface signature then you can assert the interface to get unparam to ignore:

var _ InterfaceType = (*MyType)(nil)

or if not a pointer receiver

var _ InterfaceType = MyType{}

Usually I would declare an unused parameter with the underscore name:

func foo(

name string,

_ int, // age

) {

Duncan

[Kurtis Rader]

On Mon, Mar 6, 2023 at 10:38 PM Marcello H <marc...@gmail.com> wrote:

What I do (and is a good practice), is adding behind the comment more info.

//nolint:errcheck // no need to test it, because it was tested before with stat

I appreciate that you're replying to what I hope is just a bad example but please, never do that. It's a security bug. And even without a malicious actor the path might change between the stat and the open. If you need to check some attributes of the file always open it first (and check that the open succeeded) then use the Stat method of the File object (https://pkg.go.dev/os#File.Stat). 

 

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/26a4f5d9-f590-4d40-a333-d0bfbe191ca3n%40googlegroups.com.

--

Kurtis Rader

Caretaker of the exceptional canines Junior and Hank

[Marcello H]

The point I was making is not simply to ignore a linter outcome, but add the reason behind it.

It might be a bad comment in your opinion, but that was not the point.

Op di 7 mrt 2023 om 18:42 schreef Kurtis Rader <kra...@skepticism.us>:

[Carla Pfaff]

If a linter reports false positives I don't use it. I would never uglify my source code to please a linter.

[Marcello H]

This was not about false positives ;-)
Your choice, but the linter could still find a lot of good things in the code.
Anyway: you're not typing the comment only for yourself, but also for others to understand, right?

Op woensdag 8 maart 2023 om 12:38:20 UTC+1 schreef Carla Pfaff: