refactor `AbstractDetector` to use the visitor pattern
- Dominant language
- Python
- Stars
- 6.4k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
### Describe the desired feature
Right now, each detector implements its own ad-hoc approach to iterating over a compilation unit. This leaves the analysis author to understand and decide between `contracts` vs `contracts_derived` and `functions` vs `functions_top_level` vs `functions_and_modifiers` (even more if we consider inherited vs declared). While for some detectors there is a need for specialization, the vast majority of analyses could reuse a visitor from the `AbstractDetector` like `visit_all_contracts_derived_functions`. There are several benefits to this approach:
1) These visitors require less understanding of the core API and can be clearly documented with guidance on when they should be used.
2) It will make it easier to survey which detectors and analyze what and update changes to detectors en masse e.g. some detectors likely need to be update to support top level functions.
3) I believe this would greatly impact performance positively. Rather than N detectors iterating over every comp. unit, contract, function, `AbstractDetector` would iterator over these and invoke each detector's visitor, reducing complexity.
Contributor guide
Research direction
Start by locating AbstractDetector and the existing detector loops over compilation units, contracts, and functions. Inventory which traversal variants detectors currently use and how inherited, declared, top-level, and modifier functions are handled. Done means the shared visitor approach is defined and detector behavior and performance are validated across the affected analyses.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, solidity
- Domain
- compilers, devtools
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100