tools/linters: ignore log lines

This commit adds a feature to our custom lll linter which will ignore
log lines (both single and multi-lined log lines) for the lll linter.
This commit is contained in:
Elle Mouton
2024-11-29 11:08:19 +02:00
parent 9637a8132e
commit bd55b2795b
2 changed files with 97 additions and 16 deletions

View File

@@ -12,6 +12,7 @@ import (
"go/token" "go/token"
"os" "os"
"path/filepath" "path/filepath"
"regexp"
"strings" "strings"
"unicode/utf8" "unicode/utf8"
@@ -25,12 +26,14 @@ const (
defaultMaxLineLen = 80 defaultMaxLineLen = 80
defaultTabWidthInSpaces = 8 defaultTabWidthInSpaces = 8
defaultLogRegex = `^\s*.*(L|l)og\.`
) )
// LLConfig is the configuration for the ll linter. // LLConfig is the configuration for the ll linter.
type LLConfig struct { type LLConfig struct {
LineLength int `json:"line-length"` LineLength int `json:"line-length"`
TabWidth int `json:"tab-width"` TabWidth int `json:"tab-width"`
LogRegex string `json:"log-regex"`
} }
// New creates a new LLPlugin from the given settings. It satisfies the // New creates a new LLPlugin from the given settings. It satisfies the
@@ -48,6 +51,9 @@ func New(settings any) (register.LinterPlugin, error) {
if cfg.TabWidth == 0 { if cfg.TabWidth == 0 {
cfg.TabWidth = defaultTabWidthInSpaces cfg.TabWidth = defaultTabWidthInSpaces
} }
if cfg.LogRegex == "" {
cfg.LogRegex = defaultLogRegex
}
return &LLPlugin{cfg: cfg}, nil return &LLPlugin{cfg: cfg}, nil
} }
@@ -79,13 +85,16 @@ func (l *LLPlugin) GetLoadMode() string {
} }
func (l *LLPlugin) run(pass *analysis.Pass) (any, error) { func (l *LLPlugin) run(pass *analysis.Pass) (any, error) {
var spaces = strings.Repeat(" ", l.cfg.TabWidth) var (
spaces = strings.Repeat(" ", l.cfg.TabWidth)
logRegex = regexp.MustCompile(l.cfg.LogRegex)
)
for _, f := range pass.Files { for _, f := range pass.Files {
fileName := getFileName(pass, f) fileName := getFileName(pass, f)
issues, err := getLLLIssuesForFile( issues, err := getLLLIssuesForFile(
fileName, l.cfg.LineLength, spaces, fileName, l.cfg.LineLength, spaces, logRegex,
) )
if err != nil { if err != nil {
return nil, err return nil, err
@@ -114,7 +123,7 @@ type issue struct {
} }
func getLLLIssuesForFile(filename string, maxLineLen int, func getLLLIssuesForFile(filename string, maxLineLen int,
tabSpaces string) ([]*issue, error) { tabSpaces string, logRegex *regexp.Regexp) ([]*issue, error) {
f, err := os.Open(filename) f, err := os.Open(filename)
if err != nil { if err != nil {
@@ -126,6 +135,7 @@ func getLLLIssuesForFile(filename string, maxLineLen int,
res []*issue res []*issue
lineNumber int lineNumber int
multiImportEnabled bool multiImportEnabled bool
multiLinedLog bool
) )
// Scan over each line. // Scan over each line.
@@ -167,6 +177,21 @@ func getLLLIssuesForFile(filename string, maxLineLen int,
continue continue
} }
// Check if the line matches the log pattern.
if logRegex.MatchString(line) {
multiLinedLog = !strings.HasSuffix(line, ")")
continue
}
if multiLinedLog {
// Check for the end of a multiline log call.
if strings.HasSuffix(line, ")") {
multiLinedLog = false
}
continue
}
// Otherwise, we can check the length of the line and report if // Otherwise, we can check the length of the line and report if
// it exceeds the maximum line length. // it exceeds the maximum line length.
lineLen := utf8.RuneCountInString(line) lineLen := utf8.RuneCountInString(line)

View File

@@ -2,6 +2,7 @@ package linters
import ( import (
"os" "os"
"regexp"
"strings" "strings"
"testing" "testing"
@@ -16,12 +17,14 @@ func TestGetLLLIssuesForFile(t *testing.T) {
testCases := []struct { testCases := []struct {
name string name string
content string content string
logRegex string
expectedIssue []string expectedIssue []string
}{ }{
{ {
name: "Single long line", name: "Single long line",
content: ` content: `
fmt.Println("This is a very long line that exceeds the maximum length and should be flagged by the linter.")`, fmt.Println("This is a very long line that exceeds the maximum length and should be flagged by the linter.")`,
logRegex: defaultLogRegex,
expectedIssue: []string{ expectedIssue: []string{
"the line is 140 characters long, which " + "the line is 140 characters long, which " +
"exceeds the maximum of 80 characters.", "exceeds the maximum of 80 characters.",
@@ -32,6 +35,7 @@ func TestGetLLLIssuesForFile(t *testing.T) {
content: ` content: `
fmt.Println("This is a very long line that exceeds the maximum length and should be flagged by the linter.") fmt.Println("This is a very long line that exceeds the maximum length and should be flagged by the linter.")
fmt.Println("This is a another very long line that exceeds the maximum length and should be flagged by the linter.")`, fmt.Println("This is a another very long line that exceeds the maximum length and should be flagged by the linter.")`,
logRegex: defaultLogRegex,
expectedIssue: []string{ expectedIssue: []string{
"the line is 140 characters long, which " + "the line is 140 characters long, which " +
"exceeds the maximum of 80 characters.", "exceeds the maximum of 80 characters.",
@@ -41,19 +45,23 @@ func TestGetLLLIssuesForFile(t *testing.T) {
}, },
{ {
name: "Short lines", name: "Short lines",
logRegex: defaultLogRegex,
content: ` content: `
fmt.Println("Short line")`, fmt.Println("Short line")`,
}, },
{ {
name: "Directive ignored", name: "Directive ignored",
logRegex: defaultLogRegex,
content: `//go:generate something very very very very very very very very very long and complex here wowowow`, content: `//go:generate something very very very very very very very very very long and complex here wowowow`,
}, },
{ {
name: "Long single line import", name: "Long single line import",
logRegex: defaultLogRegex,
content: `import "github.com/lightningnetwork/lnd/lnrpc/walletrpc/more/more/more/more/more/more/ok/that/is/enough"`, content: `import "github.com/lightningnetwork/lnd/lnrpc/walletrpc/more/more/more/more/more/more/ok/that/is/enough"`,
}, },
{ {
name: "Multi-line import", name: "Multi-line import",
logRegex: defaultLogRegex,
content: ` content: `
import ( import (
"os" "os"
@@ -63,19 +71,67 @@ func TestGetLLLIssuesForFile(t *testing.T) {
}, },
{ {
name: "Long single line log", name: "Long single line log",
logRegex: defaultLogRegex,
content: ` content: `
log.Infof("This is a very long log line but since it is a log line, it should be skipped by the linter.")`, log.Infof("This is a very long log line but since it is a log line, it should be skipped by the linter."),
rpcLog.Info("Another long log line with a slightly different name and should still be skipped")`,
},
{
name: "Long single line log followed by a non-log line",
logRegex: defaultLogRegex,
content: `
log.Infof("This is a very long log line but since it is a log line, it should be skipped by the linter.")
fmt.Println("This is a very long line that exceeds the maximum length and should be flagged by the linter.")`,
expectedIssue: []string{ expectedIssue: []string{
"the line is 121 characters long, which " + "the line is 140 characters long, which " +
"exceeds the maximum of 80 characters.",
},
},
{
name: "Multi-line log",
logRegex: defaultLogRegex,
content: `
log.Infof("This is a very long log line but
since it is a log line, it
should be skipped by the linter.")`,
},
{
name: "Multi-line log followed by a non-log line",
logRegex: defaultLogRegex,
content: `
log.Infof("This is a very long log line but
since it is a log line, it
should be skipped by the linter.")
fmt.Println("This is a very long line that
exceeds the maximum length and
should be flagged by the linter.")`,
expectedIssue: []string{
"the line is 82 characters long, which " +
"exceeds the maximum of 80 characters.",
},
},
{
name: "Only skip 'S' logs",
logRegex: `^\s*.*(L|l)og\.(Info|Debug|Trace|Warn|Error|Critical)S\(`,
content: `
log.Infof("A long log line but it is not an S log and so should be caught")
log.InfoS("This is a very long log line but
since it is an 'S' log line, it
should be skipped by the linter.")
log.TraceS("Another S log that should be skipped by the linter")`,
expectedIssue: []string{
"the line is 107 characters long, which " +
"exceeds the maximum of 80 characters.", "exceeds the maximum of 80 characters.",
}, },
}, },
} }
tabSpaces := strings.Repeat(" ", 8) tabSpaces := strings.Repeat(" ", defaultTabWidthInSpaces)
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
logRegex := regexp.MustCompile(tc.logRegex)
// Write content to a temporary file. // Write content to a temporary file.
tmpFile := t.TempDir() + "/test.go" tmpFile := t.TempDir() + "/test.go"
err := os.WriteFile(tmpFile, []byte(tc.content), 0644) err := os.WriteFile(tmpFile, []byte(tc.content), 0644)
@@ -83,7 +139,7 @@ func TestGetLLLIssuesForFile(t *testing.T) {
// Run the linter on the file. // Run the linter on the file.
issues, err := getLLLIssuesForFile( issues, err := getLLLIssuesForFile(
tmpFile, 80, tabSpaces, tmpFile, defaultMaxLineLen, tabSpaces, logRegex,
) )
require.NoError(t, err) require.NoError(t, err)