Skip to content

Commit

Permalink
tools/linters: ignore log lines
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ellemouton committed Nov 29, 2024
1 parent 9637a81 commit 146ca93
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 9 deletions.
35 changes: 30 additions & 5 deletions tools/linters/ll.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"go/token"
"os"
"path/filepath"
"regexp"
"strings"
"unicode/utf8"

Expand All @@ -25,12 +26,14 @@ const (

defaultMaxLineLen = 80
defaultTabWidthInSpaces = 8
defaultLogRegex = `^\s*.*(L|l)og\.`
)

// LLConfig is the configuration for the ll linter.
type LLConfig struct {
LineLength int `json:"line-length"`
TabWidth int `json:"tab-width"`
LineLength int `json:"line-length"`
TabWidth int `json:"tab-width"`
LogRegex string `json:"log-regex"`
}

// New creates a new LLPlugin from the given settings. It satisfies the
Expand All @@ -48,6 +51,9 @@ func New(settings any) (register.LinterPlugin, error) {
if cfg.TabWidth == 0 {
cfg.TabWidth = defaultTabWidthInSpaces
}
if cfg.LogRegex == "" {
cfg.LogRegex = defaultLogRegex
}

return &LLPlugin{cfg: cfg}, nil
}
Expand Down Expand Up @@ -79,13 +85,16 @@ func (l *LLPlugin) GetLoadMode() string {
}

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 {
fileName := getFileName(pass, f)

issues, err := getLLLIssuesForFile(
fileName, l.cfg.LineLength, spaces,
fileName, l.cfg.LineLength, spaces, logRegex,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -114,7 +123,7 @@ type issue struct {
}

func getLLLIssuesForFile(filename string, maxLineLen int,
tabSpaces string) ([]*issue, error) {
tabSpaces string, logRegex *regexp.Regexp) ([]*issue, error) {

f, err := os.Open(filename)
if err != nil {
Expand All @@ -126,6 +135,7 @@ func getLLLIssuesForFile(filename string, maxLineLen int,
res []*issue
lineNumber int
multiImportEnabled bool
multiLinedLog bool
)

// Scan over each line.
Expand Down Expand Up @@ -167,6 +177,21 @@ func getLLLIssuesForFile(filename string, maxLineLen int,
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
// it exceeds the maximum line length.
lineLen := utf8.RuneCountInString(line)
Expand Down
38 changes: 34 additions & 4 deletions tools/linters/ll_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package linters

import (
"os"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -64,15 +65,44 @@ func TestGetLLLIssuesForFile(t *testing.T) {
{
name: "Long single line log",
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",
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 140 characters long, which " +
"exceeds the maximum of 80 characters.",
},
},
{
name: "Multi-line log",
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",
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 121 characters long, which " +
"the line is 82 characters long, which " +
"exceeds the maximum of 80 characters.",
},
},
}

tabSpaces := strings.Repeat(" ", 8)
tabSpaces := strings.Repeat(" ", defaultTabWidthInSpaces)
logRegex := regexp.MustCompile(defaultLogRegex)

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -83,7 +113,7 @@ func TestGetLLLIssuesForFile(t *testing.T) {

// Run the linter on the file.
issues, err := getLLLIssuesForFile(
tmpFile, 80, tabSpaces,
tmpFile, defaultMaxLineLen, tabSpaces, logRegex,
)
require.NoError(t, err)

Expand Down

0 comments on commit 146ca93

Please sign in to comment.