From a0042044a5f443bc3ffcdbca9fbb8e8694b53532 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 17 Oct 2023 13:41:41 +0000 Subject: [PATCH 1/6] feat(cli): add cliutil/levenshtein package --- cli/cliutil/levenshtein/levenshtein.go | 88 +++++++++++ cli/cliutil/levenshtein/levenshtein_test.go | 165 ++++++++++++++++++++ 2 files changed, 253 insertions(+) create mode 100644 cli/cliutil/levenshtein/levenshtein.go create mode 100644 cli/cliutil/levenshtein/levenshtein_test.go diff --git a/cli/cliutil/levenshtein/levenshtein.go b/cli/cliutil/levenshtein/levenshtein.go new file mode 100644 index 0000000000000..b03b88ffbfca5 --- /dev/null +++ b/cli/cliutil/levenshtein/levenshtein.go @@ -0,0 +1,88 @@ +package levenshtein + +import ( + "golang.org/x/exp/constraints" + "golang.org/x/xerrors" +) + +// Matches returns the closest matches to the needle from the haystack. +// The maxDistance parameter is the maximum Matches distance to consider. +// If no matches are found, an empty slice is returned. +func Matches(needle string, maxDistance int, haystack ...string) (matches []string) { + for _, hay := range haystack { + if Distance(needle, hay) <= maxDistance { + matches = append(matches, hay) + } + } + + return matches +} + +// Distance returns the edit distance between a and b using the +// Wagner-Fischer algorithm. +// A and B must be less than 255 characters long. +func Distance(a, b string) int { + if len(a) > 255 || len(b) > 255 { + panic(xerrors.Errorf("levenshtein: strings too long: %q, %q", a, b)) + } + m := uint8(len(a)) + n := uint8(len(b)) + + // Special cases for empty strings + if m == 0 { + return int(n) + } + if n == 0 { + return int(m) + } + + // Allocate a matrix of size m+1 * n+1 + d := make([][]uint8, 0) + var i, j uint8 + for i = 0; i < m+1; i++ { + di := make([]uint8, n+1) + d = append(d, di) + } + + // Source prefixes + for i = 1; i < m+1; i++ { + d[i][0] = i + } + + // Target prefixes + for j = 1; j < n; j++ { + d[0][j] = j + } + + // Compute the distance + for j = 0; j < n; j++ { + for i = 0; i < m; i++ { + var subCost uint8 + // Equal + if a[i] != b[j] { + subCost = 1 + } + // Don't forget: matrix is +1 size + d[i+1][j+1] = min( + d[i][j+1]+1, // deletion + d[i+1][j]+1, // insertion + d[i][j]+subCost, // substitution + ) + } + } + + return int(d[m][n]) +} + +func min[T constraints.Ordered](ts ...T) T { + if len(ts) == 0 { + panic("min: no arguments") + } + m := ts[0] + for _, t := range ts[1:] { + if t < m { + m = t + } + } + return m +} diff --git a/cli/cliutil/levenshtein/levenshtein_test.go b/cli/cliutil/levenshtein/levenshtein_test.go new file mode 100644 index 0000000000000..907d5c98e60db --- /dev/null +++ b/cli/cliutil/levenshtein/levenshtein_test.go @@ -0,0 +1,165 @@ +package levenshtein_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/cli/cliutil/levenshtein" +) + +func Test_Levenshtein_Matches(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + Name string + Needle string + MaxDistance int + Haystack []string + Expected []string + }{ + { + Name: "empty", + Needle: "", + MaxDistance: 0, + Haystack: []string{}, + Expected: []string{}, + }, + { + Name: "empty haystack", + Needle: "foo", + MaxDistance: 0, + Haystack: []string{}, + Expected: []string{}, + }, + { + Name: "empty needle", + Needle: "", + MaxDistance: 0, + Haystack: []string{"foo"}, + Expected: []string{}, + }, + { + Name: "exact match distance 0", + Needle: "foo", + MaxDistance: 0, + Haystack: []string{"foo", "fob"}, + Expected: []string{"foo"}, + }, + { + Name: "exact match distance 1", + Needle: "foo", + MaxDistance: 1, + Haystack: []string{"foo", "bar"}, + Expected: []string{"foo"}, + }, + { + Name: "not found", + Needle: "foo", + MaxDistance: 1, + Haystack: []string{"bar"}, + Expected: []string{}, + }, + { + Name: "1 deletion", + Needle: "foo", + MaxDistance: 1, + Haystack: []string{"bar", "fo"}, + Expected: []string{"fo"}, + }, + { + Name: "one deletion, two matches", + Needle: "foo", + MaxDistance: 1, + Haystack: []string{"bar", "fo", "fou"}, + Expected: []string{"fo", "fou"}, + }, + { + Name: "one deletion, one addition", + Needle: "foo", + MaxDistance: 1, + Haystack: []string{"bar", "fo", "fou", "f"}, + Expected: []string{"fo", "fou"}, + }, + { + Name: "distance 2", + Needle: "foo", + MaxDistance: 2, + Haystack: []string{"bar", "boo", "boof"}, + Expected: []string{"boo", "boof"}, + }, + } { + tt := tt + t.Run(tt.Name, func(t *testing.T) { + t.Parallel() + actual := levenshtein.Matches(tt.Needle, tt.MaxDistance, tt.Haystack...) + require.ElementsMatch(t, tt.Expected, actual) + }) + } +} + +func Test_Levenshtein_Distance(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + Name string + A string + B string + Expected int + }{ + { + Name: "empty", + A: "", + B: "", + Expected: 0, + }, + { + Name: "a empty", + A: "", + B: "foo", + Expected: 3, + }, + { + Name: "b empty", + A: "foo", + B: "", + Expected: 3, + }, + { + Name: "a is b", + A: "foo", + B: "foo", + Expected: 0, + }, + { + Name: "one addition", + A: "foo", + B: "fooo", + Expected: 1, + }, + { + Name: "one deletion", + A: "fooo", + B: "foo", + Expected: 1, + }, + { + Name: "one substitution", + A: "foo", + B: "fou", + Expected: 1, + }, + { + Name: "different strings entirely", + A: "foo", + B: "bar", + Expected: 3, + }, + } { + tt := tt + t.Run(tt.Name, func(t *testing.T) { + t.Parallel() + actual := levenshtein.Distance(tt.A, tt.B) + require.Equal(t, tt.Expected, actual) + }) + } +} From c3079ec42c21cec67c63d36a2c4509b2c4f2a08d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 19 Oct 2023 17:07:03 +0100 Subject: [PATCH 2/6] feat(cli): attempt to catch misspelled parameter names --- cli/create_test.go | 25 +++++++++++++++++++++++++ cli/parameterresolver.go | 23 ++++++++++++++++++++--- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index 993ae9e57b441..0f3f06eb1db1d 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -391,6 +391,31 @@ func TestCreateWithRichParameters(t *testing.T) { } <-doneChan }) + + t.Run("WrongParameterName/DidYouMean", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + owner := coderdtest.CreateFirstUser(t, client) + member, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, echoResponses) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + + wrongFirstParameterName := "frst-prameter" + inv, root := clitest.New(t, "create", "my-workspace", "--template", template.Name, + "--parameter", fmt.Sprintf("%s=%s", wrongFirstParameterName, firstParameterValue), + "--parameter", fmt.Sprintf("%s=%s", secondParameterName, secondParameterValue), + "--parameter", fmt.Sprintf("%s=%s", immutableParameterName, immutableParameterValue)) + clitest.SetupConfig(t, member, root) + pty := ptytest.New(t).Attach(inv) + inv.Stdout = pty.Output() + inv.Stderr = pty.Output() + err := inv.Run() + assert.ErrorContains(t, err, "parameter \""+wrongFirstParameterName+"\" is not present in the template") + assert.ErrorContains(t, err, "Did you mean: "+firstParameterName) + }) } func TestCreateValidateRichParameters(t *testing.T) { diff --git a/cli/parameterresolver.go b/cli/parameterresolver.go index 3b8ee3a855ab3..21a31825bd0cf 100644 --- a/cli/parameterresolver.go +++ b/cli/parameterresolver.go @@ -2,14 +2,15 @@ package cli import ( "fmt" + "strings" "golang.org/x/xerrors" - "github.com/coder/pretty" - "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" + "github.com/coder/coder/v2/cli/cliutil/levenshtein" "github.com/coder/coder/v2/codersdk" + "github.com/coder/pretty" ) type WorkspaceCLIAction int @@ -163,7 +164,7 @@ func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuil for _, r := range resolved { tvp := findTemplateVersionParameter(r, templateVersionParameters) if tvp == nil { - return xerrors.Errorf("parameter %q is not present in the template", r.Name) + return templateVersionParametersNotFound(r.Name, templateVersionParameters) } if tvp.Ephemeral && !pr.promptBuildOptions && findWorkspaceBuildParameter(tvp.Name, pr.buildOptions) == nil { @@ -254,3 +255,19 @@ func isValidTemplateParameterOption(buildParameter codersdk.WorkspaceBuildParame } return false } + +func templateVersionParametersNotFound(unknown string, params []codersdk.TemplateVersionParameter) error { + var sb strings.Builder + _, _ = sb.WriteString(fmt.Sprintf("parameter %q is not present in the template.", unknown)) + // Going with a fairly generous edit distance + maxDist := len(unknown) / 2 + var paramNames []string + for _, p := range params { + paramNames = append(paramNames, p.Name) + } + matches := levenshtein.Matches(unknown, maxDist, paramNames...) + if len(matches) > 0 { + _, _ = sb.WriteString(fmt.Sprintf("\nDid you mean: %s", strings.Join(matches, ", "))) + } + return xerrors.Errorf(sb.String()) +} From 00a095b1cbad15466b6614052d3f445903cc3554 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 19 Oct 2023 17:14:00 +0100 Subject: [PATCH 3/6] levenshtein: do not panic --- cli/cliutil/levenshtein/levenshtein.go | 17 ++++++++++------- cli/cliutil/levenshtein/levenshtein_test.go | 3 ++- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cli/cliutil/levenshtein/levenshtein.go b/cli/cliutil/levenshtein/levenshtein.go index b03b88ffbfca5..b26352afe1ddb 100644 --- a/cli/cliutil/levenshtein/levenshtein.go +++ b/cli/cliutil/levenshtein/levenshtein.go @@ -10,7 +10,7 @@ import ( // If no matches are found, an empty slice is returned. func Matches(needle string, maxDistance int, haystack ...string) (matches []string) { for _, hay := range haystack { - if Distance(needle, hay) <= maxDistance { + if d, err := Distance(needle, hay); err == nil && d <= maxDistance { matches = append(matches, hay) } } @@ -21,19 +21,22 @@ func Matches(needle string, maxDistance int, haystack ...string) (matches []stri // Distance returns the edit distance between a and b using the // Wagner-Fischer algorithm. // A and B must be less than 255 characters long. -func Distance(a, b string) int { - if len(a) > 255 || len(b) > 255 { - panic(xerrors.Errorf("levenshtein: strings too long: %q, %q", a, b)) +func Distance(a, b string) (int, error) { + if len(a) > 255 { + return 0, xerrors.Errorf("levenshtein: a must be less than 255 characters long") + } + if len(b) > 255 { + return 0, xerrors.Errorf("levenshtein: b must be less than 255 characters long") } m := uint8(len(a)) n := uint8(len(b)) // Special cases for empty strings if m == 0 { - return int(n) + return int(n), nil } if n == 0 { - return int(m) + return int(m), nil } // Allocate a matrix of size m+1 * n+1 @@ -71,7 +74,7 @@ func Distance(a, b string) int { } } - return int(d[m][n]) + return int(d[m][n]), nil } func min[T constraints.Ordered](ts ...T) T { diff --git a/cli/cliutil/levenshtein/levenshtein_test.go b/cli/cliutil/levenshtein/levenshtein_test.go index 907d5c98e60db..d3bb26ff13d42 100644 --- a/cli/cliutil/levenshtein/levenshtein_test.go +++ b/cli/cliutil/levenshtein/levenshtein_test.go @@ -158,7 +158,8 @@ func Test_Levenshtein_Distance(t *testing.T) { tt := tt t.Run(tt.Name, func(t *testing.T) { t.Parallel() - actual := levenshtein.Distance(tt.A, tt.B) + actual, err := levenshtein.Distance(tt.A, tt.B) + require.NoError(t, err) require.Equal(t, tt.Expected, actual) }) } From 06fa1c3186b9ef60d715368944a4d4911d5c3f52 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 24 Oct 2023 10:42:58 -0400 Subject: [PATCH 4/6] add max distance limit --- cli/cliutil/levenshtein/levenshtein.go | 15 +++++++++--- cli/cliutil/levenshtein/levenshtein_test.go | 27 ++++++++++++++++++--- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/cli/cliutil/levenshtein/levenshtein.go b/cli/cliutil/levenshtein/levenshtein.go index b26352afe1ddb..382a10bbc0da0 100644 --- a/cli/cliutil/levenshtein/levenshtein.go +++ b/cli/cliutil/levenshtein/levenshtein.go @@ -10,7 +10,7 @@ import ( // If no matches are found, an empty slice is returned. func Matches(needle string, maxDistance int, haystack ...string) (matches []string) { for _, hay := range haystack { - if d, err := Distance(needle, hay); err == nil && d <= maxDistance { + if d, err := Distance(needle, hay, maxDistance); err == nil && d <= maxDistance { matches = append(matches, hay) } } @@ -18,10 +18,14 @@ func Matches(needle string, maxDistance int, haystack ...string) (matches []stri return matches } +var ErrMaxDist error = xerrors.New("levenshtein: maxDist exceeded") + // Distance returns the edit distance between a and b using the // Wagner-Fischer algorithm. // A and B must be less than 255 characters long. -func Distance(a, b string) (int, error) { +// maxDist is the maximum distance to consider. +// A value of -1 for maxDist means no maximum. +func Distance(a, b string, maxDist int) (int, error) { if len(a) > 255 { return 0, xerrors.Errorf("levenshtein: a must be less than 255 characters long") } @@ -54,7 +58,7 @@ func Distance(a, b string) (int, error) { // Target prefixes for j = 1; j < n; j++ { - d[0][j] = j + d[0][j] = j // nolint:gosec // this cannot overflow } // Compute the distance @@ -71,12 +75,15 @@ func Distance(a, b string) (int, error) { d[i+1][j]+1, // insertion d[i][j]+subCost, // substitution ) + // check maxDist on the diagonal + if maxDist > -1 && i == j && d[i+1][j+1] > uint8(maxDist) { + return int(d[i+1][j+1]), ErrMaxDist + } } } return int(d[m][n]), nil } - func min[T constraints.Ordered](ts ...T) T { if len(ts) == 0 { panic("min: no arguments") diff --git a/cli/cliutil/levenshtein/levenshtein_test.go b/cli/cliutil/levenshtein/levenshtein_test.go index d3bb26ff13d42..36aea0d5d288b 100644 --- a/cli/cliutil/levenshtein/levenshtein_test.go +++ b/cli/cliutil/levenshtein/levenshtein_test.go @@ -104,63 +104,84 @@ func Test_Levenshtein_Distance(t *testing.T) { Name string A string B string + MaxDist int Expected int + Error string }{ { Name: "empty", A: "", B: "", + MaxDist: -1, Expected: 0, }, { Name: "a empty", A: "", B: "foo", + MaxDist: -1, Expected: 3, }, { Name: "b empty", A: "foo", B: "", + MaxDist: -1, Expected: 3, }, { Name: "a is b", A: "foo", B: "foo", + MaxDist: -1, Expected: 0, }, { Name: "one addition", A: "foo", B: "fooo", + MaxDist: -1, Expected: 1, }, { Name: "one deletion", A: "fooo", B: "foo", + MaxDist: -1, Expected: 1, }, { Name: "one substitution", A: "foo", B: "fou", + MaxDist: -1, Expected: 1, }, { Name: "different strings entirely", A: "foo", B: "bar", + MaxDist: -1, Expected: 3, }, + { + Name: "different strings, max distance 2", + A: "foo", + B: "bar", + MaxDist: 2, + Error: levenshtein.ErrMaxDist.Error(), + }, } { tt := tt t.Run(tt.Name, func(t *testing.T) { t.Parallel() - actual, err := levenshtein.Distance(tt.A, tt.B) - require.NoError(t, err) - require.Equal(t, tt.Expected, actual) + actual, err := levenshtein.Distance(tt.A, tt.B, tt.MaxDist) + if tt.Error == "" { + require.NoError(t, err) + require.Equal(t, tt.Expected, actual) + } else { + require.EqualError(t, err, tt.Error) + } }) } } From 24a04fa10abd5e3a2d58d24a7603288f0393ae65 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 24 Oct 2023 14:57:00 -0400 Subject: [PATCH 5/6] fmt --- cli/cliutil/levenshtein/levenshtein.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/cliutil/levenshtein/levenshtein.go b/cli/cliutil/levenshtein/levenshtein.go index 382a10bbc0da0..f509e5b1000d1 100644 --- a/cli/cliutil/levenshtein/levenshtein.go +++ b/cli/cliutil/levenshtein/levenshtein.go @@ -18,7 +18,7 @@ func Matches(needle string, maxDistance int, haystack ...string) (matches []stri return matches } -var ErrMaxDist error = xerrors.New("levenshtein: maxDist exceeded") +var ErrMaxDist = xerrors.New("levenshtein: maxDist exceeded") // Distance returns the edit distance between a and b using the // Wagner-Fischer algorithm. @@ -84,6 +84,7 @@ func Distance(a, b string, maxDist int) (int, error) { return int(d[m][n]), nil } + func min[T constraints.Ordered](ts ...T) T { if len(ts) == 0 { panic("min: no arguments") From 72c07a21ce0319fc26bd65af058dee4ec70d923b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 6 Nov 2023 11:39:36 +0000 Subject: [PATCH 6/6] test with longer input --- cli/cliutil/levenshtein/levenshtein_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cli/cliutil/levenshtein/levenshtein_test.go b/cli/cliutil/levenshtein/levenshtein_test.go index 36aea0d5d288b..c635ad0564181 100644 --- a/cli/cliutil/levenshtein/levenshtein_test.go +++ b/cli/cliutil/levenshtein/levenshtein_test.go @@ -87,6 +87,13 @@ func Test_Levenshtein_Matches(t *testing.T) { Haystack: []string{"bar", "boo", "boof"}, Expected: []string{"boo", "boof"}, }, + { + Name: "longer input", + Needle: "kuberenetes", + MaxDistance: 5, + Haystack: []string{"kubernetes", "kubeconfig", "kubectl", "kube"}, + Expected: []string{"kubernetes"}, + }, } { tt := tt t.Run(tt.Name, func(t *testing.T) {