Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: unusedparams analysis does not check exported functions #65401

Closed
ruilya opened this issue Jan 31, 2024 · 1 comment
Closed

x/tools/gopls: unusedparams analysis does not check exported functions #65401

ruilya opened this issue Jan 31, 2024 · 1 comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@ruilya
Copy link

ruilya commented Jan 31, 2024

gopls version

v0.14.2

go env

GO111MODULE=''
GOARCH='amd64'
GOBIN='/home/rusyanov/go/bin'
GOCACHE='/home/rusyanov/.cache/go-build'
GOENV='/home/rusyanov/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/rusyanov/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/rusyanov/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go-1.21'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go-1.21/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/rusyanov/src/proj/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3362930225=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Defining a function that does something (for e.g. fmt.Println("sometext")) but is not using it's arguments does not raise a warning.

func F(a int) { // << no warning is raised here, although a is clearly not used and the function has body
	fmt.Printf("test")
}

What did you see happen?

No diagnostic is generated about unused function parameter. Other analyses are ran and displayed as expected.

What did you expect to see?

Warning about unused parameter.

Editor and settings

nvim v0.9.5

local capabilities = require('cmp_nvim_lsp').default_capabilities()

local lspconfig = require("lspconfig")
-- Setup language servers.
lspconfig.gopls.setup({
  capabilities = capabilities,
  settings = {
    gopls = {
      analyses = {
        unusedparams = true,
        shadow = true,
        unusedwrite = true,
        unusedvariable = true,
      },
      staticcheck = true,
      gofumpt = true,
    },
  },
})
require'lspconfig'.clangd.setup{
  capabilities = capabilities
}

full configuration: https://github.com/ilya-rusyanov/nvim-config

Logs

No response

@ruilya ruilya added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Jan 31, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jan 31, 2024
@adonovan
Copy link
Member

adonovan commented Feb 1, 2024

The unusedparams analysis, like most gopls analyzers, uses the golang.org/x/tools/go/analysis framework for modular analysis, which is analogous to separate compilation in a compiler. That means that the analysis of a given package can access the package itself, and summaries of information obtained from the package's dependencies, but it cannot access information from packages that import the current package.

For the specific problem of unused parameters, that means it cannot report that a parameter of an exported function (such as your F) is unused, because another package could assign F to a variable of type func (int) (or a method to an interface of type interface{F(int)}), in which case applying the suggested fix to remove the parameter would break the program. The fix would be unsound.

Until recently, the unusedparams checker was disabled by default because it made exactly that unsound assumption, causing it to frequently give harmful advice. We recently rewrote it so that it is completely sound (no false positives), allowing us to enable it by default. Of course this means that may miss some true positives, but we believe that is the better trade-off.

So this is working as intended. You may find it helpful to periodically run a global analysis tool for detecting dead code, unused fields, unused parameters and so on, as such tools may have better precision; but gopls cannot run such analyses in real time.

@adonovan adonovan closed this as completed Feb 1, 2024
@adonovan adonovan changed the title x/tools/gopls: unusedparams analysis does not check x/tools/gopls: unusedparams analysis does not check exported functions Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants