fix(cli): consistent output format, sorted findings, version flag
- Workflow rules now use strict if: evaluation (parse failure → skip rule, not match); fixes premature matching that blocked later rules and injected wrong variables into the context - Single = accepted as alias for == in rules:if: expressions - File/Line preserved through extends: resolution (lost during YAML encode/decode round-trip in the resolver) - Findings sorted by (File, Line, Rule) so same-file issues group together - All warnings use ruff-style path: [warning] message format (includes, extends chains, workflow non-start) - Add --version / -v flag; version shown at top of every --help output - Build injects version via ldflags using git describe Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -12,7 +12,7 @@ import (
|
||||
// - Variable references: $VAR_NAME or ${VAR_NAME}
|
||||
// - String literals: "value" or 'value'
|
||||
// - Null keyword: null
|
||||
// - Comparison: == != =~ !~
|
||||
// - Comparison: == != =~ !~ (single = is accepted as == for user convenience)
|
||||
// - Boolean: && || !
|
||||
// - Grouping: ( )
|
||||
// - Regex flags: /pattern/i (case-insensitive), /pattern/m, /pattern/s
|
||||
@@ -23,10 +23,21 @@ import (
|
||||
// used by GitLab CI. Unsupported or unparseable expressions fall back to true
|
||||
// (permissive) so the linter never silently drops jobs it cannot evaluate.
|
||||
func EvalIf(expr string, vars func(string) string) bool {
|
||||
return evalIf(expr, vars, true)
|
||||
}
|
||||
|
||||
// EvalIfStrict is like EvalIf but returns false (instead of true) when the
|
||||
// expression cannot be fully parsed. Use for workflow:rules: evaluation where
|
||||
// a failed parse should skip to the next rule rather than matching everything.
|
||||
func EvalIfStrict(expr string, vars func(string) string) bool {
|
||||
return evalIf(expr, vars, false)
|
||||
}
|
||||
|
||||
func evalIf(expr string, vars func(string) string, permissive bool) bool {
|
||||
p := &exprParser{s: strings.TrimSpace(expr), vars: vars}
|
||||
result, ok := p.parseOr()
|
||||
if !ok || p.pos < len(p.s) {
|
||||
return true // unparseable → permissive
|
||||
return permissive
|
||||
}
|
||||
return result
|
||||
}
|
||||
@@ -200,6 +211,17 @@ func (p *exprParser) parseComparison() (bool, bool) {
|
||||
return true, true // bad pattern → permissive
|
||||
}
|
||||
return !re.MatchString(leftStr), true
|
||||
|
||||
// Single = not followed by = or ~ — accepted as == (common user mistake;
|
||||
// GitLab CI only supports == but = is frequently written by accident).
|
||||
case p.peek() == '=' && !p.startsWith("==") && !p.startsWith("=~"):
|
||||
p.pos++ // consume '='
|
||||
p.skipWS()
|
||||
rightStr, ok := p.parseValue()
|
||||
if !ok {
|
||||
return false, false
|
||||
}
|
||||
return leftStr == rightStr, true
|
||||
}
|
||||
|
||||
// No operator: variable is truthy when non-empty (defined and non-null).
|
||||
|
||||
@@ -120,6 +120,12 @@ func TestEvalIf(t *testing.T) {
|
||||
// ── Permissive fallback ───────────────────────────────────────────────
|
||||
{"unparseable returns true", `this is not valid syntax %%%`, true},
|
||||
{"empty expr returns true", ``, true},
|
||||
|
||||
// ── Single = as alias for == ──────────────────────────────────────────
|
||||
{"single eq match", `$CI_COMMIT_BRANCH = "develop"`, true},
|
||||
{"single eq no match", `$CI_COMMIT_BRANCH = "main"`, false},
|
||||
{"single eq in compound", `$CI_COMMIT_BRANCH = "develop" && $CI_PIPELINE_SOURCE = "push"`, true},
|
||||
{"single eq compound false", `$CI_COMMIT_BRANCH = "main" && $CI_PIPELINE_SOURCE = "push"`, false},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
@@ -131,3 +137,48 @@ func TestEvalIf(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestEvalIfStrict(t *testing.T) {
|
||||
vars := func(key string) string {
|
||||
m := map[string]string{
|
||||
"CI_COMMIT_BRANCH": "develop",
|
||||
"CI_PIPELINE_SOURCE": "push",
|
||||
"WORKFLOW": "",
|
||||
}
|
||||
return m[key]
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
expr string
|
||||
want bool
|
||||
}{
|
||||
// Parseable expressions behave identically to EvalIf.
|
||||
{"parseable match", `$CI_COMMIT_BRANCH == "develop"`, true},
|
||||
{"parseable no match", `$CI_COMMIT_BRANCH == "main"`, false},
|
||||
{"single eq match", `$CI_COMMIT_BRANCH = "develop"`, true},
|
||||
// Empty expression: ruleIfMatchesStrict handles the empty→true case
|
||||
// before calling EvalIfStrict, so empty falls through to false here.
|
||||
{"empty expr", ``, false},
|
||||
|
||||
// Unparseable expressions return false (strict) instead of true (permissive).
|
||||
{"unparseable returns false", `this is not valid syntax %%%`, false},
|
||||
|
||||
// The key workflow-rule scenario: a complex condition with an
|
||||
// unevaluable sub-expression should not match (strict=false) so that
|
||||
// later workflow rules can be evaluated.
|
||||
{"workflow rule complex no match", `$WORKFLOW = "gitflow" && $CI_PIPELINE_SOURCE == /(push|web)/`, false},
|
||||
|
||||
// Compound with a bad second operand: strict returns false.
|
||||
{"and with bad rhs strict false", `$CI_COMMIT_BRANCH == "develop" && !(((`, false},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := EvalIfStrict(tc.expr, vars)
|
||||
if got != tc.want {
|
||||
t.Errorf("EvalIfStrict(%q) = %v, want %v", tc.expr, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -42,7 +42,11 @@ func EvalWorkflow(p *model.Pipeline, ctx *Context) (bool, map[string]string) {
|
||||
}
|
||||
vars := ctx.Get
|
||||
for _, rule := range p.Workflow.Rules {
|
||||
if !ruleIfMatches(rule.If, vars) {
|
||||
// Workflow rules use strict evaluation: an unparseable condition is
|
||||
// treated as no-match so later rules (with valid conditions or a
|
||||
// bare when:) are reached. Permissive-true would cause an early rule
|
||||
// with a complex/invalid condition to block all subsequent rules.
|
||||
if !ruleIfMatchesStrict(rule.If, vars) {
|
||||
continue
|
||||
}
|
||||
when := rule.When
|
||||
@@ -94,6 +98,13 @@ func ruleIfMatches(ifExpr string, vars func(string) string) bool {
|
||||
return EvalIf(ifExpr, vars)
|
||||
}
|
||||
|
||||
func ruleIfMatchesStrict(ifExpr string, vars func(string) string) bool {
|
||||
if ifExpr == "" {
|
||||
return true // no if: condition → rule always matches
|
||||
}
|
||||
return EvalIfStrict(ifExpr, vars)
|
||||
}
|
||||
|
||||
func whenToState(when string) JobState {
|
||||
switch when {
|
||||
case "never":
|
||||
|
||||
Reference in New Issue
Block a user