Cyfrin / aderyn-vscode

MIT License
1 stars 1 forks source link

Quick Fix: add `// @aderyn-ignore` on line above #20

Open alexroan opened 2 months ago

alexroan commented 2 months ago

This requires prerequisite aderyn work.

TilakMaddy commented 2 months ago

@alexroan Here's my 2 cents on how I envision it - If possible, I'd love to also know how you are thinking about solving this problem.

Option 1

Aderyn CLI takes up the responsibility for excluding undesired lines in the report

Pros No extra work from VSCode extension (Just render as usual) The report generated will filter out undesired line numbers even if you are not interacting via vscode extension

Cons We check all the lines across all the files in scope to find out what we should ignore. Then, after detectors capture all the issues, we filter out undesired line numbers.


Option 2

Aderyn VSCode Extension takes up the responsibility for excluding undesired lines in the report

Pros No extra work from Aderyn CLI Not much time is required for precomputation because we only have to search for the undesired line numbers in the actively opened solidity file.

Cons The report generated will not filter out undesired line numbers even if you are not interacting via vscode extension

TilakMaddy commented 2 months ago

Another comment unrelated to the above comment

The title suggests that we give // @aderyn-ignore on the line above but it may not always be feasible in situations like capturing function definitions when they extend to multiple lines, etc

I don't have a solution in my mind yet, for this (at least a full one) but I thought I would just jot it down here

New thoughts

  1. To keep it simple, we can encourage the use of 3 types of comments (maybe we can start with just 1)

Type 1

Adding aderyn-disable-next-line to disable next line as a comment in the previous line

// solhint-disable-next-line aderyn-disable-next-line
function name() external returns(uint256) {
    ....
}

Notice, how this can co-exist with other things like solhint-disable-next-line

Type 2

Adding aderyn-disable-line to disable the current line .

function 
name(bool x, bool y) // aderyn-disable-line
external {
   ...
}

Type 3 Adding /*aderyn-disable-region-start*/ and /*aderyn-disable-region-end*/ to disable a given region in the code

function name() external returns(uint256) {
    ...
   uint256 fifty6 = 56;
   uint256 fifty7 = 57;

   /*aderyn-disable-region-start*/

    .... <code region to disable> ...

   /*aderyn-disable-region-end*/

}
TilakMaddy commented 2 months ago

We can achieve the above with tokenize function in the fsloc module that we have. Because it also captures the start_line and end_line alon with token type. We can use that to inject "Lines to ignore" inside the Stats object which will be eventually injected to WorkspaceContext.. Then as post processing step we can run this filter and generate the desired output : )

I think we can start with Type 1 & 2 . Type 3 can be incorporated later

TilakMaddy commented 2 months ago

Can we have this issue in the core repo ?

TilakMaddy commented 2 months ago

@alexroan https://github.com/Cyfrin/aderyn/pull/567 should solve the issue. (Make sure to not use --skip-cloc)

alexroan commented 2 months ago

Sorry, I should have added more information to this ticket. https://github.com/Cyfrin/aderyn/pull/567 is a prerequisite for this. The actual work for this ticket is to implement the "Quick Fix" button in VSCode to auto-add the // @aderyn-ignore line. Screenshot 2024-06-26 at 14 12 13