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

x/tools/gopls: embed errors don't always appear #44342

Open
heschik opened this issue Feb 17, 2021 · 4 comments
Open

x/tools/gopls: embed errors don't always appear #44342

heschik opened this issue Feb 17, 2021 · 4 comments
Labels
Milestone

Comments

@heschik
Copy link
Contributor

@heschik heschik commented Feb 17, 2021

This program shows no error on the embed pattern:

package embedtest

import (
	_ "embed"
)

//go:embed NONEXISTANT
var foo string

When I add an import, the error appears, but only if it's not in the stdlib:

package embedtest

import (
	_ "embed"

	_ "golang.org/x/tools/imports"
)

//go:embed NONEXISTANT
var foo string

At first I thought this was because we should invalidate metadata when a //go:embed comment changes, but even when I restart gopls the IWL doesn't pick up the error. So there's something weird going on here that I didn't dig into. (We should probably still invalidate metadata on embed comment changes, or implement embed pattern checking ourselves.)

Reported by @myitcv in Slack.

@gopherbot gopherbot added this to the Unreleased milestone Feb 17, 2021
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Feb 17, 2021
@heschik
Copy link
Contributor Author

@heschik heschik commented Feb 23, 2021

go/src/go/build/read.go

Lines 174 to 273 in c73232d

// findEmbed advances the input reader to the next //go:embed comment.
// It reports whether it found a comment.
// (Otherwise it found an error or EOF.)
func (r *importReader) findEmbed(first bool) bool {
// The import block scan stopped after a non-space character,
// so the reader is not at the start of a line on the first call.
// After that, each //go:embed extraction leaves the reader
// at the end of a line.
startLine := !first
var c byte
for r.err == nil && !r.eof {
c = r.readByteNoBuf()
Reswitch:
switch c {
default:
startLine = false
case '\n':
startLine = true
case ' ', '\t':
// leave startLine alone
case '"':
startLine = false
for r.err == nil {
if r.eof {
r.syntaxError()
}
c = r.readByteNoBuf()
if c == '\\' {
r.readByteNoBuf()
if r.err != nil {
r.syntaxError()
return false
}
continue
}
if c == '"' {
c = r.readByteNoBuf()
goto Reswitch
}
}
goto Reswitch
case '`':
startLine = false
for r.err == nil {
if r.eof {
r.syntaxError()
}
c = r.readByteNoBuf()
if c == '`' {
c = r.readByteNoBuf()
goto Reswitch
}
}
case '/':
c = r.readByteNoBuf()
switch c {
default:
startLine = false
goto Reswitch
case '*':
var c1 byte
for (c != '*' || c1 != '/') && r.err == nil {
if r.eof {
r.syntaxError()
}
c, c1 = c1, r.readByteNoBuf()
}
startLine = false
case '/':
if startLine {
// Try to read this as a //go:embed comment.
for i := range goEmbed {
c = r.readByteNoBuf()
if c != goEmbed[i] {
goto SkipSlashSlash
}
}
c = r.readByteNoBuf()
if c == ' ' || c == '\t' {
// Found one!
return true
}
}
SkipSlashSlash:
for c != '\n' && r.err == nil && !r.eof {
c = r.readByteNoBuf()
}
startLine = true
}
}
}
return false
}
is the code that reads embed comments if we want to track them.

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 24, 2021

Change https://golang.org/cl/295415 mentions this issue: gopls/internal/regtest: add test for bad embed rules

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 24, 2021

Change https://golang.org/cl/295412 mentions this issue: internal/lsp/cache: parse filenames from go list errors correctly

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 25, 2021

Change https://golang.org/cl/296549 mentions this issue: internal/lsp/cache: invalidate metadata on magic comment changes

@stamblerre stamblerre added this to To Do in gopls: v1.0.0 Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants