From 4671aa5d06811176b964519fbef066f03d4bf884 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 18 Aug 2022 12:21:01 -0700 Subject: [PATCH] go/parser: remove import path string syntax checking 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 Reviewed-by: Robert Griesemer TryBot-Result: Gopher Robot Reviewed-by: Robert Findley --- src/go/parser/parser.go | 17 ------- src/go/parser/parser_test.go | 45 ------------------- .../check/importdecl0/importdecl0b.go | 9 ++-- 3 files changed, 3 insertions(+), 68 deletions(-) diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go index acb71ee9ac..564846a2e8 100644 --- a/src/go/parser/parser.go +++ b/src/go/parser/parser.go @@ -21,9 +21,6 @@ import ( "go/internal/typeparams" "go/scanner" "go/token" - "strconv" - "strings" - "unicode" ) // The parser structure holds the parser's internal state. @@ -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")) @@ -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 diff --git a/src/go/parser/parser_test.go b/src/go/parser/parser_test.go index 0c278924c9..a62dd553ce 100644 --- a/src/go/parser/parser_test.go +++ b/src/go/parser/parser_test.go @@ -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 ``. - `"\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 diff --git a/src/go/types/testdata/check/importdecl0/importdecl0b.go b/src/go/types/testdata/check/importdecl0/importdecl0b.go index 55690423b6..19b55aff76 100644 --- a/src/go/types/testdata/check/importdecl0/importdecl0b.go +++ b/src/go/types/testdata/check/importdecl0/importdecl0b.go @@ -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 -- 2.50.0