Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nolint #84

Merged
merged 6 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ build-mac:
CGO_ENABLED=0 GOOS=darwin GOARCH=amd64 $(GOBUILD) -o $(BINARY_MAC) -v $(MAIN_PACKAGE)

test:
$(GOTEST) -race -v ./...
$(GOTEST) -race -v -shuffle=on ./...

clean:
$(GOCLEAN)
Expand Down
40 changes: 34 additions & 6 deletions internal/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package internal

import (
"fmt"
"go/token"
"os"
"path/filepath"
"strings"
Expand All @@ -17,6 +18,7 @@ type Engine struct {
rules []LintRule
ignoredRules map[string]bool
defaultRules []LintRule
nolintMgr *nolintManager
}

// NewEngine creates a new lint engine.
Expand Down Expand Up @@ -72,6 +74,8 @@ func (e *Engine) Run(filename string) ([]tt.Issue, error) {
return nil, fmt.Errorf("error parsing file: %w", err)
}

e.nolintMgr = ParseNolintComments(node, fset)

var wg sync.WaitGroup
var mu sync.Mutex

Expand All @@ -80,16 +84,18 @@ func (e *Engine) Run(filename string) ([]tt.Issue, error) {
wg.Add(1)
go func(r LintRule) {
defer wg.Done()
if e.ignoredRules[rule.Name()] {
if e.ignoredRules[r.Name()] {
return
}
issues, err := rule.Check(tempFile, node, fset)
issues, err := r.Check(tempFile, node, fset)
if err != nil {
return
}

nolinted := e.filterNolintIssues(issues)

mu.Lock()
allIssues = append(allIssues, issues...)
allIssues = append(allIssues, nolinted...)
mu.Unlock()
}(rule)
}
Expand All @@ -114,6 +120,8 @@ func (e *Engine) RunSource(source []byte) ([]tt.Issue, error) {
return nil, fmt.Errorf("error parsing content: %w", err)
}

e.nolintMgr = ParseNolintComments(node, fset)

var wg sync.WaitGroup
var mu sync.Mutex

Expand All @@ -122,16 +130,18 @@ func (e *Engine) RunSource(source []byte) ([]tt.Issue, error) {
wg.Add(1)
go func(r LintRule) {
defer wg.Done()
if e.ignoredRules[rule.Name()] {
if e.ignoredRules[r.Name()] {
return
}
issues, err := rule.Check("", node, fset)
issues, err := r.Check("", node, fset)
if err != nil {
return
}

nolinted := e.filterNolintIssues(issues)

mu.Lock()
allIssues = append(allIssues, issues...)
allIssues = append(allIssues, nolinted...)
mu.Unlock()
}(rule)
}
Expand Down Expand Up @@ -195,6 +205,24 @@ func (e *Engine) filterUndefinedIssues(issues []tt.Issue) []tt.Issue {
return filtered
}

// filterNolintIssues filters issues based on nolint comments.
func (e *Engine) filterNolintIssues(issues []tt.Issue) []tt.Issue {
if e.nolintMgr == nil {
return issues
}
filtered := make([]tt.Issue, 0, len(issues))
for _, issue := range issues {
pos := token.Position{
Filename: issue.Filename,
Line: issue.Start.Line,
}
if !e.nolintMgr.IsNolint(pos, issue.Rule) {
filtered = append(filtered, issue)
}
}
return filtered
}

// createTempGoFile converts a .gno file to a .go file.
// Since golangci-lint does not support .gno file, we need to convert it to .go file.
// gno has a identical syntax to go, so it is possible to convert it to go file.
Expand Down
19 changes: 8 additions & 11 deletions internal/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,6 @@ import (
"github.com/stretchr/testify/require"
)

// createTempDir creates a temporary directory and returns its path.
// It also registers a cleanup function to remove the directory after the test.
func createTempDir(tb testing.TB, prefix string) string {
tb.Helper()
tempDir, err := os.MkdirTemp("", prefix)
require.NoError(tb, err)
tb.Cleanup(func() { os.RemoveAll(tempDir) })
return tempDir
}

func TestNewEngine(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -142,7 +132,6 @@ var testSrc = strings.Repeat("hello world", 5000)
func BenchmarkCreateTempGoFile(b *testing.B) {
tempDir := createTempDir(b, "benchmark")

// create temp go file for benchmark
gnoContent := []byte(testSrc)
gnoFile := filepath.Join(tempDir, "main.gno")
if err := os.WriteFile(gnoFile, gnoContent, 0o644); err != nil {
Expand Down Expand Up @@ -186,3 +175,11 @@ func BenchmarkRun(b *testing.B) {
}
}
}

func createTempDir(tb testing.TB, prefix string) string {
tb.Helper()
tempDir, err := os.MkdirTemp("", prefix)
require.NoError(tb, err)
tb.Cleanup(func() { os.RemoveAll(tempDir) })
return tempDir
}
195 changes: 195 additions & 0 deletions internal/nolint.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
package internal

import (
"fmt"
"go/ast"
"go/token"
"strings"
)

const nolintPrefix = "//nolint"

// nolintScope represents a range in the code where nolint applies.
type nolintScope 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)),
}
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)
if err != nil {
// ignore invalid nolint comments
continue
}
filename := scope.start.Filename
manager.scopes[filename] = append(manager.scopes[filename], scope)
}
}
return &manager
}

