diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c44827..e78c636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). This project uses [Semantic Versioning](https://semver.org). +## [0.2.24] - 2026-06-25 + +### Added + +- **`rules:needs:` validation (GL044)** — validates jobs listed in `rules:needs:` overrides (GitLab CI 16.4+). Each entry must reference a job that exists in the pipeline; `optional: true` entries that reference missing jobs are downgraded to warnings (same behaviour as GL027 for top-level `needs:`). Cross-pipeline needs (maps with a `pipeline:` key) are ignored. Skipped jobs are excluded when a context is provided. `glint explain GL044` documents the rule with a bad-YAML example and fix. + +- **`Rule.Needs` model field** — `rules:` entries now parse their `needs:` key into `model.Rule.Needs []any`, making the per-rule needs list available for validation and future evaluation. + ## [0.2.23] - 2026-06-25 ### Added diff --git a/FEATURES.md b/FEATURES.md index 4915f23..9cc618c 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -69,6 +69,7 @@ description, bad-YAML example, and fix. | GL029 | ERR | Circular dependency in `needs:` graph | | GL030 | ERR | `dependencies:` references a job that doesn't exist | | GL031 | ERR | `dependencies:` references a job in the same or a later stage | +| GL044 | ERR/WARN | `rules:needs:` references a job that doesn't exist (WARN when `optional: true`) | ### Expression & reachability diff --git a/README.md b/README.md index 4c3bf30..f4abd06 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@

License - Release + Release

> **Disclaimer:** This tool was built through iterative AI-assisted development with [Claude](https://claude.ai). It is experimental, incomplete, and not intended for production use. Coverage of GitLab CI keywords is best-effort and may lag behind GitLab's evolving spec. Use it at your own discretion — no correctness guarantees are made. Contributions and bug reports are welcome. diff --git a/ROADMAP.md b/ROADMAP.md index 5574dcf..005b9b3 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -113,7 +113,7 @@ The SVG renderer and terminal tree cover the basic layout. These would bring it ## Reliability and developer experience -- ~~**Structured rule IDs**~~ — ✓ shipped post-v0.2.0; GL001–GL031 assigned; GL032 added v0.2.11; GL033 added v0.2.15; GL034–GL041 added v0.2.16; output formats (--format json/sarif/junit/github) added v0.2.18; GL042–GL043 added v0.2.20 +- ~~**Structured rule IDs**~~ — ✓ shipped post-v0.2.0; GL001–GL031 assigned; GL032 added v0.2.11; GL033 added v0.2.15; GL034–GL041 added v0.2.16; output formats (--format json/sarif/junit/github) added v0.2.18; GL042–GL043 added v0.2.20; GL044 added v0.2.24 - ~~**`glint explain `**~~ — ✓ shipped v0.2.20; prints rule description, rationale, bad-YAML example, and fix; `glint explain` (no arg) lists all rules - ~~**Semantic versioning and first release**~~ — shipped as `v0.1.0` (2026-06-07) - ~~**Subcommand CLI**~~ — shipped as `v0.2.0` (2026-06-11); `glint check` / `glint graph [mode]` with ruff-style `--help` diff --git a/Taskfile.yml b/Taskfile.yml index ae1d576..deba163 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -113,10 +113,16 @@ tasks: ignore_error: false - cmd: ./{{.BINARY}} check testdata/inherit_dead_fields.yml ignore_error: false + - cmd: ./{{.BINARY}} check testdata/rules_needs_valid.yml + ignore_error: false + - cmd: ./{{.BINARY}} check testdata/rules_needs_invalid.yml + ignore_error: true - cmd: ./{{.BINARY}} explain GL007 ignore_error: false - cmd: ./{{.BINARY}} explain gl042 ignore_error: false + - cmd: ./{{.BINARY}} explain GL044 + ignore_error: false - cmd: ./{{.BINARY}} explain ignore_error: false diff --git a/internal/linter/explain.go b/internal/linter/explain.go index 0d7b8cf..67c2139 100644 --- a/internal/linter/explain.go +++ b/internal/linter/explain.go @@ -816,4 +816,41 @@ my-job: my-job: script: echo hi`, }, + + RuleRulesNeedsUnknown: { + Title: "'rules:needs:' references unknown job", + Severity: Error, + Description: "A job listed in a rule's 'needs:' override does not exist in " + + "the pipeline. 'rules:needs:' (GitLab CI 16.4+) overrides the top-level " + + "'needs:' list when that specific rule matches. GitLab will refuse to " + + "create the pipeline if any referenced job is missing.", + Example: `stages: [build, test] + +build: + stage: build + script: make + +test: + stage: test + script: make test + rules: + - if: $CI_COMMIT_BRANCH == "main" + needs: [build, lint] # lint does not exist`, + Fix: `stages: [build, test] + +lint: + stage: build + script: make lint + +build: + stage: build + script: make + +test: + stage: test + script: make test + rules: + - if: $CI_COMMIT_BRANCH == "main" + needs: [build, lint]`, + }, } diff --git a/internal/linter/linter.go b/internal/linter/linter.go index ce101ca..af87400 100644 --- a/internal/linter/linter.go +++ b/internal/linter/linter.go @@ -62,6 +62,7 @@ func Lint(p *model.Pipeline, skipped map[string]bool) []Finding { findings = append(findings, checkWorkflow(p)...) findings = append(findings, checkJobs(p)...) findings = append(findings, checkNeeds(p, skipped)...) + findings = append(findings, checkRulesNeeds(p, skipped)...) findings = append(findings, checkDependencies(p, skipped)...) findings = append(findings, checkVariableRefs(p)...) findings = append(findings, checkRulesIfReachability(p)...) diff --git a/internal/linter/needs.go b/internal/linter/needs.go index 737a96e..62f0e64 100644 --- a/internal/linter/needs.go +++ b/internal/linter/needs.go @@ -85,6 +85,44 @@ func checkNeeds(p *model.Pipeline, skipped map[string]bool) []Finding { return findings } +// checkRulesNeeds validates rules:needs: entries across all jobs. Each entry +// in a rule's needs: list must reference a job that exists in the pipeline. +// Cross-pipeline needs (maps with a "pipeline" key) are ignored. Skipped jobs +// are excluded from checking (same semantics as top-level needs: via GL027). +func checkRulesNeeds(p *model.Pipeline, skipped map[string]bool) []Finding { + var findings []Finding + for name, job := range p.Jobs { + if skipped[name] { + continue + } + for i, rule := range job.Rules { + if len(rule.Needs) == 0 { + continue + } + for _, entry := range parseNeedEntries(rule.Needs) { + if _, exists := p.Jobs[entry.job]; !exists { + sev := Error + if entry.optional { + sev = Warning + } + findings = append(findings, Finding{ + Severity: sev, + Rule: RuleRulesNeedsUnknown, + Job: name, + File: job.File, + Line: job.Line, + Message: fmt.Sprintf( + "rules[%d].needs: references unknown job %q", + i, entry.job, + ), + }) + } + } + } + } + return findings +} + // parseNeedEntries extracts needs entries from a needs: list, preserving the // optional flag. Each element is a plain string (job name) or a map with a // "job" key. Cross-pipeline needs (maps with a "pipeline" key) are skipped. diff --git a/internal/linter/needs_test.go b/internal/linter/needs_test.go index 631e59e..635708f 100644 --- a/internal/linter/needs_test.go +++ b/internal/linter/needs_test.go @@ -31,6 +31,129 @@ func TestCheckNeeds_SkippedJob(t *testing.T) { } } +// TestCheckRulesNeeds_UnknownJob verifies that an unknown job in rules:needs: produces GL044. +func TestCheckRulesNeeds_UnknownJob(t *testing.T) { + p := &model.Pipeline{ + Stages: []string{"build", "test"}, + Jobs: map[string]model.Job{ + "build-job": { + Name: "build-job", + Stage: "build", + Script: []any{"make"}, + }, + "test-job": { + Name: "test-job", + Stage: "test", + Script: []any{"make test"}, + Rules: []model.Rule{ + { + If: `$CI_COMMIT_BRANCH == "main"`, + Needs: []any{"build-job", "nonexistent"}, + }, + }, + }, + }, + } + findings := checkRulesNeeds(p, nil) + var got bool + for _, f := range findings { + if f.Rule == RuleRulesNeedsUnknown && f.Severity == Error { + got = true + } + } + if !got { + t.Errorf("expected GL044 error for unknown rules:needs: job; got %v", findings) + } +} + +// TestCheckRulesNeeds_KnownJob verifies no finding when all rules:needs: jobs exist. +func TestCheckRulesNeeds_KnownJob(t *testing.T) { + p := &model.Pipeline{ + Stages: []string{"build", "test"}, + Jobs: map[string]model.Job{ + "build-job": {Name: "build-job", Stage: "build", Script: []any{"make"}}, + "test-job": { + Name: "test-job", + Stage: "test", + Script: []any{"make test"}, + Rules: []model.Rule{{Needs: []any{"build-job"}}}, + }, + }, + } + if findings := checkRulesNeeds(p, nil); len(findings) != 0 { + t.Errorf("expected no findings for valid rules:needs:; got %v", findings) + } +} + +// TestCheckRulesNeeds_Optional verifies that optional: true downgrades to Warning. +func TestCheckRulesNeeds_Optional(t *testing.T) { + p := &model.Pipeline{ + Jobs: map[string]model.Job{ + "test-job": { + Name: "test-job", + Script: []any{"make test"}, + Rules: []model.Rule{ + {Needs: []any{map[string]any{"job": "ghost", "optional": true}}}, + }, + }, + }, + } + findings := checkRulesNeeds(p, nil) + if len(findings) != 1 || findings[0].Severity != Warning { + t.Errorf("optional unknown rules:needs: should produce Warning; got %v", findings) + } +} + +// TestCheckRulesNeeds_CrossPipeline verifies that cross-pipeline needs are ignored. +func TestCheckRulesNeeds_CrossPipeline(t *testing.T) { + p := &model.Pipeline{ + Jobs: map[string]model.Job{ + "test-job": { + Name: "test-job", + Script: []any{"make test"}, + Rules: []model.Rule{ + {Needs: []any{map[string]any{"pipeline": "other", "job": "j"}}}, + }, + }, + }, + } + if findings := checkRulesNeeds(p, nil); len(findings) != 0 { + t.Errorf("cross-pipeline rules:needs: should be ignored; got %v", findings) + } +} + +// TestCheckRulesNeeds_SkippedJob verifies that a skipped job's rules:needs: violations are suppressed. +func TestCheckRulesNeeds_SkippedJob(t *testing.T) { + p := &model.Pipeline{ + Jobs: map[string]model.Job{ + "test-job": { + Name: "test-job", + Script: []any{"make test"}, + Rules: []model.Rule{{Needs: []any{"nonexistent"}}}, + }, + }, + } + if findings := checkRulesNeeds(p, map[string]bool{"test-job": true}); len(findings) != 0 { + t.Errorf("skipped job: expected no findings; got %v", findings) + } +} + +// TestCheckRulesNeeds_NoNeeds verifies no findings when rules have no needs: override. +func TestCheckRulesNeeds_NoNeeds(t *testing.T) { + p := &model.Pipeline{ + Jobs: map[string]model.Job{ + "test-job": { + Name: "test-job", + Script: []any{"make test"}, + Rules: []model.Rule{{When: "on_success"}}, + }, + }, + } + if findings := checkRulesNeeds(p, nil); len(findings) != 0 { + t.Errorf("rules without needs: should produce no findings; got %v", findings) + } +} + // TestCheckNeeds_StageOrder verifies that a job needing a job in a later stage // produces RuleNeedsStageOrder (line 64-76 in needs.go). func TestCheckNeeds_StageOrder(t *testing.T) { diff --git a/internal/linter/rules.go b/internal/linter/rules.go index c43eaaa..aed68bb 100644 --- a/internal/linter/rules.go +++ b/internal/linter/rules.go @@ -154,4 +154,8 @@ const ( // pipeline has no 'default:' block, making the declaration a no-op. Also fires // when 'inherit: default: [list]' names fields not set in the default: block. RuleInheritNoDefault = "GL043" + + // GL044: a rules:needs: entry references a job that does not exist in the pipeline. + // rules:needs: overrides the top-level needs: when a specific rule matches (GitLab CI 16.4+). + RuleRulesNeedsUnknown = "GL044" ) diff --git a/internal/model/pipeline.go b/internal/model/pipeline.go index a88cb69..eec44b1 100644 --- a/internal/model/pipeline.go +++ b/internal/model/pipeline.go @@ -89,6 +89,7 @@ type Rule struct { Changes any `yaml:"changes"` // []string or {paths,compare_to} map Exists any `yaml:"exists"` // []string or map form Variables map[string]any `yaml:"variables"` // set/override variables when rule matches (GitLab CI 15.0+) + Needs []any `yaml:"needs"` // override needs: when this rule matches (GitLab CI 16.4+) } // ReservedKeys are top-level GitLab CI keys that are NOT job definitions. diff --git a/testdata/rules_needs_invalid.yml b/testdata/rules_needs_invalid.yml new file mode 100644 index 0000000..6d9de36 --- /dev/null +++ b/testdata/rules_needs_invalid.yml @@ -0,0 +1,15 @@ +stages: + - build + - test + +build-job: + stage: build + script: make + +test-job: + stage: test + script: make test + rules: + - if: '$CI_COMMIT_BRANCH == "main"' + needs: [build-job, nonexistent-job] # nonexistent-job does not exist → GL044 + - when: on_success diff --git a/testdata/rules_needs_valid.yml b/testdata/rules_needs_valid.yml new file mode 100644 index 0000000..c688893 --- /dev/null +++ b/testdata/rules_needs_valid.yml @@ -0,0 +1,19 @@ +stages: + - build + - test + +build-job: + stage: build + script: make + +lint-job: + stage: build + script: make lint + +test-job: + stage: test + script: make test + rules: + - if: '$CI_COMMIT_BRANCH == "main"' + needs: [build-job, lint-job] # both exist → clean + - when: on_success