diff --git a/CHANGELOG.md b/CHANGELOG.md index 781afe4..98bbdf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ 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.20] - 2026-06-14 + +### Added + +- **`glint explain `** — new subcommand that prints the description, rationale, a bad-YAML example, and the corrected fix for any rule ID (GL001–GL043). `glint explain` with no argument lists all rules with ID, severity, and title. Rule IDs are case-insensitive (`gl007` and `GL007` are equivalent). + +- **`rules:if:` evaluated reachability (GL042)** — warns when every `rules:if:` condition in a job evaluates to false using the values of variables declared in the pipeline YAML, meaning the job can never be included in a pipeline run. The check is conservative: it only fires when all variables referenced in the `if:` expressions are explicitly declared in the YAML; predefined `CI_*` / `GITLAB_*` variables and undeclared variables are treated as unknown (may have any runtime value) to avoid false positives. + +- **`inherit:` completeness (GL043)** — warns when a job declares `inherit: default:` (boolean or list form) but the pipeline has no `default:` block, making the declaration a no-op. Also warns when the list form names fields (e.g. `[image, before_script]`) that are not actually set in the `default:` block — those list entries have no effect. + ## [0.2.19] - 2026-06-14 ### Added diff --git a/README.md b/README.md index a649be3..ae989e4 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. @@ -39,6 +39,9 @@ A local tool to validate and lint `.gitlab-ci.yml` pipelines without needing a G - **`pages:` keyword + `artifacts.paths` (GL039)** — warns when a job uses the `pages:` keyword but `artifacts.paths` does not include the publish directory (default: `public`) - **Duplicate stage names (GL040)** — warns when a stage name appears more than once in `stages:`; GitLab silently merges duplicates, which can cause confusing ordering - **`cache.key.files` glob detection (GL041)** — warns when `cache.key.files` entries contain glob metacharacters; this field requires exact file paths, not patterns +- **`rules:if:` evaluated reachability (GL042)** — warns when every `rules:if:` condition in a job evaluates to false using the values of variables declared in the pipeline YAML, making the job statically unreachable; only fires when all referenced variables are declared (predefined `CI_*` vars are excluded to avoid false positives) +- **`inherit:` completeness (GL043)** — warns when `inherit: default:` is declared but the pipeline has no `default:` block (dead declaration), or when the list form names fields not set in the `default:` block +- **`glint explain `** — new subcommand that prints the rule description, rationale, a bad-YAML example, and the corrected fix; `glint explain` (no argument) lists all rules with their severity - **Recursive include depth limit** — include chains are capped at 100 nesting levels (matching GitLab's own limit); project and component includes are now tracked in the visited-file set to prevent cross-include cycles - **`include: inputs:` substitution** — when a `component:` entry has a `with:` block, all `$[[ inputs.KEY ]]` and `$[[ inputs.KEY | default(…) ]]` placeholders in the fetched template are substituted before parsing, so component-scoped jobs get their correct `stage:` and keyword values instead of `$[[…]]` placeholders - **Offline mode + include cache** — pass `--cache-dir DIR` to cache fetched remote templates (project: and component: includes) to disk; `--offline` serves entirely from the cache without making network calls @@ -321,6 +324,21 @@ Jobs are colour-coded by type: DAG mode (job-to-job Bézier arrows) activates automatically when any job has a `needs:` list. Classic mode draws L-shaped or straight connectors between stage columns otherwise. +### `glint explain` + +Print the documentation for a specific lint rule. + +```bash +# Show description, example, and fix for GL007 +glint explain GL007 + +# Case-insensitive: gl007 and GL007 are equivalent +glint explain gl007 + +# List all rules with their IDs and severity +glint explain +``` + ### Context simulation Pass `--branch`, `--tag`, or `--source` to `glint check` to see which jobs @@ -462,6 +480,8 @@ Every finding includes a stable rule ID (e.g. `GL003`) that can be used to filte | GL032 | WARNING | `rules:if:` references `$VAR` not declared in `variables:` (pipeline, job, or `workflow:rules:variables:`) — may be a false positive for variables set in GitLab CI/CD project settings | | GL033 | WARNING | Every rule in `rules:` has `when: never` — job is permanently excluded from the pipeline (statically provable without context) | | GL035 | WARNING | `rules:changes` / `rules:exists` path is absolute; GitLab CI paths are relative to the repo root — absolute paths will never match | +| GL042 | WARNING | Every `rules:if:` condition evaluates to false given the declared pipeline variable values — job can never be active (only fires when all referenced variables are declared in YAML) | +| GL043 | WARNING | `inherit: default:` is declared but there is no `default:` block in the pipeline, or the list form names fields not set in `default:` — declaration has no effect | ### Hidden jobs (templates) diff --git a/ROADMAP.md b/ROADMAP.md index 3f20d23..fd8fe3c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -4,53 +4,25 @@ This document tracks planned improvements to `glint`. Items are grouped by theme --- -## Context-aware validation — ✓ single-context shipped in v0.2.0; expression evaluator hardened post-v0.2.0; implicit defaults and --list-vars shipped v0.2.11; variable expansion and scalar handling shipped v0.2.13; workflow evaluation and output fixes shipped v0.2.14 +## Context simulation -Single-context simulation is fully implemented. Pass `--branch`, `--tag`, `--source`, or `--var` to either `glint check` or `glint graph`; jobs are evaluated and shown as active / manual / skipped. +Pass `--branch`, `--tag`, `--source`, or `--var` to `glint check` or `glint graph` to evaluate `rules:if:` expressions and `only`/`except` filters against a specific pipeline event. -```bash -# shipped: single-context simulation -glint check --branch develop .gitlab-ci.yml -glint check --tag v1.2.0 .gitlab-ci.yml -glint check --source merge_request_event --var CI_MERGE_REQUEST_TARGET_BRANCH_NAME=main .gitlab-ci.yml -glint graph tree --branch main .gitlab-ci.yml # tree annotated with [skipped] / [manual] -``` - -**Shipped post-v0.2.0 (unreleased)** - -- ✓ **`workflow:rules:variables:` propagation** — variables defined on the matching `workflow:rules:` entry are injected into the evaluation context before job `rules:if:` expressions are evaluated. Pipeline-level `variables:` defaults are also available. Priority chain (highest wins): `--var` > shortcuts > workflow-rule vars > pipeline defaults. -- ✓ **Expression evaluator: multi-line expressions** — newlines in block-scalar and folded YAML `if:` values are now treated as whitespace; `||` / `&&` on a continuation line evaluate correctly. -- ✓ **Expression evaluator: `${VAR}` curly-brace syntax** — `${CI_COMMIT_BRANCH}` is equivalent to `$CI_COMMIT_BRANCH` everywhere. -- ✓ **Expression evaluator: regex flags** — `/pattern/i`, `/pattern/m`, `/pattern/s` are now supported; `i` maps to `(?i)` in Go's regexp. -- ✓ **Expression evaluator: variable as regex RHS** — `$BRANCH =~ $PATTERN` where `$PATTERN` holds a `/regex/` string is evaluated correctly. -- ✓ **Expression evaluator: bare `true` / `false` keywords** — treated as the strings `"true"` / `"false"` matching GitLab CI's own behaviour; `$GATEWAY_ENABLED == true` now evaluates correctly. -- ✓ **Expression evaluator: integer literals** — `$COUNT == 4`, `$ENABLED == 1`, `$DISABLED == 0` compare as decimal strings. - -~~**Implicit default context**~~ — ✓ shipped v0.2.11; `glint check` and `glint graph` default to `--branch main --source push` when no context flag is given, so `rules:if:` expressions are always evaluated out of the box. - -~~**`--list-vars` debug flag**~~ — ✓ shipped v0.2.11; prints sorted `KEY=VALUE` of all collected variables (pipeline YAML + included files + workflow-rule union + effective context) to stderr. - -**Shipped in v0.2.13** - -- ✓ **Variable expansion** — `$VAR` / `${VAR}` references within variable values are expanded after all sources are merged; transitive chains resolve over multiple passes; visible in `--list-vars` effective-context output. -- ✓ **Non-string scalar variables** — `BUILD: true`, `RETRIES: 3` and similar bare boolean/integer values now render correctly in `--list-vars` and are injected into the evaluation context as string equivalents; previously shown as `(complex)` and silently dropped. -- ✓ **YAML `\/` escape in double-quoted strings** — regex patterns like `/^us\//` in double-quoted `if:` blocks no longer cause a parse error; the raw bytes are preprocessed before YAML unmarshalling. - -**Shipped in v0.2.14** - -- ✓ **Workflow rule strict evaluation** — workflow `rules:if:` now uses strict mode (parse failure → skip rule, not match); fixes premature matching that blocked later rules and injected wrong variables. -- ✓ **Single `=` operator** — `=` is now accepted as an alias for `==` in `rules:if:` expressions, matching common user intent. -- ✓ **Source location through `extends:` resolution** — `File` and `Line` are now preserved when a job is rebuilt via extends, so findings reference the correct source location. -- ✓ **Sorted findings output** — findings are sorted by `(File, Line, Rule)`; same-file issues group together in line order. -- ✓ **Consistent warning format** — all warnings use ruff-style `path: [warning] message` format. -- ✓ **`--version` / `-v` flag** — prints compiled version; version also shown at the top of every `--help` output. - -**Remaining work** - -- **Multi-context simulation** — run multiple contexts in one invocation and print a comparison table: - ```bash - glint check --context branch=main --context branch=develop --context tag=v1.0.0 .gitlab-ci.yml - ``` +- ~~**Single-context simulation**~~ — ✓ shipped v0.2.0; `--branch`, `--tag`, `--source`, `--var` flags on both subcommands; jobs classified as active / manual / skipped +- ~~**`workflow:rules:variables:` propagation**~~ — ✓ shipped post-v0.2.0; variables from the matching workflow rule entry injected into the evaluation context before job `rules:if:` expressions are evaluated +- ~~**Expression evaluator: multi-line `if:` values**~~ — ✓ shipped post-v0.2.0; newlines in block-scalar and folded YAML `if:` values treated as whitespace +- ~~**Expression evaluator: `${VAR}` syntax**~~ — ✓ shipped post-v0.2.0; `${CI_COMMIT_BRANCH}` equivalent to `$CI_COMMIT_BRANCH` everywhere +- ~~**Expression evaluator: regex flags**~~ — ✓ shipped post-v0.2.0; `/pattern/i`, `/pattern/m`, `/pattern/s` supported +- ~~**Expression evaluator: variable as regex RHS**~~ — ✓ shipped post-v0.2.0; `$BRANCH =~ $PATTERN` where `$PATTERN` holds a `/regex/` string evaluates correctly +- ~~**Expression evaluator: bare `true`/`false` and integer literals**~~ — ✓ shipped post-v0.2.0; `$FLAG == true`, `$COUNT == 4` compare as decimal strings matching GitLab CI behaviour +- ~~**Implicit default context**~~ — ✓ shipped v0.2.11; defaults to `--branch main --source push` when no context flag is given, so `rules:if:` is always evaluated +- ~~**`--list-vars` debug flag**~~ — ✓ shipped v0.2.11; prints sorted `KEY=VALUE` of all collected pipeline variables (root file + includes + workflow-rule union + effective context) to stderr +- ~~**Variable expansion**~~ — ✓ shipped v0.2.13; `$VAR`/`${VAR}` references within variable values expanded after all sources are merged; transitive chains resolved; visible in `--list-vars` +- ~~**Non-string scalar variables**~~ — ✓ shipped v0.2.13; `BUILD: true`, `RETRIES: 3` rendered and injected correctly instead of being silently dropped +- ~~**YAML `\/` escape in double-quoted strings**~~ — ✓ shipped v0.2.13; regex patterns like `/^us\//` in double-quoted `if:` blocks no longer cause a parse error +- ~~**Workflow rule strict evaluation**~~ — ✓ shipped v0.2.14; unparseable `if:` skips the rule instead of matching everything; prevents wrong variables being injected +- ~~**Single `=` operator**~~ — ✓ shipped v0.2.14; bare `=` accepted as alias for `==` in `rules:if:` expressions +- **Multi-context simulation** — run multiple contexts in one invocation and print a comparison table (`--context branch=main --context branch=develop --context tag=v1.0.0`) - **Context-scoped linting** — skip `needs:`/`dependencies:` cross-checks for jobs that are statically unreachable in the given context - **`rules:changes:` evaluation** — path glob evaluation against the local git tree @@ -70,7 +42,7 @@ The current rule set covers the most common sources of broken pipelines. These a - ~~**Duplicate stage names (GL040)**~~ — ✓ shipped v0.2.16; warns when a stage appears more than once in `stages:` - ~~**`cache:key:files` must be exact paths (GL041)**~~ — ✓ shipped v0.2.16; warns when entries look like glob patterns - ~~**Unreachable jobs**~~ — covered by GL033 (shipped v0.2.15); every-`when:never` rules block is statically dead -- **`inherit:` completeness** — flag when a job overrides a default field that would require `inherit: default: false` to suppress +- ~~**`inherit:` completeness (GL043)**~~ — ✓ shipped v0.2.20; warns when `inherit: default:` is declared but there's no `default:` block, or list form names fields not set in `default:` --- @@ -118,7 +90,7 @@ The SVG renderer and terminal tree cover the basic layout. These would bring it - ~~**`needs: optional: true` false-positive errors**~~ — ✓ shipped post-v0.2.0; optional missing needs are downgraded to `[WARNING]` - ~~**`extends:` jobs with missing script false errors**~~ — ✓ shipped post-v0.2.0; jobs using `extends:` that have no `script` after resolution emit `[WARNING]` (the script may come from an unfetchable remote base) -- **`rules:if:` static reachability** — report when a job's entire `rules:` block can never evaluate to `when: on_success` given the declared pipeline variables (pure static, no context required) +- ~~**`rules:if:` static reachability (GL042)**~~ — ✓ shipped v0.2.20; warns when all `rules:if:` conditions evaluate to false given declared variable values (only fires when all referenced vars are declared in YAML) --- @@ -141,8 +113,8 @@ 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 -- **`--explain `** — print the rule description, rationale, and an example fix +- ~~**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 +- ~~**`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` - **Changelog automation** — generate release notes from Conventional Commits via `git-cliff` or similar diff --git a/Taskfile.yml b/Taskfile.yml index 228cfe6..ae1d576 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -107,6 +107,18 @@ tasks: ignore_error: false - cmd: ./{{.BINARY}} check testdata/config_suppress/.gitlab-ci.yml ignore_error: false + - cmd: ./{{.BINARY}} check testdata/static_dead_rules.yml + ignore_error: false + - cmd: ./{{.BINARY}} check testdata/inherit_dead.yml + ignore_error: false + - cmd: ./{{.BINARY}} check testdata/inherit_dead_fields.yml + ignore_error: false + - cmd: ./{{.BINARY}} explain GL007 + ignore_error: false + - cmd: ./{{.BINARY}} explain gl042 + ignore_error: false + - cmd: ./{{.BINARY}} explain + ignore_error: false lint-go: desc: Run go vet on all packages diff --git a/cmd/glint/explain.go b/cmd/glint/explain.go new file mode 100644 index 0000000..bafbe5a --- /dev/null +++ b/cmd/glint/explain.go @@ -0,0 +1,69 @@ +package main + +import ( + "fmt" + "os" + "sort" + "strings" + + "git.k3nny.fr/glint/internal/linter" +) + +func cmdExplain(args []string) { + if len(args) == 0 { + printRuleList() + return + } + ruleID := strings.ToUpper(args[0]) + entry, ok := linter.RuleCatalog[ruleID] + if !ok { + fmt.Fprintf(os.Stderr, "glint explain: unknown rule %q\n\nRun 'glint explain' to list all rules.\n", ruleID) + os.Exit(2) + } + printRuleEntry(ruleID, entry) +} + +func printRuleEntry(id string, e linter.RuleEntry) { + sev := strings.ToLower(string(e.Severity)) + header := fmt.Sprintf("%s [%s] %s", id, sev, e.Title) + rule := fmt.Sprintf("\n%s\n%s\n\n%s\n", + header, + strings.Repeat("─", len(header)), + e.Description, + ) + fmt.Print(rule) + + if e.Example != "" { + fmt.Println("Example:") + fmt.Println() + for _, line := range strings.Split(e.Example, "\n") { + fmt.Printf(" %s\n", line) + } + fmt.Println() + } + + if e.Fix != "" { + fmt.Println("Fix:") + fmt.Println() + for _, line := range strings.Split(e.Fix, "\n") { + fmt.Printf(" %s\n", line) + } + fmt.Println() + } +} + +func printRuleList() { + ids := make([]string, 0, len(linter.RuleCatalog)) + for id := range linter.RuleCatalog { + ids = append(ids, id) + } + sort.Strings(ids) + + fmt.Printf("glint %s — lint rules\n\n", version) + for _, id := range ids { + e := linter.RuleCatalog[id] + sev := strings.ToLower(string(e.Severity)) + fmt.Printf(" %-6s [%-7s] %s\n", id, sev, e.Title) + } + fmt.Println("\nUse 'glint explain ' for details on a specific rule.") +} diff --git a/cmd/glint/main.go b/cmd/glint/main.go index 7af60e0..ba71e14 100644 --- a/cmd/glint/main.go +++ b/cmd/glint/main.go @@ -39,6 +39,7 @@ Usage: glint [OPTIONS] Commands: check Lint a pipeline file — exits 0 (clean) or 1 (errors found) graph Visualise the pipeline as a job tree or Mermaid graph + explain Show description and fix for a lint rule (e.g. glint explain GL007) Options: -h, --help Print help @@ -57,6 +58,8 @@ func main() { cmdCheck(os.Args[2:]) case "graph": cmdGraph(os.Args[2:]) + case "explain": + cmdExplain(os.Args[2:]) case "-h", "--help", "help": fmt.Fprintf(os.Stderr, "glint %s\n\n", version) fmt.Fprint(os.Stderr, globalUsage) diff --git a/internal/linter/explain.go b/internal/linter/explain.go new file mode 100644 index 0000000..0d7b8cf --- /dev/null +++ b/internal/linter/explain.go @@ -0,0 +1,819 @@ +package linter + +// RuleEntry holds the human-readable documentation for a single lint rule, +// shown by 'glint explain '. +type RuleEntry struct { + Title string // short title (one line) + Severity Severity // Error or Warning + Description string // what triggers the rule and why it matters + Example string // YAML snippet that triggers the rule + Fix string // corrected YAML +} + +// RuleCatalog maps stable rule IDs to their documentation entries. +var RuleCatalog = map[string]RuleEntry{ + RuleNoStages: { + Title: "no stages defined", + Severity: Warning, + Description: "The pipeline has no 'stages:' block. GitLab falls back to the " + + "built-in default stages (build, test, deploy), which may not match your " + + "intended job ordering.", + Example: `# No stages: block +build-job: + script: make build + +test-job: + script: make test`, + Fix: `stages: + - build + - test + +build-job: + stage: build + script: make build + +test-job: + stage: test + script: make test`, + }, + + RuleWorkflowWhen: { + Title: "workflow.rules.when invalid value", + Severity: Error, + Description: "'workflow.rules[n].when' only accepts 'always' or 'never'. Any " + + "other value (such as 'on_success' or 'manual') is invalid and will cause " + + "the pipeline to fail to create.", + Example: `workflow: + rules: + - if: $CI_COMMIT_BRANCH + when: on_success # invalid here`, + Fix: `workflow: + rules: + - if: $CI_COMMIT_BRANCH + when: always`, + }, + + RuleMissingScript: { + Title: "missing required 'script' field", + Severity: Error, + Description: "Every non-trigger, non-pages job must define 'script:' (or 'run:' " + + "for CI Steps). A job with no script will fail immediately when GitLab tries " + + "to run it.", + Example: `my-job: + stage: test + image: python:3.12 + # Missing script:`, + Fix: `my-job: + stage: test + image: python:3.12 + script: pytest`, + }, + + RuleUnknownStage: { + Title: "job references undeclared stage", + Severity: Error, + Description: "The job's 'stage:' value does not match any name in the pipeline's " + + "'stages:' list. GitLab will reject the pipeline configuration.", + Example: `stages: + - build + - test + +deploy-job: + stage: production # not in stages: + script: ./deploy.sh`, + Fix: `stages: + - build + - test + - production + +deploy-job: + stage: production + script: ./deploy.sh`, + }, + + RuleOnlyRulesConflict: { + Title: "'only:' and 'rules:' used together", + Severity: Error, + Description: "'only:' and 'rules:' are mutually exclusive. Using both on the " + + "same job is a configuration error that prevents the pipeline from being " + + "created.", + Example: `my-job: + only: + - main + rules: + - if: $CI_COMMIT_BRANCH + script: echo conflict`, + Fix: `# Remove only: and use rules: exclusively +my-job: + rules: + - if: $CI_COMMIT_BRANCH == "main" + script: echo ok`, + }, + + RuleExceptRulesConflict: { + Title: "'except:' and 'rules:' used together", + Severity: Error, + Description: "'except:' and 'rules:' are mutually exclusive. Using both on the " + + "same job is a configuration error that prevents the pipeline from being " + + "created.", + Example: `my-job: + except: + - tags + rules: + - if: $CI_COMMIT_BRANCH + script: echo conflict`, + Fix: `my-job: + rules: + - if: $CI_COMMIT_TAG + when: never + - if: $CI_COMMIT_BRANCH + script: echo ok`, + }, + + RuleDeprecatedOnly: { + Title: "'only:' / 'except:' deprecated", + Severity: Warning, + Description: "'only:' and 'except:' are deprecated in favour of 'rules:'. " + + "GitLab may remove support for these keywords in a future major version. " + + "'rules:' is more expressive and supports variable references, merge-request " + + "conditions, and fine-grained 'when:' control.", + Example: `my-job: + only: + - main + script: ./deploy.sh`, + Fix: `my-job: + rules: + - if: $CI_COMMIT_BRANCH == "main" + script: ./deploy.sh`, + }, + + RuleInvalidWhen: { + Title: "'when:' has invalid value", + Severity: Error, + Description: "Job-level 'when:' accepts: on_success, on_failure, always, " + + "manual, delayed, never. Any other value is rejected by GitLab.", + Example: `my-job: + when: sometimes # invalid + script: echo hi`, + Fix: `my-job: + when: on_success + script: echo hi`, + }, + + RuleDelayedNoStartIn: { + Title: "'when: delayed' without 'start_in:'", + Severity: Error, + Description: "'when: delayed' requires a 'start_in:' duration (e.g. " + + "'30 minutes', '1 hour'). Without it GitLab rejects the job configuration.", + Example: `my-job: + when: delayed + script: echo hi`, + Fix: `my-job: + when: delayed + start_in: 30 minutes + script: echo hi`, + }, + + RuleStartInNoDelayed: { + Title: "'start_in:' without 'when: delayed'", + Severity: Error, + Description: "'start_in:' is only meaningful when 'when: delayed'. Setting it " + + "alongside any other 'when:' value is a configuration error.", + Example: `my-job: + when: on_success + start_in: 30 minutes # only valid with delayed + script: echo hi`, + Fix: `my-job: + when: delayed + start_in: 30 minutes + script: echo hi`, + }, + + RuleInvalidParallel: { + Title: "'parallel:' value invalid", + Severity: Error, + Description: "'parallel:' must be an integer between 2 and 200 (inclusive), " + + "or a map with a 'matrix:' key for matrix jobs. Values outside this range " + + "or the wrong type are rejected by GitLab.", + Example: `my-job: + parallel: 1 # must be 2–200 + script: echo hi`, + Fix: `my-job: + parallel: 5 + script: echo hi`, + }, + + RuleInvalidRetry: { + Title: "'retry:' value invalid", + Severity: Error, + Description: "'retry:' accepts an integer 0–2 or a map with optional 'max:' " + + "(0–2) and 'when:' keys. Values outside this range are rejected.", + Example: `my-job: + retry: 5 # max is 2 + script: echo hi`, + Fix: `my-job: + retry: 2 + script: echo hi`, + }, + + RuleInvalidRetryWhen: { + Title: "'retry.when:' has invalid failure type", + Severity: Error, + Description: "'retry.when:' must be a recognised GitLab CI failure type " + + "(e.g. script_failure, runner_system_failure) or 'always'. Unknown " + + "values are rejected.", + Example: `my-job: + retry: + max: 2 + when: disk_full # not a valid failure type + script: echo hi`, + Fix: `my-job: + retry: + max: 2 + when: runner_system_failure + script: echo hi`, + }, + + RuleInvalidAllowFailure: { + Title: "'allow_failure:' invalid value", + Severity: Error, + Description: "'allow_failure:' must be a boolean (true/false) or a map with " + + "an 'exit_codes:' key. Other forms are rejected by GitLab.", + Example: `my-job: + allow_failure: maybe # must be true/false or map + script: echo hi`, + Fix: `my-job: + allow_failure: true + script: echo hi`, + }, + + RuleInvalidInterruptible: { + Title: "'interruptible:' is not a boolean", + Severity: Error, + Description: "'interruptible:' must be a boolean (true or false). Other types " + + "are rejected.", + Example: `my-job: + interruptible: yes # must be a YAML boolean true/false + script: echo hi`, + Fix: `my-job: + interruptible: true + script: echo hi`, + }, + + RuleTriggerWithScript: { + Title: "trigger job also defines 'script:'", + Severity: Error, + Description: "Jobs with a 'trigger:' key (downstream pipeline triggers) cannot " + + "use 'script:'. These are mutually exclusive — a trigger job has no runner " + + "environment to execute scripts in.", + Example: `trigger-job: + trigger: + project: mygroup/myproject + script: echo this will fail`, + Fix: `trigger-job: + trigger: + project: mygroup/myproject`, + }, + + RuleInvalidTrigger: { + Title: "trigger: map missing 'project:' or 'include:'", + Severity: Error, + Description: "When 'trigger:' is a map it must specify either 'project:' " + + "(downstream project trigger) or 'include:' (dynamic child pipeline). " + + "Without one of these keys GitLab cannot determine what to trigger.", + Example: `trigger-job: + trigger: + strategy: depend # missing project: or include:`, + Fix: `trigger-job: + trigger: + project: mygroup/downstream + strategy: depend`, + }, + + RuleInvalidCoverage: { + Title: "'coverage:' is not a regex pattern", + Severity: Error, + Description: "'coverage:' must be a regex pattern wrapped in forward slashes " + + "(e.g. '/Coverage: \\d+\\.?\\d*%/'). GitLab uses this pattern to extract " + + "the coverage percentage from job output.", + Example: `my-job: + coverage: "\\d+%" # missing surrounding /…/ + script: pytest --cov`, + Fix: `my-job: + coverage: '/Coverage: \d+\.?\d*%/' + script: pytest --cov`, + }, + + RuleInvalidRelease: { + Title: "'release:' missing required 'tag_name:'", + Severity: Error, + Description: "'release:' must be a map and must include a 'tag_name:' key. " + + "Without it GitLab cannot create the release.", + Example: `release-job: + script: echo releasing + release: + name: My Release # missing tag_name:`, + Fix: `release-job: + script: echo releasing + release: + tag_name: $CI_COMMIT_TAG + name: My Release`, + }, + + RuleInvalidEnvironment: { + Title: "'environment:' invalid url or action", + Severity: Error, + Description: "'environment.url' requires 'environment.name' to be set. " + + "'environment.action' must be one of: start, stop, prepare, verify, access.", + Example: `my-job: + script: ./deploy.sh + environment: + url: https://prod.example.com # name: is required`, + Fix: `my-job: + script: ./deploy.sh + environment: + name: production + url: https://prod.example.com`, + }, + + RuleInvalidArtifacts: { + Title: "'artifacts:' invalid configuration", + Severity: Error, + Description: "'artifacts.when' must be on_success, on_failure, or always. " + + "'artifacts.expose_as' requires 'artifacts.paths' to be set.", + Example: `my-job: + script: make + artifacts: + when: sometimes # invalid value`, + Fix: `my-job: + script: make + artifacts: + when: on_failure + paths: + - build/logs/`, + }, + + RulePagesPublic: { + Title: "pages job missing 'public' in artifacts.paths", + Severity: Warning, + Description: "The 'pages' job must include 'public' (or a path starting with " + + "'public/') in 'artifacts.paths' for GitLab Pages to deploy the site.", + Example: `pages: + script: hugo + artifacts: + paths: + - dist/ # should include public/`, + Fix: `pages: + script: hugo --destination public + artifacts: + paths: + - public/`, + }, + + RuleInvalidCache: { + Title: "'cache:' invalid when or policy", + Severity: Error, + Description: "'cache.when' must be on_success, on_failure, or always. " + + "'cache.policy' must be pull, push, or pull-push.", + Example: `my-job: + script: npm ci + cache: + paths: [node_modules/] + policy: read-only # invalid; use pull`, + Fix: `my-job: + script: npm ci + cache: + paths: [node_modules/] + policy: pull`, + }, + + RuleInvalidRulesWhen: { + Title: "'rules[n].when:' has invalid value", + Severity: Error, + Description: "Inside a 'rules:' block, 'when:' must be one of: on_success, " + + "on_failure, always, manual, delayed, never. Other values are rejected.", + Example: `my-job: + rules: + - if: $CI_COMMIT_BRANCH + when: conditional # not valid here + script: echo hi`, + Fix: `my-job: + rules: + - if: $CI_COMMIT_BRANCH + when: on_success + script: echo hi`, + }, + + RuleInvalidImage: { + Title: "'image:' map form missing 'name:'", + Severity: Error, + Description: "When 'image:' is a map, it must include a 'name:' key specifying " + + "the Docker image. Without it GitLab cannot resolve which image to pull.", + Example: `my-job: + image: + entrypoint: ["/bin/sh"] # missing name: + script: echo hi`, + Fix: `my-job: + image: + name: alpine:3.19 + entrypoint: ["/bin/sh"] + script: echo hi`, + }, + + RuleInvalidInherit: { + Title: "'inherit.default' or 'inherit.variables' invalid type", + Severity: Error, + Description: "'inherit.default' and 'inherit.variables' must each be a boolean " + + "(true/false) or a list of field/variable names. Other types are rejected.", + Example: `my-job: + inherit: + default: "no" # must be true/false or a list + script: echo hi`, + Fix: `my-job: + inherit: + default: false + script: echo hi`, + }, + + RuleNeedsUnknown: { + Title: "'needs:' references unknown job", + Severity: Error, + Description: "A job in the 'needs:' list does not exist in the pipeline. " + + "GitLab will refuse to create the pipeline.", + Example: `build: + stage: build + script: make + +test: + stage: test + needs: [build, lint] # lint does not exist + script: make test`, + Fix: `lint: + stage: build + script: make lint + +build: + stage: build + script: make + +test: + stage: test + needs: [build, lint] + script: make test`, + }, + + RuleNeedsStageOrder: { + Title: "'needs:' job is in a later stage", + Severity: Error, + Description: "A job listed in 'needs:' is in a later stage than the current " + + "job. GitLab does not allow needs: to reference future stages.", + Example: `stages: [build, test, deploy] + +test: + stage: test + needs: [deploy-job] # deploy-job is in a later stage + script: make test + +deploy-job: + stage: deploy + script: ./deploy.sh`, + Fix: `stages: [build, test, deploy] + +test: + stage: test + needs: [build-job] + script: make test + +build-job: + stage: build + script: make`, + }, + + RuleNeedsCycle: { + Title: "circular dependency in 'needs:' graph", + Severity: Error, + Description: "Two or more jobs in the 'needs:' graph form a cycle — A needs B " + + "and B needs A (directly or transitively). GitLab will reject the pipeline.", + Example: `job-a: + stage: test + needs: [job-b] + script: echo a + +job-b: + stage: test + needs: [job-a] + script: echo b`, + Fix: `job-a: + stage: test + script: echo a + +job-b: + stage: test + needs: [job-a] + script: echo b`, + }, + + RuleUnknownDependency: { + Title: "'dependencies:' references unknown job", + Severity: Error, + Description: "A job listed in 'dependencies:' does not exist in the pipeline. " + + "GitLab will refuse to create the pipeline.", + Example: `test: + stage: test + dependencies: [build, missing-job] + script: make test`, + Fix: `build: + stage: build + script: make + artifacts: + paths: [dist/] + +test: + stage: test + dependencies: [build] + script: make test`, + }, + + RuleDependencyStage: { + Title: "'dependencies:' job in same or later stage", + Severity: Error, + Description: "'dependencies:' can only reference jobs in earlier stages. " + + "Referencing a job in the same or a later stage is not allowed because " + + "artifacts would not yet be available.", + Example: `stages: [test, deploy] + +test: + stage: test + dependencies: [deploy-job] # deploy is a later stage + script: make test + +deploy-job: + stage: deploy + script: ./deploy.sh`, + Fix: `stages: [build, test, deploy] + +build: + stage: build + script: make + artifacts: + paths: [dist/] + +test: + stage: test + dependencies: [build] + script: make test`, + }, + + RuleUndeclaredVariable: { + Title: "'rules:if:' references undeclared variable", + Severity: Warning, + Description: "A 'rules:if:' expression references a variable that is not " + + "declared in any 'variables:' block in the pipeline YAML. This may be a " + + "typo, or the variable may be set in GitLab project settings (invisible to " + + "glint). Predefined CI_* and GITLAB_* variables are always exempt.", + Example: `variables: + APP_ENV: staging + +my-job: + rules: + - if: $DEPLOY_ENV == "prod" # DEPLOY_ENV not declared + script: ./deploy.sh`, + Fix: `variables: + APP_ENV: staging + DEPLOY_ENV: staging # declare it, or set it in GitLab project settings + +my-job: + rules: + - if: $DEPLOY_ENV == "prod" + script: ./deploy.sh`, + }, + + RuleDeadRules: { + Title: "rules: block has all 'when: never' — job permanently excluded", + Severity: Warning, + Description: "Every rule in the job's 'rules:' block has an explicit " + + "'when: never'. No matter which conditions match, the job will never " + + "be included in any pipeline run.", + Example: `my-job: + rules: + - if: $CI_COMMIT_BRANCH == "main" + when: never + - when: never + script: echo unreachable`, + Fix: `# Option 1: remove the job entirely +# Option 2: give at least one rule a non-never when: +my-job: + rules: + - if: $CI_COMMIT_BRANCH == "main" + when: on_success + - when: never + script: echo deploy`, + }, + + RuleInvalidService: { + Title: "'services:' map form missing 'name:' or invalid 'alias:'", + Severity: Error, + Description: "When a service entry is a map it must include a 'name:' key. " + + "'alias:' (if set) must be a valid DNS label: alphanumeric characters, " + + "hyphens, and dots only; must start and end with alphanumeric.", + Example: `my-job: + script: pytest + services: + - alias: my_db # underscore is not valid in a DNS label`, + Fix: `my-job: + script: pytest + services: + - name: postgres:15 + alias: my-db`, + }, + + RuleAbsoluteGlobPath: { + Title: "'rules:changes' or 'rules:exists' uses absolute path", + Severity: Warning, + Description: "Paths in 'rules:changes' and 'rules:exists' are always relative " + + "to the repository root. An absolute path starting with '/' will never " + + "match any file and will silently disable the rule.", + Example: `my-job: + rules: + - changes: + - /src/**/*.go # absolute — will never match + script: make`, + Fix: `my-job: + rules: + - changes: + - src/**/*.go + script: make`, + }, + + RuleInvalidTimeout: { + Title: "'timeout:' is not a valid duration string", + Severity: Error, + Description: "'timeout:' must be a GitLab CI duration string composed of " + + "recognised time units: weeks (w), days (d), hours (h), minutes (m/min), " + + "seconds (s). Examples: '1h 30m', '90 minutes', '2 hours'.", + Example: `my-job: + timeout: "1:30:00" # colon-separated format not supported + script: long-running-task.sh`, + Fix: `my-job: + timeout: 1h 30m + script: long-running-task.sh`, + }, + + RuleInvalidIDToken: { + Title: "'id_tokens:' entry missing required 'aud:' key", + Severity: Error, + Description: "Each entry under 'id_tokens:' must be a map with an 'aud:' " + + "(audience) key. Without 'aud:' GitLab cannot generate the ID token " + + "and the job will fail.", + Example: `my-job: + id_tokens: + MY_TOKEN: + # missing aud: + script: vault-login.sh`, + Fix: `my-job: + id_tokens: + MY_TOKEN: + aud: https://vault.example.com + script: vault-login.sh`, + }, + + RuleInvalidSecret: { + Title: "'secrets:' entry missing a provider key", + Severity: Error, + Description: "Each entry under 'secrets:' must specify a provider: one of " + + "'vault:', 'gcp_secret_manager:', or 'azure_key_vault:'. Without a " + + "provider GitLab cannot retrieve the secret.", + Example: `my-job: + secrets: + DB_PASSWORD: + path: secret/db/password # no provider key + script: run.sh`, + Fix: `my-job: + secrets: + DB_PASSWORD: + vault: + engine: + name: kv-v2 + path: secret + path: db/password + field: password + script: run.sh`, + }, + + RulePagesPublish: { + Title: "'pages:' keyword publish directory missing from 'artifacts.paths'", + Severity: Warning, + Description: "A job using the 'pages:' keyword must include its publish " + + "directory in 'artifacts.paths'. Without this, GitLab Pages will not " + + "deploy the site.", + Example: `my-pages-job: + script: hugo + pages: + publish: public + artifacts: + paths: + - dist/ # missing public/`, + Fix: `my-pages-job: + script: hugo + pages: + publish: public + artifacts: + paths: + - public/`, + }, + + RuleDuplicateStage: { + Title: "duplicate stage name in 'stages:'", + Severity: Warning, + Description: "A stage name appears more than once in the 'stages:' list. " + + "GitLab silently merges duplicate stage entries, which can make the " + + "pipeline order confusing.", + Example: `stages: + - build + - test + - test # duplicate`, + Fix: `stages: + - build + - test`, + }, + + RuleInvalidCacheKeyFiles: { + Title: "'cache.key.files' contains a glob pattern", + Severity: Warning, + Description: "'cache.key.files' must be a list of exact file paths. Glob " + + "patterns (*, ?, [...]) are not expanded — the literal glob string is " + + "used as the cache key, which probably doesn't match your intent.", + Example: `my-job: + script: npm ci + cache: + key: + files: + - package*.json # glob — use exact path`, + Fix: `my-job: + script: npm ci + cache: + key: + files: + - package.json + - package-lock.json`, + }, + + RuleStaticDeadRules: { + Title: "rules:if: block statically never activates", + Severity: Warning, + Description: "Every 'rules:if:' condition in this job evaluates to false " + + "using the variable values declared in the pipeline YAML. The job will " + + "never be included in any pipeline run. This check only fires when ALL " + + "referenced variables are declared in the YAML (predefined CI_* variables " + + "are not evaluated to avoid false positives).", + Example: `variables: + ENABLE_DEPLOY: "false" + +deploy: + rules: + - if: '$ENABLE_DEPLOY == "true"' # always false: ENABLE_DEPLOY is "false" + script: ./deploy.sh`, + Fix: `variables: + ENABLE_DEPLOY: "true" # fix the variable value + +deploy: + rules: + - if: '$ENABLE_DEPLOY == "true"' + script: ./deploy.sh + +# Or add an unconditional fallback: +deploy: + rules: + - if: '$ENABLE_DEPLOY == "true"' + - when: manual # allow manual trigger as a fallback + script: ./deploy.sh`, + }, + + RuleInheritNoDefault: { + Title: "'inherit: default:' declared but no 'default:' block", + Severity: Warning, + Description: "A job declares 'inherit: default:' but the pipeline has no " + + "'default:' block, so the declaration has no effect. This can also fire " + + "when 'inherit: default: [list]' names fields that are not set in the " + + "'default:' block.", + Example: `# No default: block exists + +my-job: + inherit: + default: false # no-op: nothing to opt out of + script: echo hi`, + Fix: `# Either add a default: block... +default: + before_script: + - source venv/bin/activate + +my-job: + inherit: + default: false + script: echo hi + +# ...or remove the pointless inherit: declaration +my-job: + script: echo hi`, + }, +} diff --git a/internal/linter/if_reachability.go b/internal/linter/if_reachability.go new file mode 100644 index 0000000..556eefa --- /dev/null +++ b/internal/linter/if_reachability.go @@ -0,0 +1,104 @@ +package linter + +import ( + "git.k3nny.fr/glint/internal/cicontext" + "git.k3nny.fr/glint/internal/model" +) + +// checkRulesIfReachability (GL042): warn when every rule in a job's rules: block +// can be statically proven to never activate given the values of variables +// declared in the pipeline YAML. Only fires when ALL variables referenced by the +// rules:if: expressions are declared in the pipeline or job variables: blocks — +// predefined CI_* / GITLAB_* variables or variables not declared in YAML are +// treated as unknown (could have any runtime value) so the check is skipped +// conservatively to avoid false positives. +func checkRulesIfReachability(p *model.Pipeline) []Finding { + if len(p.Jobs) == 0 { + return nil + } + + // Collect scalar string values from pipeline-level variables. + pipelineVars := make(map[string]string, len(p.Variables)) + for k, v := range p.Variables { + if s, ok := cicontext.ScalarString(v); ok { + pipelineVars[k] = s + } + } + + var findings []Finding + for name, job := range p.Jobs { + if len(job.Rules) == 0 { + continue + } + + // Merge pipeline vars with job-level variable overrides. + jobVars := make(map[string]string, len(pipelineVars)+len(job.Variables)) + for k, v := range pipelineVars { + jobVars[k] = v + } + for k, v := range job.Variables { + if s, ok := cicontext.ScalarString(v); ok { + jobVars[k] = s + } + } + + if f := evalRulesReachability(name, job, jobVars); f != nil { + findings = append(findings, *f) + } + } + return findings +} + +// evalRulesReachability returns a GL042 finding when the job's rules: block +// can never activate given the provided variable map, or nil if the job +// might activate (or the check cannot be applied conservatively). +func evalRulesReachability(name string, job model.Job, jobVars map[string]string) *Finding { + lookup := func(k string) string { return jobVars[k] } + + for _, rule := range job.Rules { + // Explicit when: never — this rule is provably dead; continue. + if rule.When == "never" { + continue + } + + // No if: condition → this rule always matches → job CAN activate. + if rule.If == "" { + return nil + } + + // Check whether all variables referenced in the if: expression are + // declared in the pipeline YAML. If any are not declared (predefined + // CI vars, project settings, etc.), we cannot evaluate the expression + // and must conservatively assume the job might activate. + refs := extractIfVars(rule.If) + allDeclared := true + for _, ref := range refs { + if _, ok := jobVars[ref]; !ok { + // Variable not in YAML (predefined or unknown) → cannot evaluate. + allDeclared = false + break + } + } + if !allDeclared { + return nil // conservative: job might activate + } + + // All referenced variables are declared; evaluate the expression. + // Use strict mode (unparse → false) to avoid matching unparseable expressions. + if cicontext.EvalIfStrict(rule.If, lookup) { + // This rule's condition evaluates to true → job CAN activate. + return nil + } + // Expression evaluated to false → this rule cannot activate; continue. + } + + // No rule could activate the job. + return &Finding{ + Severity: Warning, + Rule: RuleStaticDeadRules, + Job: name, + File: job.File, + Line: job.Line, + Message: "rules: block can never activate: all if: conditions evaluate to false given the declared pipeline variables", + } +} diff --git a/internal/linter/if_reachability_test.go b/internal/linter/if_reachability_test.go new file mode 100644 index 0000000..7fe0516 --- /dev/null +++ b/internal/linter/if_reachability_test.go @@ -0,0 +1,152 @@ +package linter + +import ( + "testing" + + "git.k3nny.fr/glint/internal/model" +) + +func TestCheckRulesIfReachability(t *testing.T) { + makeJob := func(vars map[string]any, rules []model.Rule) model.Job { + return model.Job{Variables: vars, Rules: rules} + } + + cases := []struct { + name string + pipelineVars map[string]any + jobs map[string]model.Job + wantHit []string // job names that should produce GL042 + wantMiss []string // job names that should NOT produce GL042 + }{ + { + name: "declared var always false — GL042 fires", + pipelineVars: map[string]any{"DEPLOY": "false"}, + jobs: map[string]model.Job{ + "deploy": makeJob(nil, []model.Rule{ + {If: `$DEPLOY == "true"`, When: "on_success"}, + }), + }, + wantHit: []string{"deploy"}, + wantMiss: nil, + }, + { + name: "declared var matches — no finding", + pipelineVars: map[string]any{"DEPLOY": "true"}, + jobs: map[string]model.Job{ + "deploy": makeJob(nil, []model.Rule{ + {If: `$DEPLOY == "true"`, When: "on_success"}, + }), + }, + wantHit: nil, + wantMiss: []string{"deploy"}, + }, + { + name: "predefined CI_ var — skip check conservatively", + pipelineVars: nil, + jobs: map[string]model.Job{ + "branch-job": makeJob(nil, []model.Rule{ + {If: `$CI_COMMIT_BRANCH == "never-exists"`, When: "on_success"}, + }), + }, + wantHit: nil, + wantMiss: []string{"branch-job"}, + }, + { + name: "undeclared var — skip check conservatively", + pipelineVars: nil, + jobs: map[string]model.Job{ + "my-job": makeJob(nil, []model.Rule{ + {If: `$UNKNOWN_VAR == "x"`, When: "on_success"}, + }), + }, + wantHit: nil, + wantMiss: []string{"my-job"}, + }, + { + name: "unconditional rule — job can always activate", + pipelineVars: map[string]any{"DEPLOY": "false"}, + jobs: map[string]model.Job{ + "my-job": makeJob(nil, []model.Rule{ + {If: `$DEPLOY == "true"`, When: "never"}, + {When: "on_success"}, + }), + }, + wantHit: nil, + wantMiss: []string{"my-job"}, + }, + { + name: "all rules when:never — GL042 fires (not GL033 territory overlap)", + pipelineVars: map[string]any{"DEPLOY": "false"}, + jobs: map[string]model.Job{ + "my-job": makeJob(nil, []model.Rule{ + {If: `$DEPLOY == "true"`, When: "on_success"}, + {When: "never"}, + }), + }, + // Rule 1 evaluates to false; Rule 2 is explicit never → all dead. + wantHit: []string{"my-job"}, + wantMiss: nil, + }, + { + name: "job-level var overrides pipeline var", + pipelineVars: map[string]any{"FLAG": "false"}, + jobs: map[string]model.Job{ + "my-job": makeJob(map[string]any{"FLAG": "true"}, []model.Rule{ + {If: `$FLAG == "true"`, When: "on_success"}, + }), + }, + // Job-level FLAG="true" makes the if: evaluate to true → no finding. + wantHit: nil, + wantMiss: []string{"my-job"}, + }, + { + name: "no rules — nothing to check", + pipelineVars: map[string]any{"DEPLOY": "false"}, + jobs: map[string]model.Job{ + "my-job": makeJob(nil, nil), + }, + wantHit: nil, + wantMiss: []string{"my-job"}, + }, + { + name: "boolean variable evaluates correctly", + pipelineVars: map[string]any{"FLAG": false}, // bool false + jobs: map[string]model.Job{ + "my-job": makeJob(nil, []model.Rule{ + {If: `$FLAG == "true"`, When: "on_success"}, + }), + }, + // ScalarString(false) → "false"; "false" == "true" → false → GL042 fires. + wantHit: []string{"my-job"}, + wantMiss: nil, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p := &model.Pipeline{ + Variables: tc.pipelineVars, + Jobs: tc.jobs, + } + findings := checkRulesIfReachability(p) + + hitSet := make(map[string]bool) + for _, f := range findings { + if f.Rule == RuleStaticDeadRules { + hitSet[f.Job] = true + } + } + + for _, want := range tc.wantHit { + if !hitSet[want] { + t.Errorf("expected GL042 for job %q but it was not reported; findings: %v", want, findings) + } + } + for _, notWant := range tc.wantMiss { + if hitSet[notWant] { + t.Errorf("unexpected GL042 for job %q", notWant) + } + } + }) + } +} diff --git a/internal/linter/inherit_completeness.go b/internal/linter/inherit_completeness.go new file mode 100644 index 0000000..9cb72dd --- /dev/null +++ b/internal/linter/inherit_completeness.go @@ -0,0 +1,112 @@ +package linter + +import ( + "fmt" + "strings" + + "git.k3nny.fr/glint/internal/model" +) + +// checkInheritCompleteness (GL043): warn when a job's 'inherit: default:' +// declaration is dead — either because there is no 'default:' block in the +// pipeline, or because the list form references fields that are not defined +// in the 'default:' block. +func checkInheritCompleteness(p *model.Pipeline) []Finding { + var findings []Finding + for name, job := range p.Jobs { + findings = append(findings, checkJobInheritCompleteness(p, name, job)...) + } + return findings +} + +func checkJobInheritCompleteness(p *model.Pipeline, name string, job model.Job) []Finding { + if job.Inherit == nil { + return nil + } + m, ok := job.Inherit.(map[string]any) + if !ok { + return nil + } + defaultVal, hasDefault := m["default"] + if !hasDefault { + return nil + } + + // Case 1: no default: block at all — the entire declaration is a no-op. + if p.Default == nil { + return []Finding{{ + Severity: Warning, + Rule: RuleInheritNoDefault, + Job: name, + File: job.File, + Line: job.Line, + Message: "'inherit: default:' is declared but the pipeline has no 'default:' block — declaration has no effect", + }} + } + + // Case 2: list form — check for field names not set in default:. + list, ok := defaultVal.([]any) + if !ok { + // bool form (true/false) with a non-nil default: block is always valid. + return nil + } + + var dead []string + for _, item := range list { + field, ok := item.(string) + if !ok { + continue + } + if !defaultBlockHasField(p.Default, field) { + dead = append(dead, field) + } + } + if len(dead) == 0 { + return nil + } + return []Finding{{ + Severity: Warning, + Rule: RuleInheritNoDefault, + Job: name, + File: job.File, + Line: job.Line, + Message: fmt.Sprintf( + "'inherit: default: [%s]': %s not defined in the 'default:' block — %s", + strings.Join(dead, ", "), + pluralIs(len(dead)), + "these entries have no effect", + ), + }} +} + +// defaultBlockHasField reports whether the given field name has a non-zero value +// in the default: block. Unknown field names return true (conservative). +func defaultBlockHasField(d *model.DefaultConfig, field string) bool { + switch field { + case "image": + return d.Image != nil + case "before_script": + return d.BeforeScript != nil + case "after_script": + return d.AfterScript != nil + case "cache": + return d.Cache != nil + case "artifacts": + return d.Artifacts != nil + case "retry": + return d.Retry != nil + case "timeout": + return d.Timeout != "" + case "tags": + return len(d.Tags) > 0 + } + // Unknown field name — conservatively assume it might be defined. + return true +} + +func pluralIs(n int) string { + if n == 1 { + return "this field is" + } + return "these fields are" +} diff --git a/internal/linter/inherit_completeness_test.go b/internal/linter/inherit_completeness_test.go new file mode 100644 index 0000000..020dfbd --- /dev/null +++ b/internal/linter/inherit_completeness_test.go @@ -0,0 +1,94 @@ +package linter + +import ( + "testing" + + "git.k3nny.fr/glint/internal/model" +) + +func TestCheckInheritCompleteness(t *testing.T) { + cases := []struct { + name string + default_ *model.DefaultConfig + inherit any // job.Inherit value + wantHit bool + }{ + { + name: "no inherit — no finding", + inherit: nil, + wantHit: false, + }, + { + name: "inherit: default: false, no default block — GL043", + inherit: map[string]any{"default": false}, + wantHit: true, + }, + { + name: "inherit: default: true, no default block — GL043", + inherit: map[string]any{"default": true}, + wantHit: true, + }, + { + name: "inherit: default: [image], no default block — GL043", + inherit: map[string]any{"default": []any{"image"}}, + wantHit: true, + }, + { + name: "inherit: default: false, default block exists — no finding", + default_: &model.DefaultConfig{Image: "node:20"}, + inherit: map[string]any{"default": false}, + wantHit: false, + }, + { + name: "inherit: default: [image], image in default — no finding", + default_: &model.DefaultConfig{Image: "node:20"}, + inherit: map[string]any{"default": []any{"image"}}, + wantHit: false, + }, + { + name: "inherit: default: [before_script], before_script NOT in default — GL043", + default_: &model.DefaultConfig{Image: "node:20"}, + inherit: map[string]any{"default": []any{"before_script"}}, + wantHit: true, + }, + { + name: "inherit: default: [image, before_script], only image in default — GL043 for before_script", + default_: &model.DefaultConfig{Image: "node:20"}, + inherit: map[string]any{"default": []any{"image", "before_script"}}, + wantHit: true, + }, + { + name: "inherit: variables only — no default check", + default_: nil, + inherit: map[string]any{"variables": false}, + wantHit: false, + }, + { + name: "inherit: unknown field in list — conservative, no finding", + default_: &model.DefaultConfig{Image: "node:20"}, + inherit: map[string]any{"default": []any{"nonexistent_field"}}, + wantHit: false, // unknown fields return true from defaultBlockHasField + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + job := model.Job{Inherit: tc.inherit} + p := &model.Pipeline{ + Default: tc.default_, + Jobs: map[string]model.Job{"test-job": job}, + } + findings := checkInheritCompleteness(p) + + hit := false + for _, f := range findings { + if f.Rule == RuleInheritNoDefault { + hit = true + } + } + if hit != tc.wantHit { + t.Errorf("GL043 hit=%v, want=%v; findings: %v", hit, tc.wantHit, findings) + } + }) + } +} diff --git a/internal/linter/linter.go b/internal/linter/linter.go index 53268d7..b1aa2a2 100644 --- a/internal/linter/linter.go +++ b/internal/linter/linter.go @@ -62,6 +62,8 @@ func Lint(p *model.Pipeline) []Finding { findings = append(findings, checkNeeds(p)...) findings = append(findings, checkDependencies(p)...) findings = append(findings, checkVariableRefs(p)...) + findings = append(findings, checkRulesIfReachability(p)...) + findings = append(findings, checkInheritCompleteness(p)...) slices.SortStableFunc(findings, func(a, b Finding) int { if c := cmp.Compare(a.File, b.File); c != 0 { return c diff --git a/internal/linter/rules.go b/internal/linter/rules.go index c7816b9..c43eaaa 100644 --- a/internal/linter/rules.go +++ b/internal/linter/rules.go @@ -143,4 +143,15 @@ const ( // GL041: cache.key.files contains a glob pattern; it must be a list of exact file paths. RuleInvalidCacheKeyFiles = "GL041" + + // GL042: every rules:if: condition in a job's rules: block evaluates to false + // given the values of variables declared in the pipeline YAML, so the job can + // never be active. Only fires when all referenced variables are declared + // (predefined CI_* / GITLAB_* vars are not evaluated to avoid false positives). + RuleStaticDeadRules = "GL042" + + // GL043: a job declares 'inherit: default:' (true, false, or list) but the + // 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" ) diff --git a/testdata/inherit_dead.yml b/testdata/inherit_dead.yml new file mode 100644 index 0000000..e54c20f --- /dev/null +++ b/testdata/inherit_dead.yml @@ -0,0 +1,26 @@ +stages: + - build + - test + +# No default: block — any inherit: default: declaration is a no-op. + +# GL043: inherit: default: false but no default: block. +isolated-job: + stage: build + inherit: + default: false + script: echo isolated + +# GL043: inherit: default: [list] but no default: block. +selective-inherit-job: + stage: test + inherit: + default: [image, tags] + script: echo selective + +# Not GL043: inherit only touches variables: (no default: check needed). +vars-inherit-job: + stage: test + inherit: + variables: false + script: echo vars-only diff --git a/testdata/inherit_dead_fields.yml b/testdata/inherit_dead_fields.yml new file mode 100644 index 0000000..bf41d73 --- /dev/null +++ b/testdata/inherit_dead_fields.yml @@ -0,0 +1,20 @@ +stages: + - build + - test + +default: + image: node:20 + +# GL043: before_script is in the list but not defined in default:. +selective-bad: + stage: build + inherit: + default: [image, before_script] # before_script not in default: + script: echo hi + +# Not GL043: image IS defined in default: — list is valid. +selective-good: + stage: test + inherit: + default: [image] + script: echo hi diff --git a/testdata/keywords_valid.yml b/testdata/keywords_valid.yml index 9de6f72..8e98f96 100644 --- a/testdata/keywords_valid.yml +++ b/testdata/keywords_valid.yml @@ -9,6 +9,10 @@ workflow: when: always - when: never +default: + tags: + - docker + variables: GO_VERSION: "1.26" diff --git a/testdata/static_dead_rules.yml b/testdata/static_dead_rules.yml new file mode 100644 index 0000000..9b78f0b --- /dev/null +++ b/testdata/static_dead_rules.yml @@ -0,0 +1,48 @@ +stages: + - deploy + - test + +variables: + DEPLOY: "false" + SKIP_TEST: "true" + +# GL042: both if: conditions always evaluate to false given the declared variables. +always-dead-deploy: + stage: deploy + rules: + - if: '$DEPLOY == "true"' + when: on_success + script: echo deploy + +# GL042: if: is false, explicit when:on_success rule — still never fires. +always-dead-test: + stage: test + rules: + - if: '$SKIP_TEST == "false"' + when: on_success + script: echo test + +# Not GL042: references a predefined CI_ variable → unknown value → skip check. +predefined-var-job: + stage: test + rules: + - if: '$CI_COMMIT_BRANCH == "nonexistent-branch"' + when: on_success + script: echo branch + +# Not GL042: has an unconditional rule (no if: → always matches). +unconditional-job: + stage: test + rules: + - if: '$DEPLOY == "true"' + when: never + - when: on_success + script: echo always + +# Not GL042: references undeclared variable → unknown → conservative. +undeclared-var-job: + stage: test + rules: + - if: '$UNDECLARED_VAR == "something"' + when: on_success + script: echo undeclared