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 <noreply@anthropic.com>
This commit is contained in:
@@ -9,10 +9,14 @@ This project uses [Semantic Versioning](https://semver.org).
|
|||||||
|
|
||||||
### Added
|
### 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.
|
- **`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
|
### 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.
|
- **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.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.
|
- **`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.
|
||||||
|
|||||||
@@ -24,6 +24,8 @@ func checkDependencies(p *model.Pipeline) []Finding {
|
|||||||
findings = append(findings, Finding{
|
findings = append(findings, Finding{
|
||||||
Severity: Error,
|
Severity: Error,
|
||||||
Job: name,
|
Job: name,
|
||||||
|
File: job.File,
|
||||||
|
Line: job.Line,
|
||||||
Message: fmt.Sprintf("'dependencies' references unknown job %q", dep),
|
Message: fmt.Sprintf("'dependencies' references unknown job %q", dep),
|
||||||
})
|
})
|
||||||
continue
|
continue
|
||||||
@@ -34,6 +36,8 @@ func checkDependencies(p *model.Pipeline) []Finding {
|
|||||||
findings = append(findings, Finding{
|
findings = append(findings, Finding{
|
||||||
Severity: Error,
|
Severity: Error,
|
||||||
Job: name,
|
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),
|
Message: fmt.Sprintf("'dependencies' job %q must be in an earlier stage (in %q, current job is in %q)", dep, depJob.Stage, job.Stage),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -17,12 +17,25 @@ const (
|
|||||||
type Finding struct {
|
type Finding struct {
|
||||||
Severity Severity
|
Severity Severity
|
||||||
Job string // empty for pipeline-level findings
|
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
|
Message string
|
||||||
}
|
}
|
||||||
|
|
||||||
func (f Finding) String() 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 != "" {
|
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)
|
return fmt.Sprintf("[%s] %s", f.Severity, f.Message)
|
||||||
}
|
}
|
||||||
@@ -43,6 +56,7 @@ func checkStages(p *model.Pipeline) []Finding {
|
|||||||
if len(p.Stages) == 0 {
|
if len(p.Stages) == 0 {
|
||||||
findings = append(findings, Finding{
|
findings = append(findings, Finding{
|
||||||
Severity: Warning,
|
Severity: Warning,
|
||||||
|
File: p.SourceFile,
|
||||||
Message: "no stages defined; GitLab will use default stages (build, test, deploy)",
|
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] {
|
if rule.When != "" && !validWorkflowRuleWhen[rule.When] {
|
||||||
findings = append(findings, Finding{
|
findings = append(findings, Finding{
|
||||||
Severity: Error,
|
Severity: Error,
|
||||||
|
File: p.SourceFile,
|
||||||
Message: fmt.Sprintf("workflow.rules[%d].when has invalid value %q; valid: always, never", i, rule.When),
|
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.
|
// After extends resolution, a job with no script/run is an error.
|
||||||
// Exceptions: trigger jobs, pages jobs (use pages: keyword), and template jobs.
|
// 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
|
hasScript := scriptNonEmpty(job.Script) || job.Run != nil
|
||||||
if !isTemplate && !isTrigger && job.Pages == nil && !hasScript {
|
if !isTemplate && !isTrigger && job.Pages == nil && !hasScript {
|
||||||
|
sev := Error
|
||||||
|
if job.Extends != nil {
|
||||||
|
sev = Warning
|
||||||
|
}
|
||||||
findings = append(findings, Finding{
|
findings = append(findings, Finding{
|
||||||
Severity: Error,
|
Severity: sev,
|
||||||
Job: name,
|
Job: name,
|
||||||
Message: "missing required field 'script' (or 'run')",
|
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)...)
|
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
|
return findings
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -35,6 +35,8 @@ func checkNeeds(p *model.Pipeline) []Finding {
|
|||||||
findings = append(findings, Finding{
|
findings = append(findings, Finding{
|
||||||
Severity: Error,
|
Severity: Error,
|
||||||
Job: name,
|
Job: name,
|
||||||
|
File: job.File,
|
||||||
|
Line: job.Line,
|
||||||
Message: fmt.Sprintf("needs unknown job %q", needed),
|
Message: fmt.Sprintf("needs unknown job %q", needed),
|
||||||
})
|
})
|
||||||
continue
|
continue
|
||||||
@@ -47,6 +49,8 @@ func checkNeeds(p *model.Pipeline) []Finding {
|
|||||||
findings = append(findings, Finding{
|
findings = append(findings, Finding{
|
||||||
Severity: Error,
|
Severity: Error,
|
||||||
Job: name,
|
Job: name,
|
||||||
|
File: job.File,
|
||||||
|
Line: job.Line,
|
||||||
Message: fmt.Sprintf(
|
Message: fmt.Sprintf(
|
||||||
"needs %q which is in a later stage (%q after %q)",
|
"needs %q which is in a later stage (%q after %q)",
|
||||||
needed, neededJob.Stage, job.Stage,
|
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
|
return findings
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -82,7 +86,7 @@ func parseNeedJobNames(needs []any) []string {
|
|||||||
return names
|
return names
|
||||||
}
|
}
|
||||||
|
|
||||||
func detectNeedsCycles(graph map[string][]string) []Finding {
|
func detectNeedsCycles(graph map[string][]string, jobs map[string]model.Job) []Finding {
|
||||||
const (
|
const (
|
||||||
unvisited = 0
|
unvisited = 0
|
||||||
visiting = 1
|
visiting = 1
|
||||||
@@ -101,9 +105,12 @@ func detectNeedsCycles(graph map[string][]string) []Finding {
|
|||||||
case visiting:
|
case visiting:
|
||||||
if !reported[name] {
|
if !reported[name] {
|
||||||
reported[name] = true
|
reported[name] = true
|
||||||
|
j := jobs[name]
|
||||||
findings = append(findings, Finding{
|
findings = append(findings, Finding{
|
||||||
Severity: Error,
|
Severity: Error,
|
||||||
Job: name,
|
Job: name,
|
||||||
|
File: j.File,
|
||||||
|
Line: j.Line,
|
||||||
Message: fmt.Sprintf("circular dependency in needs: %v → %s", path, name),
|
Message: fmt.Sprintf("circular dependency in needs: %v → %s", path, name),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -13,14 +13,22 @@ func Parse(path string) (*Pipeline, error) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("reading file: %w", err)
|
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.
|
// ParseBytes parses YAML from an in-memory byte slice.
|
||||||
func ParseBytes(data []byte) (*Pipeline, error) {
|
func ParseBytes(data []byte) (*Pipeline, error) {
|
||||||
// First pass: decode into a raw map to extract job keys.
|
// First pass: parse into a yaml.Node document to extract job keys with
|
||||||
var raw map[string]yaml.Node
|
// their exact source line numbers (key nodes carry the line, value nodes
|
||||||
if err := yaml.Unmarshal(data, &raw); err != nil {
|
// 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)
|
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.Jobs = make(map[string]Job)
|
||||||
p.RawJobs = make(map[string]map[string]any)
|
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] {
|
if ReservedKeys[key] {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
var rawMap map[string]any
|
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)
|
return nil, fmt.Errorf("parsing raw job %q: %w", key, err)
|
||||||
}
|
}
|
||||||
p.RawJobs[key] = rawMap
|
p.RawJobs[key] = rawMap
|
||||||
|
|
||||||
var j Job
|
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)
|
return nil, fmt.Errorf("parsing job %q: %w", key, err)
|
||||||
}
|
}
|
||||||
j.Name = key
|
j.Name = key
|
||||||
|
j.Line = keyNode.Line // exact line of the job name key
|
||||||
p.Jobs[key] = j
|
p.Jobs[key] = j
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package model
|
|||||||
// Pipeline represents the top-level structure of a .gitlab-ci.yml file.
|
// Pipeline represents the top-level structure of a .gitlab-ci.yml file.
|
||||||
// Unknown top-level keys are collected into Jobs.
|
// Unknown top-level keys are collected into Jobs.
|
||||||
type Pipeline struct {
|
type Pipeline struct {
|
||||||
|
SourceFile string // path of the root pipeline file; set by Parse
|
||||||
Stages []string `yaml:"stages"`
|
Stages []string `yaml:"stages"`
|
||||||
Variables map[string]any `yaml:"variables"` // string or {value,description,options} map
|
Variables map[string]any `yaml:"variables"` // string or {value,description,options} map
|
||||||
Default *DefaultConfig `yaml:"default"`
|
Default *DefaultConfig `yaml:"default"`
|
||||||
@@ -13,6 +14,17 @@ type Pipeline struct {
|
|||||||
RawJobs map[string]map[string]any `yaml:"-"` // pre-resolution raw maps, used by the resolver
|
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 {
|
type DefaultConfig struct {
|
||||||
Image any `yaml:"image"` // string or {name,pull_policy,...} map
|
Image any `yaml:"image"` // string or {name,pull_policy,...} map
|
||||||
BeforeScript any `yaml:"before_script"` // []string or string (block scalar)
|
BeforeScript any `yaml:"before_script"` // []string or string (block scalar)
|
||||||
@@ -30,6 +42,8 @@ type Workflow struct {
|
|||||||
|
|
||||||
type Job 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"`
|
Stage string `yaml:"stage"`
|
||||||
Script any `yaml:"script"` // []string or string (block scalar)
|
Script any `yaml:"script"` // []string or string (block scalar)
|
||||||
Run any `yaml:"run"` // alternative to script (CI steps)
|
Run any `yaml:"run"` // alternative to script (CI steps)
|
||||||
|
|||||||
@@ -117,6 +117,7 @@ func resolveLocalInclude(p *model.Pipeline, rawPath string, cfg fetcher.GitLabCo
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return []IncludeWarning{{Label: label, Err: fmt.Errorf("parsing YAML: %w", 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
|
// Recursively resolve the included file's own includes first, merging
|
||||||
// everything into `included` before we merge it into the parent `p`.
|
// 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)})
|
warnings = append(warnings, IncludeWarning{Label: label, Err: fmt.Errorf("parsing YAML: %w", err)})
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
included.SetJobOrigin(label)
|
||||||
|
|
||||||
if len(included.Include) > 0 {
|
if len(included.Include) > 0 {
|
||||||
w, ew := resolveIncludes(included, included.Include, cfg, rootDir, visited)
|
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 {
|
if err != nil {
|
||||||
return IncludeWarning{Label: label, Err: fmt.Errorf("parsing component YAML: %w", err)}, nil, true
|
return IncludeWarning{Label: label, Err: fmt.Errorf("parsing component YAML: %w", err)}, nil, true
|
||||||
}
|
}
|
||||||
|
included.SetJobOrigin(label)
|
||||||
|
|
||||||
var extWarnings []ExtendWarning
|
var extWarnings []ExtendWarning
|
||||||
if len(included.Include) > 0 {
|
if len(included.Include) > 0 {
|
||||||
|
|||||||
Reference in New Issue
Block a user