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

fix(scripts): unused import regex and misc refactoring #362

Merged
merged 3 commits into from
Nov 6, 2024

Conversation

dnut
Copy link
Contributor

@dnut dnut commented Nov 6, 2024

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.

…st for checking, it does autofix as well. --check is a cli option and confusingly redundant with the filename
@dnut dnut marked this pull request as ready for review November 6, 2024 23:03
@dnut dnut requested a review from Rexicon226 November 6, 2024 23:03
Copy link
Contributor

@Rexicon226 Rexicon226 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching my mistake and improving it!

@dnut dnut merged commit 0e0ece4 into main Nov 6, 2024
6 checks passed
@dnut dnut deleted the dnut/check_style-fixes branch November 6, 2024 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants