From c74c76d06217875f88a9df58eca3eaa79070e8e3 Mon Sep 17 00:00:00 2001 From: Lee ByeongJun Date: Tue, 8 Oct 2024 15:40:57 +0900 Subject: [PATCH] refactor --- formatter/builder.go | 93 +++++----- formatter/doc.go | 2 - formatter/formatter_test.go | 16 +- internal/checker/deprecate.go | 175 +++++++++++-------- internal/checker/deprecate_test.go | 26 ++- internal/engine.go | 7 +- internal/lints/deprecate_func.go | 58 +++++- internal/lints/format_emit.go | 10 +- internal/lints/lint_test.go | 16 -- internal/lints/missing_package_mod.go | 14 +- internal/lints/regex_lint_test.go | 8 +- internal/lints/repeated_regex_compilation.go | 10 +- internal/{ => nolint}/nolint.go | 42 ++--- internal/{ => nolint}/nolint_test.go | 8 +- internal/rule_set.go | 2 +- testdata/demo.gno | 22 --- 16 files changed, 293 insertions(+), 216 deletions(-) rename internal/{ => nolint}/nolint.go (80%) rename internal/{ => nolint}/nolint_test.go (94%) delete mode 100644 testdata/demo.gno diff --git a/formatter/builder.go b/formatter/builder.go index ec4d25c..182f04e 100644 --- a/formatter/builder.go +++ b/formatter/builder.go @@ -86,15 +86,37 @@ func getFormatter(rule string) IssueFormatter { /***** Issue Formatter Builder *****/ type IssueFormatterBuilder struct { - result strings.Builder - issue tt.Issue - snippet *internal.SourceCode + result strings.Builder + issue tt.Issue + snippet *internal.SourceCode + startLine int + endLine int + maxLineNumWidth int + padding string + commonIndent string } func NewIssueFormatterBuilder(issue tt.Issue, snippet *internal.SourceCode) *IssueFormatterBuilder { + startLine := issue.Start.Line + endLine := issue.End.Line + maxLineNumWidth := calculateMaxLineNumWidth(endLine) + padding := strings.Repeat(" ", maxLineNumWidth+1) + + var commonIndent string + if startLine-1 < 0 || endLine > len(snippet.Lines) || startLine > endLine { + commonIndent = "" + } else { + commonIndent = findCommonIndent(snippet.Lines[startLine-1 : endLine]) + } + return &IssueFormatterBuilder{ - issue: issue, - snippet: snippet, + issue: issue, + snippet: snippet, + startLine: startLine, + endLine: endLine, + maxLineNumWidth: maxLineNumWidth, + padding: padding, + commonIndent: commonIndent, } } @@ -118,11 +140,8 @@ func (b *IssueFormatterBuilder) AddHeader(kind headerType) *IssueFormatterBuilde b.result.WriteString(ruleStyle.Sprintln(b.issue.Rule)) - endLine := b.issue.End.Line - maxLineNumWidth := calculateMaxLineNumWidth(endLine) - padding := strings.Repeat(" ", maxLineNumWidth) - // add file name + padding := strings.Repeat(" ", b.maxLineNumWidth) b.result.WriteString(lineStyle.Sprint(fmt.Sprintf("%s--> ", padding))) b.result.WriteString(fileStyle.Sprintln(b.issue.Filename)) @@ -130,29 +149,17 @@ func (b *IssueFormatterBuilder) AddHeader(kind headerType) *IssueFormatterBuilde } func (b *IssueFormatterBuilder) AddCodeSnippet() *IssueFormatterBuilder { - startLine := b.issue.Start.Line - endLine := b.issue.End.Line - maxLineNumWidth := calculateMaxLineNumWidth(endLine) - - var commonIndent string - if startLine-1 < 0 || endLine > len(b.snippet.Lines) || startLine > endLine { - commonIndent = "" - } else { - commonIndent = findCommonIndent(b.snippet.Lines[startLine-1 : endLine]) - } - // add separator - padding := strings.Repeat(" ", maxLineNumWidth+1) - b.result.WriteString(lineStyle.Sprintf("%s|\n", padding)) + b.result.WriteString(lineStyle.Sprintf("%s|\n", b.padding)) - for i := startLine; i <= endLine; i++ { + for i := b.startLine; i <= b.endLine; i++ { if i-1 < 0 || i-1 >= len(b.snippet.Lines) { continue } line := b.snippet.Lines[i-1] - line = strings.TrimPrefix(line, commonIndent) - lineNum := fmt.Sprintf("%*d", maxLineNumWidth, i) + line = strings.TrimPrefix(line, b.commonIndent) + lineNum := fmt.Sprintf("%*d", b.maxLineNumWidth, i) b.result.WriteString(lineStyle.Sprintf("%s | ", lineNum)) b.result.WriteString(line + "\n") @@ -162,35 +169,29 @@ func (b *IssueFormatterBuilder) AddCodeSnippet() *IssueFormatterBuilder { } func (b *IssueFormatterBuilder) AddUnderlineAndMessage() *IssueFormatterBuilder { - startLine := b.issue.Start.Line - endLine := b.issue.End.Line - maxLineNumWidth := calculateMaxLineNumWidth(endLine) - padding := strings.Repeat(" ", maxLineNumWidth+1) - - b.result.WriteString(lineStyle.Sprintf("%s| ", padding)) + b.result.WriteString(lineStyle.Sprintf("%s| ", b.padding)) - if startLine <= 0 || startLine > len(b.snippet.Lines) || endLine <= 0 || endLine > len(b.snippet.Lines) || startLine > endLine { + if !b.isValidLineRange() { b.result.WriteString(messageStyle.Sprintf("%s\n\n", b.issue.Message)) return b } - commonIndent := findCommonIndent(b.snippet.Lines[startLine-1 : endLine]) - commonIndentWidth := calculateVisualColumn(commonIndent, len(commonIndent)+1) + commonIndentWidth := calculateVisualColumn(b.commonIndent, len(b.commonIndent)+1) // calculate underline start position - underlineStart := calculateVisualColumn(b.snippet.Lines[startLine-1], b.issue.Start.Column) - commonIndentWidth + underlineStart := calculateVisualColumn(b.snippet.Lines[b.startLine-1], b.issue.Start.Column) - commonIndentWidth if underlineStart < 0 { underlineStart = 0 } // calculate underline end position - underlineEnd := calculateVisualColumn(b.snippet.Lines[endLine-1], b.issue.End.Column) - commonIndentWidth + underlineEnd := calculateVisualColumn(b.snippet.Lines[b.endLine-1], b.issue.End.Column) - commonIndentWidth underlineLength := underlineEnd - underlineStart + 1 b.result.WriteString(strings.Repeat(" ", underlineStart)) b.result.WriteString(messageStyle.Sprintf("%s\n", strings.Repeat("~", underlineLength))) - b.result.WriteString(lineStyle.Sprintf("%s| ", padding)) + b.result.WriteString(lineStyle.Sprintf("%s= ", b.padding)) b.result.WriteString(messageStyle.Sprintf("%s\n\n", b.issue.Message)) return b @@ -208,20 +209,17 @@ func (b *IssueFormatterBuilder) AddSuggestion() *IssueFormatterBuilder { return b } - maxLineNumWidth := calculateMaxLineNumWidth(b.issue.End.Line) - padding := strings.Repeat(" ", maxLineNumWidth+1) - b.result.WriteString(suggestionStyle.Sprint("Suggestion:\n")) - b.result.WriteString(lineStyle.Sprintf("%s|\n", padding)) + b.result.WriteString(lineStyle.Sprintf("%s|\n", b.padding)) suggestionLines := strings.Split(b.issue.Suggestion, "\n") for i, line := range suggestionLines { - lineNum := fmt.Sprintf("%*d", maxLineNumWidth, b.issue.Start.Line+i) + lineNum := fmt.Sprintf("%*d", b.maxLineNumWidth, b.issue.Start.Line+i) b.result.WriteString(lineStyle.Sprintf("%s | ", lineNum)) b.result.WriteString(line + "\n") } - b.result.WriteString(lineStyle.Sprintf("%s|\n", padding)) + b.result.WriteString(lineStyle.Sprintf("%s|\n", b.padding)) b.result.WriteString("\n") return b @@ -245,6 +243,14 @@ func (b *IssueFormatterBuilder) Build() string { return b.result.String() } +func (b *IssueFormatterBuilder) isValidLineRange() bool { + return b.startLine > 0 && + b.endLine > 0 && + b.startLine <= b.endLine && + b.startLine <= len(b.snippet.Lines) && + b.endLine <= len(b.snippet.Lines) +} + func calculateMaxLineNumWidth(endLine int) int { return len(fmt.Sprintf("%d", endLine)) } @@ -278,7 +284,6 @@ func findCommonIndent(lines []string) string { // find first non-empty line's indent var firstIndent []rune for _, line := range lines { - // trimmed := strings.TrimSpace(line) trimmed := strings.TrimLeftFunc(line, unicode.IsSpace) if trimmed != "" { firstIndent = []rune(line[:len(line)-len(trimmed)]) diff --git a/formatter/doc.go b/formatter/doc.go index f41c657..f451866 100644 --- a/formatter/doc.go +++ b/formatter/doc.go @@ -2,5 +2,3 @@ // in a human-readable format. It includes various formatters for different // types of issues and utility functions for text manipulation. package formatter - -type TemplateFormatter struct{} diff --git a/formatter/formatter_test.go b/formatter/formatter_test.go index ec3c046..af6d9d9 100644 --- a/formatter/formatter_test.go +++ b/formatter/formatter_test.go @@ -44,14 +44,14 @@ func TestFormatIssuesWithArrows(t *testing.T) { | 4 | x := 1 | ~~ - | x declared but not used + = x declared but not used error: empty-if --> test.go | 5 | if true {} | ~~~~~~~~~ - | empty branch + = empty branch ` @@ -76,14 +76,14 @@ error: empty-if | 4 | x := 1 | ~~ - | x declared but not used + = x declared but not used error: empty-if --> test.go | 5 | if true {} | ~~~~~~~~~ - | empty branch + = empty branch ` @@ -138,21 +138,21 @@ func TestFormatIssuesWithArrows_MultipleDigitsLineNumbers(t *testing.T) { | 4 | x := 1 // unused variable | ~~ - | x declared but not used + = x declared but not used error: empty-if --> test.go | 5 | if true {} // empty if statement | ~~~~~~~~~ - | empty branch + = empty branch error: example --> test.go | 10 | println("end") | ~~~~~~~~ - | example issue + = example issue ` @@ -246,7 +246,7 @@ func TestUnnecessaryTypeConversionFormatter(t *testing.T) { | 5 | result := int(myInt) | ~~~~~~~~~~~ - | unnecessary type conversion + = unnecessary type conversion Suggestion: | diff --git a/internal/checker/deprecate.go b/internal/checker/deprecate.go index 06c8600..6681598 100644 --- a/internal/checker/deprecate.go +++ b/internal/checker/deprecate.go @@ -1,34 +1,49 @@ package checker import ( + "fmt" "go/ast" "go/token" "strconv" "strings" ) -// pkgPath -> funcName -> alternative -type deprecatedFuncMap map[string]map[string]string +// PkgFuncMap maps package paths to function names and their alternatives +type PkgFuncMap map[string]map[string]string -// DeprecatedFunc represents a deprecated function. +// DeprecatedFunc represents a deprecated function type DeprecatedFunc struct { Package string Function string Alternative string - Position token.Position + Start token.Position + End token.Position } -// DeprecatedFuncChecker checks for deprecated functions. +// DeprecatedFuncChecker checks for deprecated functions type DeprecatedFuncChecker struct { - deprecatedFuncs deprecatedFuncMap + deprecatedFuncs PkgFuncMap } +// NewDeprecatedFuncChecker creates a new DeprecatedFuncChecker func NewDeprecatedFuncChecker() *DeprecatedFuncChecker { return &DeprecatedFuncChecker{ - deprecatedFuncs: make(deprecatedFuncMap), + deprecatedFuncs: make(PkgFuncMap), } } +// Register adds a deprecated function to the checker +// +// @notJoon [10/08/2024]: The deprecated functions are currently beign updated manually +// in the [internal/lints/deprecate_func.go] file. We could consider automatically recognizing +// and updating the map if the comments include the string `Deprecated:` in accordance with +// the `godoc` style. +// +// However, this approach would require additional file traversal, which I believe +// would be mostly unnecessary computation and may not be worth the effort. +// +// Therefore, currently, it seems more efficient to manually update the deprecated functions +// to only handle the deprecated items in gno's standard library. func (d *DeprecatedFuncChecker) Register(pkgName, funcName, alternative string) { if _, ok := d.deprecatedFuncs[pkgName]; !ok { d.deprecatedFuncs[pkgName] = make(map[string]string) @@ -36,86 +51,98 @@ func (d *DeprecatedFuncChecker) Register(pkgName, funcName, alternative string) d.deprecatedFuncs[pkgName][funcName] = alternative } -// Check checks a AST node for deprecated functions. -// -// TODO: use this in the linter rule implementation -func (d *DeprecatedFuncChecker) Check( - filename string, - node *ast.File, - fset *token.FileSet, -) ([]DeprecatedFunc, error) { +// Check checks an AST node for deprecated functions +func (d *DeprecatedFuncChecker) Check(filename string, node *ast.File, fset *token.FileSet) ([]DeprecatedFunc, error) { + packageAliases, err := d.getPackageAliases(node) + if err != nil { + return nil, fmt.Errorf("error getting package aliases: %w", err) + } + var found []DeprecatedFunc + ast.Inspect(node, func(n ast.Node) bool { + if call, ok := n.(*ast.CallExpr); ok { + if deprecatedFunc := d.checkCall(call, packageAliases, fset); deprecatedFunc != nil { + found = append(found, *deprecatedFunc) + } + } + return true + }) + return found, nil +} + +func (d *DeprecatedFuncChecker) getPackageAliases(node *ast.File) (map[string]string, error) { packageAliases := make(map[string]string) for _, imp := range node.Imports { path, err := strconv.Unquote(imp.Path.Value) if err != nil { - continue - } - name := "" - if imp.Name != nil { - name = imp.Name.Name - } else { - parts := strings.Split(path, "/") - name = parts[len(parts)-1] + return nil, fmt.Errorf("error unquoting import path: %w", err) } + name := d.getImportName(imp, path) packageAliases[name] = path } + return packageAliases, nil +} - ast.Inspect(node, func(n ast.Node) bool { - call, ok := n.(*ast.CallExpr) - if !ok { - return true - } +func (d *DeprecatedFuncChecker) getImportName(imp *ast.ImportSpec, path string) string { + if imp.Name != nil { + return imp.Name.Name + } + parts := strings.Split(path, "/") + return parts[len(parts)-1] +} - switch fun := call.Fun.(type) { - case *ast.SelectorExpr: - ident, ok := fun.X.(*ast.Ident) - if !ok { - return true - } - pkgAlias := ident.Name - funcName := fun.Sel.Name +func (d *DeprecatedFuncChecker) checkCall(call *ast.CallExpr, packageAliases map[string]string, fset *token.FileSet) *DeprecatedFunc { + switch fun := call.Fun.(type) { + case *ast.SelectorExpr: + return d.checkSelectorExpr(fun, packageAliases, fset, call) + case *ast.Ident: + return d.checkIdent(fun, packageAliases, fset, call) + } + return nil +} - pkgPath, ok := packageAliases[pkgAlias] - if !ok { - // Not a package alias, possibly a method call - return true - } +func (d *DeprecatedFuncChecker) checkSelectorExpr(fun *ast.SelectorExpr, packageAliases map[string]string, fset *token.FileSet, call *ast.CallExpr) *DeprecatedFunc { + ident, ok := fun.X.(*ast.Ident) + if !ok { + return nil + } + pkgAlias := ident.Name + funcName := fun.Sel.Name - if funcs, ok := d.deprecatedFuncs[pkgPath]; ok { - if alt, ok := funcs[funcName]; ok { - found = append(found, DeprecatedFunc{ - Package: pkgPath, - Function: funcName, - Alternative: alt, - Position: fset.Position(call.Pos()), - }) - } - } - case *ast.Ident: - // Handle functions imported via dot imports - funcName := fun.Name - // Check dot-imported packages - for alias, pkgPath := range packageAliases { - if alias != "." { - continue - } - if funcs, ok := d.deprecatedFuncs[pkgPath]; ok { - if alt, ok := funcs[funcName]; ok { - found = append(found, DeprecatedFunc{ - Package: pkgPath, - Function: funcName, - Alternative: alt, - Position: fset.Position(call.Pos()), - }) - break - } - } - } + pkgPath, ok := packageAliases[pkgAlias] + if !ok { + return nil // Not a package alias, possibly a method call + } + + return d.createDeprecatedFuncIfFound(pkgPath, funcName, fset, call) +} + +func (d *DeprecatedFuncChecker) checkIdent(fun *ast.Ident, packageAliases map[string]string, fset *token.FileSet, call *ast.CallExpr) *DeprecatedFunc { + funcName := fun.Name + // Check dot-imported packages + for alias, pkgPath := range packageAliases { + if alias != "." { + continue } - return true - }) + if deprecatedFunc := d.createDeprecatedFuncIfFound(pkgPath, funcName, fset, call); deprecatedFunc != nil { + return deprecatedFunc + } + } + return nil +} - return found, nil +func (d *DeprecatedFuncChecker) createDeprecatedFuncIfFound(pkgPath, funcName string, fset *token.FileSet, call *ast.CallExpr) *DeprecatedFunc { + if funcs, ok := d.deprecatedFuncs[pkgPath]; ok { + if alt, ok := funcs[funcName]; ok { + return &DeprecatedFunc{ + Package: pkgPath, + Function: funcName, + Alternative: alt, + Start: fset.Position(call.Pos()), + End: fset.Position(call.End()), + } + } + } + return nil } diff --git a/internal/checker/deprecate_test.go b/internal/checker/deprecate_test.go index bc1a491..27241fd 100644 --- a/internal/checker/deprecate_test.go +++ b/internal/checker/deprecate_test.go @@ -15,7 +15,7 @@ func TestRegisterDeprecatedFunctions(t *testing.T) { checker.Register("fmt", "Println", "fmt.Print") checker.Register("os", "Remove", "os.RemoveAll") - expected := deprecatedFuncMap{ + expected := PkgFuncMap{ "fmt": {"Println": "fmt.Print"}, "os": {"Remove": "os.RemoveAll"}, } @@ -59,23 +59,35 @@ func main() { Package: "fmt", Function: "Println", Alternative: "fmt.Print", - Position: token.Position{ + Start: token.Position{ Filename: "example.go", Offset: 55, Line: 10, Column: 2, }, + End: token.Position{ + Filename: "example.go", + Offset: 83, + Line: 10, + Column: 30, + }, }, { Package: "os", Function: "Remove", Alternative: "os.RemoveAll", - Position: token.Position{ + Start: token.Position{ Filename: "example.go", Offset: 85, Line: 11, Column: 2, }, + End: token.Position{ + Filename: "example.go", + Offset: 111, + Line: 11, + Column: 28, + }, }, } @@ -243,8 +255,8 @@ func assertDeprecatedFuncEqual(t *testing.T, expected, actual DeprecatedFunc) { assert.Equal(t, expected.Package, actual.Package) assert.Equal(t, expected.Function, actual.Function) assert.Equal(t, expected.Alternative, actual.Alternative) - assert.NotEmpty(t, actual.Position.Filename) - assert.Greater(t, actual.Position.Offset, 0) - assert.Greater(t, actual.Position.Line, 0) - assert.Greater(t, actual.Position.Column, 0) + assert.NotEmpty(t, actual.Start.Filename) + assert.Greater(t, actual.Start.Offset, 0) + assert.Greater(t, actual.Start.Line, 0) + assert.Greater(t, actual.Start.Column, 0) } diff --git a/internal/engine.go b/internal/engine.go index aab91ba..5d802d7 100644 --- a/internal/engine.go +++ b/internal/engine.go @@ -9,6 +9,7 @@ import ( "sync" "github.com/gnolang/tlin/internal/lints" + "github.com/gnolang/tlin/internal/nolint" tt "github.com/gnolang/tlin/internal/types" ) @@ -18,7 +19,7 @@ type Engine struct { rules []LintRule ignoredRules map[string]bool defaultRules []LintRule - nolintMgr *nolintManager + nolintMgr *nolint.Manager } // NewEngine creates a new lint engine. @@ -75,7 +76,7 @@ func (e *Engine) Run(filename string) ([]tt.Issue, error) { return nil, fmt.Errorf("error parsing file: %w", err) } - e.nolintMgr = ParseNolintComments(node, fset) + e.nolintMgr = nolint.ParseComments(node, fset) var wg sync.WaitGroup var mu sync.Mutex @@ -121,7 +122,7 @@ func (e *Engine) RunSource(source []byte) ([]tt.Issue, error) { return nil, fmt.Errorf("error parsing content: %w", err) } - e.nolintMgr = ParseNolintComments(node, fset) + e.nolintMgr = nolint.ParseComments(node, fset) var wg sync.WaitGroup var mu sync.Mutex diff --git a/internal/lints/deprecate_func.go b/internal/lints/deprecate_func.go index 058b7e5..c1814c9 100644 --- a/internal/lints/deprecate_func.go +++ b/internal/lints/deprecate_func.go @@ -9,16 +9,39 @@ import ( tt "github.com/gnolang/tlin/internal/types" ) +func register() *checker.DeprecatedFuncChecker { + deprecated := checker.NewDeprecatedFuncChecker() + + deprecated.Register("std", "SetOrigCaller", "std.PrevRealm") + deprecated.Register("std", "GetOrigCaller", "std.PrevRealm") + deprecated.Register("std", "TestSetOrigCaller", "") + + return deprecated +} + func DetectDeprecatedFunctions( filename string, node *ast.File, fset *token.FileSet, ) ([]tt.Issue, error) { - deprecated := checker.NewDeprecatedFuncChecker() + deprecated := register() - deprecated.Register("std", "SetOrigCaller", "std.PrevRealm") - deprecated.Register("std", "GetOrigCaller", "std.PrevRealm") - deprecated.Register("std", "TestSetOrigCaller", "") + imports := extractDeprecatedImports(node) + if len(imports) == 0 { + return nil, nil + } + + hasDeprecatedPackage := false + for imp := range imports { + if deprecatedPackages[imp] { + hasDeprecatedPackage = true + break + } + } + + if !hasDeprecatedPackage { + return nil, nil + } dfuncs, err := deprecated.Check(filename, node, fset) if err != nil { @@ -30,8 +53,8 @@ func DetectDeprecatedFunctions( issues = append(issues, tt.Issue{ Rule: "deprecated", Filename: filename, - Start: df.Position, - End: df.Position, + Start: df.Start, + End: df.End, Message: createDeprecationMessage(df), Suggestion: df.Alternative, }) @@ -49,3 +72,26 @@ func createDeprecationMessage(df checker.DeprecatedFunc) string { msg = fmt.Sprintf("%s. Please remove it.", msg) return msg } + +type pkgContainsDeprecatedMap map[string]bool + +var deprecatedPackages = pkgContainsDeprecatedMap{ + "std": true, +} + +func extractDeprecatedImports(node *ast.File) pkgContainsDeprecatedMap { + return extractImports(node, func(path string) bool { + return true + }) +} + +func extractImports[T any](node *ast.File, valueFunc func(string) T) map[string]T { + imports := make(map[string]T) + + for _, imp := range node.Imports { + path := imp.Path.Value[1 : len(imp.Path.Value)-1] + imports[path] = valueFunc(path) + } + + return imports +} diff --git a/internal/lints/format_emit.go b/internal/lints/format_emit.go index 754b8b5..4089eba 100644 --- a/internal/lints/format_emit.go +++ b/internal/lints/format_emit.go @@ -9,7 +9,15 @@ import ( ) func DetectEmitFormat(filename string, node *ast.File, fset *token.FileSet) ([]tt.Issue, error) { - var issues []tt.Issue + imports := extractImports(node, func(path string) bool { + return path == "std" + }) + + if !imports["std"] { + return nil, nil + } + + issues := make([]tt.Issue, 0) ast.Inspect(node, func(n ast.Node) bool { call, ok := n.(*ast.CallExpr) if !ok { diff --git a/internal/lints/lint_test.go b/internal/lints/lint_test.go index 5c9bc40..a6322db 100644 --- a/internal/lints/lint_test.go +++ b/internal/lints/lint_test.go @@ -253,22 +253,6 @@ func main() { }`, expected: 2, }, - // { - // name: "Variable allocation in loop", - // // ref: https://stackoverflow.com/questions/77180437/understanding-short-variable-declaration-in-loop-resulting-unnecessary-memory-a - // code: ` - // package main - - // import "fmt" - - // func main() { - // for i:=0; i<10; i++ { - // a:=i+1 // BAD!: allocates memory in every iteration - // fmt.Printf("i-val: %d, i-addr: %p, a-val: %d, a-addr: %p\n", i, &i, a, &a) - // } - // }`, - // expected: 1, - // }, { name: "No allocation in loop", code: ` diff --git a/internal/lints/missing_package_mod.go b/internal/lints/missing_package_mod.go index 7dddff1..50a64d3 100644 --- a/internal/lints/missing_package_mod.go +++ b/internal/lints/missing_package_mod.go @@ -7,6 +7,7 @@ import ( "go/token" "os" "path/filepath" + "strconv" "strings" tt "github.com/gnolang/tlin/internal/types" @@ -16,9 +17,9 @@ func DetectMissingModPackage(filename string, node *ast.File, fset *token.FileSe dir := filepath.Dir(filename) modFile := filepath.Join(dir, "gno.mod") - requiredPackages, err := extractImports(node) + requiredPackages, err := extractGnoImports(node) if err != nil { - return nil, fmt.Errorf("failed to extract imports: %w", err) + return nil, fmt.Errorf("failed to extract gno imports: %w", err) } declaredPackages, err := extractDeclaredPackages(modFile) @@ -26,7 +27,7 @@ func DetectMissingModPackage(filename string, node *ast.File, fset *token.FileSe return nil, fmt.Errorf("failed to extract declared packages: %w", err) } - var issues []tt.Issue + issues := make([]tt.Issue, 0) var unusedPackages []string for pkg := range declaredPackages { @@ -62,11 +63,14 @@ func DetectMissingModPackage(filename string, node *ast.File, fset *token.FileSe return issues, nil } -func extractImports(node *ast.File) (map[string]bool, error) { +func extractGnoImports(node *ast.File) (map[string]bool, error) { imports := make(map[string]bool) for _, imp := range node.Imports { if imp.Path != nil { - path := strings.Trim(imp.Path.Value, "\"") + path, err := strconv.Unquote(imp.Path.Value) + if err != nil { + return nil, fmt.Errorf("failed to unquote import path: %w", err) + } if strings.HasPrefix(path, "gno.land/p/") || strings.HasPrefix(path, "gno.land/r/") { imports[path] = true } diff --git a/internal/lints/regex_lint_test.go b/internal/lints/regex_lint_test.go index ad77561..aa7d088 100644 --- a/internal/lints/regex_lint_test.go +++ b/internal/lints/regex_lint_test.go @@ -1,6 +1,8 @@ package lints import ( + "go/parser" + "go/token" "os" "path/filepath" "testing" @@ -79,7 +81,11 @@ func multipleRepeats() { err = os.WriteFile(tempFile, []byte(tt.code), 0o644) require.NoError(t, err) - issues, err := DetectRepeatedRegexCompilation(tempFile) + fset := token.NewFileSet() + node, err := parser.ParseFile(fset, tempFile, nil, parser.ParseComments) + require.NoError(t, err) + + issues, err := DetectRepeatedRegexCompilation(tempFile, node) require.NoError(t, err) assert.Len(t, issues, tt.expected) diff --git a/internal/lints/repeated_regex_compilation.go b/internal/lints/repeated_regex_compilation.go index 96cd1e9..b33afc6 100644 --- a/internal/lints/repeated_regex_compilation.go +++ b/internal/lints/repeated_regex_compilation.go @@ -15,7 +15,15 @@ var RepeatedRegexCompilationAnalyzer = &analysis.Analyzer{ Run: runRepeatedRegexCompilation, } -func DetectRepeatedRegexCompilation(filename string) ([]tt.Issue, error) { +func DetectRepeatedRegexCompilation(filename string, node *ast.File) ([]tt.Issue, error) { + imports := extractImports(node, func(path string) bool { + return path == "regexp" + }) + + if !imports["regexp"] { + return nil, nil + } + issues, err := runAnalyzer(filename, RepeatedRegexCompilationAnalyzer) if err != nil { return nil, err diff --git a/internal/nolint.go b/internal/nolint/nolint.go similarity index 80% rename from internal/nolint.go rename to internal/nolint/nolint.go index 540e965..c862172 100644 --- a/internal/nolint.go +++ b/internal/nolint/nolint.go @@ -1,4 +1,4 @@ -package internal +package nolint import ( "fmt" @@ -9,29 +9,29 @@ import ( const nolintPrefix = "//nolint" -// nolintScope represents a range in the code where nolint applies. -type nolintScope struct { +// Manager manages nolint scopes and checks if a position is nolinted. +type Manager struct { + scopes map[string][]scope // filename to scopes +} + +// scope represents a range in the code where nolint applies. +type scope struct { start token.Position end token.Position rules map[string]struct{} // empty, null => apply to all lint rules } -// nolintManager manages nolint scopes and checks if a position is nolinted. -type nolintManager struct { - scopes map[string][]nolintScope // filename to scopes -} - -// ParseNolintComments parses nolint comments in the given AST file and returns a nolintManager. -func ParseNolintComments(f *ast.File, fset *token.FileSet) *nolintManager { - manager := nolintManager{ - scopes: make(map[string][]nolintScope, len(f.Comments)), +// ParseComments parses nolint comments in the given AST file and returns a nolintManager. +func ParseComments(f *ast.File, fset *token.FileSet) *Manager { + manager := Manager{ + scopes: make(map[string][]scope, len(f.Comments)), } stmtMap := indexStatementsByLine(f, fset) packageLine := fset.Position(f.Package).Line for _, cg := range f.Comments { for _, comment := range cg.List { - scope, err := parseNolintComment(comment, f, fset, stmtMap, packageLine) + scope, err := parseComment(comment, f, fset, stmtMap, packageLine) if err != nil { // ignore invalid nolint comments continue @@ -43,15 +43,15 @@ func ParseNolintComments(f *ast.File, fset *token.FileSet) *nolintManager { return &manager } -// parseNolintComment parses a single nolint comment and determines its scope. -func parseNolintComment( +// parseComment parses a single nolint comment and determines its scope. +func parseComment( comment *ast.Comment, f *ast.File, fset *token.FileSet, stmtMap map[int]ast.Stmt, packageLine int, -) (nolintScope, error) { - var scope nolintScope +) (scope, error) { + var scope scope text := comment.Text if !strings.HasPrefix(text, nolintPrefix) { @@ -75,7 +75,7 @@ func parseNolintComment( return scope, fmt.Errorf("invalid nolint comment: expected colon after 'nolint'") } - scope.rules = parseNolintRuleNames(rest) + scope.rules = parseIgnoreRuleNames(rest) pos := fset.Position(comment.Slash) // check if the comment is before the package declaration @@ -119,8 +119,8 @@ func parseNolintComment( return scope, nil } -// parseNolintRuleNames parses the rule list from the nolint comment more efficiently. -func parseNolintRuleNames(text string) map[string]struct{} { +// parseIgnoreRuleNames parses the rule list from the nolint comment more efficiently. +func parseIgnoreRuleNames(text string) map[string]struct{} { rulesMap := make(map[string]struct{}) if text == "" { @@ -175,7 +175,7 @@ func findFunctionAfterLine(fset *token.FileSet, f *ast.File, line int) *ast.Func } // IsNolint checks if a given position and rule are nolinted. -func (m *nolintManager) IsNolint(pos token.Position, ruleName string) bool { +func (m *Manager) IsNolint(pos token.Position, ruleName string) bool { scopes, exists := m.scopes[pos.Filename] if !exists { return false diff --git a/internal/nolint_test.go b/internal/nolint/nolint_test.go similarity index 94% rename from internal/nolint_test.go rename to internal/nolint/nolint_test.go index 259be9f..39656a6 100644 --- a/internal/nolint_test.go +++ b/internal/nolint/nolint_test.go @@ -1,4 +1,4 @@ -package internal +package nolint import ( "go/parser" @@ -10,7 +10,7 @@ func TestParseNolintRules(t *testing.T) { t.Parallel() input := "rule1,rule2,rule3" expected := []string{"rule1", "rule2", "rule3"} - result := parseNolintRuleNames(input) + result := parseIgnoreRuleNames(input) if len(result) != len(expected) { t.Errorf("Expected %d rules, got %d", len(expected), len(result)) } @@ -40,7 +40,7 @@ var x int t.Fatalf("Failed to parse file: %v", err) } - manager := ParseNolintComments(f, fset) + manager := ParseComments(f, fset) if manager == nil { t.Fatal("Expected nolintManager, got nil") } @@ -76,7 +76,7 @@ func main() { t.Fatalf("Failed to parse source: %v", err) } - manager := ParseNolintComments(node, fset) + manager := ParseComments(node, fset) tests := []struct { line int diff --git a/internal/rule_set.go b/internal/rule_set.go index e9acf25..ce9ec81 100644 --- a/internal/rule_set.go +++ b/internal/rule_set.go @@ -147,7 +147,7 @@ func (r *MissingModPackageRule) Name() string { type RepeatedRegexCompilationRule struct{} func (r *RepeatedRegexCompilationRule) Check(filename string, node *ast.File, fset *token.FileSet) ([]tt.Issue, error) { - return lints.DetectRepeatedRegexCompilation(filename) + return lints.DetectRepeatedRegexCompilation(filename, node) } func (r *RepeatedRegexCompilationRule) Name() string { diff --git a/testdata/demo.gno b/testdata/demo.gno deleted file mode 100644 index 3cf147a..0000000 --- a/testdata/demo.gno +++ /dev/null @@ -1,22 +0,0 @@ -package main - -import "strings" - -func foo(x, y bool) int { - if x { - return 1 - } else { - return 2 - } -} - -func bar() { - x := 5 - println("hello") -} - -func main() { - x := foo(true) - bar() - println(x) -}