Skip to content

Optimistically match shellcheck errors to source #556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type String struct {
Quoted bool
// Pos is a position of the string in source.
Pos *Pos
// If string is a literal block which preserves newlines and indentation. Helpful for error messages
Literal bool
}

// ContainsExpression checks if the given string contains a ${{ }} placeholder or not. This function
Expand Down
5 changes: 3 additions & 2 deletions parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ func isNull(n *yaml.Node) bool {

func newString(n *yaml.Node) *String {
quoted := n.Style&(yaml.DoubleQuotedStyle|yaml.SingleQuotedStyle) != 0
return &String{n.Value, quoted, posAt(n)}
literal := n.Style == yaml.LiteralStyle
return &String{n.Value, quoted, posAt(n), literal}
}

type workflowKeyVal struct {
Expand Down Expand Up @@ -137,7 +138,7 @@ func (p *parser) mayParseExpression(n *yaml.Node) *String {

func (p *parser) parseString(n *yaml.Node, allowEmpty bool) *String {
if !p.checkString(n, allowEmpty) {
return &String{"", false, posAt(n)}
return &String{"", false, posAt(n), false}
}
return newString(n)
}
Expand Down
4 changes: 2 additions & 2 deletions rule_runner_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func (rule *RuleRunnerLabel) tryToGetLabelsInMatrix(label *String, m *Matrix) []
if row, ok := m.Rows[prop]; ok {
for _, v := range row.Values {
if s, ok := v.(*RawYAMLString); ok && !ContainsExpression(s.Value) {
labels = append(labels, &String{s.Value, false, s.Pos()})
labels = append(labels, &String{s.Value, false, s.Pos(), false})
}
}
}
Expand All @@ -277,7 +277,7 @@ func (rule *RuleRunnerLabel) tryToGetLabelsInMatrix(label *String, m *Matrix) []
if combi.Assigns != nil {
if assign, ok := combi.Assigns[prop]; ok {
if s, ok := assign.Value.(*RawYAMLString); ok && !ContainsExpression(s.Value) {
labels = append(labels, &String{s.Value, false, s.Pos()})
labels = append(labels, &String{s.Value, false, s.Pos(), false})
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions rule_runner_label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func TestRuleRunnerLabelCheckLabels(t *testing.T) {
pos := &Pos{}
labels := make([]*String, 0, len(tc.labels))
for _, l := range tc.labels {
labels = append(labels, &String{l, false, pos})
labels = append(labels, &String{l, false, pos, false})
}
node := &Job{
RunsOn: &Runner{
Expand All @@ -264,7 +264,7 @@ func TestRuleRunnerLabelCheckLabels(t *testing.T) {
}

if tc.matrix != nil {
n := &String{"os", false, pos}
n := &String{"os", false, pos, false}
row := make([]RawYAMLValue, 0, len(tc.matrix))
for _, m := range tc.matrix {
row = append(row, &RawYAMLString{m, pos})
Expand Down
17 changes: 14 additions & 3 deletions rule_shellcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package actionlint
import (
"encoding/json"
"fmt"
"os"
"strings"
"sync"
)
Expand Down Expand Up @@ -57,7 +58,7 @@ func (rule *RuleShellcheck) VisitStep(n *Step) error {
return nil
}

rule.runShellcheck(run.Run.Value, rule.getShellName(run), run.RunPos)
rule.runShellcheck(run.Run, rule.getShellName(run), run.RunPos)
return nil
}

Expand Down Expand Up @@ -160,7 +161,7 @@ func sanitizeExpressionsInScript(src string) string {
}
}

func (rule *RuleShellcheck) runShellcheck(src, shell string, pos *Pos) {
func (rule *RuleShellcheck) runShellcheck(srcAst *String, shell string, pos *Pos) {
var sh string
if shell == "bash" || shell == "sh" {
sh = shell
Expand All @@ -172,6 +173,7 @@ func (rule *RuleShellcheck) runShellcheck(src, shell string, pos *Pos) {
return // Skip checking this shell script since shellcheck doesn't support it
}

src := srcAst.Value
src = sanitizeExpressionsInScript(src)
rule.Debug("%s: Run shellcheck for %s script:\n%s", pos, sh, src)

Expand Down Expand Up @@ -214,6 +216,8 @@ func (rule *RuleShellcheck) runShellcheck(src, shell string, pos *Pos) {
return nil
}

rule.EnableDebug(os.Stdout)

// Synchronize rule.Errorf calls
rule.mu.Lock()
defer rule.mu.Unlock()
Expand All @@ -227,7 +231,14 @@ func (rule *RuleShellcheck) runShellcheck(src, shell string, pos *Pos) {
// Consider the first line is setup for running shell which was implicitly added for better check
line := err.Line - 1
msg := strings.TrimSuffix(err.Message, ".") // Trim period aligning style of error message
rule.Errorf(pos, "shellcheck reported issue in this script: SC%d:%s:%d:%d: %s", err.Code, err.Level, line, err.Column, msg)

var errorLocation Pos
if srcAst.Literal {
errorLocation = Pos{line + srcAst.Pos.Line, err.Column + srcAst.Pos.Col - 4}
} else {
errorLocation = *pos
}
rule.Errorf(&errorLocation, "shellcheck reported issue in this script: SC%d:%s:%d:%d: %s", err.Code, err.Level, line, err.Column, msg)
}

return nil
Expand Down