adamroyjones
Repos
6
Followers
2
Following
2

Events

delete branch
adamroyjones delete branch any
Created at 2 weeks ago
delete branch
adamroyjones delete branch add-gofmt-support
Created at 2 weeks ago
adamroyjones delete branch clarify-issue-template
Created at 3 weeks ago
issue comment
U1000: The "unused" linting rule has misleading behaviour when interacting with generated code

Thanks for the quick and thoughtful response! Thanks, as well, for all of your work on staticcheck—I find it such a valuable tool.

I haven't yet verified that behavior, but that would indeed be a bug. If bar gets special treatment because it's defined in auto-generated code, then anything it uses should also be used, and foo shouldn't get flagged.

The special treatment bar gets is that its U1000-related error is not reported because it's in generated code. I think in the example there are three reasonable options:

  1. to neither flag foo nor bar, as foo is only called by bar and staticcheck chooses, at least by default, to not make determinations about generated code;
  2. to flag both foo and bar, since it is probably an error—even in generated code—for there to be non-exported functions that aren't called; or
  3. to flag only foo, but to note in the error message that it's called in generated code that isn't exported (which is perhaps a bit clumsy).

My instinct would be to have a piece of configuration that toggles between options 1 and 2, since both seem reasonable in different contexts. In the private example that triggered this investigation, we have control of the code generation, so option 2 would have (correctly) flagged an issue in the code generator. If we were at the mercy of uncontrollable code generation, then I think option 1 might feel a bit better.

Those are just feelings, though. You'll have a much better sense than I have of what would be best for staticcheck as a tool.

That is not a conclusion that U1000 could make automatically. We don't know whether the unexported function (bar) has become genuinely unused over time, or if it was meant to be exported. More often than not, a web of unexported functions that have used each other become unused over time, and the correct solution would be to delete both foo and bar.

Yes, quite right—if there's a whole subgraph that's unused (as in this case), it's quite right of staticcheck to report not just any leaf nodes but the whole set of nodes in the subgraph. It absolutely should do this: if foo and bar mutually depended on each other, then staticcheck should tell us that we can delete both, not that we can delete neither.

I'll edit the OP to strike that through. Thanks for setting me straight!

Created at 3 weeks ago

docs: Fix typo in readme; add a detail about why this exists

Created at 3 weeks ago
pull request opened
chore: Add a clarifying note to the bug report template

As I wasted @ldez's time in issue #3354, the least I can do is add a note to the issue template that should disambiguate things a touch and prevent this from reoccurring.

Created at 3 weeks ago

chore: Add a clarifying note to the bug report template

As I wasted @ldez's time in issue #3354, the least I can do is add a note to the issue template that should disambiguate things a touch and prevent this from reoccurring.

Created at 3 weeks ago

chore: Add a clarifying note to the bug report template

As I wasted @ldez's time in issue #3354, the least I can do is add a note to the issue template that should disambiguate things a touch.

Created at 3 weeks ago

chore: Add a clarifying note to the bug report template

As I wasted @ldez's time in issue #3354, the least I can do is add a note to the issue template that should disambiguate things a touch.

Created at 3 weeks ago
adamroyjones create branch clarify-issue-template
Created at 3 weeks ago
opened issue
U1000: The "unused" linting rule has misleading behaviour when interacting with generated code

staticcheck has, in my judgement, some misleading behaviour when it interacts with generated code.

Under some conditions, it produces an error message that doesn't suffice to resolve the underlying problem. (This was first reported here, which led me to here.)

Reproduction

The following example was produced with staticcheck 2022.1.3 (v0.3.3).

The "details" block at the bottom of this section describes the following directory structure (which I've also attached as a tarball here):

.
├── bar.go
├── Dockerfile
├── foo.go
├── go.mod
└── rc.sh

First, recreate this directory structure and set rc.sh to be executable.

  1. If you run ./rc.sh, then staticcheck will report that the function foo is unused.

  2. If you remove the first line from bar.go (that says that the file is generated) and run ./rc.sh, then staticcheck will report that both foo and bar are unused.

  3. If you replace bar with Bar, then, irrespective of the presence of the "first line" in bar.go, no errors are reported.

In my view, the messages in 1 and 2 are misleading. In both cases, the issue is that bar isn't public (and therefore isn't used); foo is only unused as a consequence of bar not being used. This is especially a problem in 1, because there's no way to resolve the problem from the error message alone.

// bar.go
// Code generated by hand. DO NOT EDIT.
package foo

