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 {