Skip to content

Commit

Permalink
Tweak the full interface specification code
Browse files Browse the repository at this point in the history
* Add tests.
* Be stricter about validating the data.
* We already have the data, so don’t bother sending it into the parser. This does mean that we’ll accept “packages” like “a b c/foo.Bar”, but that will result in typeSpec returning an error like
“couldn't find package “a b c”: …”, which is perfectly sufficient.
  • Loading branch information
josharian committed May 21, 2016
1 parent f059da9 commit 43306ba
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 26 deletions.
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ func (f *File) Close() error {
panic("not implemented")
}

# or provide a full name by specifying the namespace
impl 'ts *Source' golang.org/x/oauth2.TokenSource
func (ts *Source) Token() (*oauth2.Token, error) {
# You can also provide a full name by specifying the package path.
# This helps in cases where the interface can't be guessed
# just from the package name and interface name.
$ impl 's *Source' golang.org/x/oauth2.TokenSource
func (s *Source) Token() (*oauth2.Token, error) {
panic("not implemented")
}
```
Expand Down
43 changes: 20 additions & 23 deletions impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,40 +35,37 @@ to prevent shell globbing.
// findInterface returns the import path and identifier of an interface.
// For example, given "http.ResponseWriter", findInterface returns
// "net/http", "ResponseWriter".
// If a fully qualified interface is given, such as "net/http.ResponseWriter",
// it simply parses the input.
func findInterface(iface string) (path string, id string, err error) {
if len(strings.Fields(iface)) != 1 {
return "", "", fmt.Errorf("couldn't parse interface: %s", iface)
}

var importPath string
liStash := strings.LastIndex(iface, "/")
liDot := strings.LastIndex(iface, ".")
if liStash > -1 {
// make sure iface is not ending with "/" (e.g. reject net/http/)
if liStash+1 == len(iface) {
if slash := strings.LastIndex(iface, "/"); slash > -1 {
// package path provided
dot := strings.LastIndex(iface, ".")
// make sure iface does not end with "/" (e.g. reject net/http/)
if slash+1 == len(iface) {
return "", "", fmt.Errorf("interface name cannot end with a '/' character: %s", iface)
}
// make sure iface has a "." after "/" (e.g. reject net/http/httputil)
if liDot < liStash {
// make sure iface does not end with "." (e.g. reject net/http.)
if dot+1 == len(iface) {
return "", "", fmt.Errorf("interface name cannot end with a '.' character: %s", iface)
}
// make sure iface has exactly one "." after "/" (e.g. reject net/http/httputil)
if strings.Count(iface[slash:], ".") != 1 {
return "", "", fmt.Errorf("invalid interface name: %s", iface)
}
importPath = iface[:liDot]
iface = iface[liStash+1:]
return iface[:dot], iface[dot+1:], nil
}

var imp []byte
if importPath == "" {
src := []byte("package hack\n" + "var i " + iface)
// If we couldn't determine the import path, goimports will
// auto fix the import path.
imp, err = imports.Process(".", src, nil)
if err != nil {
return "", "", fmt.Errorf("couldn't parse interface: %s", iface)
}
} else {
imp = []byte(fmt.Sprintf(`package hack
import "%s"
var i %s`, importPath, iface))
src := []byte("package hack\n" + "var i " + iface)
// If we couldn't determine the import path, goimports will
// auto fix the import path.
imp, err := imports.Process(".", src, nil)
if err != nil {
return "", "", fmt.Errorf("couldn't parse interface: %s", iface)
}

// imp should now contain an appropriate import.
Expand Down
5 changes: 5 additions & 0 deletions impl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ func TestFindInterface(t *testing.T) {
{iface: "http.ResponseWriter", path: "net/http", id: "ResponseWriter"},
{iface: "net.Tennis", wantErr: true},
{iface: "a + b", wantErr: true},
{iface: "a/b/c/", wantErr: true},
{iface: "a/b/c/pkg", wantErr: true},
{iface: "a/b/c/pkg.", wantErr: true},
{iface: "a/b/c/pkg.Typ", path: "a/b/c/pkg", id: "Typ"},
{iface: "a/b/c/pkg.Typ.Foo", wantErr: true},
}

for _, tt := range cases {
Expand Down

0 comments on commit 43306ba

Please sign in to comment.