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

Jump to bottom

x/tools/internal/imports: import embed in files containing go:embed directives #50414

Open
zephyrtronium opened this issue Jan 3, 2022 · 4 comments
Open

x/tools/internal/imports: import embed in files containing go:embed directives #50414

zephyrtronium opened this issue Jan 3, 2022 · 4 comments
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@zephyrtronium
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.17.5 linux/amd64

Gopls version info:

$ gopls -v version
Build info
----------
golang.org/x/tools/gopls v0.7.4
    golang.org/x/tools/gopls@v0.7.4 h1:hw8cpqjio1iMwIKbbDkG3MeW4l8R9dY/yqOHqv7HImA=
    github.com/BurntSushi/toml@v0.4.1 h1:GaI7EiDXDRfa8VshkTj7Fym7ha+y8/XxIgD2okUIjLw=
    github.com/google/go-cmp@v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.5.1 h1:OJxoQ/rynoF0dcCdI7cLPktw/hR2cueqYfjm43oqK38=
    golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
    golang.org/x/sys@v0.0.0-20211019181941-9d821ace8654 h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0=
    golang.org/x/text@v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
    golang.org/x/tools@v0.1.9-0.20211209172050-90a85b2969be h1:JRBiPXZpZ1FsceyPRRme0vX394zXC3xlhqu705k9nzM=
    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    honnef.co/go/tools@v0.2.1 h1:/EPr//+UMMXwMTkXvCCoaJDq8cpjMO80Ou+L4PDo2mY=
    mvdan.cc/gofumpt@v0.1.1 h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=
    mvdan.cc/xurls/v2@v2.3.0 h1:59Olnbt67UKpxF1EwVBopJvkSUBmgtb468E4GVWIZ1I=

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/zephyr/.cache/go-build"
GOENV="/home/zephyr/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/zephyr/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/zephyr/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.5"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/zephyr/mygo/cl/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2601496310=/tmp/go-build -gno-record-gcc-switches"

What did you do?

import (
	"strings"
	"testing"
)

//go:embed generate/CL/cl.h
var clSource string

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
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jan 3, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jan 3, 2022
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 7, 2022
@cagedmantis
Copy link
Contributor

/cc @heschi

@heschi
Copy link
Contributor

heschi commented Jan 7, 2022

I think this would be a good feature. I don't expect to get to it myself any time soon but CLs are welcome.

@heschi heschi added help wanted NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 7, 2022
@rhnvrm
Copy link
Contributor

rhnvrm commented Jan 13, 2022

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.

https://github.com/golang/tools/blob/b894a3290fff7ed8373c3156460600f8216a6c2d/internal/imports/fix.go#L317

And in the collect references, add it to the list of references returned by this method:

https://github.com/golang/tools/blob/b894a3290fff7ed8373c3156460600f8216a6c2d/internal/imports/fix.go#L154

Now, I guess if we pass this forward, it will get passed and fixed?

Am I on the right path?

@heschi
Copy link
Contributor

heschi commented Jan 13, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@cagedmantis @rhnvrm @zephyrtronium @gopherbot @heschi