]> Cypherpunks repositories - gostls13.git/commitdiff
context: don't depend on fmt
authorBrad Fitzpatrick <bradfitz@golang.org>
Wed, 27 Feb 2019 20:08:36 +0000 (20:08 +0000)
committerBrad Fitzpatrick <bradfitz@golang.org>
Wed, 27 Mar 2019 02:37:56 +0000 (02:37 +0000)
So the net package doesn't indirectly depend on unicode tables.

But we're still not quite there, because a new test added in this CL
reveals that we still have a path to unicode via:

deps_test.go:570:
  TODO(issue 30440): policy violation: net => sort => reflect => unicode

Updates #30440

Change-Id: I710c2061dfbaa8e866c92e6c824bd8df35784165
Reviewed-on: https://go-review.googlesource.com/c/go/+/169080
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/context/context.go
src/context/context_test.go
src/go/build/deps_test.go
src/internal/reflectlite/type.go

index 36f83c7b5b2b202708654a3d3840bc3af9da6394..77298f6531c7cb9cb458573fc85c537ec8815ea2 100644 (file)
@@ -49,7 +49,6 @@ package context
 
 import (
        "errors"
-       "fmt"
        "internal/reflectlite"
        "sync"
        "time"
@@ -338,8 +337,19 @@ func (c *cancelCtx) Err() error {
        return err
 }
 
+type stringer interface {
+       String() string
+}
+
+func contextName(c Context) string {
+       if s, ok := c.(stringer); ok {
+               return s.String()
+       }
+       return reflectlite.TypeOf(c).String()
+}
+
 func (c *cancelCtx) String() string {
-       return fmt.Sprintf("%v.WithCancel", c.Context)
+       return contextName(c.Context) + ".WithCancel"
 }
 
 // cancel closes c.done, cancels each of c's children, and, if
@@ -420,7 +430,9 @@ func (c *timerCtx) Deadline() (deadline time.Time, ok bool) {
 }
 
 func (c *timerCtx) String() string {
-       return fmt.Sprintf("%v.WithDeadline(%s [%s])", c.cancelCtx.Context, c.deadline, time.Until(c.deadline))
+       return contextName(c.cancelCtx.Context) + ".WithDeadline(" +
+               c.deadline.String() + " [" +
+               time.Until(c.deadline).String() + "])"
 }
 
 func (c *timerCtx) cancel(removeFromParent bool, err error) {
@@ -481,8 +493,23 @@ type valueCtx struct {
        key, val interface{}
 }
 
+// stringify tries a bit to stringify v, without using fmt, since we don't
+// want context depending on the unicode tables. This is only used by
+// *valueCtx.String().
+func stringify(v interface{}) string {
+       if s, ok := v.(stringer); ok {
+               return s.String()
+       }
+       if s, ok := v.(string); ok {
+               return s
+       }
+       return "<not Stringer>"
+}
+
 func (c *valueCtx) String() string {
-       return fmt.Sprintf("%v.WithValue(%#v, %#v)", c.Context, c.key, c.val)
+       return contextName(c.Context) + ".WithValue(type " +
+               reflectlite.TypeOf(c.key).String() +
+               ", val " + stringify(c.val) + ")"
 }
 
 func (c *valueCtx) Value(key interface{}) interface{} {
index f73f2837b80e1d23209f5936859c3c2fae828755..0cec1699158189785c498796b0ec8366e5da0cf0 100644 (file)
@@ -343,7 +343,7 @@ func XTestValues(t testingT) {
        c1 := WithValue(Background(), k1, "c1k1")
        check(c1, "c1", "c1k1", "", "")
 
-       if got, want := fmt.Sprint(c1), `context.Background.WithValue(1, "c1k1")`; got != want {
+       if got, want := fmt.Sprint(c1), `context.Background.WithValue(type context.key1, val c1k1)`; got != want {
                t.Errorf("c.String() = %q want %q", got, want)
        }
 
index e9ea0fabd80cfd559232ee02659a0ce8bcf5d0f3..92b115eb5353c7570222441829cf561366598753 100644 (file)
@@ -249,7 +249,7 @@ var pkgDeps = map[string][]string{
        "compress/gzip":                  {"L4", "compress/flate"},
        "compress/lzw":                   {"L4"},
        "compress/zlib":                  {"L4", "compress/flate"},
-       "context":                        {"errors", "fmt", "internal/reflectlite", "sync", "time"},
+       "context":                        {"errors", "internal/reflectlite", "sync", "time"},
        "database/sql":                   {"L4", "container/list", "context", "database/sql/driver", "database/sql/internal"},
        "database/sql/driver":            {"L4", "context", "time", "database/sql/internal"},
        "debug/dwarf":                    {"L4"},
@@ -520,15 +520,21 @@ func TestDependencies(t *testing.T) {
        }
        sort.Strings(all)
 
+       sawImport := map[string]map[string]bool{} // from package => to package => true
+
        for _, pkg := range all {
                imports, err := findImports(pkg)
                if err != nil {
                        t.Error(err)
                        continue
                }
+               if sawImport[pkg] == nil {
+                       sawImport[pkg] = map[string]bool{}
+               }
                ok := allowed(pkg)
                var bad []string
                for _, imp := range imports {
+                       sawImport[pkg][imp] = true
                        if !ok[imp] {
                                bad = append(bad, imp)
                        }
@@ -537,6 +543,34 @@ func TestDependencies(t *testing.T) {
                        t.Errorf("unexpected dependency: %s imports %v", pkg, bad)
                }
        }
+
+       // depPath returns the path between the given from and to packages.
+       // It returns the empty string if there's no dependency path.
+       var depPath func(string, string) string
+       depPath = func(from, to string) string {
+               if sawImport[from][to] {
+                       return from + " => " + to
+               }
+               for pkg := range sawImport[from] {
+                       if p := depPath(pkg, to); p != "" {
+                               return from + " => " + p
+                       }
+               }
+               return ""
+       }
+
+       // Also test some high-level policy goals are being met by not finding
+       // these dependency paths:
+       badPaths := []struct{ from, to string }{
+               {"net", "unicode"},
+       }
+
+       for _, path := range badPaths {
+               if how := depPath(path.from, path.to); how != "" {
+                       t.Logf("TODO(issue 30440): policy violation: %s", how)
+               }
+       }
+
 }
 
 var buildIgnore = []byte("\n// +build ignore")
index faecb8755d1140ee54de07608028a8f9fe251d8c..33754646477b2035e8cad7bb15ea3968b5665c14 100644 (file)
@@ -47,6 +47,13 @@ type Type interface {
        // Comparable reports whether values of this type are comparable.
        Comparable() bool
 
+       // String returns a string representation of the type.
+       // The string representation may use shortened package names
+       // (e.g., base64 instead of "encoding/base64") and is not
+       // guaranteed to be unique among types. To test for type identity,
+       // compare the Types directly.
+       String() string
+
        // Elem returns a type's element type.
        // It panics if the type's Kind is not Ptr.
        Elem() Type