x/tools/gopls: report unnecessarily exported symbols

[adonovan]

After a code reorganization, many symbols may continue to be exported even though they are no longer referenced from another package. Just as it is helpful to report dead code within an application, it would be useful to report symbols that could safely be unexported.

Gopls has all the information required to do this; we should expose it on request in the UI somehow.

(Or this could be a feature of cmd/deadcode. But cmd/deadcode could be a feature of gopls, too.)

[gopherbot]

Change https://go.dev/cl/586781 mentions this issue: gopls/internal/golang: unexport several functions

[adonovan]

We could easily add this as a code action "Show unnecessarily exported symbols" whose associated command should indicate one of two things (that don't currently exist in LSP, but should):

• that its result is a set of generalized references; or
• that its progress messages can be treated like a mixed stdout/stderr stream to a terminal-like viewer (e.g. VS Code output window, Emacs non-file buffer, etc). Automated linkification should suffice.

@findleyr had a neat idea: produce the report as a web page (which we could easily do today), with a list of checkboxes, one per function. The user could select a subset of them then hit an "Apply changes" button, which would cause the server to send one or more ApplyEdit downcalls to do the renamings.

[ian-h-chamberlain]

👍 to this feature.

As a companion feature, a code action to easily toggle exported-ness of a single symbol (or struct member) would also be helpful! That would make it easier to fix one-off instances as you see them, without needing to explicitly run the analysis for the entire package/workspace. I imagine the necessary pieces of functionality would be very similar to those needed for a report like this.

[adonovan]

Some edge cases one might not expect:

• a single enum const (A = iota, B, C) may be unreferenced, but that doesn't mean it should be unexported. Constants should be treated as a group: a reference to one is a reference to all.
• users of T in func NewT() *T { ... }; func (*T) Method() {} may never reference T but that doesn't mean it should be unexported. A reference to a public symbol NewT should count as a reference to its associated type T.

Here are some of the findings of a quick experiment:

gopls/internal/util/frob/frob.go:42:6: Codec is unnecessarily exported from golang.org/x/tools/gopls/internal/util/frob
gopls/internal/cache/methodsets/methodsets.go:96:6: Key is unnecessarily exported from golang.org/x/tools/gopls/internal/cache/methodsets
gopls/internal/cache/methodsets/methodsets.go:111:6: Result is unnecessarily exported from golang.org/x/tools/gopls/internal/cache/methodsets
gopls/internal/cache/typerefs/refs.go:53:6: Class is unnecessarily exported from golang.org/x/tools/gopls/internal/cache/typerefs
gopls/internal/util/lru/lru.go:16:6: Cache is unnecessarily exported from golang.org/x/tools/gopls/internal/util/lru
gopls/internal/filecache/filecache.go:233:6: SetBudget is unnecessarily exported from golang.org/x/tools/gopls/internal/filecache
gopls/internal/vulncheck/types.go:44:2: ModeInvalid is unnecessarily exported from golang.org/x/tools/gopls/internal/vulncheck
gopls/internal/protocol/command/interface.go:508:6: CallStack is unnecessarily exported from golang.org/x/tools/gopls/internal/protocol/command
gopls/internal/protocol/command/interface.go:42:6: Interface is unnecessarily exported from golang.org/x/tools/gopls/internal/protocol/command
gopls/internal/protocol/command/interface.go:615:6: PackagesMode is unnecessarily exported from golang.org/x/tools/gopls/internal/protocol/command
gopls/internal/protocol/command/interface.go:511:6: StackEntry is unnecessarily exported from golang.org/x/tools/gopls/internal/protocol/command
gopls/internal/protocol/command/interface.go:679:6: TestCase is unnecessarily exported from golang.org/x/tools/gopls/internal/protocol/command
gopls/internal/protocol/command/interface.go:660:6: TestFile is unnecessarily exported from golang.org/x/tools/gopls/internal/protocol/command
gopls/internal/fuzzy/matcher.go:16:2: MaxInputSize is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/fuzzy/matcher.go:19:2: MaxPatternSize is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/fuzzy/input.go:24:2: RHead is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/fuzzy/input.go:16:2: RNone is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/fuzzy/input.go:18:2: RSep is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/fuzzy/input.go:20:2: RTail is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/fuzzy/input.go:22:2: RUCTail is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/fuzzy/symbol.go:32:6: SymbolMatcher is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/fuzzy/input.go:160:6: WordConsumer is unnecessarily exported from golang.org/x/tools/gopls/internal/fuzzy
gopls/internal/analysis/fillreturns/fillreturns.go:256:6: FixesError is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/fillreturns
gopls/internal/analysis/infertypeargs/infertypeargs.go:19:7: Doc is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/infertypeargs
gopls/internal/analysis/nonewvars/nonewvars.go:87:6: FixesError is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/nonewvars
gopls/internal/analysis/noresultvalues/noresultvalues.go:83:6: FixesError is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/noresultvalues
gopls/internal/analysis/stubmethods/stubmethods.go:77:6: DiagnosticForError is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/stubmethods
gopls/internal/analysis/stubmethods/stubmethods.go:69:6: MatchesMessage is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/stubmethods
gopls/internal/analysis/stubmethods/stubmethods.go:105:6: StubInfo is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/stubmethods
gopls/internal/util/slices/slices.go:67:6: Grow is unnecessarily exported from golang.org/x/tools/gopls/internal/util/slices
gopls/internal/util/slices/slices.go:29:6: IndexFunc is unnecessarily exported from golang.org/x/tools/gopls/internal/util/slices
gopls/internal/analysis/unusedvariable/unusedvariable.go:22:7: Doc is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/unusedvariable
gopls/internal/analysis/useany/useany.go:20:7: Doc is unnecessarily exported from golang.org/x/tools/gopls/internal/analysis/useany
gopls/internal/util/constraints/constraint.go:42:6: Complex is unnecessarily exported from golang.org/x/tools/gopls/internal/util/constraints
gopls/internal/util/constraints/constraint.go:35:6: Float is unnecessarily exported from golang.org/x/tools/gopls/internal/util/constraints
gopls/internal/util/constraints/constraint.go:28:6: Integer is unnecessarily exported from golang.org/x/tools/gopls/internal/util/constraints
gopls/internal/util/constraints/constraint.go:14:6: Signed is unnecessarily exported from golang.org/x/tools/gopls/internal/util/constraints
gopls/internal/util/constraints/constraint.go:21:6: Unsigned is unnecessarily exported from golang.org/x/tools/gopls/internal/util/constraints
gopls/internal/cache/diagnostics.go:86:2: ConsistencyInfo is unnecessarily exported from golang.org/x/tools/gopls/internal/cache
gopls/internal/cache/diagnostics.go:75:2: UnknownError is unnecessarily exported from golang.org/x/tools/gopls/internal/cache
gopls/internal/debug/log/log.go:25:2: Info is unnecessarily exported from golang.org/x/tools/gopls/internal/debug/log
gopls/internal/protocol/semtok/semtok.go:26:2: TokInterface is unnecessarily exported from golang.org/x/tools/gopls/internal/protocol/semtok
gopls/internal/golang/snapshot.go:65:6: WidestPackageForFile is unnecessarily exported from golang.org/x/tools/gopls/internal/golang
gopls/internal/mod/diagnostics.go:98:6: ModParseDiagnostics is unnecessarily exported from golang.org/x/tools/gopls/internal/mod
gopls/internal/mod/diagnostics.go:110:6: ModTidyDiagnostics is unnecessarily exported from golang.org/x/tools/gopls/internal/mod
gopls/internal/mod/diagnostics.go:137:6: ModUpgradeDiagnostics is unnecessarily exported from golang.org/x/tools/gopls/internal/mod
gopls/internal/mod/diagnostics.go:185:6: ModVulnerabilityDiagnostics is unnecessarily exported from golang.org/x/tools/gopls/internal/mod
gopls/internal/util/browser/browser.go:16:6: Commands is unnecessarily exported from golang.org/x/tools/gopls/internal/util/browser
gopls/internal/protocol/command/commandmeta/meta.go:37:6: Field is unnecessarily exported from golang.org/x/tools/gopls/internal/protocol/command/commandmeta
gopls/internal/test/integration/options.go:80:6: NoLogsOnError is unnecessarily exported from golang.org/x/tools/gopls/internal/test/integration
gopls/internal/vulncheck/vulntest/report.go:145:5: ReferenceTypes is unnecessarily exported from golang.org/x/tools/gopls/internal/vulncheck/vulntest