fix(rules): Make rules: if: operands match GitLab

What this MR does and why?

Three divergences in the rules: if: expression language, all in how operands are parsed. Each one is silent: the pipeline stays green and either the wrong jobs run, or a config GitLab accepts is refused locally.

! bound looser than the comparison operators (#161 (closed)). glci read !$FOO == "bar" as !($FOO == "bar"); GitLab reads it as (!$FOO) == "bar". Lexeme::Not has precedence 1 against Lexeme::Equals's 10, so the shunting-yard pops Not before Equals is pushed, and Not#evaluate is !operand.evaluate.present? — a boolean, which compare_with_coercion then renders as "true"/"false". The two readings coincide only when the right operand equals the string form of the negated left one, which is why this survived casual testing. glci variables showed it as an apparent contradiction:

#0  !$FOO (baz) == "bar"  → true

A negation was rejected on the right of a comparison (#175 (closed)), and a parenthesised group was rejected as an operand at all (#176 (closed)). GitLab accepts "false" == !$FOO, ($FOO) == "baz" and $FOO == ($FOO). glci refused each as invalid expression syntax — and since !174 (merged) made an unparseable if: a hard error, that refuses the whole pipeline rather than dropping one job.

The common cause was that the evaluator carried booleans. parsePrimary returned a bool, so a negated or grouped operand had no value to hand to compare, and the right-hand side could only ever be a bare value. It now carries values and applies truthiness once, at the top. One parseOperand serves both sides.

That also settles what a group is. Parentheses are transparent grouping on GitLab, not a cast: ($FOO) is the value of $FOO, so ($FOO) == "baz" compares "baz" against "baz". Reducing a group to a boolean would have made that false. && and || yield an operand rather than a boolean for the same reason, so ($UNSET || $FOO) == "baz" matches.

Operand provenance moved onto exprValue as part of this. The regex-error messages infer whether a pattern came from a variable — a value that must never be printed — and used to do it by checking whether the source text started with $. A parenthesised pattern defeated that: ($SECRET) read as a literal and put one character of the value into the unsupported-flag message. The value now carries its own provenance, so a group propagates it and a negation clears it.

Expected values throughout are GitLab's own, from ci/lint dry-runs of the same expressions — 40-odd of them across the two test tables.

Steps to reproduce

variables:
  FOO: "baz"

anchor:
  script: echo anchor

neg_eq_bar:
  script: echo neg_eq_bar
  rules:
    - if: '!$FOO == "bar"'

neg_eq_false:
  script: echo neg_eq_false
  rules:
    - if: '!$FOO == "false"'

grouped:
  script: echo grouped
  rules:
    - if: '($FOO) == "baz"'

negated_right:
  script: echo negated_right
  rules:
    - if: '"false" == !$FOO'
glci lint
glci jobs

Before: lint fails with jobs:grouped:rules:rule if invalid expression syntax. Remove the last two jobs and it passes, but jobs then lists neg_eq_bar, which GitLab does not create.

After: lint exits 0, and jobs lists anchor, neg_eq_false, grouped and negated_right — the same four GitLab creates.

Closes #161 (closed), closes #175 (closed), closes #176 (closed).

  • Follows !174 (merged) (#154 (closed)), which made an unparseable if: a hard error and so raised the cost of refusing a valid expression.
  • GitLab source: lib/gitlab/ci/pipeline/expression/lexeme/{not,equals,and,or,parenthesis_open,parenthesis_close}.rb and statement.rb.

Two divergences found while reviewing this are not addressed here, because both need the evaluator to tell an unset variable from one defined empty — a change to variable lookup rather than to operands:

  • $EMPTY == null is true here and false on GitLab, and != likewise inverted.
  • &&/|| pick a side using present?, where Ruby counts "" as truthy, so a defined-but-empty left operand selects the wrong operand.

Both are pinned by TestEvalIf_EmptyOperandLogicalIsADivergence so the gap is deliberate rather than accidental, and named in the evalIf grammar comment and the docs. Note this MR gives the first one a new surface: ($EMPTY) == null now parses where it used to error.

Edited by Paweł Farys

Merge request reports

Loading
Loading