Skip to content

Commit

Permalink
feat(luacheck) new lint
Browse files Browse the repository at this point in the history
  • Loading branch information
xwen-winnie committed Feb 2, 2024
1 parent 5499d30 commit 8606d2c
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 115 deletions.
8 changes: 5 additions & 3 deletions config/config.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# This is the default config file, you can override it by creating a config.yaml in the same directory
# by default, all linters are enabled if you don't specify. You can disable them by setting enable to false
# example1: disable staticcheck for org
# qbox:
#qbox:
# staticcheck:
# enable: false
#
# enable: true
# luacheck:
# enable: true

# example2: disable staticcheck for repo
# qbox/kodo:
# staticcheck:
Expand Down
107 changes: 4 additions & 103 deletions internal/linters/go/staticcheck/staticcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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", "./...")
}
Expand All @@ -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.
Expand Down Expand Up @@ -115,96 +99,13 @@ 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)
// gslb-api/utils.go:162:2: this value of err is never used (SA4006)
// 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)
}
107 changes: 107 additions & 0 deletions internal/linters/linters_util.go
Original file line number Diff line number Diff line change
@@ -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))
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
Loading

0 comments on commit 8606d2c

Please sign in to comment.