func bar() {
	foo()
}
# Dockerfile
FROM golang:1.19-bullseye
RUN go install honnef.co/go/tools/cmd/staticcheck@latest
RUN mkdir /reprex
WORKDIR /reprex
// foo.go
package foo

func foo() {}
// go.mod
module staticcheck-reprex

go 1.19
# rc.sh
#! /usr/bin/env sh

docker build --tag staticcheck-reprex .

docker container run \
  --mount type=bind,source=$(pwd),destination=/reprex \
  -it \
  staticcheck-reprex \
  staticcheck ./...
Created at 3 weeks ago
closed issue
The "unused" linting rule has misleading behaviour when interacting with generated code

Welcome

  • [X] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • [X] Yes, I've searched similar issues on GitHub and didn't find any.
  • [X] Yes, I've included all information below (version, config, etc).
  • [X] Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

One of the default linters, unused, is described on the golangci-lint site as something that

[c]hecks Go code for unused constants, variables, functions and types

but if a constant, variable, function, or type is only referred to in a generated file, then golangci-lint will act in a misleading way.

I think this should be categorised as a bug, but it may be that I've missed a piece of configuration that I can set.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.50.1 built from 8926a95f on 2022-10-22T10:50:47Z

Configuration file

$ cat .golangci.yml
---
run:
  timeout: 20m

linters:
  disable-all: true
  enable:
    - unused

Go environment

The reproducing example at the end uses a Dockerfile.

Verbose output of running

The reproducing example at the end uses a Dockerfile.

Code example or link to a public repository

The "details" block below describes the following directory structure (which I've also attached as a tarball here):

.
├── bar.go
├── Dockerfile
├── foo.go
├── .golangci.yml
├── go.mod
└── rc.sh

First, recreate this directory structure and set rc.sh to be executable.

  1. If you run ./rc.sh, then golangci-lint will report that the function foo is unused.

  2. If you remove the first line from foo/bar.go (that says the file is generated) and then run ./rc.sh, then golangci-lint will now report that both foo and bar are unused.

  3. If you replace bar with Bar, then, irrespective of the line at the top about the file being generated being present, everything is (correctly) declared as being used.

In my view, the messages in 1 and 2 are misleading. In both cases, the issue is that bar isn't public (and therefore isn't used); foo is only unused as a consequence of bar not being used. This is especially awkward in 1, because there's no way to resolve the problem from the error message alone.

// bar.go
// Code generated by hand. DO NOT EDIT.
package foo

func bar() {
	foo()
}
# Dockerfile
FROM golang:1.19-bullseye

RUN apt-get update && \
  apt-get install curl && \
  curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.50.1

RUN mkdir /reprex
WORKDIR /reprex
// foo.go
package foo

func foo() {}
# .golangci.yml
---
run:
  timeout: 20m

linters:
  disable-all: true
  enable:
    - unused
// go.mod
module golangci-lint-reprex

go 1.19
# rc.sh
#! /usr/bin/env sh

docker build --tag golangci-lint-reprex .

docker container run \
  --mount type=bind,source=$(pwd),destination=/reprex \
  -it \
  golangci-lint-reprex \
  golangci-lint run ./...
Created at 3 weeks ago
issue comment
The "unused" linting rule has misleading behaviour when interacting with generated code

Oh! I didn't realise that was what was meant by "standalone"; golangci-lint seems to stand alone quite well...

I've reproduced the same issues with staticcheck, so I'll port this across to there. Thanks for the pointer!

Created at 3 weeks ago
reopened issue
The "unused" linting rule has misleading behaviour when interacting with generated code

Welcome

  • [X] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • [X] Yes, I've searched similar issues on GitHub and didn't find any.
  • [X] Yes, I've included all information below (version, config, etc).
  • [X] Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

One of the default linters, unused, is described on the golangci-lint site as something that

[c]hecks Go code for unused constants, variables, functions and types

but if a constant, variable, function, or type is only referred to in a generated file, then golangci-lint will act in a misleading way.

I think this should be categorised as a bug, but it may be that I've missed a piece of configuration that I can set.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.50.1 built from 8926a95f on 2022-10-22T10:50:47Z

Configuration file

$ cat .golangci.yml
---
run:
  timeout: 20m

linters:
  disable-all: true
  enable:
    - unused

Go environment

The reproducing example at the end uses a Dockerfile.

Verbose output of running

The reproducing example at the end uses a Dockerfile.

Code example or link to a public repository

