diff --git a/config/config.yaml b/config/config.yaml index 6fa6f4c9..0d2f6b59 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -4,7 +4,7 @@ # qbox: # staticcheck: # enable: false -# + # example2: disable staticcheck for repo # qbox/kodo: # staticcheck: diff --git a/internal/linters/go/staticcheck/staticcheck.go b/internal/linters/go/staticcheck/staticcheck.go index e6c5bc0f..b3280388 100644 --- a/internal/linters/go/staticcheck/staticcheck.go +++ b/internal/linters/go/staticcheck/staticcheck.go @@ -17,18 +17,12 @@ package staticcheck import ( - "fmt" - "os/exec" - "path/filepath" - "regexp" - "strconv" - "strings" - "github.com/google/go-github/v57/github" "github.com/qiniu/x/log" "github.com/qiniu/x/xlog" "github.com/reviewbot/config" "github.com/reviewbot/internal/linters" + "os/exec" ) var lintName = "staticcheck" @@ -44,7 +38,7 @@ func staticcheckHandler(log *xlog.Logger, linterConfig config.Linter, agent lint return nil, err } - if isEmpty(linterConfig.Args...) { + if linters.IsEmpty(linterConfig.Args...) { // turn off compile errors by default linterConfig.Args = append([]string{}, "-debug.no-compile-errors=true", "./...") } @@ -64,16 +58,6 @@ func staticcheckHandler(log *xlog.Logger, linterConfig config.Linter, agent lint return parsedOutput, nil } -func isEmpty(args ...string) bool { - for _, arg := range args { - if arg != "" { - return false - } - } - - return true -} - // Staticcheck is an executor that knows how to execute Staticcheck commands. type Staticcheck struct { // dir is the location of this repo. @@ -115,10 +99,6 @@ func (e *Staticcheck) Run(log *xlog.Logger, args ...string) ([]byte, error) { return b, nil } -func (e *Staticcheck) Parse(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { - return formatStaticcheckOutput(log, output) -} - // formatStaticcheckOutput formats the output of staticcheck // example： // gslb-api/utils.go:149:6: func dealWithRecordVipsId is unused (U1000) @@ -126,85 +106,6 @@ func (e *Staticcheck) Parse(log *xlog.Logger, output []byte) (map[string][]linte // domain/repo/image.go:70:7: receiver name should be a reflection of its identity; don't use generic names such as "this" or "self" (ST1006) // // output: map[file][]linters.LinterOutput -func formatStaticcheckOutput(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { - lines := strings.Split(string(output), "\n") - - var result = make(map[string][]linters.LinterOutput) - for _, line := range lines { - if line == "" { - continue - } - output, err := formatStaticcheckLine(line) - if err != nil { - log.Warnf("unexpected staticcheck output: %v", line) - // 不直接退出 - continue - } - - if output == nil { - continue - } - - if isGopAutoGeneratedFile(output.File) { - log.Infof("skip auto generated file by go+ : %s", output.File) - continue - } - - 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) - } - } - } - - return result, nil -} - -func formatStaticcheckLine(line string) (*linters.LinterOutput, error) { - pattern := `^(.*):(\d+):(\d+): (.*)$` - regex, err := regexp.Compile(pattern) - if err != nil { - log.Errorf("compile regex failed: %v", err) - return nil, err - } - matches := regex.FindStringSubmatch(line) - if len(matches) != 5 { - return nil, fmt.Errorf("unexpected format, original: %s", line) - } - - lineNumber, err := strconv.ParseInt(matches[2], 10, 64) - if err != nil { - return nil, err - } - - columnNumber, err := strconv.ParseInt(matches[3], 10, 64) - if err != nil { - return nil, err - } - - return &linters.LinterOutput{ - File: matches[1], - Line: int(lineNumber), - Column: int(columnNumber), - Message: matches[4], - }, nil -} - -var gop_auto_generated_file_pattern = `^.*_autogen.*.go$` -var gopGeneratedFileRegexp = regexp.MustCompile(gop_auto_generated_file_pattern) - -func isGopAutoGeneratedFile(file string) bool { - // TODO: need a more accurate way to determine whether it is a go+ auto generated file - return gopGeneratedFileRegexp.MatchString(filepath.Base(file)) +func (e *Staticcheck) Parse(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { + return linters.FormatLinterOutput(log, output, linters.FormatLinterLine) } diff --git a/internal/linters/lua/luacheck/luacheck.go b/internal/linters/lua/luacheck/luacheck.go new file mode 100644 index 00000000..40d974e3 --- /dev/null +++ b/internal/linters/lua/luacheck/luacheck.go @@ -0,0 +1,97 @@ +package luacheck + +import ( + "bytes" + "github.com/google/go-github/v57/github" + "github.com/qiniu/x/xlog" + "github.com/reviewbot/config" + "github.com/reviewbot/internal/linters" + "os/exec" + "strings" +) + +var lintName = "luacheck" + +func init() { + linters.RegisterCodeReviewHandler(lintName, luaCheckHandler) +} + +func luaCheckHandler(log *xlog.Logger, linterConfig config.Linter, _ linters.Agent, _ github.PullRequestEvent) (map[string][]linters.LinterOutput, error) { + executor, err := NewLuaCheckExecutor(linterConfig.WorkDir) + if err != nil { + log.Errorf("init luacheck executor failed: %v", err) + return nil, err + } + + if linters.IsEmpty(linterConfig.Args...) { + linterConfig.Args = append([]string{}, ".") + } + + output, err := executor.Run(log, linterConfig.Args...) + if err != nil { + log.Errorf("luacheck run failed: %v", err) + return nil, err + } + + parsedOutput, err := executor.Parse(log, output) + if err != nil { + log.Errorf("luacheck parse output failed: %v", err) + return nil, err + } + + return parsedOutput, nil +} + +// luacheck is an executor that knows how to execute luacheck commands. +type luacheck struct { + // dir is the location of this repo. + dir string + // luacheck is the path to the luacheck binary. + luacheck string +} + +func NewLuaCheckExecutor(dir string) (linters.Linter, error) { + g, err := exec.LookPath("luacheck") + if err != nil { + return nil, err + } + return &luacheck{ + dir: dir, + luacheck: g, + }, nil +} + +func (e *luacheck) Run(log *xlog.Logger, args ...string) ([]byte, error) { + c := exec.Command(e.luacheck, args...) + c.Dir = e.dir + var out bytes.Buffer + c.Stdout = &out + c.Stderr = &out + err := c.Run() + if err != nil { + log.Errorf("luacheck run with status: %v, mark and continue", err) + } else { + log.Infof("luacheck succeeded") + } + + return out.Bytes(), nil +} + +func (e *luacheck) Parse(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { + return linters.FormatLinterOutput(log, output, formatLuaCheckLine) +} + +func formatLuaCheckLine(line string) (*linters.LinterOutput, error) { + + lineResult, err := linters.FormatLinterLine(line) + if err != nil { + return nil, err + + } + return &linters.LinterOutput{ + File: strings.TrimLeft(lineResult.File, " "), + Line: lineResult.Line, + Column: lineResult.Column, + Message: strings.ReplaceAll(strings.ReplaceAll(lineResult.Message, "\x1b[0m\x1b[1m", ""), "\x1b[0m", ""), + }, nil +} diff --git a/internal/linters/lua/luacheck/luacheck_test.go b/internal/linters/lua/luacheck/luacheck_test.go new file mode 100644 index 00000000..1e3d5bf0 --- /dev/null +++ b/internal/linters/lua/luacheck/luacheck_test.go @@ -0,0 +1,79 @@ +/* + Copyright 2024 Qiniu Cloud (qiniu.com). + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package luacheck + +import ( + "github.com/reviewbot/internal/linters" + "testing" +) + +func TestFormatLuaCheckLine(t *testing.T) { + tc := []struct { + input string + expected *linters.LinterOutput + }{ + {" video/mp4/libs/mp4lib.lua:184:11: value assigned to variable mem_data is overwritten on line 202 before use", &linters.LinterOutput{ + File: " video/mp4/libs/mp4lib.lua:184:11", + Line: 184, + Column: 11, + Message: "value assigned to variable mem_data is overwritten on line 202 before use", + }}, + {" utils/jsonschema.lua:723:121: line is too long (142 > 120)", &linters.LinterOutput{ + File: " utils/jsonschema.lua", + Line: 723, + Column: 121, + Message: "line is too long (142 > 120)", + }}, + {" utils/httpc/http_simple.lua:24:1: setting read-only global variable _VERSION", &linters.LinterOutput{ + File: " utils/httpc/http_simple.lua", + Line: 24, + Column: 1, + Message: "setting read-only global variable _VERSION", + }}, + {" test/qtest_access.lua:1220:1: inconsistent indentation (SPACE followed by TAB)", &linters.LinterOutput{ + File: " test/qtest_access.lua", + Line: 1220, + Column: 1, + Message: "inconsistent indentation (SPACE followed by TAB)", + }}, + {"Checking test/qtest_mgrconf.lua", nil}, + } + + for _, c := range tc { + output, err := formatLuaCheckLine(c.input) + if c.expected == nil && output != nil { + t.Errorf("expected error, got: %v", output) + } else { + continue + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if output == nil { + if c.expected != nil { + t.Errorf("expected: %v, got: %v", c.expected, output) + } + continue + } + + if output.File != c.expected.File || output.Line != c.expected.Line || output.Column != c.expected.Column || output.Message != c.expected.Message { + t.Errorf("expected: %v, got: %v", c.expected, output) + } + } +} diff --git a/internal/linters/util.go b/internal/linters/util.go new file mode 100644 index 00000000..0c51635f --- /dev/null +++ b/internal/linters/util.go @@ -0,0 +1,107 @@ +package linters + +import ( + "fmt" + "github.com/qiniu/x/log" + "github.com/qiniu/x/xlog" + "path/filepath" + "regexp" + "strconv" + "strings" +) + +func IsEmpty(args ...string) bool { + for _, arg := range args { + if arg != "" { + return false + } + } + + return true +} + +type FormatLinterLineFunc func(line string) (*LinterOutput, error) + +func FormatLinterOutput(log *xlog.Logger, output []byte, FormatLinter FormatLinterLineFunc) (map[string][]LinterOutput, error) { + lines := strings.Split(string(output), "\n") + + var result = make(map[string][]LinterOutput) + for _, line := range lines { + if line == "" { + continue + } + output, err := FormatLinter(line) + if err != nil { + log.Warnf("unexpected linter check output: %v", line) + // 不直接退出 + continue + } + + if output == nil { + continue + } + + if isGopAutoGeneratedFile(output.File) { + log.Infof("skip auto generated file by go+ : %s", output.File) + continue + } + + if outs, ok := result[output.File]; !ok { + result[output.File] = []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) + } + } + } + + return result, nil +} + +// common format LinterLine +func FormatLinterLine(line string) (*LinterOutput, error) { + pattern := `^(.*):(\d+):(\d+): (.*)$` + regex, err := regexp.Compile(pattern) + if err != nil { + log.Errorf("compile regex failed: %v", err) + return nil, err + } + matches := regex.FindStringSubmatch(line) + if len(matches) != 5 { + return nil, fmt.Errorf("unexpected format, original: %s", line) + } + + lineNumber, err := strconv.ParseInt(matches[2], 10, 64) + if err != nil { + return nil, err + } + + columnNumber, err := strconv.ParseInt(matches[3], 10, 64) + if err != nil { + return nil, err + } + + return &LinterOutput{ + File: matches[1], + Line: int(lineNumber), + Column: int(columnNumber), + Message: matches[4], + }, nil +} + +var gop_auto_generated_file_pattern = `^.*_autogen.*.go$` +var gopGeneratedFileRegexp = regexp.MustCompile(gop_auto_generated_file_pattern) + +func isGopAutoGeneratedFile(file string) bool { + // TODO: need a more accurate way to determine whether it is a go+ auto generated file + return gopGeneratedFileRegexp.MatchString(filepath.Base(file)) +} diff --git a/internal/linters/go/staticcheck/staticcheck_test.go b/internal/linters/util_test.go similarity index 90% rename from internal/linters/go/staticcheck/staticcheck_test.go rename to internal/linters/util_test.go index 3ea198c0..d9eeb9f7 100644 --- a/internal/linters/go/staticcheck/staticcheck_test.go +++ b/internal/linters/util_test.go @@ -14,32 +14,30 @@ limitations under the License. */ -package staticcheck +package linters import ( "testing" - - "github.com/reviewbot/internal/linters" ) func TestFormatStaticcheckLine(t *testing.T) { tc := []struct { input string - expected *linters.LinterOutput + expected *LinterOutput }{ - {"cdn-admin.v2/client/dns/dnsapi.go:59:3: assignment to err", &linters.LinterOutput{ + {"cdn-admin.v2/client/dns/dnsapi.go:59:3: assignment to err", &LinterOutput{ File: "cdn-admin.v2/client/dns/dnsapi.go", Line: 59, Column: 3, Message: "assignment to err", }}, - {"smart_scheduler/provider_scheduler/provider_manager/provider_manager.go:207:31: should use make([]float64, len(result.CDNLog.Points)) instead (S1019)", &linters.LinterOutput{ + {"smart_scheduler/provider_scheduler/provider_manager/provider_manager.go:207:31: should use make([]float64, len(result.CDNLog.Points)) instead (S1019)", &LinterOutput{ File: "smart_scheduler/provider_scheduler/provider_manager/provider_manager.go", Line: 207, Column: 31, Message: "should use make([]float64, len(result.CDNLog.Points)) instead (S1019)", }}, - {"cdn-admin.v2/api/api_line.go:342:3: should replace loop with ret = append(ret, scope.EdgeNodes...) (S1011)", &linters.LinterOutput{ + {"cdn-admin.v2/api/api_line.go:342:3: should replace loop with ret = append(ret, scope.EdgeNodes...) (S1011)", &LinterOutput{ File: "cdn-admin.v2/api/api_line.go", Line: 342, Column: 3, @@ -49,7 +47,7 @@ func TestFormatStaticcheckLine(t *testing.T) { } for _, c := range tc { - output, err := formatStaticcheckLine(c.input) + output, err := FormatLinterLine(c.input) if c.expected == nil && output != nil { t.Errorf("expected error, got: %v", output) } else { @@ -84,7 +82,7 @@ func TestIsEmpty(t *testing.T) { {[]string{"a"}, false}, } for _, tc := range tcs { - actual := isEmpty(tc.input...) + actual := IsEmpty(tc.input...) if actual != tc.expected { t.Errorf("expected: %v, got: %v", tc.expected, actual) } diff --git a/main.go b/main.go index 84a6388b..e3bcf917 100644 --- a/main.go +++ b/main.go @@ -32,6 +32,7 @@ import ( // linters import _ "github.com/reviewbot/internal/linters/git-flow/commit-check" _ "github.com/reviewbot/internal/linters/go/staticcheck" + _ "github.com/reviewbot/internal/linters/lua/luacheck" ) type options struct {