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/go/packages: undeprecate Load* mode flags (or fix Need* flags) #70470

Closed
findleyr opened this issue Nov 20, 2024 · 6 comments
Closed
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

Especially with the 'deprecated' gopls analyzer, I keep bumping up against the fact that go/packages.LoadAllSyntax is deprecated, yet we continue to use it, and keep adding more uses. The stated reason for continuing to use it are usually (1) it is convenient, and (2) the Need* flags interact surprisingly in complicated ways. (To be honest, I can't recall exactly how NeedFlags don't behave as expected -- perhaps @adonovan remembers).

Assertions:

  • We should definitely fix bugs related to the Need* mode flags, if there are any. I couldn't find any filed.
  • If we prefer to use Load* mode flags, we shouldn't advertise them as deprecated.

Since we're unlikely to have time to prioritize digging deeply into this problem, I think we should just remove the deprecation notices from these mode flags. Given that the original addition did not have a proposal (https://go.dev/cl/173959), I don't think we need a proposal here. (deprecation is a reversible decision).

CC @adonovan @matloob

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 20, 2024
@gopherbot gopherbot added this to the Unreleased milestone Nov 20, 2024
@gabyhelp
Copy link

@adonovan
Copy link
Member

I agree that we should undeprecate them, recommend their use, and improve their documentation.

The Load flags are not only simpler, but they have fewer combinations, which means fewer "colors" (really: subtle Pantone shades) of Packages data structures that algorithms need to deal with.

(To be honest, I can't recall exactly how NeedFlags don't behave as expected -- perhaps @adonovan remembers).

See this list of need-related issues for starters:

@findleyr
Copy link
Contributor Author

Thank you. Somehow this github search fails to return any of those...
https://github.com/golang/go/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+go%2Fpackages+need

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 20, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/630435 mentions this issue: go/packages: undeprecate Load* style flags

@xieyuschen
Copy link
Contributor

xieyuschen commented Nov 21, 2024

@findleyr I think the issues are listed in the comments of LoadMode and here is an add-on. I have sent you a CL for the doc change and hope you like it:)

gopherbot pushed a commit to golang/tools that referenced this issue Nov 21, 2024
Updates golang/go#70470

Change-Id: Ia254b993b301fcc708d07b3773fad31a971c5997
Reviewed-on: https://go-review.googlesource.com/c/tools/+/630435
Reviewed-by: Robert Findley <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@findleyr
Copy link
Contributor Author

Thanks for the contribution! I think we can actually close this issue now, since we do have other issues filed for the broken Need flags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants