feat(search): Allow partial IN list values (#27922)

This allows values like `key:[one, ]` to match the parser, making it
easier to complete values in the lists.

The token will be invalid if left like this.

src/sentry/api/event_search.py

@@ -123,7 +123,7 @@ key                    = ~r"[a-zA-Z0-9_.-]+"
 quoted_key             = '"' ~r"[a-zA-Z0-9_.:-]+" '"'
 explicit_tag_key       = "tags" open_bracket search_key closed_bracket
 aggregate_key          = key open_paren spaces function_args? spaces closed_paren
-function_args          = aggregate_param (spaces comma spaces aggregate_param)*
+function_args          = aggregate_param (spaces comma spaces !comma aggregate_param?)*
 aggregate_param        = quoted_aggregate_param / raw_aggregate_param
 raw_aggregate_param    = ~r"[^()\t\n, \"]+"
 quoted_aggregate_param = '"' ('\\"' / ~r'[^\t\n\"]')* '"'
@@ -136,8 +136,8 @@ text_in_value          = quoted_value / in_value
 search_value           = quoted_value / value
 numeric_value          = "-"? numeric ~r"[kmb]"? &(end_value / comma / closed_bracket)
 boolean_value          = ~r"(true|1|false|0)"i &end_value
-text_in_list           = open_bracket text_in_value (spaces comma spaces text_in_value)* closed_bracket &end_value
-numeric_in_list        = open_bracket numeric_value (spaces comma spaces numeric_value)* closed_bracket &end_value
+text_in_list           = open_bracket text_in_value (spaces comma spaces !comma text_in_value?)* closed_bracket &end_value
+numeric_in_list        = open_bracket numeric_value (spaces comma spaces !comma numeric_value?)* closed_bracket &end_value
 
 # See: https://stackoverflow.com/a/39617181/790169
 in_value_termination = in_value_char (!in_value_end in_value_char)* in_value_end
@@ -258,9 +258,13 @@ def remove_space(children):
 
 
 def process_list(first, remaining):
+    # Empty values become blank nodes
+    if any(isinstance(item[4], Node) for item in remaining):
+        raise InvalidSearchQuery("Lists should not have empty values")
+
     return [
         first,
-        *[item[3] for item in remaining],
+        *[item[4][0] for item in remaining],
     ]
 
 

static/app/components/searchSyntax/grammar.pegjs

@@ -218,7 +218,7 @@ aggregate_key
 
 function_args
   = arg1:aggregate_param
-    args:(spaces comma spaces aggregate_param)* {
+    args:(spaces comma spaces !comma aggregate_param?)* {
       return tc.tokenKeyAggregateArgs(arg1, args);
     }
 
@@ -277,7 +277,7 @@ boolean_value
 text_in_list
   = open_bracket
     item1:text_in_value
-    items:(spaces comma spaces text_in_value)*
+    items:(spaces comma spaces !comma text_in_value?)*
     closed_bracket
     &end_value {
       return tc.tokenValueTextList(item1, items);
@@ -286,7 +286,7 @@ text_in_list
 numeric_in_list
   = open_bracket
     item1:numeric_value
-    items:(spaces comma spaces numeric_value)*
+    items:(spaces comma spaces !comma numeric_value?)*
     closed_bracket
     &end_value {
       return tc.tokenValueNumberList(item1, items);

static/app/components/searchSyntax/parser.tsx

@@ -14,14 +14,15 @@ import {getKeyName} from './utils';
 type TextFn = () => string;
 type LocationFn = () => LocationRange;
 
-type ListItem<K> = [
+type ListItem<V> = [
   space: ReturnType<TokenConverter['tokenSpaces']>,
   comma: string,
   space: ReturnType<TokenConverter['tokenSpaces']>,
-  key: K
+  notComma: undefined,
+  value: V | null
 ];
 
-const listJoiner = <K,>([s1, comma, s2, value]: ListItem<K>) => ({
+const listJoiner = <K,>([s1, comma, s2, _, value]: ListItem<K>) => ({
   separator: [s1.value, comma, s2.value].join(''),
   value,
 });
@@ -284,6 +285,7 @@ type FilterMap = {
 };
 
 type TextFilter = FilterMap[FilterType.Text];
+type InFilter = FilterMap[FilterType.TextIn] | FilterMap[FilterType.NumericIn];
 
 /**
  * The Filter type discriminates on the FilterType enum using the `filter` key.
@@ -530,6 +532,9 @@ export class TokenConverter {
 
     const {isNumeric, isDuration, isBoolean, isDate, isPercentage} = this.keyValidation;
 
+    const checkAggregate = (check: (s: string) => boolean) =>
+      aggregateKey.args?.args.some(arg => check(arg?.value?.value ?? ''));
+
     switch (type) {
       case FilterType.Numeric:
       case FilterType.NumericIn:
@@ -547,13 +552,13 @@ export class TokenConverter {
         return isDate(keyName);
 
       case FilterType.AggregateDuration:
-        return aggregateKey.args?.args.some(arg => isDuration(arg.value.value));
+        return checkAggregate(isDuration);
 
       case FilterType.AggregateDate:
-        return aggregateKey.args?.args.some(arg => isDate(arg.value.value));
+        return checkAggregate(isDate);
 
       case FilterType.AggregatePercentage:
-        return aggregateKey.args?.args.some(arg => isPercentage(arg.value.value));
+        return checkAggregate(isPercentage);
 
       default:
         return true;
@@ -574,16 +579,20 @@ export class TokenConverter {
     key: FilterMap[T]['key'],
     value: FilterMap[T]['value']
   ) => {
-    // Only text filters may currently be invalid, since the text filter is the
-    // "fall through" filter that will match when other filter predicates fail.
-    if (filter !== FilterType.Text) {
-      return null;
+    // Text filter is the "fall through" filter that will match when other
+    // filter predicates fail.
+    if (filter === FilterType.Text) {
+      return this.checkInvalidTextFilter(
+        key as TextFilter['key'],
+        value as TextFilter['value']
+      );
+    }
+
+    if ([FilterType.TextIn, FilterType.NumericIn].includes(filter)) {
+      return this.checkInvalidInFilter(value as InFilter['value']);
     }
 
-    return this.checkInvalidTextFilter(
-      key as TextFilter['key'],
-      value as TextFilter['value']
-    );
+    return null;
   };
 
   /**
@@ -653,6 +662,19 @@ export class TokenConverter {
 
     return null;
   };
+
+  /**
+   * Validates IN filter values do not have an missing elements
+   */
+  checkInvalidInFilter = ({items}: InFilter['value']) => {
+    const hasEmptyValue = items.some(item => item.value === null);
+
+    if (hasEmptyValue) {
+      return {reason: t('Lists should not have empty values')};
+    }
+
+    return null;
+  };
 }
 
 /**

static/app/components/searchSyntax/renderer.tsx

@@ -174,7 +174,7 @@ const ListToken = ({
   <InList>
     {token.items.map(({value, separator}) => [
       <ListComma key="comma">{separator}</ListComma>,
-      renderToken(value, cursor),
+      value && renderToken(value, cursor),
     ])}
   </InList>
 );

static/app/components/searchSyntax/utils.tsx

@@ -69,7 +69,11 @@ export function treeResultLocator<T>({
 }: TreeResultLocatorOpts): T {
   const returnResult = (result: any) => new TokenResultFound(result);
 
-  const nodeVisitor = (token: TokenResult<Token>) => {
+  const nodeVisitor = (token: TokenResult<Token> | null) => {
+    if (token === null) {
+      return;
+    }
+
     const result = visitorTest({token, returnResult, skipToken: skipTokenMarker});
 
     // Bubble the result back up.
@@ -142,7 +146,11 @@ type TreeTransformerOpts = {
  * a transform to those nodes.
  */
 export function treeTransformer({tree, transform}: TreeTransformerOpts) {
-  const nodeVisitor = (token: TokenResult<Token>) => {
+  const nodeVisitor = (token: TokenResult<Token> | null) => {
+    if (token === null) {
+      return null;
+    }
+
     switch (token.type) {
       case Token.Filter:
         return transform({

tests/fixtures/search-syntax/numeric_in_filter.json

@@ -276,5 +276,47 @@
       },
       {"type": "spaces", "value": ""}
     ]
+  },
+  {
+    "query": "project_id:[500,501,]",
+    "result": [
+      {"type": "spaces", "value": ""},
+      {
+        "type": "filter",
+        "filter": "numericIn",
+        "invalid": {"reason": "Lists should not have empty values"},
+        "negated": false,
+        "key": {"type": "keySimple", "value": "project_id", "quoted": false},
+        "operator": "",
+        "value": {
+          "type": "valueNumberList",
+          "items": [
+            {
+              "separator": "",
+              "value": {
+                "type": "valueNumber",
+                "value": "500",
+                "rawValue": 500,
+                "unit": null
+              }
+            },
+            {
+              "separator": ",",
+              "value": {
+                "type": "valueNumber",
+                "value": "501",
+                "rawValue": 501,
+                "unit": null
+              }
+            },
+            {
+              "separator": ",",
+              "value": null
+            }
+          ]
+        }
+      },
+      {"type": "spaces", "value": ""}
+    ]
   }
 ]

tests/fixtures/search-syntax/simple_in.json

@@ -357,5 +357,33 @@
       },
       {"type": "spaces", "value": ""}
     ]
+  },
+  {
+    "query": "user.email:[test@test.com, ]",
+    "result": [
+      {"type": "spaces", "value": ""},
+      {
+        "type": "filter",
+        "filter": "textIn",
+        "invalid": {"reason": "Lists should not have empty values"},
+        "negated": false,
+        "key": {"type": "keySimple", "value": "user.email", "quoted": false},
+        "operator": "",
+        "value": {
+          "type": "valueTextList",
+          "items": [
+            {
+              "separator": "",
+              "value": {"type": "valueText", "value": "test@test.com", "quoted": false}
+            },
+            {
+              "separator": ", ",
+              "value": null
+            }
+          ]
+        }
+      },
+      {"type": "spaces", "value": ""}
+    ]
   }
 ]