Skip to content

Handle Quoted Paths #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
EliSauder wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Handle Quoted Paths #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0390322
feat(impl.stripPaths): handle quoted paths
EliSauder Dec 10, 2025
11942c5
feat(impl.findInterface): handle quoted paths
EliSauder Dec 10, 2025
7325163
Revert misc formatting change in isPathRune
EliSauder Dec 10, 2025
86ebc9b
feat: add trimPathPrefix and trimPathSuffix functions
EliSauder Dec 10, 2025
51198be
refactor: replace tests containing \" with ``
EliSauder Dec 10, 2025
3b11bc0
feat: add in unbalanced and double quoted path tests
EliSauder Dec 12, 2025
41929ca
feat(stripPaths): error check unbalanced and double quoted paths
EliSauder Dec 12, 2025
c8ed998
test(findInterface): remove unsupported test cases
EliSauder Dec 12, 2025
7542082
refactor(stripPaths): clean up and document stripPaths
EliSauder Dec 12, 2025
02eeb5f
refactor(stripPaths): rename runes to remain for clarity
EliSauder Dec 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 113 additions & 25 deletions impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,38 +80,123 @@ func parseType(in string) (Type, error) {
// "Iface[a/b.T, c/d.U]" -> "Iface[b.T, d.U]"
// "Iface[a/b.Other[c/d.T]]" -> "Iface[b.Other[d.T]]"
// "Iface[*a/b.T]" -> "Iface[*b.T]"
func stripPaths(in string) string {
runes := []rune(in)
out := make([]rune, 0, len(runes))
for len(runes) > 0 {
// Find extent of path-like segment
n := slices.IndexFunc(runes, isNonPathRune)
seg := runes
if n >= 0 {
seg = seg[:n]
}
if slash := lastIndex(seg, '/'); slash >= 0 {
seg = seg[slash+1:]
}
// For more examples, see the tests in impl_test.go
//
// Because of the staggered parsing, the handling of quoted paths is done in
// a way that supports seeing the start and end quote in separate loop
// iterations.
//
// Algorithm Example:
// Given: "github.com/foo".Iface
//
// 1. First iteration
// a. Grab all path characters (none, because " is first)
// b. Grab all non-path characters (")
// c. Strip quotes (remove only the one)
// 2. Second iteration
// a. Grab all path characters (github.com/foo)
// b. Grab all non-path characters (")
// c. Strip second quote
// 3. etc
func stripPaths(in string) (string, error) {
remain := []rune(in)
out := make([]rune, 0, len(remain))
quotesRemoved := 0

for len(remain) > 0 {
var seg []rune
var more bool
Copy link
Owner

@josharian josharian Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very optional: change more to done, because done is marginally nicer than !more as a condition.


seg, remain, more = getPathSeg(remain)
out = append(out, seg...)
if n == -1 {
if !more {
break
}
runes = runes[n:]

// Copy non-path runes verbatim
n = slices.IndexFunc(runes, isPathRune)
seg = runes
if n >= 0 {
seg = seg[:n]
seg, remain, more = getNonPathSeg(remain, &quotesRemoved)
// Check for remaining quote in segment. This is to handle
// double quotes
if checkForQuote(seg) {
return "", fmt.Errorf("double quotes")
}
out = append(out, seg...)
if n == -1 {
if !more {
break
}
runes = runes[n:]
}
return string(out)

// We want balanced quotes for our paths
if quotesRemoved % 2 != 0 {
Copy link
Owner

@josharian josharian Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

return "", fmt.Errorf("unbalanced quotes")
}

return string(out), nil
}

func getNonPathSeg(runes []rune, quotesRemoved *int) (seg []rune, remain []rune, more bool) {
Copy link
Owner

@josharian josharian Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please return quotesRemoved instead of passing pointer. and it looks like it can be a bool (singular quoteRemoved), not an int, which makes it clearer.

// Get index of next path character
n := slices.IndexFunc(runes, isPathRune)
// Copy all characters before the path character
seg = runes
if n >= 0 {
seg = seg[:n]
}
// Trim a quote from the segment
lenPreTrim := len(seg)
seg = trimPathSeg(seg)
// If a quote was removed, increment the number of quotes removed
// This is for checking that the quotations are balanced
*quotesRemoved += lenPreTrim - len(seg)
// If there are no path like characters, we are done
if n == -1 {
return seg, []rune{}, false
Copy link
Owner

@josharian josharian Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use nil instead of []rune{}

}
remain = runes[n:]
return seg, remain, true
}

func getPathSeg(runes []rune) (seg []rune, remain []rune, more bool) {
Copy link
Owner

@josharian josharian Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seg, remain []rune

// Find first index of a non-path character
n := slices.IndexFunc(runes, isNonPathRune)
// Get characters up to the non-path character
seg = runes
if n >= 0 {
seg = seg[:n]
}
// If there is a path separator, get the segment at
// the end of the path
if slash := lastIndex(seg, '/'); slash >= 0 {
seg = seg[slash+1:]
}
// if there is no non-path like characters, we are done
if n == -1 {
return seg, []rune{}, false
}
remain = runes[n:]
return seg, remain, true
}

func checkForQuote(p []rune) bool {
Copy link
Owner

@josharian josharian Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call this hasQuote

if len(p) == 0 {
return false
}
return p[0] == '"' || p[len(p)-1] == '"'
}

func trimPathSeg(p []rune) []rune {
Copy link
Owner

@josharian josharian Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trimQuote

if len(p) == 0 {
return p
}

if p[0] == '"' {
return p[1:]
}

if p[len(p)-1] == '"' {
return p[:len(p)-1]
}

return p
}

// lastIndex returns the index of the last occurrence of v in s, or -1 if not present.
Expand Down Expand Up @@ -175,8 +260,11 @@ func findInterface(input string, srcDir string) (path string, iface Type, err er
if dot <= slash {
return "", Type{}, fmt.Errorf("invalid interface name: %s", input)
}
path = baseInput[:dot]
id := stripPaths(input[dot+1:])
path = strings.Trim(baseInput[:dot], "\"")
id, err := stripPaths(input[dot+1:])
if err != nil {
return "", Type{}, err
}
iface, err = parseType(id)
if err != nil {
return "", Type{}, err
Expand Down
41 changes: 35 additions & 6 deletions impl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func TestFindInterface(t *testing.T) {
{input: "a/b/c/pkg", wantErr: true},
{input: "a/b/c/pkg.", wantErr: true},
{input: "a/b/c/pkg.Typ", path: "a/b/c/pkg", typ: Type{Name: "Typ"}},
{input: `"a/b/c/pkg".Typ`, path: "a/b/c/pkg", typ: Type{Name: "Typ"}},
{input: "gopkg.in/yaml.v2.Unmarshaler", path: "gopkg.in/yaml.v2", typ: Type{Name: "Unmarshaler"}},
{input: "github.com/josharian/impl/testdata.GenericInterface1[string]", path: "github.com/josharian/impl/testdata", typ: Type{Name: "GenericInterface1", Params: []string{"string"}}},
{input: "github.com/josharian/impl/testdata.GenericInterface1[*string]", path: "github.com/josharian/impl/testdata", typ: Type{Name: "GenericInterface1", Params: []string{"*string"}}},
Expand All @@ -55,6 +56,17 @@ func TestFindInterface(t *testing.T) {
{input: "github.com/josharian/impl/testdata.GenericInterface1[*github.com/josharian/impl/testdata.Struct5]", path: "github.com/josharian/impl/testdata", typ: Type{Name: "GenericInterface1", Params: []string{"*testdata.Struct5"}}},
// Hyphenated package path (hyphens in path segments, not in final package name)
{input: "github.com/go-chi/chi.Router[github.com/some-org/pkg.SomeType]", path: "github.com/go-chi/chi", typ: Type{Name: "Router", Params: []string{"pkg.SomeType"}}},

// Quoted path edge cases - unbalanced/double quotes
//// missing closing quote
Copy link
Owner

@josharian josharian Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment above these explaining why they are here-but-commented-out.

//{input: `"a/b/c/pkg.Typ`, wantErr: true},
//// missing opening quote
//{input: `a/b/c/pkg".Typ`, wantErr: true},
//// quote after type name
//{input: `"a/b/c/pkg.Typ"`, wantErr: true},
//// double quotes
//{input: `""a/b/c/pkg"".Typ`, wantErr: true},
//{input: `"github.com/josharian/impl/testdata".Interface1`, path: "github.com/josharian/impl/testdata", typ: Type{Name: "Interface1"}},
}

for _, tt := range cases {
Expand Down Expand Up @@ -693,35 +705,35 @@ func TestStubGenerationForImplemented(t *testing.T) {
want string
}{
{
desc: "without implemeted methods",
desc: "without implemented methods",
iface: "github.com/josharian/impl/testdata.Interface3",
recv: "r *Implemented",
recvPkg: "testdata",
want: testdata.Interface4Output,
},
{
desc: "without implemeted methods with trailing space",
desc: "without implemented methods with trailing space",
iface: "github.com/josharian/impl/testdata.Interface3",
recv: "r *Implemented ",
recvPkg: "testdata",
want: testdata.Interface4Output,
},
{
desc: "without implemeted methods, with generic receiver",
desc: "without implemented methods, with generic receiver",
iface: "github.com/josharian/impl/testdata.Interface3",
recv: "r *ImplementedGeneric[Type1]",
recvPkg: "testdata",
want: testdata.Interface4GenericOutput,
},
{
desc: "without implemeted methods, with generic receiver with multiple params",
desc: "without implemented methods, with generic receiver with multiple params",
iface: "github.com/josharian/impl/testdata.Interface3",
recv: "r *ImplementedGenericMultipleParams[Type1, Type2]",
recvPkg: "testdata",
want: testdata.Interface4GenericMultipleParamsOutput,
},
{
desc: "without implemeted methods and receiver variable",
desc: "without implemented methods and receiver variable",
iface: "github.com/josharian/impl/testdata.Interface3",
recv: "*Implemented",
recvPkg: "testdata",
Expand Down Expand Up @@ -911,11 +923,22 @@ func TestStripPaths(t *testing.T) {
desc string
input string
want string
wantErr bool
}{
{desc: "no path", input: "Iface", want: "Iface"},
{desc: "simple path", input: "a/b.T", want: "b.T"},
{desc: "simple quoted path", input: `"a/b".T`, want: "b.T"},
{desc: "simple unbalacned quote path", input: "\"a/b.T", wantErr: true},
{desc: "simple unbalacned quote path 2", input: "a/b\".T", wantErr: true},
{desc: "simple double quote path", input: "\"\"a/b\"\".T", wantErr: true},
{desc: "simple unbalanced double quote path", input: "a/b\"\".T", wantErr: true},
{desc: "simple unbalanced double quote path 2", input: "\"\"a/b.T", wantErr: true},
{desc: "deep path", input: "github.com/foo/bar.T", want: "bar.T"},
{desc: "deep quoted path", input: `"github.com/foo/bar".T`, want: "bar.T"},
{desc: "generic with path param", input: "Iface[github.com/foo/bar.T]", want: "Iface[bar.T]"},
{desc: "generic and interface with path param", input: "github.com/foo.Iface[github.com/foo/bar.T]", want: "foo.Iface[bar.T]"},
{desc: "generic and interface with quoted path param", input: `"github.com/foo".Iface["github.com/foo/bar".T]`, want: "foo.Iface[bar.T]"},
{desc: "generic with quoted path param", input: `Iface["github.com/foo/bar".T]`, want: "Iface[bar.T]"},
{desc: "multiple path params", input: "Iface[a/b.T, c/d.U]", want: "Iface[b.T, d.U]"},
{desc: "nested generic with paths", input: "Iface[a/b.Other[c/d.T]]", want: "Iface[b.Other[d.T]]"},
{desc: "pointer to path type", input: "Iface[*a/b.T]", want: "Iface[*b.T]"},
Expand Down Expand Up @@ -962,7 +985,13 @@ func TestStripPaths(t *testing.T) {
for _, tt := range cases {
t.Run(tt.desc, func(t *testing.T) {
t.Parallel()
got := stripPaths(tt.input)
got, err := stripPaths(tt.input)
if err == nil && tt.wantErr {
t.Errorf("stripPaths(%q) = %q, want error", tt.input, got)
}
if err != nil && !tt.wantErr {
t.Errorf("stripPaths(%q) = got error %v, want %q", tt.input, err, tt.want)
}
if got != tt.want {
t.Errorf("stripPaths(%q) = %q, want %q", tt.input, got, tt.want)
}
Expand Down