Skip to content

Commit

Permalink
fix(luacheck) filter out unnecessary lines
Browse files Browse the repository at this point in the history
  • Loading branch information
xwen-winnie committed Jun 21, 2024
1 parent c4ca1ce commit c55f903
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 13 deletions.
43 changes: 31 additions & 12 deletions internal/linters/lua/luacheck/luacheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,43 @@ func luacheckHandler(log *xlog.Logger, a linters.Agent) error {
if linters.IsEmpty(a.LinterConfig.Args...) {
// identify global variables for Redis and Nginx modules.
// disable the maximum line length check, which is no need.
a.LinterConfig.Args = append([]string{}, ".", "--globals='ngx KEYS ARGV table redis cjson'", "--no-max-line-length")
// luacheck execute the command "--globals='ngx KEYS ARGV table redis cjson'", which does not take effect. So the parameter should be changed to an array-based approach.
cmdArgs := []string{
".",
"--globals=ngx",
"--globals=KEYS",
"--globals=ARGV",
"--globals=table",
"--globals=redis",
"--globals=cjson",
"--no-max-line-length",
"--no-color",
}
a.LinterConfig.Args = append([]string{}, cmdArgs...)
}

return linters.GeneralHandler(log, a, func(_ *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) {
return linters.Parse(log, output, luacheckParser)
return linters.Parse(log, output, luacheckLineParser)
})
}

func luacheckParser(line string) (*linters.LinterOutput, error) {
lineResult, err := linters.GeneralLineParser(line)
if err != nil {
return nil, err
func luacheckLineParser(line string) (*linters.LinterOutput, error) {
filteredLine := luacheckLineProcess(line)
if filteredLine != "" {
return linters.GeneralLineParser(filteredLine)
}
return nil, nil

Check warning on line 47 in internal/linters/lua/luacheck/luacheck.go

View check run for this annotation

qiniu-x / golangci-lint

internal/linters/lua/luacheck/luacheck.go#L47

return both the `nil` error and invalid value: use a sentinel error instead (nilnil)
}

func luacheckLineProcess(line string) string {
// Luacheck will output lines starting with 'Total ' or 'Checking ', which are no meaningful for the reviewbot scenario, so we discard them
// Such as:
// 1. Total: 0 warnings / 0 errors in 0 files
// 2. Checking cmd/jarviswsserver/etc/get_node_wsserver.lua 11 warnings
if strings.HasPrefix(line, "Total: ") || strings.HasPrefix(line, "Checking ") {
return ""
}
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
// Luacheck will output lines starting with unrecognized characters, so we discard them
//re := regexp.MustCompile(`\x1b\[0m\x1b\[1m|\x1b\[0m`)

Check warning on line 59 in internal/linters/lua/luacheck/luacheck.go

View check run for this annotation

qiniu-x / golangci-lint

internal/linters/lua/luacheck/luacheck.go#L59

commentFormatting: put a space between `//` and comment text (gocritic)
return strings.TrimLeft(line, " ")
}
2 changes: 1 addition & 1 deletion internal/linters/lua/luacheck/luacheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestFormatLuaCheckLine(t *testing.T) {
}

for _, c := range tc {
output, err := luacheckParser(c.input)
output, err := luacheckLineParser(c.input)
if output == nil {
if c.expected != nil {
t.Errorf("expected: %v, got: %v", c.expected, output)
Expand Down

0 comments on commit c55f903

Please sign in to comment.