Reject an empty positive list, field in (), as a compile error

What does this MR do and why?

field in () compiled to an empty GraphQL list and was accepted. What happened next depended on the backend: the analytics engines compiled it to an always-false predicate, so the panel showed zero rows with no error; the standard API drops a blank list parameter and matches everything, so id in () on work items returned every row. Neither is what the author meant, and both are silent. The negated spelling, not in (), was already rejected by !537 (merged), so the two forms also disagreed about whether an empty list is a mistake.

This adds reject_empty_positive_list next to the negated guard in ExpressionAnalyzer, the one stage that sees operator and value together. It fires on in () — and only in () — in every mode.

Why the guard keys on in alone

Only field in () is ever a genuine empty filter. field = () and field != () are wildcard spellings: transformer::transform_expression rewrites (_, Equal, List([])) to NONE and (_, NotEqual, List([])) to ANY unconditionally, for every field, before codegen runs. The field type is consulted only afterwards, to pick the *WildcardId argument. So the analyzer leaves = () / != () alone and they keep compiling exactly as before (label = () → labelName: "NONE", label != () → labelName: "ANY").

in () has no such rewrite. An empty membership list names no members, so it cannot be a filter in any mode, and what each backend did with it before this change differed: the analytics engines compiled it to an always-false predicate (column.in([]), which Arel renders as 1=0) and the panel showed zero rows; the standard API drops a blank list parameter (params[:iids].present? ? items.iid_in(...) : items) and returned every row. Neither outcome is what the author meant, and neither surfaced as an error. The guard therefore applies across GLQL, not only to analytics: on work items id in (), type in () and author in (), and on merge requests id in (), targetBranch in (), sourceBranch in () and reviewState in (), are rejected alongside flowType in () and status in ().

The guard lives in ExpressionAnalyzer, next to the existing not in () guard, because it is the one stage that sees operator and value together. It runs after the field analyzer's operator gate, so a field that does not admit in still reports the unsupported operator rather than the empty list. Its order relative to the negated guard does not matter — the two test disjoint operators — and both now share a has_empty_list predicate.

Before / after

Before (main):

  type = duoworkflow and flowType in ()      ->  duoWorkflows(workflowDefinition: [])   # engine: always-false predicate, zero rows, no error
  type = duoworkflow and status in ()        ->  duoWorkflows(status: [])               # same
  type = Issue and group = "g" and id in ()  ->  workItems(iids: [])                    # API drops the blank parameter: every row
  type in () and group = "g"                 ->  workItems(types: [])                   # same
  type = MergeRequest and targetBranch in () ->  mergeRequests(targetBranches: [])      # same
  type = duoworkflow and flowType not in ()  ->  Error: `flowType not in (...)` needs at least one value to exclude. ...

After:

  type = duoworkflow and flowType in ()      ->  Error: `flowType in (...)` needs at least one value to match. An empty list leaves nothing to match against, so it is almost certainly not what was meant.
  type = duoworkflow and status in ()        ->  Error: `status in (...)` needs at least one value to match. ...
  type = Issue and group = "g" and id in ()  ->  Error: `id in (...)` needs at least one value to match. ...
  type in () and group = "g"                 ->  Error: `type in (...)` needs at least one value to match. ...
  type = Issue and group = "g" and author in () -> Error: `author in (...)` needs at least one value to match. ...
  type = MergeRequest and targetBranch in () ->  Error: `targetBranch in (...)` needs at least one value to match. ...
  type = duoworkflow and flowType not in ()  ->  Error: `flowType not in (...)` needs at least one value to exclude. ...   (unchanged)

Unchanged (wildcard spellings, rewritten by the transformer for every field):

  type = Issue and label = ()                ->  workItems(types: ISSUE, labelName: "NONE", ...)
  type = Issue and label != ()               ->  workItems(types: ISSUE, labelName: "ANY", ...)
  type = MergeRequest and label = ()         ->  mergeRequests(labelName: "NONE", ...)
  type = MergeRequest and label != ()        ->  mergeRequests(labelName: "ANY", ...)

Unchanged (operator gate answers before the guard):

  type = duoworkflow and flowType = ()       ->  Error: `flowType` does not support the equals (`=`) operator for `()`. ...
  type = Issue and author = ()               ->  Error: `author` does not support the equals (`=`) operator for `()`. ...

The message mirrors the tone of the existing not in (...) message and describes neither backend's behaviour — only that there is nothing to match against. The error kind is EmptyPositiveList { field }, the twin of EmptyNegatedList { field }.

How to set up and validate locally

cargo fmt -- --check
cargo clippy --all-targets -- -D warnings
cargo test

1920 tests pass, 0 failures. src/schema/schema.json is unchanged: this adds an error kind and a guard, and no analyzer declaration moved.

Tests

Unit (src/analyzer/expression.rs):

  • test_empty_positive_list_is_rejected — flowType in () → EmptyPositiveList { field: FlowType }
  • test_non_empty_positive_list_is_accepted — flowType in ("chat") passes
  • test_empty_list_under_equals_is_left_to_the_wildcard_rewrite — label = () passes the analyzer; the guard keys on in, = () belongs to the transformer's NONE rewrite
  • test_unsupported_operator_is_reported_before_the_empty_list (pre-existing) — operator gate answers before either empty-list guard

Integration, analytics (tests/duo_workflows_tests.rs):

  • test_list_filters_reject_an_empty_inclusion_list — flowType in (), status in (), user in () all rejected
  • test_flow_type_rejects_an_empty_exclusion_list (pre-existing) — not in () message unchanged
  • test_flow_type_rejects_negated_wildcards_and_empty_equality (pre-existing) — flowType = () still reports the operator

Integration, standard (tests/work_items_tests.rs):

  • test_id_in_empty_list, test_author_in_empty_list — exact new error
  • test_label_equals_empty_array / test_label_not_equals_empty_array (pre-existing) — label = () → "NONE", label != () → "ANY": pin the transformer's wildcard path

Integration, standard (tests/merge_requests_tests.rs):

  • test_label_equals_empty_list, test_label_not_equals_empty_list — label = () → labelName: "NONE", label != () → labelName: "ANY"
  • test_id_in_empty_list, test_target_branch_in_empty_list, test_source_branch_in_empty_list, test_review_state_in_empty_list — exact new error

Closes #238 (closed)

Edited by Chandra Saripaka

Merge request reports

Loading
Loading