// parseNolintComment parses a single nolint comment and determines its scope.
func parseNolintComment(
comment *ast.Comment,
f *ast.File,
fset *token.FileSet,
stmtMap map[int]ast.Stmt,
packageLine int,
) (nolintScope, error) {
var scope nolintScope
text := comment.Text

if !strings.HasPrefix(text, nolintPrefix) {
return scope, fmt.Errorf("invalid nolint comment")
}

prefixLen := len(nolintPrefix)
rest := text[prefixLen:]

if len(rest) > 0 && rest[0] != ':' {
return scope, fmt.Errorf("invalid nolint comment format")
}

if len(rest) > 0 && rest[0] == ':' {
rest = strings.TrimPrefix(rest, ":")
rest = strings.TrimSpace(rest)
if rest == "" {
return scope, fmt.Errorf("invalid nolint comment: no rules specified after colon")
}
} else if len(rest) > 0 {
return scope, fmt.Errorf("invalid nolint comment: expected colon after 'nolint'")
}

scope.rules = parseNolintRuleNames(rest)
pos := fset.Position(comment.Slash)

// check if the comment is before the package declaration
if isBeforePackageDecl(pos.Line, packageLine) {
scope.start = fset.Position(f.Pos())
scope.end = fset.Position(f.End())
return scope, nil
}

// check if the comment is at the end of a line (inline comment)
if pos.Line == fset.File(comment.Slash).Line(comment.Slash) {
// Inline comment, applies to the statement on the same line
if stmt, exists := stmtMap[pos.Line]; exists {
scope.start = fset.Position(stmt.Pos())
scope.end = fset.Position(stmt.End())
return scope, nil
}
}

// check if the comment is above a statement
nextLine := pos.Line + 1
if stmt, exists := stmtMap[nextLine]; exists {
scope.start = fset.Position(stmt.Pos())
scope.end = fset.Position(stmt.End())
return scope, nil
}

// check if the comment is above a function declaration
if decl := findFunctionAfterLine(fset, f, pos.Line); decl != nil {
funcPos := fset.Position(decl.Pos())
if funcPos.Line == pos.Line+1 {
scope.start = funcPos
scope.end = fset.Position(decl.End())
return scope, nil
}
}

// Default case: apply to the line of the comment
scope.start = pos
scope.end = pos
return scope, nil
}

// parseNolintRuleNames parses the rule list from the nolint comment more efficiently.
func parseNolintRuleNames(text string) map[string]struct{} {
rulesMap := make(map[string]struct{})

if text == "" {
return rulesMap
}

rules := strings.Split(text, ",")
for _, rule := range rules {
rule = strings.TrimSpace(rule)
if rule != "" {
rulesMap[rule] = struct{}{}
}
}
return rulesMap
}

// indexStatementsByLine traverses the AST once and maps each line to its corresponding statement.
func indexStatementsByLine(f *ast.File, fset *token.FileSet) map[int]ast.Stmt {
stmtMap := make(map[int]ast.Stmt)
ast.Inspect(f, func(n ast.Node) bool {
if n == nil {
return false
}
if stmt, ok := n.(ast.Stmt); ok {
line := fset.Position(stmt.Pos()).Line
// save only the first statement of each line
if _, exists := stmtMap[line]; !exists {
stmtMap[line] = stmt
}
}
return true
})
return stmtMap
}

// isBeforePackageDecl checks if a given line is before the package declaration.
func isBeforePackageDecl(line, packageLine int) bool {
return line < packageLine
}

// findFunctionAfterLine finds the first function declaration after a given line.
func findFunctionAfterLine(fset *token.FileSet, f *ast.File, line int) *ast.FuncDecl {
for _, decl := range f.Decls {
if funcDecl, ok := decl.(*ast.FuncDecl); ok {
funcLine := fset.Position(funcDecl.Pos()).Line
if funcLine >= line {
return funcDecl
}
}
}
return nil
}

// IsNolint checks if a given position and rule are nolinted.
func (m *nolintManager) IsNolint(pos token.Position, ruleName string) bool {
scopes, exists := m.scopes[pos.Filename]
if !exists {
return false
}
for _, scope := range scopes {
if pos.Line < scope.start.Line || pos.Line > scope.end.Line {
continue
}
if len(scope.rules) == 0 {
return true
}
if _, exists := scope.rules[ruleName]; exists {
return true
}
}
return false
}
Loading
Loading