Skip to content

Commit

Permalink
nolint (#84)
Browse files Browse the repository at this point in the history
close #83
  • Loading branch information
notJoon authored Oct 7, 2024
1 parent be21dd1 commit ecce4c3
Show file tree
Hide file tree
Showing 7 changed files with 375 additions and 21 deletions.
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

0 comments on commit ecce4c3

Please sign in to comment.