From a303f63a5e76bd9e45a8b7594751c6f7fd7a0a60 Mon Sep 17 00:00:00 2001 From: k3nny Date: Thu, 11 Jun 2026 21:24:18 +0200 Subject: [PATCH] feat(linter): add file/line to findings; downgrade extends missing-script to warning Every finding now carries the source file and exact line number of the job key in its YAML file. Format: [ERROR] job "name" (file.yml:12): message. Pipeline-level findings (workflow rules, no stages) reference p.SourceFile. Cross-file include jobs (local, project, component) carry the include source as their File, set via Pipeline.SetJobOrigin after each ParseBytes call in the resolver. Line numbers come from the yaml.Node key node (exact job-name line) in a new document-level first pass in ParseBytes, replacing the previous map[string]yaml.Node approach which only gave value-node lines. Also: jobs that declare extends: but have no script after resolution now emit WARNING instead of ERROR. The script may come from a base in a remote include that was not fetched (no token, offline), making the error a false positive in common project setups. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 4 ++++ internal/linter/dependencies.go | 4 ++++ internal/linter/linter.go | 32 ++++++++++++++++++++++++++-- internal/linter/needs.go | 11 ++++++++-- internal/model/parser.go | 37 ++++++++++++++++++++++++++------- internal/model/pipeline.go | 26 +++++++++++++++++------ internal/resolver/includes.go | 3 +++ 7 files changed, 100 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be00460..a20ad35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,14 @@ This project uses [Semantic Versioning](https://semver.org). ### Added +- **File and line numbers on findings** — every finding now includes the source file and line where the job is defined, e.g. `[ERROR] job "deploy" (src/deploy.yml:14): …`. For jobs that come from local or fetched includes the file reflects the include source. Pipeline-level findings (workflow rules, missing stages) reference the root pipeline file. + - **`glint graph includes` shows jobs per file** — each node in the Mermaid include dependency graph now shows the jobs defined directly in that file. Jobs are rendered as rounded nodes (`(name)`) in a distinct light-purple style, connected with dashed arrows (`-.->`) to distinguish ownership from the include hierarchy (solid `-->` arrows). The root pipeline file always shows its direct jobs; local and fetched project/component nodes show theirs when the file can be read. ### Fixed +- **`extends:` jobs with missing script downgraded to warning** — a job that declares `extends:` but has no `script` after resolution now emits `[WARNING]` instead of `[ERROR]`. The script may legitimately come from a base job in a remote include that could not be fetched at lint time (e.g. no token configured). + - **Variable map form now parses correctly** — `variables:` entries that use the extended `{value, description, options}` form (GitLab CI 13.7+) no longer cause `yaml: cannot unmarshal !!map into string`. Both `Pipeline.Variables` and per-job `Variables` now accept either plain strings or map-form declarations. - **`default.image` map form now parses correctly** — `default: image: {name: ..., pull_policy: ...}` used to cause `yaml: cannot unmarshal !!map into string`; `DefaultConfig.Image` is now typed as `any` to match `Job.Image`. - **`default.before_script` / `default.after_script` now accept both list and scalar forms** — previously `DefaultConfig.BeforeScript` and `DefaultConfig.AfterScript` were `[]string`, causing a parse error when the field was written as a block scalar string. They are now typed as `any` to match the corresponding `Job` fields. diff --git a/internal/linter/dependencies.go b/internal/linter/dependencies.go index 1e0d601..51c641b 100644 --- a/internal/linter/dependencies.go +++ b/internal/linter/dependencies.go @@ -24,6 +24,8 @@ func checkDependencies(p *model.Pipeline) []Finding { findings = append(findings, Finding{ Severity: Error, Job: name, + File: job.File, + Line: job.Line, Message: fmt.Sprintf("'dependencies' references unknown job %q", dep), }) continue @@ -34,6 +36,8 @@ func checkDependencies(p *model.Pipeline) []Finding { findings = append(findings, Finding{ Severity: Error, Job: name, + File: job.File, + Line: job.Line, Message: fmt.Sprintf("'dependencies' job %q must be in an earlier stage (in %q, current job is in %q)", dep, depJob.Stage, job.Stage), }) } diff --git a/internal/linter/linter.go b/internal/linter/linter.go index 1330838..b51c08b 100644 --- a/internal/linter/linter.go +++ b/internal/linter/linter.go @@ -17,12 +17,25 @@ const ( type Finding struct { Severity Severity Job string // empty for pipeline-level findings + File string // source file where the finding originates + Line int // line number in File (0 = unknown) Message string } func (f Finding) String() string { + loc := "" + if f.File != "" { + if f.Line > 0 { + loc = fmt.Sprintf(" (%s:%d)", f.File, f.Line) + } else { + loc = fmt.Sprintf(" (%s)", f.File) + } + } if f.Job != "" { - return fmt.Sprintf("[%s] job %q: %s", f.Severity, f.Job, f.Message) + return fmt.Sprintf("[%s] job %q%s: %s", f.Severity, f.Job, loc, f.Message) + } + if loc != "" { + return fmt.Sprintf("[%s]%s: %s", f.Severity, loc, f.Message) } return fmt.Sprintf("[%s] %s", f.Severity, f.Message) } @@ -43,6 +56,7 @@ func checkStages(p *model.Pipeline) []Finding { if len(p.Stages) == 0 { findings = append(findings, Finding{ Severity: Warning, + File: p.SourceFile, Message: "no stages defined; GitLab will use default stages (build, test, deploy)", }) } @@ -58,6 +72,7 @@ func checkWorkflow(p *model.Pipeline) []Finding { if rule.When != "" && !validWorkflowRuleWhen[rule.When] { findings = append(findings, Finding{ Severity: Error, + File: p.SourceFile, Message: fmt.Sprintf("workflow.rules[%d].when has invalid value %q; valid: always, never", i, rule.When), }) } @@ -88,10 +103,16 @@ func checkJob(name string, job model.Job, stageSet map[string]bool) []Finding { // After extends resolution, a job with no script/run is an error. // Exceptions: trigger jobs, pages jobs (use pages: keyword), and template jobs. + // When the job has extends:, the script may come from a base that couldn't be + // fetched (e.g. a remote include without a token), so downgrade to warning. hasScript := scriptNonEmpty(job.Script) || job.Run != nil if !isTemplate && !isTrigger && job.Pages == nil && !hasScript { + sev := Error + if job.Extends != nil { + sev = Warning + } findings = append(findings, Finding{ - Severity: Error, + Severity: sev, Job: name, Message: "missing required field 'script' (or 'run')", }) @@ -137,6 +158,13 @@ func checkJob(name string, job model.Job, stageSet map[string]bool) []Finding { findings = append(findings, checkJobKeywords(name, job)...) + // Attach source location to every job-scoped finding collected above. + for i := range findings { + if findings[i].Job != "" && findings[i].File == "" { + findings[i].File = job.File + findings[i].Line = job.Line + } + } return findings } diff --git a/internal/linter/needs.go b/internal/linter/needs.go index 94fa653..dbe2ab2 100644 --- a/internal/linter/needs.go +++ b/internal/linter/needs.go @@ -35,6 +35,8 @@ func checkNeeds(p *model.Pipeline) []Finding { findings = append(findings, Finding{ Severity: Error, Job: name, + File: job.File, + Line: job.Line, Message: fmt.Sprintf("needs unknown job %q", needed), }) continue @@ -47,6 +49,8 @@ func checkNeeds(p *model.Pipeline) []Finding { findings = append(findings, Finding{ Severity: Error, Job: name, + File: job.File, + Line: job.Line, Message: fmt.Sprintf( "needs %q which is in a later stage (%q after %q)", needed, neededJob.Stage, job.Stage, @@ -57,7 +61,7 @@ func checkNeeds(p *model.Pipeline) []Finding { } } - findings = append(findings, detectNeedsCycles(needsGraph)...) + findings = append(findings, detectNeedsCycles(needsGraph, p.Jobs)...) return findings } @@ -82,7 +86,7 @@ func parseNeedJobNames(needs []any) []string { return names } -func detectNeedsCycles(graph map[string][]string) []Finding { +func detectNeedsCycles(graph map[string][]string, jobs map[string]model.Job) []Finding { const ( unvisited = 0 visiting = 1 @@ -101,9 +105,12 @@ func detectNeedsCycles(graph map[string][]string) []Finding { case visiting: if !reported[name] { reported[name] = true + j := jobs[name] findings = append(findings, Finding{ Severity: Error, Job: name, + File: j.File, + Line: j.Line, Message: fmt.Sprintf("circular dependency in needs: %v → %s", path, name), }) } diff --git a/internal/model/parser.go b/internal/model/parser.go index bf6de02..8ea3f7b 100644 --- a/internal/model/parser.go +++ b/internal/model/parser.go @@ -13,14 +13,22 @@ func Parse(path string) (*Pipeline, error) { if err != nil { return nil, fmt.Errorf("reading file: %w", err) } - return ParseBytes(data) + p, err := ParseBytes(data) + if err != nil { + return nil, err + } + p.SourceFile = path + p.SetJobOrigin(path) + return p, nil } // ParseBytes parses YAML from an in-memory byte slice. func ParseBytes(data []byte) (*Pipeline, error) { - // First pass: decode into a raw map to extract job keys. - var raw map[string]yaml.Node - if err := yaml.Unmarshal(data, &raw); err != nil { + // First pass: parse into a yaml.Node document to extract job keys with + // their exact source line numbers (key nodes carry the line, value nodes + // carry the body we decode into Job / map[string]any). + var doc yaml.Node + if err := yaml.Unmarshal(data, &doc); err != nil { return nil, fmt.Errorf("parsing YAML: %w", err) } @@ -32,21 +40,36 @@ func ParseBytes(data []byte) (*Pipeline, error) { p.Jobs = make(map[string]Job) p.RawJobs = make(map[string]map[string]any) - for key, node := range raw { + + if doc.Kind != yaml.DocumentNode || len(doc.Content) == 0 { + return p, nil + } + root := doc.Content[0] + if root.Kind != yaml.MappingNode { + return p, nil + } + + // Walk root mapping in key/value pairs. + for i := 0; i+1 < len(root.Content); i += 2 { + keyNode := root.Content[i] + valNode := root.Content[i+1] + key := keyNode.Value if ReservedKeys[key] { continue } + var rawMap map[string]any - if err := node.Decode(&rawMap); err != nil { + if err := valNode.Decode(&rawMap); err != nil { return nil, fmt.Errorf("parsing raw job %q: %w", key, err) } p.RawJobs[key] = rawMap var j Job - if err := node.Decode(&j); err != nil { + if err := valNode.Decode(&j); err != nil { return nil, fmt.Errorf("parsing job %q: %w", key, err) } j.Name = key + j.Line = keyNode.Line // exact line of the job name key p.Jobs[key] = j } diff --git a/internal/model/pipeline.go b/internal/model/pipeline.go index e7c971c..b5869ed 100644 --- a/internal/model/pipeline.go +++ b/internal/model/pipeline.go @@ -3,16 +3,28 @@ package model // Pipeline represents the top-level structure of a .gitlab-ci.yml file. // Unknown top-level keys are collected into Jobs. type Pipeline struct { - Stages []string `yaml:"stages"` - Variables map[string]any `yaml:"variables"` // string or {value,description,options} map - Default *DefaultConfig `yaml:"default"` - Include []any `yaml:"include"` - Workflow *Workflow `yaml:"workflow"` + SourceFile string // path of the root pipeline file; set by Parse + Stages []string `yaml:"stages"` + Variables map[string]any `yaml:"variables"` // string or {value,description,options} map + Default *DefaultConfig `yaml:"default"` + Include []any `yaml:"include"` + Workflow *Workflow `yaml:"workflow"` // Jobs holds every non-reserved top-level key (i.e. job definitions). Jobs map[string]Job `yaml:"-"` RawJobs map[string]map[string]any `yaml:"-"` // pre-resolution raw maps, used by the resolver } +// SetJobOrigin sets the File field on all jobs that don't already have one. +// Called after ParseBytes to record which file each job came from. +func (p *Pipeline) SetJobOrigin(file string) { + for name, j := range p.Jobs { + if j.File == "" { + j.File = file + } + p.Jobs[name] = j + } +} + type DefaultConfig struct { Image any `yaml:"image"` // string or {name,pull_policy,...} map BeforeScript any `yaml:"before_script"` // []string or string (block scalar) @@ -29,7 +41,9 @@ type Workflow struct { } type Job struct { - Name string // set by parser, not from YAML + Name string // set by parser, not from YAML + File string // source file; set by Parse / resolver + Line int // line of the job key in its source file; set by parser Stage string `yaml:"stage"` Script any `yaml:"script"` // []string or string (block scalar) Run any `yaml:"run"` // alternative to script (CI steps) diff --git a/internal/resolver/includes.go b/internal/resolver/includes.go index dbd3fff..998e276 100644 --- a/internal/resolver/includes.go +++ b/internal/resolver/includes.go @@ -117,6 +117,7 @@ func resolveLocalInclude(p *model.Pipeline, rawPath string, cfg fetcher.GitLabCo if err != nil { return []IncludeWarning{{Label: label, Err: fmt.Errorf("parsing YAML: %w", err)}}, nil } + included.SetJobOrigin(absPath) // Recursively resolve the included file's own includes first, merging // everything into `included` before we merge it into the parent `p`. @@ -161,6 +162,7 @@ func resolveProjectInclude(p *model.Pipeline, entry map[string]any, project stri warnings = append(warnings, IncludeWarning{Label: label, Err: fmt.Errorf("parsing YAML: %w", err)}) continue } + included.SetJobOrigin(label) if len(included.Include) > 0 { w, ew := resolveIncludes(included, included.Include, cfg, rootDir, visited) @@ -200,6 +202,7 @@ func resolveComponentInclude(p *model.Pipeline, ref string, cfg fetcher.GitLabCo if err != nil { return IncludeWarning{Label: label, Err: fmt.Errorf("parsing component YAML: %w", err)}, nil, true } + included.SetJobOrigin(label) var extWarnings []ExtendWarning if len(included.Include) > 0 {