search: separate parameters and pattern types in parse tree
Created by: rvantonder
TL;DR:
The change separates a singular datatype Parameter
that currently represents both search patterns and field:value
parameters to be separate (a new Pattern
type is created). Everything else is updated around that change (separate functions for parsing these, visitor interface, tree reduction, etc.).
We can have confidence in the change because tests pass, and affected tests are updated to the correct desired behavior.
Long description so I can trace my thinking and how this changed.
The thing motivating this change is that we currently use a convention that search patterns are represented as a type Parameter
in the AST where field is ""
and value represents the pattern value. This same convention is in the existing (older) parser and can simplify some implementation details. Initially I didn't think we need to separate field:value
versus patterns ("":value
) at the parsing level, but since https://github.com/sourcegraph/sourcegraph/pull/9844 combining the two in the same datatype started bothering me because we do have some properties that we semantically distinguish in patterns versus parameters:
Adding Quoted is not the desired end goal for the Parameter types. It currently pollutes the visitor interface a bit. I will be turning parameter values into better structures with type members later.
Basically, we want to track whether a search pattern was quoted (it has semantic implications for our search: quoting in regex mode implies literal search) but we do not care about that for field:value
parameters, since we will process field:value
versus field:"value"
or field:'value'
all the same.
Thus, while this change adds extra logic to deal with the extra Pattern
case, it produces a final tree that better introspection, simplifies the visitor interface, and simplifies other parts of query validation.