Open zephyrtronium opened 2 years ago
/cc @heschi
I think this would be a good feature. I don't expect to get to it myself any time soon but CLs are welcome.
Hey
I wanted to work on this and create a CL for this. Just wanted to check if I am on the right path. I think we need to in the first pass figure out if there exists //go:embed
in the AST.
And in the collect references, add it to the list of references returned by this method:
Now, I guess if we pass this forward, it will get passed and fixed?
Am I on the right path?
I haven't thought about this, so I don't have an answer ready. But the embed package doesn't follow the usual rules, so I don't think that approach will work well. For one thing, only the stdlib "embed" package is a valid choice, so the selection logic shouldn't run. For another, renamed imports are still sufficient as far as I know. Consider:
package main
import embed2 "embed"
//go:embed foo.txt
var foo string
var _ embed2.FS
I'm afraid this may be a very special case.
What version of Go are you using (
go version
)?Gopls version info:
Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
What did you expect to see?
gopls via x/tools/internal/imports should add an
import _ "embed"
, because the file requires it for the package to build.What did you see instead?
No import added. The package fails to compile when running
go test
because./manual_test.go:8:3: go:embed only allowed in Go files that import "embed"
.@gopherbot Please add label FeatureRequest