golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.21k stars 17.7k forks source link

x/tools/go/packages: undeprecate Load* mode flags (or fix Need* flags) #70470

Closed findleyr closed 14 hours ago

findleyr commented 1 day ago

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:

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

gabyhelp commented 1 day ago

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

adonovan commented 1 day ago

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 commented 1 day ago

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

gopherbot commented 23 hours ago

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

xieyuschen commented 23 hours ago

@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:)

findleyr commented 14 hours ago

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.