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

Refactor More Detector Names to suit //aderyn-ignore.. pattern #739

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

TilakMaddy
Copy link
Collaborator

@TilakMaddy TilakMaddy commented Sep 19, 2024

Follow ups from
#737

Please feel free to improve them even more.

Basically what I'm looking for is that when we write //aderyn-ignore(rule-name)

It should be CONSISTENT!
We "ignore" the "wrong" thing.
Therefore rule-name must specify the WRONG thing.

For example, //aderyn-ignore(require-without-string)
instead of //aderyn-ignore(require-with-string)

I get it that we want to have a string as explanation in the require statement. But .. that's where the description/recommendation/hint comes into play.

Tldr;
It should read like .... Here's the problem. and we're ignoring it
Not like .. Here's the correct thing to do .. and we're ignoring it.

Other examples that would make sense

//aderyn-allow-next-line(require-without-string)
//aderyn-allow-next-line(require-without-explanation)

ETC, ETC

@alexroan Please feel free to change it up as you see makes sense

@TilakMaddy TilakMaddy marked this pull request as ready for review September 19, 2024 06:57
@TilakMaddy
Copy link
Collaborator Author

TilakMaddy commented Oct 4, 2024

@alexroan Since we haven't announced the ignore pattern yet, I think we have a lot of room for improvement / flexibility. We need to take action quickly here otherwise once people start using //aderyn-ignore(detector-name) it would be a pain in the neck to change all those to //aderyn-ignore(insert-new-detector-name) ..

Although I'm merging this, please take a careful look once again and propose new changes (if any) before we release 0.3.1

I'll assign this to you in linear as well :)

@TilakMaddy
Copy link
Collaborator Author

New name list here

ArrayLengthNotCached
StateChangeInAssert
HashCollisionDueToAbiEncodePacked
SignatureMalleabilityDueToRawEcrecover
NoZeroAddressCheck
SendsEtherAwayWithoutCheckingAddress
DelegateCallOnUncheckedAddress
RedundantBooleanEquality

@TilakMaddy TilakMaddy merged commit 8d5fdc3 into dev Oct 4, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant