Merge branch 'issue-409' (fix #409)

rule_pyflakes.go

@@ -3,6 +3,7 @@ package actionlint
 import (
 	"bytes"
 	"fmt"
+	"strings"
 	"sync"
 )
 
@@ -18,7 +19,7 @@ func getShellIsPythonKind(shell *String) shellIsPythonKind {
 	if shell == nil {
 		return shellIsPythonKindUnspecified
 	}
-	if shell.Value == "python" {
+	if shell.Value == "python" || strings.HasPrefix(shell.Value, "python ") {
 		return shellIsPythonKindPython
 	}
 	return shellIsPythonKindNotPython
@@ -34,15 +35,8 @@ type RulePyflakes struct {
 	mu                    sync.Mutex
 }
 
-// NewRulePyflakes creates new RulePyflakes instance. Parameter executable can be command name
-// or relative/absolute file path. When the given executable is not found in system, it returns
-// an error.
-func NewRulePyflakes(executable string, proc *concurrentProcess) (*RulePyflakes, error) {
-	cmd, err := proc.newCommandRunner(executable)
-	if err != nil {
-		return nil, err
-	}
-	r := &RulePyflakes{
+func newRulePyflakes(cmd *externalCommand) *RulePyflakes {
+	return &RulePyflakes{
 		RuleBase: RuleBase{
 			name: "pyflakes",
 			desc: "Checks for Python script when \"shell: python\" is configured using Pyflakes",
@@ -51,7 +45,17 @@ func NewRulePyflakes(executable string, proc *concurrentProcess) (*RulePyflakes,
 		workflowShellIsPython: shellIsPythonKindUnspecified,
 		jobShellIsPython:      shellIsPythonKindUnspecified,
 	}
-	return r, nil
+}
+
+// NewRulePyflakes creates new RulePyflakes instance. Parameter executable can be command name
+// or relative/absolute file path. When the given executable is not found in system, it returns
+// an error.
+func NewRulePyflakes(executable string, proc *concurrentProcess) (*RulePyflakes, error) {
+	cmd, err := proc.newCommandRunner(executable)
+	if err != nil {
+		return nil, err
+	}
+	return newRulePyflakes(cmd), nil
 }
 
 // VisitJobPre is callback when visiting Job node before visiting its children.
@@ -98,8 +102,8 @@ func (rule *RulePyflakes) VisitStep(n *Step) error {
 }
 
 func (rule *RulePyflakes) isPythonShell(r *ExecRun) bool {
-	if r.Shell != nil {
-		return r.Shell.Value == "python"
+	if k := getShellIsPythonKind(r.Shell); k != shellIsPythonKindUnspecified {
+		return k == shellIsPythonKindPython
 	}
 
 	if rule.jobShellIsPython != shellIsPythonKindUnspecified {

rule_pyflakes_test.go

@@ -0,0 +1,84 @@
+package actionlint
+
+import (
+	"testing"
+)
+
+func TestRulePyflakesDetectPythonShell(t *testing.T) {
+	tests := []struct {
+		what     string
+		isPython bool
+		workflow string // Shell name set at 'defaults' in Workflow node
+		job      string // Shell name set at 'defaults' in Job node
+		step     string // Shell name set at 'shell' in Step node
+	}{
+		{
+			what: "no default shell",
+			isPython: false,
+		},
+		{
+			what: "workflow default",
+			isPython: true,
+			workflow: "python",
+		},
+		{
+			what: "job default",
+			isPython: true,
+			job: "python",
+		},
+		{
+			what: "step shell",
+			isPython: true,
+			step: "python",
+		},
+		{
+			what: "custom shell",
+			isPython: true,
+			workflow: "python {0}",
+		},
+		{
+			what: "other shell",
+			isPython: false,
+			workflow: "pwsh",
+		},
+		{
+			what: "other custom shell",
+			isPython: false,
+			workflow: "bash -e {0}",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.what, func(t *testing.T) {
+			r := newRulePyflakes(&externalCommand{})
+
+			w := &Workflow{}
+			if tc.workflow != "" {
+				w.Defaults = &Defaults{
+					Run: &DefaultsRun{
+						Shell: &String{Value: tc.workflow},
+					},
+				}
+			}
+			r.VisitWorkflowPre(w)
+
+			j := &Job{}
+			if tc.job != "" {
+				j.Defaults = &Defaults{
+					Run: &DefaultsRun{
+						Shell: &String{Value: tc.job},
+					},
+				}
+			}
+			r.VisitJobPre(j)
+
+			e := &ExecRun{}
+			if tc.step != "" {
+				e.Shell = &String{Value: tc.step}
+			}
+			if have := r.isPythonShell(e); have != tc.isPython {
+				t.Fatalf("Actual isPython=%v but wanted isPython=%v", have, tc.isPython)
+			}
+		})
+	}
+}

rule_shellcheck.go

@@ -22,27 +22,32 @@ type RuleShellcheck struct {
 	cmd           *externalCommand
 	workflowShell string
 	jobShell      string
+	runnerShell   string
 	mu            sync.Mutex
 }
 
-// NewRuleShellcheck creates new RuleShellcheck instance. Parameter executable can be command name
-// or relative/absolute file path. When the given executable is not found in system, it returns an
-// error as 2nd return value.
-func NewRuleShellcheck(executable string, proc *concurrentProcess) (*RuleShellcheck, error) {
-	cmd, err := proc.newCommandRunner(executable)
-	if err != nil {
-		return nil, err
-	}
-	r := &RuleShellcheck{
+func newRuleShellcheck(cmd *externalCommand) *RuleShellcheck {
+	return &RuleShellcheck{
 		RuleBase: RuleBase{
 			name: "shellcheck",
 			desc: "Checks for shell script sources in \"run:\" using shellcheck",
 		},
 		cmd:           cmd,
 		workflowShell: "",
 		jobShell:      "",
+		runnerShell:   "",
 	}
-	return r, nil
+}
+
+// NewRuleShellcheck creates new RuleShellcheck instance. The executable argument can be command
+// name or relative/absolute file path. When the given executable is not found in system, it returns
+// an error as 2nd return value.
+func NewRuleShellcheck(executable string, proc *concurrentProcess) (*RuleShellcheck, error) {
+	cmd, err := proc.newCommandRunner(executable)
+	if err != nil {
+		return nil, err
+	}
+	return newRuleShellcheck(cmd), nil
 }
 
 // VisitStep is callback when visiting Step node.
@@ -52,44 +57,35 @@ func (rule *RuleShellcheck) VisitStep(n *Step) error {
 		return nil
 	}
 
-	name := rule.getShellName(run)
-	if name != "bash" && name != "sh" {
-		return nil
-	}
-
-	rule.runShellcheck(run.Run.Value, name, run.RunPos)
+	rule.runShellcheck(run.Run.Value, rule.getShellName(run), run.RunPos)
 	return nil
 }
 
 // VisitJobPre is callback when visiting Job node before visiting its children.
 func (rule *RuleShellcheck) VisitJobPre(n *Job) error {
 	if n.Defaults != nil && n.Defaults.Run != nil && n.Defaults.Run.Shell != nil {
 		rule.jobShell = n.Defaults.Run.Shell.Value
-		return nil
 	}
 
-	if n.RunsOn == nil {
-		return nil
-	}
-
-	for _, label := range n.RunsOn.Labels {
-		l := strings.ToLower(label.Value)
-		// Default shell on Windows is PowerShell.
-		// https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#using-a-specific-shell
-		if l == "windows" || strings.HasPrefix(l, "windows-") {
-			return nil
+	if n.RunsOn != nil {
+		for _, label := range n.RunsOn.Labels {
+			l := strings.ToLower(label.Value)
+			// Default shell on Windows is PowerShell.
+			// https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#using-a-specific-shell
+			if l == "windows" || strings.HasPrefix(l, "windows-") {
+				rule.runnerShell = "pwsh"
+				break
+			}
 		}
 	}
 
-	// TODO: When bash is not found, GitHub-hosted runner fallbacks to sh. What OSes require this behavior?
-	rule.jobShell = "bash"
-
 	return nil
 }
 
 // VisitJobPost is callback when visiting Job node after visiting its children.
 func (rule *RuleShellcheck) VisitJobPost(n *Job) error {
 	rule.jobShell = ""
+	rule.runnerShell = ""
 	return nil
 }
 
@@ -114,7 +110,15 @@ func (rule *RuleShellcheck) getShellName(exec *ExecRun) string {
 	if rule.jobShell != "" {
 		return rule.jobShell
 	}
-	return rule.workflowShell
+	if rule.workflowShell != "" {
+		return rule.workflowShell
+	}
+	if rule.runnerShell != "" {
+		return rule.runnerShell
+	}
+	// Note: Default shell on Windows is pwsh so this value is not always correct.
+	// Note: When bash is not found, GitHub-hosted runner fallbacks to sh.
+	return "bash"
 }
 
 // Replace ${{ ... }} with underscores like __________
@@ -156,7 +160,18 @@ func sanitizeExpressionsInScript(src string) string {
 	}
 }
 
-func (rule *RuleShellcheck) runShellcheck(src, sh string, pos *Pos) {
+func (rule *RuleShellcheck) runShellcheck(src, shell string, pos *Pos) {
+	var sh string
+	if shell == "bash" || shell == "sh" {
+		sh = shell
+	} else if strings.HasPrefix(shell, "bash ") {
+		sh = "bash"
+	} else if strings.HasPrefix(shell, "sh ") {
+		sh = "sh"
+	} else {
+		return // Skip checking this shell script since shellcheck doesn't support it
+	}
+
 	src = sanitizeExpressionsInScript(src)
 	rule.Debug("%s: Run shellcheck for %s script:\n%s", pos, sh, src)