Skip to content

Commit 5d413a0

Browse files
committed
fix: name the spelling a vocab term wants, not its pattern
Signed-off-by: Joseph Kato <joseph@jdkato.io>
1 parent 5f071ef commit 5d413a0

3 files changed

Lines changed: 232 additions & 15 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package check
2+
3+
import "testing"
4+
5+
func BenchmarkRecaseToTerm(b *testing.B) {
6+
for _, c := range []struct{ name, term, observed string }{
7+
{"literal", "OpenAPI", "openapi"},
8+
{"optional", "OAuth2?", "Oauth"},
9+
{"alternation", "Docker(file|ize)", "dockerfile"},
10+
{"bails", "[Pp]ython", "python"},
11+
} {
12+
b.Run(c.name, func(b *testing.B) {
13+
b.ReportAllocs()
14+
for i := 0; i < b.N; i++ {
15+
_ = recaseToTerm(c.term, c.observed)
16+
}
17+
})
18+
}
19+
}

internal/check/substitution.go

Lines changed: 133 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"fmt"
55
"sort"
66
"strings"
7+
"sync"
8+
"unicode"
79

810
"golang.org/x/exp/maps"
911

@@ -237,17 +239,139 @@ func literalSkeleton(pattern string) string {
237239
return b.String()
238240
}
239241

