From b4483497f96bc5933e5bb6436587dc846843b7b9 Mon Sep 17 00:00:00 2001 From: wwcchh0123 <1170142389@qq.com> Date: Thu, 14 Mar 2024 17:20:03 +0800 Subject: [PATCH] feat: new shellcheck linter --- .../linters/shell/shellcheck/shellcheck.go | 167 ++++++++++++++++++ .../shell/shellcheck/shellcheck_test.go | 62 +++++++ .../shell/shellcheck/testdata/test.txt | 7 + main.go | 1 + 4 files changed, 237 insertions(+) create mode 100644 internal/linters/shell/shellcheck/shellcheck.go create mode 100644 internal/linters/shell/shellcheck/shellcheck_test.go create mode 100644 internal/linters/shell/shellcheck/testdata/test.txt diff --git a/internal/linters/shell/shellcheck/shellcheck.go b/internal/linters/shell/shellcheck/shellcheck.go new file mode 100644 index 00000000..08579f10 --- /dev/null +++ b/internal/linters/shell/shellcheck/shellcheck.go @@ -0,0 +1,167 @@ +package shellcheck + +import ( + "os/exec" + "regexp" + "strconv" + "strings" + + "github.com/qiniu/reviewbot/internal/linters" + "github.com/qiniu/x/log" + "github.com/qiniu/x/xlog" +) + +var lintName = "shellcheck" + +func init() { + linters.RegisterPullRequestHandler(lintName, shellcheckHandler) +} + +func shellcheckHandler(log *xlog.Logger, a linters.Agent) error { + /* + * Shellcheck can not support check directories recursively, it is usually used in conjunction with the "find" command + * Info: + * https://github.com/koalaman/shellcheck/issues/143 + * https://github.com/koalaman/shellcheck/wiki/Recursiveness + */ + if linters.IsEmpty(a.LinterConfig.Args...) { + paths := FindShellFilesPath(a.LinterConfig.WorkDir) + a.LinterConfig.Args = append(a.LinterConfig.Args, paths...) + } + + executor, err := NewShellcheckExecutor(a.LinterConfig.WorkDir) + if err != nil { + log.Errorf("init shellcheck executor failed: %v", err) + return err + } + + output, err := executor.Run(log, a.LinterConfig.Args...) + if err != nil { + log.Errorf("shellcheck run failed: %v", err) + return err + } + parsedOutput, err := executor.Parse(log, output) + if err != nil { + log.Errorf("shellcheck parse output failed: %v", err) + return err + } + + return linters.Report(log, a, parsedOutput) +} + +func FindShellFilesPath(dir string) []string { + c := exec.Command("find", ".", "-type", "f", "-name", "*.sh") + c.Dir = dir + out, err := c.Output() + if err != nil { + log.Errorf("find the path of shell files failed, err: %v", err) + } + fps := strings.Split(string(out), "\n") + for i := 0; i < len(fps); i++ { + fps[i] = strings.TrimPrefix(fps[i], "./") + } + return fps +} + +type Shellcheck struct { + dir string + shellcheck string + execute func(dir, command string, args ...string) ([]byte, error) +} + +func NewShellcheckExecutor(dir string) (linters.Linter, error) { + log.Infof("shellcheck executor init") + g, err := exec.LookPath("shellcheck") + if err != nil { + return nil, err + } + return &Shellcheck{ + dir: dir, + shellcheck: g, + execute: func(dir, command string, args ...string) ([]byte, error) { + c := exec.Command(command, args...) + c.Dir = dir + log.Printf("final command: %v \n", c) + return c.Output() + }, + }, nil +} + +func (g *Shellcheck) Run(log *xlog.Logger, args ...string) ([]byte, error) { + b, err := g.execute(g.dir, g.shellcheck, args...) + if err != nil { + log.Errorf("shellcheck run with status: %v, mark and continue", err) + } else { + log.Infof("shellcheck running succeeded") + } + return b, nil +} + +func (g *Shellcheck) Parse(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { + log.Infof("shellcheck output is being parsed") + return formatShellcheckOutput(output) +} + +func formatShellcheckOutput(output []byte) (map[string][]linters.LinterOutput, error) { + //format for tty format output + var result = make(map[string][]linters.LinterOutput) + lines := strings.Split(string(output), "\n") + var filename string + var location int + fileErr := make(map[string]map[int][]string) + re := regexp.MustCompile(`In (\S+) line (\d+):`) + for _, line := range lines { + if _, ok := fileErr[filename]; !ok { + fileErr[filename] = make(map[int][]string) + } + if strings.HasPrefix(line, "In") { + match := re.FindStringSubmatch(line) + if len(match) >= 3 { + filename = strings.TrimPrefix(match[1], "./") + location, _ = strconv.Atoi(match[2]) + } + + } else if strings.HasPrefix(line, "For more information:") { + break + } else { + fileErr[filename][location] = append(fileErr[filename][location], line) + } + } + + for filename, errs := range fileErr { + for locationLine, msgs := range errs { + sendMsg := strings.Join(msgs, "\n") + sendMsg = "Is there some potential issue with your shell code?" + "\n```\n" + sendMsg + "\n```\n" + addShellcheckOutput(result, filename, locationLine, sendMsg) + } + } + + return result, nil +} + +func addShellcheckOutput(result map[string][]linters.LinterOutput, filename string, line int, message string) { + output := &linters.LinterOutput{ + File: filename, + Line: int(line), + Column: int(line), + Message: message, + } + + if outs, ok := result[output.File]; !ok { + result[output.File] = []linters.LinterOutput{*output} + } else { + // remove duplicate + var existed bool + for _, v := range outs { + if v.File == output.File && v.Line == output.Line && v.Column == output.Column && v.Message == output.Message { + existed = true + break + } + } + + if !existed { + result[output.File] = append(result[output.File], *output) + } + } + +} diff --git a/internal/linters/shell/shellcheck/shellcheck_test.go b/internal/linters/shell/shellcheck/shellcheck_test.go new file mode 100644 index 00000000..bdcb7bd5 --- /dev/null +++ b/internal/linters/shell/shellcheck/shellcheck_test.go @@ -0,0 +1,62 @@ +package shellcheck + +import ( + "os" + "testing" + + "github.com/qiniu/reviewbot/internal/linters" +) + +func TestShell(t *testing.T) { + content, err := os.ReadFile("./test.txt") + if err != nil { + t.Errorf("open file failed ,the err is : %v", err) + return + } + expect := []linters.LinterOutput{ + { + File: "lua-ut.sh", + Line: 22, + Column: 22, + Message: "", + }, + { + File: "util.sh", + Line: 13, + Column: 13, + Message: "", + }, + } + tc := []struct { + input []byte + expected []linters.LinterOutput + }{ + { + content, + expect, + }, + } + for _, c := range tc { + outputMap, err := formatShellcheckOutput([]byte(c.input)) + for _, outputs := range outputMap { + for _, output := range outputs { + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if output.File == "util.sh" { + if output.File != c.expected[1].File || output.Line != c.expected[1].Line || output.Column != c.expected[1].Column { + t.Errorf("expected: %v, got: %v", c.expected[1], output) + } + } else { + if output.File != c.expected[0].File || output.Line != c.expected[0].Line || output.Column != c.expected[0].Column { + t.Errorf("expected: %v, got: %v", c.expected[0], output) + } + } + + } + + } + } +} diff --git a/internal/linters/shell/shellcheck/testdata/test.txt b/internal/linters/shell/shellcheck/testdata/test.txt new file mode 100644 index 00000000..19d6384c --- /dev/null +++ b/internal/linters/shell/shellcheck/testdata/test.txt @@ -0,0 +1,7 @@ +In lua-ut.sh line 22: +for svc in ${svcs[@]} + ^--------^ SC2068 (error): Double quote array expansions to avoid re-splitting elements. + +In util.sh line 13: + echo -e "[$(date +'%Y-%m-%dT%H:%M:%S.%N%z')] WARN: $@" >&2 + ^-- SC2145 (error): Argument mixes string and array. Use * or separate argument. diff --git a/main.go b/main.go index 3c1ce017..eb020e38 100644 --- a/main.go +++ b/main.go @@ -35,6 +35,7 @@ import ( _ "github.com/qiniu/reviewbot/internal/linters/go/golangci_lint" _ "github.com/qiniu/reviewbot/internal/linters/go/staticcheck" _ "github.com/qiniu/reviewbot/internal/linters/lua/luacheck" + _ "github.com/qiniu/reviewbot/internal/linters/shell/shellcheck" ) type options struct {