mirror of
https://github.com/lightninglabs/lightning-terminal.git
synced 2026-08-13 12:33:36 +02:00
makefile+tools: add custom 'll' linter for extended line length checks
- Replace the default `lll` with a custom `ll` linter, enabling configurable exclusions for specific `S` log lines. - Integrate custom `ll` linter into the build system and `Makefile`. - Include relevant test cases and configuration for `golangci-lint`.
This commit is contained in:
parent
6e6b5522ff
commit
e96424cc4a
9 changed files with 474 additions and 3 deletions
4
.custom-gcl.yml
Normal file
4
.custom-gcl.yml
Normal file
|
|
@ -0,0 +1,4 @@
|
|||
version: v1.64.6
|
||||
plugins:
|
||||
- module: 'github.com/lightninglabs/lightning-terminal/tools/linters'
|
||||
path: ./tools/linters
|
||||
|
|
@ -14,6 +14,17 @@ run:
|
|||
- dev
|
||||
|
||||
linters-settings:
|
||||
custom:
|
||||
ll:
|
||||
type: "module"
|
||||
description: "Custom lll linter with 'S' log line exclusion."
|
||||
settings:
|
||||
# Max line length, lines longer will be reported.
|
||||
line-length: 80
|
||||
# Tab width in spaces.
|
||||
tab-width: 8
|
||||
# The regex that we will use to detect the start of an `S` log line.
|
||||
log-regex: "^\\s*.*(L|l)og\\.(Info|Debug|Trace|Warn|Error|Critical)S\\("
|
||||
govet:
|
||||
# Don't report about shadowed variables
|
||||
check-shadowing: false
|
||||
|
|
@ -39,7 +50,7 @@ linters-settings:
|
|||
|
||||
linters:
|
||||
enable:
|
||||
- lll
|
||||
- ll
|
||||
- gofmt
|
||||
- tagliatelle
|
||||
- whitespace
|
||||
|
|
|
|||
2
Makefile
2
Makefile
|
|
@ -304,7 +304,7 @@ check-go-version: check-go-version-dockerfile check-go-version-yaml
|
|||
|
||||
lint: check-go-version docker-tools
|
||||
@$(call print, "Linting source.")
|
||||
$(DOCKER_TOOLS) golangci-lint run -v $(LINT_WORKERS)
|
||||
$(DOCKER_TOOLS) custom-gcl run -v $(LINT_WORKERS)
|
||||
|
||||
mod:
|
||||
@$(call print, "Tidying modules.")
|
||||
|
|
|
|||
4
tools/.custom-gcl.yml
Normal file
4
tools/.custom-gcl.yml
Normal file
|
|
@ -0,0 +1,4 @@
|
|||
version: v1.64.6
|
||||
plugins:
|
||||
- module: 'github.com/lightninglabs/lightning-terminal/tools/linters'
|
||||
path: ./linters
|
||||
|
|
@ -11,7 +11,9 @@ RUN cd /tmp \
|
|||
&& mkdir -p /tmp/build/.cache \
|
||||
&& mkdir -p /tmp/build/.modcache \
|
||||
&& cd /tmp/tools \
|
||||
&& go install -trimpath github.com/golangci/golangci-lint/cmd/golangci-lint \
|
||||
&& CGO_ENABLED=0 go install -trimpath github.com/golangci/golangci-lint/cmd/golangci-lint \
|
||||
&& CGO_ENABLED=0 golangci-lint custom \
|
||||
&& mv ./custom-gcl /usr/local/bin/custom-gcl \
|
||||
&& chmod -R 777 /tmp/build/
|
||||
|
||||
WORKDIR /build
|
||||
|
|
|
|||
15
tools/linters/go.mod
Normal file
15
tools/linters/go.mod
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
module github.com/lightninglabs/lightning-terminal/tools/linters
|
||||
|
||||
go 1.24.9
|
||||
|
||||
require (
|
||||
github.com/golangci/plugin-module-register v0.1.1
|
||||
github.com/stretchr/testify v1.10.0
|
||||
golang.org/x/tools v0.30.0
|
||||
)
|
||||
|
||||
require (
|
||||
github.com/davecgh/go-spew v1.1.1 // indirect
|
||||
github.com/pmezard/go-difflib v1.0.0 // indirect
|
||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
||||
)
|
||||
14
tools/linters/go.sum
Normal file
14
tools/linters/go.sum
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
|
||||
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
|
||||
github.com/golangci/plugin-module-register v0.1.1 h1:TCmesur25LnyJkpsVrupv1Cdzo+2f7zX0H6Jkw1Ol6c=
|
||||
github.com/golangci/plugin-module-register v0.1.1/go.mod h1:TTpqoB6KkwOJMV8u7+NyXMrkwwESJLOkfl9TxR1DGFc=
|
||||
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
|
||||
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
|
||||
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
|
||||
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
|
||||
golang.org/x/tools v0.30.0 h1:BgcpHewrV5AUp2G9MebG4XPFI1E2W41zU1SaqVA9vJY=
|
||||
golang.org/x/tools v0.30.0/go.mod h1:c347cR/OJfw5TI+GfX7RUPNMdDRRbjvYTS0jPyvsVtY=
|
||||
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
|
||||
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
|
||||
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
|
||||
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
|
||||
266
tools/linters/ll.go
Normal file
266
tools/linters/ll.go
Normal file
|
|
@ -0,0 +1,266 @@
|
|||
// The following code is based on code from GolangCI.
|
||||
// Source: https://github.com/golangci-lint/pkg/golinters/lll/lll.go
|
||||
// License: GNU
|
||||
|
||||
package linters
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"errors"
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/token"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/golangci/plugin-module-register/register"
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
||||
const (
|
||||
linterName = "ll"
|
||||
goCommentDirectivePrefix = "//go:"
|
||||
|
||||
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"`
|
||||
LogRegex string `json:"log-regex"`
|
||||
}
|
||||
|
||||
// New creates a new LLPlugin from the given settings. It satisfies the
|
||||
// signature required by the golangci-lint linter for plugins.
|
||||
func New(settings any) (register.LinterPlugin, error) {
|
||||
cfg, err := register.DecodeSettings[LLConfig](settings)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Fill in default config values if they are not set.
|
||||
if cfg.LineLength == 0 {
|
||||
cfg.LineLength = defaultMaxLineLen
|
||||
}
|
||||
if cfg.TabWidth == 0 {
|
||||
cfg.TabWidth = defaultTabWidthInSpaces
|
||||
}
|
||||
if cfg.LogRegex == "" {
|
||||
cfg.LogRegex = defaultLogRegex
|
||||
}
|
||||
|
||||
return &LLPlugin{cfg: cfg}, nil
|
||||
}
|
||||
|
||||
// LLPlugin is a golangci-linter plugin that can be used to check that code line
|
||||
// lengths do not exceed a certain limit.
|
||||
type LLPlugin struct {
|
||||
cfg LLConfig
|
||||
}
|
||||
|
||||
// BuildAnalyzers creates the analyzers for the ll linter.
|
||||
//
|
||||
// NOTE: This is part of the register.LinterPlugin interface.
|
||||
func (l *LLPlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
|
||||
return []*analysis.Analyzer{
|
||||
{
|
||||
Name: linterName,
|
||||
Doc: "Reports long lines",
|
||||
Run: l.run,
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
|
||||
// GetLoadMode returns the load mode for the ll linter.
|
||||
//
|
||||
// NOTE: This is part of the register.LinterPlugin interface.
|
||||
func (l *LLPlugin) GetLoadMode() string {
|
||||
return register.LoadModeSyntax
|
||||
}
|
||||
|
||||
func (l *LLPlugin) run(pass *analysis.Pass) (any, error) {
|
||||
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, logRegex,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
file := pass.Fset.File(f.Pos())
|
||||
for _, issue := range issues {
|
||||
pos := file.LineStart(issue.pos.Line)
|
||||
|
||||
pass.Report(analysis.Diagnostic{
|
||||
Pos: pos,
|
||||
End: 0,
|
||||
Category: linterName,
|
||||
Message: issue.text,
|
||||
})
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
type issue struct {
|
||||
pos token.Position
|
||||
text string
|
||||
}
|
||||
|
||||
func getLLLIssuesForFile(filename string, maxLineLen int,
|
||||
tabSpaces string, logRegex *regexp.Regexp) ([]*issue, error) {
|
||||
|
||||
f, err := os.Open(filename)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("can't open file %s: %w", filename, err)
|
||||
}
|
||||
defer f.Close()
|
||||
|
||||
var (
|
||||
res []*issue
|
||||
lineNumber int
|
||||
multiImportEnabled bool
|
||||
multiLinedLog bool
|
||||
)
|
||||
|
||||
// Scan over each line.
|
||||
scanner := bufio.NewScanner(f)
|
||||
for scanner.Scan() {
|
||||
lineNumber++
|
||||
|
||||
// Replace all tabs with spaces.
|
||||
line := scanner.Text()
|
||||
line = strings.ReplaceAll(line, "\t", tabSpaces)
|
||||
|
||||
// Ignore any //go: directives since these cant be wrapped onto
|
||||
// a new line.
|
||||
if strings.HasPrefix(line, goCommentDirectivePrefix) {
|
||||
continue
|
||||
}
|
||||
|
||||
// We never want the linter to run on imports since these cannot
|
||||
// be wrapped onto a new line. If this is a single line import
|
||||
// we can skip the line entirely. If this is a multi-line import
|
||||
// skip until the closing bracket.
|
||||
//
|
||||
// NOTE: We trim the line space around the line here purely for
|
||||
// the purpose of being able to test this part of the linter
|
||||
// without the risk of the `gosimports` tool reformatting the
|
||||
// test case and removing the import.
|
||||
if strings.HasPrefix(strings.TrimSpace(line), "import") {
|
||||
multiImportEnabled = strings.HasSuffix(line, "(")
|
||||
continue
|
||||
}
|
||||
|
||||
// If we have marked the start of a multi-line import, we should
|
||||
// skip until the closing bracket of the import block.
|
||||
if multiImportEnabled {
|
||||
if line == ")" {
|
||||
multiImportEnabled = false
|
||||
}
|
||||
|
||||
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)
|
||||
if lineLen > maxLineLen {
|
||||
res = append(res, &issue{
|
||||
pos: token.Position{
|
||||
Filename: filename,
|
||||
Line: lineNumber,
|
||||
},
|
||||
text: fmt.Sprintf("the line is %d "+
|
||||
"characters long, which exceeds the "+
|
||||
"maximum of %d characters.", lineLen,
|
||||
maxLineLen),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
if err := scanner.Err(); err != nil {
|
||||
if errors.Is(err, bufio.ErrTooLong) &&
|
||||
maxLineLen < bufio.MaxScanTokenSize {
|
||||
|
||||
// scanner.Scan() might fail if the line is longer than
|
||||
// bufio.MaxScanTokenSize. In the case where the
|
||||
// specified maxLineLen is smaller than
|
||||
// bufio.MaxScanTokenSize we can return this line as a
|
||||
// long line instead of returning an error. The reason
|
||||
// for this change is that this case might happen with
|
||||
// autogenerated files. The go-bindata tool for instance
|
||||
// might generate a file with a very long line. In this
|
||||
// case, as it's an auto generated file, the warning
|
||||
// returned by lll will be ignored.
|
||||
// But if we return a linter error here, and this error
|
||||
// happens for an autogenerated file the error will be
|
||||
// discarded (fine), but all the subsequent errors for
|
||||
// lll will be discarded for other files, and we'll miss
|
||||
// legit error.
|
||||
res = append(res, &issue{
|
||||
pos: token.Position{
|
||||
Filename: filename,
|
||||
Line: lineNumber,
|
||||
Column: 1,
|
||||
},
|
||||
text: fmt.Sprintf("line is more than "+
|
||||
"%d characters",
|
||||
bufio.MaxScanTokenSize),
|
||||
})
|
||||
} else {
|
||||
return nil, fmt.Errorf("can't scan file %s: %w",
|
||||
filename, err)
|
||||
}
|
||||
}
|
||||
|
||||
return res, nil
|
||||
}
|
||||
|
||||
func getFileName(pass *analysis.Pass, file *ast.File) string {
|
||||
fileName := pass.Fset.PositionFor(file.Pos(), true).Filename
|
||||
ext := filepath.Ext(fileName)
|
||||
if ext != "" && ext != ".go" {
|
||||
// The position has been adjusted to a non-go file,
|
||||
// revert to original file.
|
||||
position := pass.Fset.PositionFor(file.Pos(), false)
|
||||
fileName = position.Filename
|
||||
}
|
||||
|
||||
return fileName
|
||||
}
|
||||
|
||||
func init() {
|
||||
// Register the linter with the plugin module register.
|
||||
register.Plugin(linterName, New)
|
||||
}
|
||||
155
tools/linters/ll_test.go
Normal file
155
tools/linters/ll_test.go
Normal file
|
|
@ -0,0 +1,155 @@
|
|||
package linters
|
||||
|
||||
import (
|
||||
"os"
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestGetLLLIssuesForFile tests the line-too-long linter.
|
||||
//
|
||||
//nolint:ll
|
||||
func TestGetLLLIssuesForFile(t *testing.T) {
|
||||
// Test data
|
||||
testCases := []struct {
|
||||
name string
|
||||
content string
|
||||
logRegex string
|
||||
expectedIssue []string
|
||||
}{
|
||||
{
|
||||
name: "Single long line",
|
||||
content: `
|
||||
fmt.Println("This is a very long line that exceeds the maximum length and should be flagged by the linter.")`,
|
||||
logRegex: defaultLogRegex,
|
||||
expectedIssue: []string{
|
||||
"the line is 140 characters long, which " +
|
||||
"exceeds the maximum of 80 characters.",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Multiple long lines",
|
||||
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 another very long line that exceeds the maximum length and should be flagged by the linter.")`,
|
||||
logRegex: defaultLogRegex,
|
||||
expectedIssue: []string{
|
||||
"the line is 140 characters long, which " +
|
||||
"exceeds the maximum of 80 characters.",
|
||||
"the line is 148 characters long, which " +
|
||||
"exceeds the maximum of 80 characters.",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Short lines",
|
||||
logRegex: defaultLogRegex,
|
||||
content: `
|
||||
fmt.Println("Short line")`,
|
||||
},
|
||||
{
|
||||
name: "Directive ignored",
|
||||
logRegex: defaultLogRegex,
|
||||
content: `//go:generate something very very very very very very very very very long and complex here wowowow`,
|
||||
},
|
||||
{
|
||||
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"`,
|
||||
},
|
||||
{
|
||||
name: "Multi-line import",
|
||||
logRegex: defaultLogRegex,
|
||||
content: `
|
||||
import (
|
||||
"os"
|
||||
"fmt"
|
||||
"github.com/lightningnetwork/lnd/lnrpc/walletrpc/more/ok/that/is/enough"
|
||||
)`,
|
||||
},
|
||||
{
|
||||
name: "Long single 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."),
|
||||
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{
|
||||
"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.",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
tabSpaces := strings.Repeat(" ", defaultTabWidthInSpaces)
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
logRegex := regexp.MustCompile(tc.logRegex)
|
||||
|
||||
// Write content to a temporary file.
|
||||
tmpFile := t.TempDir() + "/test.go"
|
||||
err := os.WriteFile(tmpFile, []byte(tc.content), 0644)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Run the linter on the file.
|
||||
issues, err := getLLLIssuesForFile(
|
||||
tmpFile, defaultMaxLineLen, tabSpaces, logRegex,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Len(t, issues, len(tc.expectedIssue))
|
||||
|
||||
for i, issue := range issues {
|
||||
require.Equal(
|
||||
t, tc.expectedIssue[i], issue.text,
|
||||
)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue