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

feat(ct): add checks for unused imports #12348

Merged
merged 1 commit 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
24 changes: 18 additions & 6 deletions packages/contracts-bedrock/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ gas-snapshot-no-build:
# Generates a gas snapshot.
gas-snapshot: build-go-ffi gas-snapshot-no-build

# Checks that the state diff is up to date.
statediff:
./scripts/utils/statediff.sh && git diff --exit-code

# Generates default Kontrol summary.
kontrol-summary:
./test/kontrol/scripts/make-summary-deployment.sh
Expand Down Expand Up @@ -176,6 +172,13 @@ lint-forge-tests-check:
lint-check:
forge fmt --check

# Checks for unused imports in Solidity contracts. Does not build contracts.
unused-imports-check-no-build:
go run ./scripts/checks/unused-imports

# Checks for unused imports in Solidity contracts.
unused-imports-check: build unused-imports-check-no-build

# Checks that the deploy configs are valid.
validate-deploy-configs:
./scripts/checks/check-deploy-configs.sh
Expand All @@ -189,8 +192,17 @@ validate-spacers: build validate-spacers-no-build

# TODO: Also run lint-forge-tests-check but we need to fix the test names first.
# Runs all checks.
check: gas-snapshot-check-no-build kontrol-deployment-check snapshots-check-no-build lint-check semver-diff-check-no-build semver-natspec-check-no-build validate-deploy-configs validate-spacers-no-build interfaces-check-no-build

check:
@just gas-snapshot-check-no-build \
unused-imports-check-no-build \
kontrol-deployment-check \
snapshots-check-no-build \
lint-check \
semver-diff-check-no-build \
semver-natspec-check-no-build \
validate-deploy-configs \
validate-spacers-no-build \
interfaces-check-no-build

########################################################
# DEV TOOLS #
Expand Down
1 change: 0 additions & 1 deletion packages/contracts-bedrock/scripts/Artifacts.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ pragma solidity ^0.8.0;
import { console2 as console } from "forge-std/console2.sol";
import { stdJson } from "forge-std/StdJson.sol";
import { Vm } from "forge-std/Vm.sol";
import { Executables } from "scripts/libraries/Executables.sol";
import { Predeploys } from "src/libraries/Predeploys.sol";
import { Config } from "scripts/libraries/Config.sol";
import { StorageSlot } from "scripts/libraries/ForgeArtifacts.sol";
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts-bedrock/scripts/DeployOPChain.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { IDisputeGameFactory } from "src/dispute/interfaces/IDisputeGameFactory.
import { IAnchorStateRegistry } from "src/dispute/interfaces/IAnchorStateRegistry.sol";
import { IFaultDisputeGame } from "src/dispute/interfaces/IFaultDisputeGame.sol";
import { IPermissionedDisputeGame } from "src/dispute/interfaces/IPermissionedDisputeGame.sol";
import { Claim, Duration, GameType, GameTypes, Hash, OutputRoot } from "src/dispute/lib/Types.sol";
import { Claim, Duration, GameType, GameTypes, Hash } from "src/dispute/lib/Types.sol";

import { OPContractsManager } from "src/L1/OPContractsManager.sol";
import { IOptimismPortal2 } from "src/L1/interfaces/IOptimismPortal2.sol";
Expand Down
3 changes: 0 additions & 3 deletions packages/contracts-bedrock/scripts/L2Genesis.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,12 @@
pragma solidity 0.8.15;

// Testing
import { Script } from "forge-std/Script.sol";
import { console2 as console } from "forge-std/console2.sol";
import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol";

// Scripts
import { Deployer } from "scripts/deploy/Deployer.sol";
import { Config, OutputMode, OutputModeUtils, Fork, ForkUtils, LATEST_FORK } from "scripts/libraries/Config.sol";
import { Artifacts } from "scripts/Artifacts.s.sol";
import { DeployConfig } from "scripts/deploy/DeployConfig.s.sol";
import { Process } from "scripts/libraries/Process.sol";
import { SetPreinstalls } from "scripts/SetPreinstalls.s.sol";

Expand Down
149 changes: 149 additions & 0 deletions packages/contracts-bedrock/scripts/checks/unused-imports/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package main

import (
"bufio"
"errors"
"fmt"
"os"
"path/filepath"
"regexp"
"runtime"
"strings"
"sync"
"sync/atomic"
)

var importPattern = regexp.MustCompile(`import\s*{([^}]+)}`)
var asPattern = regexp.MustCompile(`(\S+)\s+as\s+(\S+)`)

func main() {
if err := run(); err != nil {
writeStderr("an error occurred: %v", err)
os.Exit(1)
}
}

