diff --git a/.custom-gcl.yml b/.custom-gcl.yml new file mode 100644 index 00000000..926af744 --- /dev/null +++ b/.custom-gcl.yml @@ -0,0 +1,4 @@ +version: v1.64.6 +plugins: + - module: 'github.com/lightninglabs/lightning-terminal/tools/linters' + path: ./tools/linters diff --git a/.golangci.yml b/.golangci.yml index 8e007198..17f66266 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -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 diff --git a/Makefile b/Makefile index 4c89d813..29cd76f9 100644 --- a/Makefile +++ b/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.") diff --git a/tools/.custom-gcl.yml b/tools/.custom-gcl.yml new file mode 100644 index 00000000..38332c00 --- /dev/null +++ b/tools/.custom-gcl.yml @@ -0,0 +1,4 @@ +version: v1.64.6 +plugins: + - module: 'github.com/lightninglabs/lightning-terminal/tools/linters' + path: ./linters diff --git a/tools/Dockerfile b/tools/Dockerfile index f4a20c3b..f5eb9043 100644 --- a/tools/Dockerfile +++ b/tools/Dockerfile @@ -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 diff --git a/tools/linters/go.mod b/tools/linters/go.mod new file mode 100644 index 00000000..e73a1150 --- /dev/null +++ b/tools/linters/go.mod @@ -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 +) diff --git a/tools/linters/go.sum b/tools/linters/go.sum new file mode 100644 index 00000000..1d2bc3d2 --- /dev/null +++ b/tools/linters/go.sum @@ -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= diff --git a/tools/linters/ll.go b/tools/linters/ll.go new file mode 100644 index 00000000..7170447d --- /dev/null +++ b/tools/linters/ll.go @@ -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) +} diff --git a/tools/linters/ll_test.go b/tools/linters/ll_test.go new file mode 100644 index 00000000..1ab4ae35 --- /dev/null +++ b/tools/linters/ll_test.go @@ -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, + ) + } + }) + } +}