Simplify analysis and tool reporting

analysis_engine.go

@@ -96,10 +96,10 @@ func (l *analysisEngine) check(
 		}
 
 		v := NewVisitor()
-		v.actions = localActions
 		for _, rule := range rules {
 			v.AddPass(rule)
 		}
+		v.composites = &compositeAnalyzer{ctx: l.ctx, actions: localActions, passes: v.passes}
 		if dbg != nil {
 			v.EnableDebug(dbg)
 			for _, r := range rules {
@@ -122,7 +122,7 @@ func (l *analysisEngine) check(
 			l.debug("%s found %d errors", rule.Name(), len(errs))
 			all = append(all, errs...)
 		}
-		for _, composite := range v.compositeRules {
+		for _, composite := range v.composites.rules {
 			for _, rule := range composite.rules {
 				for _, finding := range rule.Errs() {
 					if finding.Filepath == "" {

composite_analysis.go

@@ -0,0 +1,97 @@
+package actionlint
+
+import (
+	"context"
+	"errors"
+)
+
+// compositeAnalyzer owns script-analyzer policy and invocation-specific state,
+// leaving Visitor responsible for the workflow's ordinary pass lifecycle.
+type compositeAnalyzer struct {
+	ctx           context.Context
+	actions       *LocalActionsCache
+	passes        []Pass
+	rules         []compositeScriptRules
+	workflowStart int
+	actionPathErr error
+}
+
+func (analysis *compositeAnalyzer) cancelled() error {
+	if analysis.ctx == nil {
+		return nil
+	}
+	return analysis.ctx.Err()
+}
+
+func (analysis *compositeAnalyzer) beginWorkflow() {
+	analysis.workflowStart = len(analysis.rules)
+	analysis.actionPathErr = nil
+}
+
+func (analysis *compositeAnalyzer) deferWorkflowError(pass Pass, err error) bool {
+	if _, shellcheck := pass.(*RuleShellcheck); shellcheck && errors.Is(err, errConfigActionPathUnavailable) {
+		// Validate action-only configuration when entering the actual composite.
+		analysis.actionPathErr = err
+		return true
+	}
+	return false
+}
+
+func (analysis *compositeAnalyzer) validateWorkflow() error {
+	if analysis.actionPathErr == nil {
+		return nil
+	}
+	for _, composite := range analysis.rules[analysis.workflowStart:] {
+		for _, rule := range composite.rules {
+			if _, shellcheck := rule.(*RuleShellcheck); shellcheck {
+				return nil
+			}
+		}
+	}
+	return analysis.actionPathErr
+}
+
+func (analysis *compositeAnalyzer) beginJob(job *Job) {
+	analysis.actions.setCheckout(runDirectory{})
+	analysis.actions.platform = runnerPlatform(job.RunsOn)
+	analysis.actions.caseInsensitive = analysis.actions.platform == platformKindWindows || macOSRunner(job.RunsOn)
+}
+
+func (analysis *compositeAnalyzer) invalidateCheckout() {
+	analysis.actions.setCheckout(runDirectory{kind: directoryUnknown})
+}
+
+func (analysis *compositeAnalyzer) visitStep(step *Step) error {
+	if err := analysis.cancelled(); err != nil {
+		return err
+	}
+	analysis.actions.observeCheckout(step)
+	for _, pass := range analysis.passes {
+		if shellcheck, ok := pass.(*RuleShellcheck); ok {
+			compositeCheckoutPaths(shellcheck, analysis.actions)
+		}
+		if _, script := pass.(*RuleExecutableBit); script {
+			if _, action := step.Exec.(*ExecAction); action {
+				continue // Preserve checkout state while entering a local composite.
+			}
+		}
+		if err := pass.VisitStep(step); err != nil {
+			return err
+		}
+	}
+	if _, action := step.Exec.(*ExecAction); !action {
+		return nil
+	}
+	var scripts []Rule
+	for _, pass := range analysis.passes {
+		switch rule := pass.(type) {
+		case *RuleShellcheck:
+			scripts = append(scripts, rule)
+		case *RulePyflakes:
+			scripts = append(scripts, rule)
+		case *RuleExecutableBit:
+			scripts = append(scripts, rule)
+		}
+	}
+	return analysis.visitActionScripts(step, scripts, make(map[string]bool))
+}

composite_analysis_test.go

@@ -0,0 +1,59 @@
+package actionlint
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestCompositeAnalysisCancellation(t *testing.T) {
+	for _, actionRule := range []bool{false, true} {
+		name := "metadata steps"
+		if actionRule {
+			name = "invocation after action validation"
+		}
+		t.Run(name, func(t *testing.T) {
+			root := t.TempDir()
+			outer := writeShellcheckFixture(t, root, "outer/action.yml", "name: outer\ndescription: test\nruns:\n  using: composite\n  steps:\n    - uses: $/inner\n")
+			inner := writeShellcheckFixture(t, root, "inner/action.yml", "name: inner\ndescription: test\nruns:\n  using: composite\n  steps:\n    - shell: bash\n      run: echo ok\n")
+			ctx, cancel := context.WithCancel(t.Context())
+			defer cancel()
+			outerReads, innerReads := 0, 0
+			result, err := Analyze(ctx, AnalysisRequest{
+				WorkingDir: root,
+				Sources: []SourceUnit{{
+					Path: "workflow.yml", Project: &Project{root: root},
+					Content: []byte("on: push\njobs:\n  test:\n    runs-on: ubuntu-latest\n    steps:\n      - uses: $/outer\n"),
+				}},
+				OnRulesCreated: func(rules []Rule) []Rule {
+					if actionRule {
+						for _, rule := range rules {
+							if _, ok := rule.(*RuleAction); ok {
+								return []Rule{rule}
+							}
+						}
+					}
+					return nil
+				},
+				ReadFile: func(path string) ([]byte, error) {
+					switch filepath.Clean(path) {
+					case outer:
+						outerReads++
+						cancel()
+					case inner:
+						innerReads++
+					}
+					return os.ReadFile(path)
+				},
+			})
+			if result != nil || !errors.Is(err, context.Canceled) {
+				t.Fatalf("wanted cancelled analysis, got result=%+v, error=%v", result, err)
+			}
+			if outerReads != 1 || innerReads != 0 {
+				t.Fatalf("analysis continued after cancellation: outer reads=%d, inner reads=%d", outerReads, innerReads)
+			}
+		})
+	}
+}

composite_scripts.go

@@ -1,7 +1,6 @@
 package actionlint
 
 import (
-	"maps"
 	"path/filepath"
 	"strings"
 
@@ -15,7 +14,10 @@ type compositeScriptRules struct {
 	rules []Rule
 }
 
-func (v *Visitor) visitActionScripts(call *Step, parents []Rule, active map[string]bool) error {
+func (analysis *compositeAnalyzer) visitActionScripts(call *Step, parents []Rule, active map[string]bool) error {
+	if err := analysis.cancelled(); err != nil {
+		return err
+	}
 	action, ok := call.Exec.(*ExecAction)
 	if !ok {
 		return nil
@@ -25,11 +27,11 @@ func (v *Visitor) visitActionScripts(call *Step, parents []Rule, active map[stri
 		spec := action.Uses.Value
 		// Metadata-load diagnostics belong to RuleAction. An unresolved action
 		// still invalidates executable-bit assumptions below.
-		meta, _, _ = v.actions.FindMetadata(spec)
+		meta, _, _ = analysis.actions.FindMetadata(spec)
 	}
 	if meta == nil || !strings.EqualFold(meta.Runs.Using, "composite") || active[meta.Path()] || len(active) >= 10 {
 		for _, rule := range parents {
-			compositeCheckoutPaths(rule, v.actions)
+			compositeCheckoutPaths(rule, analysis.actions)
 			if err := rule.VisitStep(call); err != nil {
 				return err
 			}
@@ -38,88 +40,62 @@ func (v *Visitor) visitActionScripts(call *Step, parents []Rule, active map[stri
 	}
 	active[meta.Path()] = true
 	defer delete(active, meta.Path())
-	checkout := v.actions.checkoutState()
+	checkout := analysis.actions.checkoutState()
 	defer func() {
 		enabled, known := invocationCondition(call.If)
 		if known && !enabled {
-			v.actions.restoreCheckout(checkout)
-		} else if (!known || boolMayBeTrue(call.ContinueOnError) || boolMayBeTrue(call.Background)) && checkout != v.actions.checkoutState() {
-			v.actions.setCheckout(runDirectory{kind: directoryUnknown})
+			analysis.actions.restoreCheckout(checkout)
+		} else if (!known || boolMayBeTrue(call.ContinueOnError) || boolMayBeTrue(call.Background)) && checkout != analysis.actions.checkoutState() {
+			analysis.actions.setCheckout(runDirectory{kind: directoryUnknown})
 		}
 	}()
 
 	children := make([]Rule, 0, len(parents))
 	for _, parent := range parents {
-		child := compositeScriptRule(parent, call, filepath.Dir(meta.Path()), v.actions)
+		child := compositeScriptRule(parent, call, filepath.Dir(meta.Path()), analysis.actions)
 		if shellcheck, ok := child.(*RuleShellcheck); ok {
 			if err := shellcheck.prepareConfigPath(); err != nil {
 				return err
 			}
 		}
 		children = append(children, child)
 	}
-	v.compositeRules = append(v.compositeRules, compositeScriptRules{meta, children})
+	analysis.rules = append(analysis.rules, compositeScriptRules{meta, children})
 	parser := &parser{sourceLines: splitSourceLines(meta.src)}
 	for _, metadataStep := range meta.Runs.Steps {
+		if err := analysis.cancelled(); err != nil {
+			return err
+		}
 		step := compositeScriptStep(metadataStep, parser)
-		v.actions.observeCheckout(step)
+		analysis.actions.observeCheckout(step)
 		if _, action := step.Exec.(*ExecAction); action {
-			for _, pass := range v.passes {
+			for _, pass := range analysis.passes {
 				if parent, enabled := pass.(*RuleAction); enabled {
-					validation := NewRuleAction(v.actions)
+					validation := NewRuleAction(analysis.actions)
 					validation.SetConfig(parent.Config())
 					if err := validation.VisitStep(step); err != nil {
 						return err
 					}
-					v.compositeRules = append(v.compositeRules, compositeScriptRules{meta, []Rule{validation}})
+					analysis.rules = append(analysis.rules, compositeScriptRules{meta, []Rule{validation}})
 					break
 				}
 			}
-			if err := v.visitActionScripts(step, children, active); err != nil {
+			if err := analysis.visitActionScripts(step, children, active); err != nil {
 				return err
 			}
 			continue
 		}
 		for _, rule := range children {
-			compositeCheckoutPaths(rule, v.actions)
+			compositeCheckoutPaths(rule, analysis.actions)
 			if err := rule.VisitStep(step); err != nil {
 				return err
 			}
 		}
 	}
-	enabled, conditionKnown := invocationCondition(call.If)
 	for i, parent := range parents {
 		if outer, ok := parent.(*RuleExecutableBit); ok {
 			if inner, ok := children[i].(*RuleExecutableBit); ok {
-				if conditionKnown && !enabled {
-					continue
-				}
-				if !conditionKnown {
-					// Either branch can retain changed modes; a child checkout only
-					// resets them when that branch actually runs.
-					maps.Copy(outer.changed, inner.changed)
-					maps.Copy(outer.actionChanged, inner.actionChanged)
-				}
-				outer.actionPristine = outer.actionPristine && inner.actionPristine
-				outer.repositoryUnknown = inner.repositoryUnknown
-				if boolMayBeTrue(call.ContinueOnError) {
-					// The caller may continue after a failed child checkout.
-					outer.pristine, outer.repositoryUnknown = false, true
-					outer.sequential = inner.sequential
-					continue
-				}
-				if !conditionKnown {
-					// A conditional checkout may never have run. Do not promote
-					// one branch's filesystem assumptions to the caller.
-					outer.pristine = false
-					outer.sequential = inner.sequential
-					continue
-				}
-				outer.pristine, outer.sequential = inner.pristine, inner.sequential
-				inner.paths.actionPath = outer.paths.actionPath
-				inner.paths.actionRunnerPath = outer.paths.actionRunnerPath
-				inner.paths.actionIndependent = outer.paths.actionIndependent
-				outer.paths, outer.changed, outer.actionChanged = inner.paths, inner.changed, inner.actionChanged
+				outer.joinComposite(call, inner)
 			}
 		}
 	}
@@ -175,33 +151,7 @@ func compositeScriptRule(parent Rule, call *Step, actionPath string, actions *Lo
 	case *RulePyflakes:
 		child = newRulePyflakes(rule.cmd)
 	case *RuleExecutableBit:
-		scoped := newRuleExecutableBit(rule.context)
-		scoped.unix, scoped.sequential, scoped.pristine = rule.unix, rule.sequential, rule.pristine
-		scoped.caseInsensitive = rule.caseInsensitive
-		scoped.shIsDash = rule.shIsDash
-		scoped.repositoryUnknown = rule.repositoryUnknown
-		scoped.actionPristine = rule.actionPristine
-		if stepCanRunAfterFailure(call.If) {
-			scoped.pristine, scoped.repositoryUnknown = false, true
-			scoped.actionPristine = false
-		}
-		scoped.paths, scoped.changed, scoped.actionChanged = rule.paths, rule.changed, rule.actionChanged
-		compositeCheckoutPaths(scoped, actions)
-		compositeActionOrigin(&scoped.paths, call, actionPath)
-		enabled, conditionKnown := invocationCondition(call.If)
-		scoped.skipFindings = rule.skipFindings || conditionKnown && !enabled
-		if !conditionKnown || !enabled {
-			scoped.changed = maps.Clone(rule.changed)
-			scoped.actionChanged = maps.Clone(rule.actionChanged)
-		}
-		scoped.jobEnv = rule.jobEnv || shellEnvironmentUnknown(call.Env)
-		scoped.jobGitEnv = rule.jobGitEnv || checkoutEnvironmentUnknown(call.Env)
-		scoped.jobPathUnknown = rule.jobPathUnknown || shellPathUnknown(call.Env)
-		if conditionKnown && !enabled || boolMayBeTrue(call.Background) {
-			scoped.sequential, scoped.pristine = false, false
-			scoped.actionPristine = false
-		}
-		child = scoped
+		child = rule.forkComposite(call, actionPath, actions)
 	default:
 		panic("unsupported composite script rule")
 	}

config_shellcheck_test.go

@@ -92,6 +92,8 @@ func TestShellcheckInlineConfigAnalysis(t *testing.T) {
 		{"code disabled", "config: {disable: [SC2086]}", "echo $VALUE", ""},
 		{"range disabled", "config: {disable: [SC2000-SC3000]}", "echo $VALUE", ""},
 		{"dialect override", "config: {shell: sh}", `[[ -n "$HOME" ]]`, "SC3010"},
+		{"safe variable default", "enabled: true", "value=hello\necho $value", ""},
+		{"optional safe variable quoting", "config: {enable: [quote-safe-variables]}", "value=hello\necho $value", "SC2248"},
 		{"multiple prefix lines", "config: {disable: [SC2016], enable: [quote-safe-variables], extended-analysis: false}", "echo $VALUE", "SC2086"},
 		{"source path", `config: {source-path: ["lib's directory"], external-sources: true}`, ". config.sh\necho $VALUE", ""},
 		{"external sources disabled", `config: {source-path: ["lib's directory"], external-sources: false}`, ". config.sh\necho $VALUE", "SC2086"},