240-
// recaseToTerm re-cases observed to a vocab term's canonical form when the term
241-
// is a regex describing a single fixed-length spelling (e.g. `OAuth2?` against
242-
// `oauth2` yields `OAuth2`). It returns term unchanged when the pattern can't
243-
// be cleanly aligned, so the raw regex is only ever shown as a last resort.
244-
// See #997.
242+
// Alternations multiply out fast; past a handful the term is no longer one
243+
// word spelled several ways.
244+
const maxExpansions = 64
245+
246+
// expandPattern enumerates every spelling a vocab term can match -- `OAuth2?`
247+
// gives OAuth2 and OAuth -- or nil if the pattern is not a finite set.
248+
func expandPattern(pattern string) []string {
249+
runes := []rune(pattern)
250+
251+
// parse reads an alternation at i, returning its spellings and the index
252+
// just past what it consumed.
253+
var parse func(i int, nested bool) (out []string, next int, ok bool)
254+
parse = func(i int, nested bool) ([]string, int, bool) {
255+
branches := []string{} // completed alternatives
256+
current := []string{""} // spellings of the branch being read
257+
258+
// cross appends every element of add to every spelling so far.
259+
cross := func(add []string) bool {
260+
if len(current)*len(add) > maxExpansions {
261+
return false
262+
}
263+
next := make([]string, 0, len(current)*len(add))
264+
for _, prefix := range current {
265+
for _, suffix := range add {
266+
next = append(next, prefix+suffix)
267+
}
268+
}
269+
current = next
270+
return true
271+
}
272+
273+
for i < len(runes) {
274+
var atom []string
275+
276+
switch r := runes[i]; r {
277+
case ')':
278+
if !nested {
279+
return nil, 0, false // unbalanced
280+
}
281+
return append(branches, current...), i + 1, true
282+
case '|':
283+
branches = append(branches, current...)
284+
current = []string{""}
285+
i++
286+
continue
287+
case '(':
288+
start := i + 1
289+
// Capturing or not makes no difference to the spellings.
290+
if strings.HasPrefix(string(runes[start:]), "?:") {
291+
start += 2
292+
} else if start < len(runes) && runes[start] == '?' {
293+
return nil, 0, false // lookaround, named group, flags
294+
}
295+
inner, next, ok := parse(start, true)
296+
if !ok {
297+
return nil, 0, false
298+
}
299+
atom, i = inner, next
300+
case '\\':
301+
if i+1 >= len(runes) {
302+
return nil, 0, false
303+
}
304+
// `\d`, `\w`, `\b` are not fixed spellings.
305+
next := runes[i+1]
306+
if unicode.IsLetter(next) || unicode.IsDigit(next) {
307+
return nil, 0, false
308+
}
309+
atom = []string{string(next)}
310+
i += 2
311+
case '[', ']', '{', '}', '.', '*', '+', '^', '$':
312+
return nil, 0, false
313+
default:
314+
atom = []string{string(r)}
315+
i++
316+
}
317+
318+
// `?` makes the atom optional: add the empty spelling too.
319+
if i < len(runes) && runes[i] == '?' {
320+
atom = append(append([]string{}, atom...), "")
321+
i++
322+
} else if i < len(runes) && (runes[i] == '*' || runes[i] == '+' || runes[i] == '{') {
323+
return nil, 0, false
324+
}
325+
326+
if !cross(atom) {
327+
return nil, 0, false
328+
}
329+
}
330+
331+
if nested {
332+
return nil, 0, false // unterminated group
333+
}
334+
return append(branches, current...), i, true
335+
}
336+
337+
out, next, ok := parse(0, false)
338+
if !ok || next != len(runes) || len(out) == 0 {
339+
return nil
340+
}
341+
342+
return out
343+
}
344+
345+
// Computed once per term, not once per alert; a run can raise hundreds of
346+
// thousands against the same few terms.
347+
var expansions sync.Map // pattern -> []string
348+
349+
func expansionsFor(term string) []string {
350+
if cached, ok := expansions.Load(term); ok {
351+
return cached.([]string)
352+
}
353+
354+
out := expandPattern(term)
355+
if out == nil {
356+
if skel := literalSkeleton(term); skel != "" {
357+
out = []string{skel}
358+
}
359+
}
360+
361+
expansions.Store(term, out)
362+
return out
363+
}
364+
365+
// recaseToTerm re-cases observed to a vocab term's canonical spelling, e.g.
366+
// `OAuth2?` against `Oauth` yields `OAuth`. It returns term unchanged when no
367+
// spelling matches, so the raw regex is only ever a last resort. See #997.
245368
func recaseToTerm(term, observed string) string {
246-
skel := literalSkeleton(term)
247-
if skel == "" || !strings.EqualFold(skel, observed) {
248-
return term
369+
for _, candidate := range expansionsFor(term) {
370+
if strings.EqualFold(candidate, observed) {
371+
return candidate
372+
}
249373
}
250-
return skel
374+
return term
251375
}
252376

253377
func convertMessage(s string) string {

internal/check/substitution_test.go

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package check
22

33
import (
4+
"slices"
5+
"sort"
6+
"strings"
47
"testing"
58

69
"github.com/errata-ai/vale/v3/internal/core"
@@ -164,12 +167,22 @@ func TestRecaseToTerm(t *testing.T) {
164167
cases := []struct {
165168
term, observed, want string
166169
}{
167-
{"OAuth2?", "oauth2", "OAuth2"}, // optional char present
168-
{"OpenAPI", "openapi", "OpenAPI"}, // plain literal
169-
{`Wi\-?Fi`, "wi-fi", "Wi-Fi"}, // escaped literal hyphen, aligned
170-
{"Wi-?Fi", "wifi", "Wi-?Fi"}, // optional char absent -> fall back
171-
{"[Pp]ython", "python", "[Pp]ython"}, // class -> fall back
172-
{"(?:foo|bar)", "foo", "(?:foo|bar)"}, // group/alternation -> fall back
170+
{"OAuth2?", "oauth2", "OAuth2"}, // optional char present
171+
{"OAuth2?", "Oauth", "OAuth"}, // optional char absent -- see #997
172+
{"OpenAPI", "openapi", "OpenAPI"}, // plain literal
173+
{`Wi\-?Fi`, "wi-fi", "Wi-Fi"}, // escaped literal hyphen
174+
{"Wi-?Fi", "wifi", "WiFi"}, // optional char absent
175+
{"Docker(file|ize)", "dockerfile", "Dockerfile"}, // alternation -- see #997
176+
{"Docker(file|ize)", "DOCKERIZE", "Dockerize"},
177+
{"(?:foo|bar)", "foo", "foo"}, // non-capturing group
178+
179+
// No spelling in the set matches, so the term stands.
180+
{"Docker(file|ize)", "docker", "Docker(file|ize)"},
181+
182+
// Not a finite set of spellings: nothing to name.
183+
{"[Pp]ython", "python", "[Pp]ython"},
184+
{`Py.*\b`, "pythonic", `Py.*\b`},
185+
{"Go+gle", "google", "Go+gle"},
173186
}
174187
for _, c := range cases {
175188
if got := recaseToTerm(c.term, c.observed); got != c.want {
@@ -178,6 +191,67 @@ func TestRecaseToTerm(t *testing.T) {
178191
}
179192
}
180193

194+
func TestExpandPattern(t *testing.T) {
195+
cases := []struct {
196+
pattern string
197+
want []string
198+
}{
199+
{"OAuth", []string{"OAuth"}},
200+
{"OAuth2?", []string{"OAuth2", "OAuth"}},
201+
{"Docker(file|ize)", []string{"Dockerfile", "Dockerize"}},
202+
{"Docker(?:file|ize)", []string{"Dockerfile", "Dockerize"}},
203+
{"foo|bar", []string{"foo", "bar"}},
204+
{`Wi\-Fi`, []string{"Wi-Fi"}},
205+
{"a(b|c)d?", []string{"abd", "ab", "acd", "ac"}},
206+
207+
// Optional group: the whole group drops out.
208+
{"Java(Script)?", []string{"JavaScript", "Java"}},
209+
210+
// Unbounded or class-based -- no finite set of spellings.
211+
{"[Pp]ython", nil},
212+
{"Go+gle", nil},
213+
{"Py.*", nil},
214+
{`\d+`, nil},
215+
{"a{2,3}", nil},
216+
{"(unclosed", nil},
217+
{"unopened)", nil},
218+
{"(?=lookahead)", nil},
219+
}
220+
221+
for _, c := range cases {
222+
got := expandPattern(c.pattern)
223+
if c.want == nil {
224+
if got != nil {
225+
t.Errorf("expandPattern(%q) = %v, want nil", c.pattern, got)
226+
}
227+
continue
228+
}
229+
if !slices.Equal(sorted(got), sorted(c.want)) {
230+
t.Errorf("expandPattern(%q) = %v, want %v", c.pattern, got, c.want)
231+
}
232+
}
233+
}
234+
235+
// A blown budget has to return nil rather than a truncated set: a partial list
236+
// would let recaseToTerm miss the spelling the writer actually used and report
237+
// the raw pattern, which is the bug this exists to avoid.
238+
func TestExpandPatternBudget(t *testing.T) {
239+
// 2^7 = 128 spellings, past maxExpansions.
240+
if got := expandPattern(strings.Repeat("(a|b)", 7)); got != nil {
241+
t.Errorf("expected nil past the budget, got %d spellings", len(got))
242+
}
243+
// 2^5 = 32, inside it.
244+
if got := expandPattern(strings.Repeat("(a|b)", 5)); len(got) != 32 {
245+
t.Errorf("expected 32 spellings, got %d", len(got))
246+
}
247+
}
248+
249+
func sorted(s []string) []string {
250+
out := append([]string{}, s...)
251+
sort.Strings(out)
252+
return out
253+
}
254+
181255
func TestOptions(t *testing.T) {
182256
cases := map[string][]string{
183257
"foo|bar": {"foo", "bar"},

0 commit comments

Comments
 (0)