The "details" block below describes the following directory structure (which I've also attached as a tarball here):

.
├── bar.go
├── Dockerfile
├── foo.go
├── .golangci.yml
├── go.mod
└── rc.sh

First, recreate this directory structure and set rc.sh to be executable.

  1. If you run ./rc.sh, then golangci-lint will report that the function foo is unused.

  2. If you remove the first line from foo/bar.go (that says the file is generated) and then run ./rc.sh, then golangci-lint will now report that both foo and bar are unused.

  3. If you replace bar with Bar, then, irrespective of the line at the top about the file being generated being present, everything is (correctly) declared as being used.

In my view, the messages in 1 and 2 are misleading. In both cases, the issue is that bar isn't public (and therefore isn't used); foo is only unused as a consequence of bar not being used. This is especially awkward in 1, because there's no way to resolve the problem from the error message alone.

// bar.go
// Code generated by hand. DO NOT EDIT.
package foo

func bar() {
	foo()
}
# Dockerfile
FROM golang:1.19-bullseye

RUN apt-get update && \
  apt-get install curl && \
  curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.50.1

RUN mkdir /reprex
WORKDIR /reprex
// foo.go
package foo

func foo() {}
# .golangci.yml
---
run:
  timeout: 20m

linters:
  disable-all: true
  enable:
    - unused
// go.mod
module golangci-lint-reprex

go 1.19
# rc.sh
#! /usr/bin/env sh

docker build --tag golangci-lint-reprex .

docker container run \
  --mount type=bind,source=$(pwd),destination=/reprex \
  -it \
  golangci-lint-reprex \
  golangci-lint run ./...
Created at 3 weeks ago
issue comment
The "unused" linting rule has misleading behaviour when interacting with generated code

Right—apologies for the noise above. I've replaced the example with someone that should be clearer.

Created at 3 weeks ago
closed issue
"unused" linting rule doesn't examine generated code, leading to errant behaviour

Welcome

  • [X] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • [X] Yes, I've searched similar issues on GitHub and didn't find any.
  • [X] Yes, I've included all information below (version, config, etc).
  • [X] Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

One of the default linters, unused, is described on the golangci-lint site as something that

[c]hecks Go code for unused constants, variables, functions and types

but if a constant, variable, function, or type is only referred to in a generated file, then golangci-lint will not act correctly. This is because golangci-lint ignores generated files.

I think this should be categorised as a bug, but it may be that I've missed a piece of configuration that I can set.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.50.1 built from 8926a95f on 2022-10-22T10:50:47Z

Configuration file

$ cat .golangci.yml
---
run:
  timeout: 20m

linters:
  disable-all: true
  enable:
    - unused

Go environment

The reproducing example at the end uses a Dockerfile.

Verbose output of running

The reproducing example at the end uses a Dockerfile.

Code example or link to a public repository