func writeStderr(msg string, args ...any) {
_, _ = 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 int32
var outMtx sync.Mutex
smartcontracts marked this conversation as resolved.
Show resolved Hide resolved
fail := func(msg string, args ...any) {
outMtx.Lock()
writeStderr("❌ "+msg, args...)
outMtx.Unlock()
atomic.StoreInt32(&hasErr, 1)
}

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 atomic.LoadInt32(&hasErr) == 1 {
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)
if err != nil {
fail("%s: failed to read file: %v", filePath, err)
return
}

imports := findImports(string(content))
unusedImports := checkUnusedImports(imports, string(content))

if len(unusedImports) > 0 {
fail("File: %s\nUnused imports:", filePath)
for _, unused := range unusedImports {
fail(" - %s", unused)
}
}
}

func findImports(content string) []string {
var imports []string
matches := importPattern.FindAllStringSubmatch(content, -1)
for _, match := range matches {
if len(match) > 1 {
importList := strings.Split(match[1], ",")
for _, imp := range importList {
imports = append(imports, strings.TrimSpace(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 unusedImports
}

func isImportUsed(imp, content string) bool {
scanner := bufio.NewScanner(strings.NewReader(content))
for scanner.Scan() {
line := scanner.Text()
if strings.HasPrefix(strings.TrimSpace(line), "//") {
continue
}
if strings.Contains(line, "import") {
continue
}
if strings.Contains(line, imp) {
return true
}
}
return false
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import { console2 as console } from "forge-std/console2.sol";

// Scripts
import { DeployConfig } from "scripts/deploy/DeployConfig.s.sol";
import { Deployer } from "scripts/deploy/Deployer.sol";
import { ISystemConfigV0 } from "scripts/interfaces/ISystemConfigV0.sol";
import { ISystemConfigInterop } from "src/L1/interfaces/ISystemConfigInterop.sol";

// Libraries
Expand Down
5 changes: 1 addition & 4 deletions packages/contracts-bedrock/scripts/deploy/Deploy.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ pragma solidity ^0.8.0;

// Testing
import { VmSafe } from "forge-std/Vm.sol";
import { Script } from "forge-std/Script.sol";
import { console2 as console } from "forge-std/console2.sol";
import { stdJson } from "forge-std/StdJson.sol";
import { AlphabetVM } from "test/mocks/AlphabetVM.sol";
Expand Down Expand Up @@ -40,17 +39,15 @@ import { IProxy } from "src/universal/interfaces/IProxy.sol";
import { IProxyAdmin } from "src/universal/interfaces/IProxyAdmin.sol";
import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol";
import { IOptimismPortal2 } from "src/L1/interfaces/IOptimismPortal2.sol";
import { IOptimismPortalInterop } from "src/L1/interfaces/IOptimismPortalInterop.sol";
import { ICrossDomainMessenger } from "src/universal/interfaces/ICrossDomainMessenger.sol";
import { IL1CrossDomainMessenger } from "src/L1/interfaces/IL1CrossDomainMessenger.sol";
import { IL2OutputOracle } from "src/L1/interfaces/IL2OutputOracle.sol";
import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol";
import { ISystemConfig } from "src/L1/interfaces/ISystemConfig.sol";
import { ISystemConfigInterop } from "src/L1/interfaces/ISystemConfigInterop.sol";
import { IDataAvailabilityChallenge } from "src/L1/interfaces/IDataAvailabilityChallenge.sol";
import { IL1ERC721Bridge } from "src/L1/interfaces/IL1ERC721Bridge.sol";
import { IL1StandardBridge } from "src/L1/interfaces/IL1StandardBridge.sol";
import { IProtocolVersions, ProtocolVersion } from "src/L1/interfaces/IProtocolVersions.sol";
import { ProtocolVersion } from "src/L1/interfaces/IProtocolVersions.sol";
import { IBigStepper } from "src/dispute/interfaces/IBigStepper.sol";
import { IDisputeGameFactory } from "src/dispute/interfaces/IDisputeGameFactory.sol";
import { IDisputeGame } from "src/dispute/interfaces/IDisputeGame.sol";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { console2 as console } from "forge-std/console2.sol";
import { stdJson } from "forge-std/StdJson.sol";
import { Executables } from "scripts/libraries/Executables.sol";
import { Process } from "scripts/libraries/Process.sol";
import { Chains } from "scripts/libraries/Chains.sol";
import { Config, Fork, ForkUtils } from "scripts/libraries/Config.sol";

/// @title DeployConfig
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity ^0.8.0;

import { console2 as console } from "forge-std/console2.sol";
import { stdJson } from "forge-std/StdJson.sol";

import { GnosisSafe as Safe } from "safe-contracts/GnosisSafe.sol";
import { GnosisSafeProxyFactory as SafeProxyFactory } from "safe-contracts/proxies/GnosisSafeProxyFactory.sol";
Expand Down
Loading