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 6, 2024
1 parent 5499d30 commit a3b7587
Show file tree
Hide file tree
Showing 7 changed files with 296 additions and 113 deletions.
2 changes: 1 addition & 1 deletion config/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# qbox:
# staticcheck:
# enable: false
#

# 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)
}
97 changes: 97 additions & 0 deletions internal/linters/lua/luacheck/luacheck.go
Original file line number Diff line number Diff line change
@@ -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
}
79 changes: 79 additions & 0 deletions internal/linters/lua/luacheck/luacheck_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
Loading

0 comments on commit a3b7587

Please sign in to comment.