search

dominikh / go-tools

Staticcheck - The advanced Go linter
https://staticcheck.dev
MIT License
6.21k stars 377 forks source link

New check: warn when packages gets moved from golang.org/exp to the stdlib #1584

Open Foxboron opened 3 months ago

Foxboron commented 3 months ago

I just debugged an issue where one part of the codebase was importing log/slog and an older part of the codebase was importing golang.org/x/exp/slog causing me to get very confused why setting the default logger didn't work.

Neither go vet ./... nor staticcheck ./... gave me a warning so I thought this would be a cool thing to check for.

arp242 commented 3 months ago

I wonder if it would make sense to mark x/exp/slog as deprecated?

Foxboron commented 3 months ago

I suspect that would be annoying for everyone that doesn't have a new enough stdlib? Could you avoid false positives?

arp242 commented 3 months ago

Yeah, that's a potential issue.

stdlib's log/slog is more or less a drop-in replacement for x/exp/slog, but for example x/exp/maps has some stuff that didn't make it in stdlib, so "replace this package with stdlib" isn't always so straight-forward.

Something, somewhere needs to keep a list of things, whether that's in stasticcheck or "// Deprecated: ..." comments, or somewhere else.

Somewhat related issue: #499 (stylecheck: configurable import blacklist).

Foxboron commented 3 months ago

I suspect this depends on how you want to solve the check.

You could just check if slog is occupied by multiple packages in your code base, or you could check if you are using slog.Debug from two different packages.