-
Notifications
You must be signed in to change notification settings - Fork 18k
x/tools/cmd/deadcode: Deadcode not reported with different argument order in workspace #73652
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
Comments
cc @adonovan |
Related Issues
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.) |
Hi @nicholascapo , thanks for reporting the issue, but I believe you have missed the For example:
@adonovan , I believe we may add a support for go.work support, as in this scenario, the module name of the first package cannot covermultiple go modules under the workspace. We need to make the |
Change https://go.dev/cl/671916 mentions this issue: |
@xieyuschen You are correct, I had missed The only remaining strange thing is why it worked for so long, and only "broke" when I added a module. 🤷 Adding (automatic) workspace support (like the above CR) would be excellent! |
The deadcode command is actually working as intended:
The simplest workaround would be to set the -filter flag explicitly to whatever pattern you prefer. However, I recognize that this behavior may not be what you want. Would it make more sense if instead of privileging the first package on the command line, it looked at all the initial packages and allowed any of their modules? If so, then the necessary code change would be a loop over if *filterFlag == "<module>" {
if mod := initial[0].Module; mod != nil && mod.Path != "" {
*filterFlag = "^" + regexp.QuoteMeta(mod.Path) + "\\b"
} else {
*filterFlag = "" // match any
}
} |
Go version
go version go1.24.2 linux/amd64
Output of
go env
in your module/workspace:What did you do?
Argument order can hide some dead code
Real Life example
At
$work
we use a private monorepo with several services, along with library packages that are shared across services.Each service may contain several other packages that are not shared with other services.
In addition we has a "tools" module that doesn't import much from the library.
Each service is a module, tools is a module, and the library is a module.
The root of the repo is a workspace.
Linter
We have an internal linter that runs
deadcode
and allows us to ignore certain dead functions (etc) based on//nolint:deadcode
comments.So we expect some deadcode to be reported as dead, and then our linter can ignore it.
The linter also reports when code with a
//nolint:deadcode
comment was not reported as dead (that way we can remove the comment).This works pretty well, but isn't actually relevant to the issue.
Problem
It was discovered that running
deadcode
across all the modules sometimes produces a much smaller report than other times.This appears to happen only in specific circumstances, but it is reliable to reproduce.
Additionally, we were able to reproduce it with some package names, but not others.
In one specific case, adding
tools/a/a.go
with a singlepackage a
line (no added dead code), causesdeadcode
to fail to report dead code (from another module).But moving that same file to
tools/x/x.go
does not trigger the bug.While debugging, we discovered that the order of package arguments to
deadcode
reliably causes it to fail.The difference is remarkable too: Depending on the order of the arguments,
deadcode
will report either one or several thousand lines.Minimal Reproduction
Setup
$ tree . ├── go.work ├── lib │ ├── a │ │ └── a.go │ └── go.mod └── svc ├── go.mod └── s └── main.go
This is an example workspace with a service and library module, which each contain a single package.
The only
main()
is insvc/s/main.go
and it does not importlib/a
.Therefore
lib/a.A()
is unused and dead.But this is only found based on the order of the arguments to
deadcode
:What did you see happen?
Based on argument order, in a workspace,
deadcode
may fail to report dead codeWhat did you expect to see?
Reporting of dead code is consistent even with different argument orders
The text was updated successfully, but these errors were encountered: