go/printer, cmd/gofmt: print import paths in double quotes

Fixes #9644.

Change-Id: Ia2e42befa20233107ac5409e79f9dce794983a3f
Reviewed-on: https://go-review.googlesource.com/3200
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go
index d5a69349beba8bdcf72ea914e8ed2a74a84ad983..fe047053afb01a934e66eb710051f7acea2b2876 100644 (file)
--- a/src/go/printer/nodes.go
+++ b/src/go/printer/nodes.go
@@ -12,6 +12,9 @@ import (
        "bytes"
        "go/ast"
        "go/token"
+       "strconv"
+       "strings"
+       "unicode"
        "unicode/utf8"
 )
 
@@ -1334,6 +1337,49 @@ func (p *printer) valueSpec(s *ast.ValueSpec, keepType bool) {
        }
 }
 
+func sanitizeImportPath(lit *ast.BasicLit) *ast.BasicLit {
+       // Note: An unmodified AST generated by go/parser will already
+       // contain a backward- or double-quoted path string that does
+       // not contain any invalid characters, and most of the work
+       // here is not needed. However, a modified or generated AST
+       // may possibly contain non-canonical paths. Do the work in
+       // all cases since it's not too hard and not speed-critical.
+
+       // if we don't have a proper string, be conservative and return whatever we have
+       if lit.Kind != token.STRING {
+               return lit
+       }
+       s, err := strconv.Unquote(lit.Value)
+       if err != nil {
+               return lit
+       }
+
+       // if the string is an invalid path, return whatever we have
+       //
+       // spec: "Implementation restriction: A compiler may restrict
+       // ImportPaths to non-empty strings using only characters belonging
+       // to Unicode's L, M, N, P, and S general categories (the Graphic
+       // characters without spaces) and may also exclude the characters
+       // !"#$%&'()*,:;<=>?[\]^`{|} and the Unicode replacement character
+       // U+FFFD."
+       if s == "" {
+               return lit
+       }
+       const illegalChars = `!"#$%&'()*,:;<=>?[\]^{|}` + "`\uFFFD"
+       for _, r := range s {
+               if !unicode.IsGraphic(r) || unicode.IsSpace(r) || strings.ContainsRune(illegalChars, r) {
+                       return lit
+               }
+       }
+
+       // otherwise, return the double-quoted path
+       s = strconv.Quote(s)
+       if s == lit.Value {
+               return lit // nothing wrong with lit
+       }
+       return &ast.BasicLit{ValuePos: lit.ValuePos, Kind: token.STRING, Value: s}
+}
+
 // The parameter n is the number of specs in the group. If doIndent is set,
 // multi-line identifier lists in the spec are indented when the first
 // linebreak is encountered.
@@ -1346,7 +1392,7 @@ func (p *printer) spec(spec ast.Spec, n int, doIndent bool) {
                        p.expr(s.Name)
                        p.print(blank)
                }
-               p.expr(s.Path)
+               p.expr(sanitizeImportPath(s.Path))
                p.setComment(s.Comment)
                p.print(s.EndPos)
 

diff --git a/src/go/printer/testdata/declarations.golden b/src/go/printer/testdata/declarations.golden
index 9acd41b7d288626f124ef7028dea85800ac7a12c..b7be6f80ad88cacec43250e655efa14b92097292 100644 (file)
--- a/src/go/printer/testdata/declarations.golden
+++ b/src/go/printer/testdata/declarations.golden
@@ -110,6 +110,12 @@ import (
        "package_dddd"  // comment
 )
 
+// print import paths as double-quoted strings
+import (
+       "fmt"
+       "math"
+)
+
 // at least one empty line between declarations of different kind
 import _ "io"
 

diff --git a/src/go/printer/testdata/declarations.input b/src/go/printer/testdata/declarations.input
index 45beec25fc7ff4f182d16e6bf9e3ef58497be2c6..a0a3783b846fbed661713caee409bdc41ec1afef 100644 (file)
--- a/src/go/printer/testdata/declarations.input
+++ b/src/go/printer/testdata/declarations.input
@@ -111,6 +111,15 @@ import (
        "package_dddd" // comment
 )
 
+// print import paths as double-quoted strings
+// (we would like more test cases but the go/parser
+// already excludes most incorrect paths, and we don't
+// bother setting up test-ASTs manually)
+import (
+       `fmt`
+       "math"
+)
+
 // at least one empty line between declarations of different kind
 import _ "io"
 var _ int