Skip to content

Commit fcc9d81

Browse files
committed
internal/telemetry/cmd/stacks: anchored literals
Literals in predicates are re-interpreted as matching at word boundaries. A literal like "fu+12" will no longer match "fu+123" or "snafu+12". For golang/go#71045. Change-Id: Id5b6c8ad536dadebdb9593cbfa13ff8dd81b6645 Reviewed-on: https://go-review.googlesource.com/c/tools/+/643835 Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 684910f commit fcc9d81

File tree

3 files changed

+36
-9
lines changed

3 files changed

+36
-9
lines changed

gopls/internal/doc/api.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@
110110
"EnumValues": null,
111111
"Default": "[]",
112112
"Status": "",
113-
"Hierarchy": "build"
113+
"Hierarchy": "build",
114+
"DeprecationMessage": ""
114115
},
115116
{
116117
"Name": "hoverKind",

gopls/internal/telemetry/cmd/stacks/stacks.go

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,10 @@
4141
// > | expr && expr
4242
// > | expr || expr
4343
//
44-
// Each string literal implies a substring match on the stack;
44+
// Each string literal must match complete words on the stack;
4545
// the other productions are boolean operations.
46+
// As an example of literal matching, "fu+12" matches "x:fu+12 "
47+
// but not "fu:123" or "snafu+12".
4648
//
4749
// The stacks command gathers all such predicates out of the
4850
// labelled issues and evaluates each one against each new stack.
@@ -76,6 +78,7 @@ import (
7678
"os"
7779
"os/exec"
7880
"path/filepath"
81+
"regexp"
7982
"runtime"
8083
"sort"
8184
"strconv"
@@ -424,11 +427,21 @@ func readIssues(pcfg ProgramConfig) ([]*Issue, error) {
424427
// | ! expr
425428
// | expr && expr
426429
// | expr || expr
430+
//
431+
// The value of a string literal is whether it is a substring of the stack, respecting word boundaries.
432+
// That is, a literal L behaves like the regular expression \bL'\b, where L' is L with
433+
// regexp metacharacters quoted.
427434
func parsePredicate(s string) (func(string) bool, error) {
428435
expr, err := parser.ParseExpr(s)
429436
if err != nil {
430437
return nil, fmt.Errorf("parse error: %w", err)
431438
}
439+
440+
// Cache compiled regexps since we need them more than once.
441+
literalRegexps := make(map[*ast.BasicLit]*regexp.Regexp)
442+
443+
// Check for errors in the predicate so we can report them now,
444+
// ensuring that evaluation is error-free.
432445
var validate func(ast.Expr) error
433446
validate = func(e ast.Expr) error {
434447
switch e := e.(type) {
@@ -454,9 +467,15 @@ func parsePredicate(s string) (func(string) bool, error) {
454467
if e.Kind != token.STRING {
455468
return fmt.Errorf("invalid literal (%s)", e.Kind)
456469
}
457-
if _, err := strconv.Unquote(e.Value); err != nil {
470+
lit, err := strconv.Unquote(e.Value)
471+
if err != nil {
458472
return err
459473
}
474+
// The literal should match complete words. It may match multiple words,
475+
// if it contains non-word runes like whitespace; but it must match word
476+
// boundaries at each end.
477+
// The constructed regular expression is always valid.
478+
literalRegexps[e] = regexp.MustCompile(`\b` + regexp.QuoteMeta(lit) + `\b`)
460479

461480
default:
462481
return fmt.Errorf("syntax error (%T)", e)
@@ -485,8 +504,7 @@ func parsePredicate(s string) (func(string) bool, error) {
485504
return eval(e.X)
486505

487506
case *ast.BasicLit:
488-
substr, _ := strconv.Unquote(e.Value)
489-
return strings.Contains(stack, substr)
507+
return literalRegexps[e].MatchString(stack)
490508
}
491509
panic("unreachable")
492510
}

gopls/internal/telemetry/cmd/stacks/stacks_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,23 @@ func TestParsePredicate(t *testing.T) {
8383
want bool
8484
}{
8585
{`"x"`, `"x"`, true},
86-
{`"x"`, `"axe"`, true}, // literals match by strings.Contains
86+
{`"x"`, `"axe"`, false}, // literals match whole words
87+
{`"x"`, "val:x+5", true},
88+
{`"fu+12"`, "x:fu+12,", true},
89+
{`"fu+12"`, "snafu+12,", false},
90+
{`"fu+12"`, "x:fu+123,", false},
91+
{`"a.*b"`, "a.*b", true}, // regexp metachars are escaped
92+
{`"a.*b"`, "axxb", false}, // ditto
8793
{`"x"`, `"y"`, false},
8894
{`!"x"`, "x", false},
8995
{`!"x"`, "y", true},
90-
{`"x" && "y"`, "xy", true},
96+
{`"x" && "y"`, "xy", false},
97+
{`"x" && "y"`, "x y", true},
9198
{`"x" && "y"`, "x", false},
9299
{`"x" && "y"`, "y", false},
93-
{`"xz" && "zy"`, "xzy", true}, // matches need not be disjoint
94-
{`"x" || "y"`, "xy", true},
100+
{`"xz" && "zy"`, "xzy", false},
101+
{`"xz" && "zy"`, "zy,xz", true},
102+
{`"x" || "y"`, "x\ny", true},
95103
{`"x" || "y"`, "x", true},
96104
{`"x" || "y"`, "y", true},
97105
{`"x" || "y"`, "z", false},

0 commit comments

Comments
 (0)