Commit 21827a2f authored by smartcontracts's avatar smartcontracts Committed by GitHub

maint: update unused imports check to use new pattern (#13227)

Updates the unused imports contract check to use the new
framework that we recently merged.
parent ec05937c
...@@ -2,101 +2,51 @@ package main ...@@ -2,101 +2,51 @@ package main
import ( import (
"bufio" "bufio"
"errors"
"fmt" "fmt"
"os" "os"
"path/filepath"
"regexp" "regexp"
"runtime"
"strings" "strings"
"sync"
"sync/atomic" "github.com/ethereum-optimism/optimism/packages/contracts-bedrock/scripts/checks/common"
) )
var importPattern = regexp.MustCompile(`import\s*{([^}]+)}`) var importPattern = regexp.MustCompile(`import\s*{([^}]+)}`)
var asPattern = regexp.MustCompile(`(\S+)\s+as\s+(\S+)`) var asPattern = regexp.MustCompile(`(\S+)\s+as\s+(\S+)`)
func main() { func main() {
if err := run(); err != nil { if err := common.ProcessFilesGlob(
writeStderr("an error occurred: %v", err) []string{"src/**/*.sol", "scripts/**/*.sol", "test/**/*.sol"},
[]string{},
processFile,
); err != nil {
fmt.Printf("error: %v\n", err)
os.Exit(1) os.Exit(1)
} }
} }
func writeStderr(msg string, args ...any) { func processFile(filePath string) []error {
_, _ = fmt.Fprintf(os.Stderr, msg+"\n", args...)
}
func run() error {
cwd, err := os.Getwd()
if err != nil {
return fmt.Errorf("failed to get current working directory: %w", err)
}
var hasErr atomic.Bool
var outMtx sync.Mutex
fail := func(msg string, args ...any) {
outMtx.Lock()
writeStderr("❌ "+msg, args...)
outMtx.Unlock()
hasErr.Store(true)
}
dirs := []string{"src", "scripts", "test"}
sem := make(chan struct{}, runtime.NumCPU())
for _, dir := range dirs {
dirPath := filepath.Join(cwd, dir)
if _, err := os.Stat(dirPath); errors.Is(err, os.ErrNotExist) {
continue
}
err := filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if !info.IsDir() && strings.HasSuffix(info.Name(), ".sol") {
sem <- struct{}{}
go func() {
defer func() { <-sem }()
processFile(path, fail)
}()
}
return nil
})
if err != nil {
return fmt.Errorf("failed to walk directory %s: %w", dir, err)
}
}
for i := 0; i < cap(sem); i++ {
sem <- struct{}{}
}
if hasErr.Load() {
return errors.New("unused imports check failed, see logs above")
}
return nil
}
func processFile(filePath string, fail func(string, ...any)) {
content, err := os.ReadFile(filePath) content, err := os.ReadFile(filePath)
if err != nil { if err != nil {
fail("%s: failed to read file: %v", filePath, err) return []error{fmt.Errorf("%s: failed to read file: %w", filePath, err)}
return
} }
imports := findImports(string(content)) imports := findImports(string(content))
unusedImports := checkUnusedImports(imports, string(content)) var unusedImports []string
for _, imp := range imports {
if !isImportUsed(imp, string(content)) {
unusedImports = append(unusedImports, imp)
}
}
if len(unusedImports) > 0 { if len(unusedImports) > 0 {
fail("File: %s\nUnused imports:", filePath) var errors []error
for _, unused := range unusedImports { for _, unused := range unusedImports {
fail(" - %s", unused) errors = append(errors, fmt.Errorf("%s", unused))
} }
return errors
} }
return nil
} }
func findImports(content string) []string { func findImports(content string) []string {
...@@ -106,29 +56,17 @@ func findImports(content string) []string { ...@@ -106,29 +56,17 @@ func findImports(content string) []string {
if len(match) > 1 { if len(match) > 1 {
importList := strings.Split(match[1], ",") importList := strings.Split(match[1], ",")
for _, imp := range importList { for _, imp := range importList {
imports = append(imports, strings.TrimSpace(imp)) imp = strings.TrimSpace(imp)
if asMatch := asPattern.FindStringSubmatch(imp); len(asMatch) > 2 {
// Use the renamed identifier (after 'as')
imports = append(imports, strings.TrimSpace(asMatch[2]))
} else {
imports = append(imports, imp)
} }
} }
} }
return imports
}
func checkUnusedImports(imports []string, content string) []string {
var unusedImports []string
for _, imp := range imports {
searchTerm := imp
displayName := imp
if match := asPattern.FindStringSubmatch(imp); len(match) > 2 {
searchTerm = match[2]
displayName = fmt.Sprintf("%s as %s", match[1], match[2])
}
if !isImportUsed(searchTerm, content) {
unusedImports = append(unusedImports, displayName)
} }
} return imports
return unusedImports
} }
func isImportUsed(imp, content string) bool { func isImportUsed(imp, content string) bool {
......
package main
import (
"testing"
"github.com/stretchr/testify/require"
)
func Test_findImports(t *testing.T) {
tests := []struct {
name string
content string
expected []string
}{
{
name: "finds single named import",
content: `
pragma solidity ^0.8.0;
import { Contract } from "./Contract.sol";
contract Test {}
`,
expected: []string{"Contract"},
},
{
name: "finds multiple named imports",
content: `
pragma solidity ^0.8.0;
import { Contract1, Contract2 } from "./Contracts.sol";
contract Test {}
`,
expected: []string{"Contract1", "Contract2"},
},
{
name: "handles import with as keyword",
content: `
pragma solidity ^0.8.0;
import { Contract as Renamed } from "./Contract.sol";
contract Test {}
`,
expected: []string{"Renamed"},
},
{
name: "handles multiple imports with as keyword",
content: `
pragma solidity ^0.8.0;
import { Contract1 as C1, Contract2 as C2 } from "./Contracts.sol";
contract Test {}
`,
expected: []string{"C1", "C2"},
},
{
name: "ignores regular imports",
content: `
pragma solidity ^0.8.0;
import "./Contract.sol";
contract Test {}
`,
expected: nil,
},
{
name: "empty content",
content: "",
expected: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := findImports(tt.content)
require.Equal(t, tt.expected, result)
})
}
}
func Test_isImportUsed(t *testing.T) {
tests := []struct {
name string
importedName string
content string
expected bool
}{
{
name: "import used in contract",
importedName: "UsedContract",
content: `
contract Test {
UsedContract used;
}
`,
expected: true,
},
{
name: "import used in inheritance",
importedName: "BaseContract",
content: `
contract Test is BaseContract {
}
`,
expected: true,
},
{
name: "import used in function",
importedName: "Utility",
content: `
contract Test {
function test() {
Utility.doSomething();
}
}
`,
expected: true,
},
{
name: "import not used",
importedName: "UnusedContract",
content: `
contract Test {
OtherContract other;
}
`,
expected: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := isImportUsed(tt.importedName, tt.content)
require.Equal(t, tt.expected, result)
})
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment