diff --git a/internal/linter/rules.go b/internal/linter/rules.go index 3e09479..dfb31c5 100644 --- a/internal/linter/rules.go +++ b/internal/linter/rules.go @@ -204,12 +204,37 @@ func (r *UndefinedVariableRule) Check(wast *WorkflowAST) []LintIssue { defined[param.Name] = true } + // Collect variables provided by trigger inputs + triggerVars := make(map[string]bool) + hasEventTrigger := false + for _, trigger := range w.Triggers { + if trigger.On == core.TriggerEvent { + hasEventTrigger = true + } + if trigger.Input.HasVars() { + for varName := range trigger.Input.Vars { + triggerVars[varName] = true + } + } + // Legacy syntax + if trigger.Input.Name != "" { + triggerVars[trigger.Input.Name] = true + } + } + + // If workflow has event triggers, add event envelope variables as defined + if hasEventTrigger { + for _, v := range []string{"EventEnvelope", "EventTopic", "EventSource", "EventDataType", "EventTimestamp", "EventData"} { + defined[v] = true + } + } + // Check each step for undefined variables for i, step := range w.Steps { stepPrefix := fmt.Sprintf("steps[%d]", i) // Check all fields of this step - checkStepFieldsForUndefinedVars(&step, stepPrefix, defined, wast, r, &issues) + checkStepFieldsForUndefinedVars(&step, stepPrefix, defined, triggerVars, hasEventTrigger, wast, r, &issues) // Recurse into foreach inner step with scoped defined copy if step.Step != nil { @@ -219,12 +244,12 @@ func (r *UndefinedVariableRule) Check(wast *WorkflowAST) []LintIssue { } // _id_ is always available in foreach inner steps innerDefined["_id_"] = true - checkStepFieldsForUndefinedVars(step.Step, stepPrefix+".step", innerDefined, wast, r, &issues) + checkStepFieldsForUndefinedVars(step.Step, stepPrefix+".step", innerDefined, triggerVars, hasEventTrigger, wast, r, &issues) } // Recurse into parallel_steps for j := range step.ParallelSteps { - checkStepFieldsForUndefinedVars(&step.ParallelSteps[j], fmt.Sprintf("%s.parallel_steps[%d]", stepPrefix, j), defined, wast, r, &issues) + checkStepFieldsForUndefinedVars(&step.ParallelSteps[j], fmt.Sprintf("%s.parallel_steps[%d]", stepPrefix, j), defined, triggerVars, hasEventTrigger, wast, r, &issues) } // After processing this step, add its exports to defined @@ -699,9 +724,9 @@ func copyDefinedMap(m map[string]bool) map[string]bool { // checkStepFieldsForUndefinedVars checks ALL template-renderable fields of a single step. // stepPrefix is the field path prefix (e.g. "steps[0]" or "steps[0].step"). -func checkStepFieldsForUndefinedVars(step *core.Step, stepPrefix string, defined map[string]bool, wast *WorkflowAST, rule *UndefinedVariableRule, issues *[]LintIssue) { +func checkStepFieldsForUndefinedVars(step *core.Step, stepPrefix string, defined map[string]bool, triggerVars map[string]bool, hasEventTrigger bool, wast *WorkflowAST, rule *UndefinedVariableRule, issues *[]LintIssue) { check := func(s, field string) { - checkStringForUndefinedVars(s, field, defined, wast, rule, issues) + checkStringForUndefinedVars(s, field, defined, triggerVars, hasEventTrigger, wast, rule, issues) } checkSlice := func(slice []string, fieldBase string) { for j, s := range slice { @@ -802,24 +827,46 @@ func checkStepFieldsForUndefinedVars(step *core.Step, stepPrefix string, defined } } -func checkStringForUndefinedVars(s, field string, defined map[string]bool, wast *WorkflowAST, rule *UndefinedVariableRule, issues *[]LintIssue) { +func checkStringForUndefinedVars(s, field string, defined map[string]bool, triggerVars map[string]bool, hasEventTrigger bool, wast *WorkflowAST, rule *UndefinedVariableRule, issues *[]LintIssue) { if s == "" { return } for _, v := range extractVariables(s) { if !defined[v] && !builtInVariables[v] { - line, col := wast.GetNodePosition(field) - suggestion := findSimilarVariable(v, defined) - *issues = append(*issues, LintIssue{ - Rule: rule.Name(), - Severity: rule.Severity(), - Message: fmt.Sprintf("Variable '%s' is not defined", v), - Suggestion: suggestion, - Line: line, - Column: col, - Field: field, - }) + if triggerVars[v] { + line, col := wast.GetNodePosition(field) + *issues = append(*issues, LintIssue{ + Rule: rule.Name(), + Severity: SeverityInfo, + Message: fmt.Sprintf("Variable '%s' is provided by trigger input (not statically defined)", v), + Line: line, + Column: col, + Field: field, + }) + } else if hasEventTrigger { + line, col := wast.GetNodePosition(field) + *issues = append(*issues, LintIssue{ + Rule: rule.Name(), + Severity: SeverityInfo, + Message: fmt.Sprintf("Variable '%s' may be provided by event data at runtime", v), + Line: line, + Column: col, + Field: field, + }) + } else { + line, col := wast.GetNodePosition(field) + suggestion := findSimilarVariable(v, defined) + *issues = append(*issues, LintIssue{ + Rule: rule.Name(), + Severity: rule.Severity(), + Message: fmt.Sprintf("Variable '%s' is not defined", v), + Suggestion: suggestion, + Line: line, + Column: col, + Field: field, + }) + } } } } diff --git a/internal/linter/rules_test.go b/internal/linter/rules_test.go index e3e75b9..9f724a0 100644 --- a/internal/linter/rules_test.go +++ b/internal/linter/rules_test.go @@ -919,3 +919,249 @@ func searchSubstring(s, substr string) bool { } return false } + +func TestUndefinedVariableRule_TriggerInputVars(t *testing.T) { + rule := &UndefinedVariableRule{} + + t.Run("trigger input vars downgraded to info", func(t *testing.T) { + ast := parseTestWorkflow(t, ` +name: event-handler +kind: module +params: + - name: target +triggers: + - name: on-new-asset + on: event + enabled: true + event: + topic: assets.new + input: + source: event_data.source + description: event_data.desc +steps: + - name: process + type: bash + command: echo "{{target}} {{source}} {{description}}" +`) + issues := rule.Check(ast) + + // Should have issues for source and description, but at info severity + var infoIssues, warningIssues []LintIssue + for _, issue := range issues { + if issue.Rule == "undefined-variable" { + if issue.Severity == SeverityInfo { + infoIssues = append(infoIssues, issue) + } else if issue.Severity == SeverityWarning { + warningIssues = append(warningIssues, issue) + } + } + } + + assert.Len(t, infoIssues, 2, "should have 2 info-level issues for trigger vars") + assert.Empty(t, warningIssues, "should have no warning-level undefined-variable issues") + + // Verify messages mention trigger input + for _, issue := range infoIssues { + assert.Contains(t, issue.Message, "provided by trigger input") + } + }) + + t.Run("legacy trigger input syntax downgraded to info", func(t *testing.T) { + ast := parseTestWorkflow(t, ` +name: event-handler-legacy +kind: module +triggers: + - name: on-webhook + on: event + enabled: true + event: + topic: webhook.received + input: + type: event_data + field: url + name: webhook_target +steps: + - name: process + type: bash + command: echo "{{webhook_target}}" +`) + issues := rule.Check(ast) + + var infoIssues, warningIssues []LintIssue + for _, issue := range issues { + if issue.Rule == "undefined-variable" { + if issue.Severity == SeverityInfo { + infoIssues = append(infoIssues, issue) + } else if issue.Severity == SeverityWarning { + warningIssues = append(warningIssues, issue) + } + } + } + + assert.Len(t, infoIssues, 1, "should have 1 info-level issue for legacy trigger var") + assert.Empty(t, warningIssues, "should have no warning-level issues") + assert.Contains(t, infoIssues[0].Message, "webhook_target") + }) + + t.Run("event envelope variables recognized in event-triggered workflow", func(t *testing.T) { + ast := parseTestWorkflow(t, ` +name: event-handler-envelope +kind: module +triggers: + - name: on-asset + on: event + enabled: true + event: + topic: assets.new + input: + source: event_data.source +steps: + - name: process + type: bash + command: echo "{{EventTopic}} {{EventSource}} {{EventTimestamp}} {{EventData}} {{EventEnvelope}} {{EventDataType}}" +`) + issues := rule.Check(ast) + + // Event envelope variables should NOT produce any undefined-variable issues + var undefinedWarnings []LintIssue + for _, issue := range issues { + if issue.Rule == "undefined-variable" && issue.Severity == SeverityWarning { + undefinedWarnings = append(undefinedWarnings, issue) + } + } + assert.Empty(t, undefinedWarnings, "event envelope variables should be recognized as defined") + }) + + t.Run("truly undefined vars still warned alongside trigger vars", func(t *testing.T) { + ast := parseTestWorkflow(t, ` +name: mixed-vars +kind: module +triggers: + - name: on-event + on: event + enabled: true + event: + topic: test.topic + input: + source: event_data.source +steps: + - name: process + type: bash + command: echo "{{source}} {{truly_undefined}}" +`) + issues := rule.Check(ast) + + var infoIssues, warningIssues []LintIssue + for _, issue := range issues { + if issue.Rule == "undefined-variable" { + if issue.Severity == SeverityInfo { + infoIssues = append(infoIssues, issue) + } else if issue.Severity == SeverityWarning { + warningIssues = append(warningIssues, issue) + } + } + } + + // truly_undefined should be info (downgraded because event trigger exists) + // source is in triggerVars so also info + assert.Len(t, infoIssues, 2, "both source and truly_undefined should be info due to event trigger") + assert.Empty(t, warningIssues, "no warnings expected when event trigger is present") + }) + + t.Run("no trigger means no downgrade", func(t *testing.T) { + ast := parseTestWorkflow(t, ` +name: no-triggers +kind: module +steps: + - name: step1 + type: bash + command: echo "{{source}}" +`) + issues := rule.Check(ast) + + // source should be a normal warning, not info + var warningIssues []LintIssue + for _, issue := range issues { + if issue.Rule == "undefined-variable" && issue.Severity == SeverityWarning { + warningIssues = append(warningIssues, issue) + } + } + assert.Len(t, warningIssues, 1) + assert.Contains(t, warningIssues[0].Message, "source") + }) + + t.Run("event trigger downgrades unlisted vars to info", func(t *testing.T) { + // Scenario: input only maps "target", but steps also use source, description, + // asset_type — these should be SeverityInfo, not SeverityWarning. + ast := parseTestWorkflow(t, ` +name: event-handler-partial-input +kind: module +triggers: + - name: on-new-asset + on: event + enabled: true + event: + topic: assets.new + input: + target: event_data.url +steps: + - name: process + type: bash + command: echo "{{target}} {{source}} {{description}} {{asset_type}}" +`) + issues := rule.Check(ast) + + var infoIssues, warningIssues []LintIssue + for _, issue := range issues { + if issue.Rule == "undefined-variable" { + if issue.Severity == SeverityInfo { + infoIssues = append(infoIssues, issue) + } else if issue.Severity == SeverityWarning { + warningIssues = append(warningIssues, issue) + } + } + } + + // target is a built-in variable so no issue is emitted + // source, description, asset_type are NOT in triggerVars but hasEventTrigger → info ("may be provided by event data") + assert.Len(t, infoIssues, 3, "source, description, asset_type should be info-level") + assert.Empty(t, warningIssues, "no warnings when event trigger is present") + + // Verify the messages indicate possible event data + for _, issue := range infoIssues { + assert.Contains(t, issue.Message, "may be provided by event data") + } + }) + + t.Run("no event trigger means undefined vars are warnings", func(t *testing.T) { + // Workflow with a cron trigger (not event) — undefined vars should stay as warnings + ast := parseTestWorkflow(t, ` +name: cron-handler +kind: module +triggers: + - name: nightly + on: cron + enabled: true + schedule: "0 0 * * *" +steps: + - name: process + type: bash + command: echo "{{source}} {{description}}" +`) + issues := rule.Check(ast) + + var infoIssues, warningIssues []LintIssue + for _, issue := range issues { + if issue.Rule == "undefined-variable" { + if issue.Severity == SeverityInfo { + infoIssues = append(infoIssues, issue) + } else if issue.Severity == SeverityWarning { + warningIssues = append(warningIssues, issue) + } + } + } + + assert.Empty(t, infoIssues, "no info downgrades without event trigger") + assert.Len(t, warningIssues, 2, "source and description should be warnings") + }) +}