Skip to content

Commit

Permalink
fix(scripts): unused import regex and misc refactoring (#362)
Browse files Browse the repository at this point in the history
* fix(scripts): check_style.py regex issues, avoid early exit, and refactoring

* refactor(scripts) rename check_style.py to style.py since it's not just for checking, it does autofix as well. --check is a cli option and confusingly redundant with the filename

* refactor: remove unused imports

I noticed the regex for detecting unused imports was changed in a recent PR. This broke the detection of unused imports. The change made it so you would only detect unused lines that explicitly say `@import("...")`. But those are not the only lines we want to check. We also want to check for things like `const Thing = sig.utils.Thing;`. The recent change also broke detection of identifiers with numbers and underscores. So I fixed these issues.

I also made the regex slightly better at detecting file paths within `@import` lines by including `/` as a valid character. This identified few more unused imports which I also removed in this PR.

I consolidated the code to a main function so it's clearer what's actually running when you run the script.

I used return values to indicate failed checks, instead of directly calling `exit(1)` within the check function. That way it always runs all the checks, instead of exiting early.

I renamed the file to `style.py`. `check_style.py` is misleading because it's not just a check. By default this script will automatically edit the code to adjust its style. Only if you provide the `--check` flag does it limit itself to checks. At that point, the name is redundant with the flag. I see "style" as analogous to zig's "fmt" subcommand. By default `zig fmt` will autoformat the code, and then it limits itself to checks if you use `--check`.
  • Loading branch information
dnut authored Nov 6, 2024
1 parent c9b77a1 commit 0e0ece4
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 34 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
python-version: "3.10"

- name: check style
run: python scripts/check_style.py --check src
run: python scripts/style.py --check src

test:
strategy:
Expand Down
62 changes: 34 additions & 28 deletions scripts/check_style.py → scripts/style.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,29 @@

MAX_LINE_LENGTH = 100

arg_parser = argparse.ArgumentParser()
arg_parser.add_argument("dirs", action="append")
arg_parser.add_argument("--check", action="store_true")
arg_parser.add_argument("-v", "--verbose", action="store_true")
args = arg_parser.parse_args()

def main():
arg_parser = argparse.ArgumentParser()
arg_parser.add_argument("dirs", action="append")
arg_parser.add_argument("--check", action="store_true")
arg_parser.add_argument("-v", "--verbose", action="store_true")
args = arg_parser.parse_args()

def get_files():
files_to_check = get_files(args)

checks = [
unused_imports,
line_length,
]
fails = 0
for check in checks:
print("Running check: ", check.__name__)
fails += check(args, files_to_check)
if fails:
exit(1)


def get_files(args):
files_to_check = []
dirs = [*args.dirs]
while len(dirs) > 0:
Expand All @@ -30,19 +45,18 @@ def get_files():
return files_to_check


# Checks for unused imports in files.
def unused_imports():
files_to_check = get_files()
def unused_imports(args, files_to_check):
"""Checks for unused imports in files."""
import_line_regex = re.compile(
r'const\s+([a-zA-Z]+)\s+=\s+@import\("([a-zA-Z.]+)"\);'
r'const\s+([a-zA-Z0-9_]+)\s+=\s+(@import\("[a-zA-Z0-9_./]+"\))?[a-zA-Z0-9_.]*;'
)

total_lines_removed = 0
lines_removed = 0

while True:
lines_removed = 0
for path in files_to_check:
# get all lines of code
with open(path) as f:
orig_file = f.read()
orig_lines = orig_file.split("\n")
Expand Down Expand Up @@ -103,11 +117,11 @@ def unused_imports():
print("Total unused imports found:", total_lines_removed)
else:
print("Total unused imports removed:", total_lines_removed)
if total_lines_removed > 0:
exit(1)

return total_lines_removed


excluded_files = [
files_excluded_from_line_length_check = [
"src/accountsdb/accounts_file.zig",
"src/accountsdb/bank.zig",
"src/accountsdb/cache.zig",
Expand Down Expand Up @@ -232,15 +246,14 @@ def unused_imports():
]


# Enforces rows to be at most 100 characters long.
def row_size():
files_to_check = get_files()
def line_length(args, files_to_check):
"""Enforces lines of code to be at most 100 characters long."""
unique_files = []

lines_found = 0

for path in files_to_check:
if path in excluded_files:
if path in files_excluded_from_line_length_check:
continue
with open(path) as f:
lines = f.readlines()
Expand All @@ -258,15 +271,8 @@ def row_size():
for file in unique_files:
print(f'"{file}",')

if lines_found > 0:
exit(1)

return lines_found

checks = [
unused_imports,
row_size,
]

for check in checks:
print("Running check: ", check.__name__)
check()
if __name__ == '__main__':
main()
3 changes: 0 additions & 3 deletions src/accountsdb/sysvars.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@ const ArrayList = std.ArrayList;
const Slot = @import("../core/time.zig").Slot;
const Epoch = @import("../core/time.zig").Epoch;
const Pubkey = @import("../core/pubkey.zig").Pubkey;
const Hash = @import("../core/hash.zig").Hash;

const StakeHistoryEntry = @import("./snapshots.zig").StakeHistoryEntry;
const UnixTimestamp = @import("genesis_config.zig").UnixTimestamp;
const ThreadPool = @import("../sync/thread_pool.zig").ThreadPool;

const BitVec = @import("../bloom/bit_vec.zig").BitVec;
const DynamicArrayBitSet = @import("../bloom/bit_set.zig").DynamicArrayBitSet;
const BitVecConfig = @import("../bloom/bit_vec.zig").BitVecConfig;
const bincode = @import("../bincode/bincode.zig");
Expand Down
1 change: 0 additions & 1 deletion src/shred_collector/shred_tracker.zig
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const std = @import("std");
const sig = @import("../sig.zig");
const shred_collector = @import("lib.zig");

const Allocator = std.mem.Allocator;
const ArrayList = std.ArrayList;
Expand Down
1 change: 0 additions & 1 deletion src/shred_collector/shred_verifier.zig
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const std = @import("std");
const sig = @import("../sig.zig");
const shred_collector = @import("lib.zig");

const shred_layout = sig.ledger.shred.layout;

Expand Down

0 comments on commit 0e0ece4

Please sign in to comment.