The "details" block below describes the following directory structure (which I've also attached as a tarball here):

.
├── Dockerfile
├── foo
│   ├── bar.go
│   └── foo.go
├── .golangci.yml
├── go.mod
├── main.go
└── rc.sh

If you recreate this directory structure, set rc.sh to be executable, and run ./rc.sh, then golangci-lint will report that only the constant unused is unused.

If you remove the first line from foo/bar.go (that says the file is generated) and then run ./rc.sh, then golangci-lint will now report that qux is also unused.

(I've also seen the dual behaviour of something being described as unused when it's in fact referred to in a generated file.)

# rc.sh
#! /usr/bin/env sh

docker build --tag golangci-lint-reprex .

docker container run \
  --mount type=bind,source=$(pwd),destination=/reprex \
  -it \
  golangci-lint-reprex \
  golangci-lint run ./...
# Dockerfile
FROM golang:1.19-bullseye

RUN apt-get update && \
  apt-get install curl && \
  curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.50.1

RUN mkdir /reprex
WORKDIR /reprex
# .golangci.yml
---
run:
  timeout: 20m

linters:
  disable-all: true
  enable:
    - unused
// go.mod
module golangci-lint-reprex

go 1.19
// main.go
package main

import (
	"golangci-lint-reprex/foo"
)

func main() {
	foo.Foo()
}
// foo/foo.go
package foo

import "fmt"

const unused = "unused"

func Foo() {
	fmt.Println("fooing")
	bar()
}

func baz() {
	fmt.Println("bazzing")
}
// foo/bar.go
// Code generated by hand DO NOT EDIT.
package foo

import (
	"fmt"
)

func bar() {
	fmt.Println("barring")
	baz()
}

func qux() {
	fmt.Println("bazzing")
}
Created at 3 weeks ago
issue comment
"unused" linting rule doesn't examine generated code, leading to errant behaviour

(Closing for now, as I need to clean up the examples and should produce the dual example. Everything's a bit mangled. I'll try to reopen things later today.)

Created at 3 weeks ago
issue comment
"unused" linting rule doesn't examine generated code, leading to errant behaviour

I'll see if I can put together a reproduction of what I mentioned above as the "dual" behaviour, since that type of problem is a bigger concern.

Created at 3 weeks ago
opened issue
"unused" linting rule doesn't examine generated code, leading to false positives

Welcome

  • [X] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • [X] Yes, I've searched similar issues on GitHub and didn't find any.
  • [X] Yes, I've included all information below (version, config, etc).
  • [X] Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

One of the default linters, unused, is described on the golangci-lint site as something that

[c]hecks Go code for unused constants, variables, functions and types

If a constant, variable, function, or type is only referred to in a generated file, then golangci-lint will produce a false positive as it ignores generated files.

It's unclear how to resolve the problem without annotating the code with nolint:unused, which is undesirable.

I think this should be categorised as a bug, but it may be that I've missed a piece of configuration that I can set.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.50.1 built from 8926a95f on 2022-10-22T10:50:47Z

Configuration file

$ cat .golangci.yml
---
run:
  timeout: 20m

linters:
  disable-all: true
  enable:
    - unused

Go environment

The reproducing example at the end uses a Dockerfile.

Verbose output of running

The reproducing example at the end uses a Dockerfile.

Code example or link to a public repository

The below describes the following directory structure (which I've also attached as a tarball here):

.
├── Dockerfile
├── foo
│   ├── bar.go
│   └── foo.go
├── go.mod
├── main.go
└── rc.sh

If you recreate the directory structure, set rc.sh to be executable, and run ./rc.sh, then golangci-lint will report that only the constant unused is unused.

If you remove the first line from foo/bar.go (that says the file is generated) and then run ./rc.sh, then golangci-lint will now report that qux is also unused.

docker build --tag golangci-lint-reprex .

docker container run
--mount type=bind,source=$(pwd),destination=/reprex
-it
golangci-lint-reprex
golangci-lint run ./...


```go
// add your code here
FROM golang:1.19-bullseye

RUN apt-get update && \
  apt-get install curl && \
  curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.50.1

RUN mkdir /reprex
WORKDIR /reprex
// go.mod
module golangci-lint-reprex

go 1.19
// main.go
package main

import (
	"golangci-lint-reprex/foo"
)

func main() {
	foo.Foo()
}
// foo/foo.go
package foo

import "fmt"

const unused = "unused"

func Foo() {
	fmt.Println("fooing")
	bar()
}

func baz() {
	fmt.Println("bazzing")
}
// foo/bar.go
// Code generated by hand DO NOT EDIT.
package foo

import (
	"fmt"
)

func bar() {
	fmt.Println("barring")
	baz()
}

func qux() {
	fmt.Println("bazzing")
}
Created at 3 weeks ago
adamroyjones create tag v4.0.3
Created at 3 weeks ago

fix?: Remove module references to github.com/shuLhan/go-bindata

This also runs go mod tidy and updates the documentation.

Created at 3 weeks ago
adamroyjones create tag v4.0.2
Created at 3 weeks ago

fix: Fix module name

Created at 3 weeks ago
delete branch
adamroyjones delete branch update-dependencies
Created at 3 weeks ago

Update internal dependencies; set Go version to 1.17

Our internal projects use recent versions of several packages in /x/. This commit manually upgrades the versions of those package in this library so that the library is compatible with the versions of those packages that we use.

This is all being done so that our reliance on io/ioutil can be purged.

The following commands were run:

go get -u golang.org/x/crypto
go get -u golang.org/x/mod
go get -u golang.org/x/net
go get -u golang.org/x/text
go get -u golang.org/x/tools

Remove io/ioutil

To quote from the documentation (https://pkg.go.dev/io/ioutil):

Deprecated: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details.

Remove non-v4 files

This is unneeded bloat.

Merge pull request #1 from adamroyjones/update-dependencies

Update dependencies; remove io/ioutil; remove non-v4 files

Created at 3 weeks ago
pull request closed
Update dependencies; remove io/ioutil; remove non-v4 files
Created at 3 weeks ago
pull request opened
Update dependencies; remove io/ioutil; remove non-v4 files
Created at 3 weeks ago
create branch
adamroyjones create branch update-dependencies
Created at 3 weeks ago