Skip to content

Commit

Permalink
go/parser: remove import path string syntax checking
Browse files Browse the repository at this point in the history
The validity of an import path string is checked by the type checker
(and possibly other tools); it doesn't need to be done by the parser.
Remove the respective code and tests.

Also, adjust a corresponding go/types test which resolves a TODO.

For #54511.

Change-Id: Id1fc80df4e3e83be3ef123da3946ccb8f759779f
Reviewed-on: https://go-review.googlesource.com/c/go/+/424855
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
griesemer committed Aug 19, 2022
1 parent 4a954fa commit 4671aa5
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 68 deletions.
17 changes: 0 additions & 17 deletions src/go/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ import (
"go/internal/typeparams"
"go/scanner"
"go/token"
"strconv"
"strings"
"unicode"
)

// The parser structure holds the parser's internal state.
Expand Down Expand Up @@ -2547,17 +2544,6 @@ func (p *parser) parseStmt() (s ast.Stmt) {

type parseSpecFunction func(doc *ast.CommentGroup, pos token.Pos, keyword token.Token, iota int) ast.Spec

func isValidImport(lit string) bool {
const illegalChars = `!"#$%&'()*,:;<=>?[\]^{|}` + "`\uFFFD"
s, _ := strconv.Unquote(lit) // go/scanner returns a legal string literal
for _, r := range s {
if !unicode.IsGraphic(r) || unicode.IsSpace(r) || strings.ContainsRune(illegalChars, r) {
return false
}
}
return s != ""
}

func (p *parser) parseImportSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token, _ int) ast.Spec {
if p.trace {
defer un(trace(p, "ImportSpec"))
Expand All @@ -2576,9 +2562,6 @@ func (p *parser) parseImportSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Tok
var path string
if p.tok == token.STRING {
path = p.lit
if !isValidImport(path) {
p.error(pos, "invalid import path: "+path)
}
p.next()
} else {
p.expect(token.STRING) // use expect() error handling
Expand Down
45 changes: 0 additions & 45 deletions src/go/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,51 +310,6 @@ type s3b struct { a, b *s3b; c []float }
}
}

var imports = map[string]bool{
`"a"`: true,
"`a`": true,
`"a/b"`: true,
`"a.b"`: true,
`"m\x61th"`: true,
`"greek/αβ"`: true,
`""`: false,

// Each of these pairs tests both `` vs "" strings
// and also use of invalid characters spelled out as
// escape sequences and written directly.
// For example `"\x00"` tests import "\x00"
// while "`\x00`" tests import `<actual-NUL-byte>`.
`"\x00"`: false,
"`\x00`": false,
`"\x7f"`: false,
"`\x7f`": false,
`"a!"`: false,
"`a!`": false,
`"a b"`: false,
"`a b`": false,
`"a\\b"`: false,
"`a\\b`": false,
"\"`a`\"": false,
"`\"a\"`": false,
`"\x80\x80"`: false,
"`\x80\x80`": false,
`"\xFFFD"`: false,
"`\xFFFD`": false,
}

func TestImports(t *testing.T) {
for path, isValid := range imports {
src := fmt.Sprintf("package p; import %s", path)
_, err := ParseFile(token.NewFileSet(), "", src, 0)
switch {
case err != nil && isValid:
t.Errorf("ParseFile(%s): got %v; expected no error", src, err)
case err == nil && !isValid:
t.Errorf("ParseFile(%s): got no error; expected one", src)
}
}
}

func TestCommentGroups(t *testing.T) {
f, err := ParseFile(token.NewFileSet(), "", `
package p /* 1a */ /* 1b */ /* 1c */ // 1d
Expand Down
9 changes: 3 additions & 6 deletions src/go/types/testdata/check/importdecl0/importdecl0b.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,9 @@ import . /* ERROR .unsafe. imported but not used */ "unsafe"
import . "fmt" // declares Println in file scope

import (
// TODO(gri) At the moment, 2 errors are reported because both go/parser
// and the type checker report it. Eventually, this test should not be
// done by the parser anymore.
"" /* ERROR invalid import path */ /* ERROR invalid import path */
"a!b" /* ERROR invalid import path */ /* ERROR invalid import path */
"abc\xffdef" /* ERROR invalid import path */ /* ERROR invalid import path */
"" /* ERROR invalid import path */
"a!b" /* ERROR invalid import path */
"abc\xffdef" /* ERROR invalid import path */
)

// using "math" in this file doesn't affect its use in other files
Expand Down

0 comments on commit 4671aa5

Please sign in to comment.