Fix inheritance printer rebase #2153
Conversation
0xPhaze
requested review from
montyly,
0xalpharush and
smonicas
as code owners
DarkaMaul
force-pushed
the
fix-inheritance-printer-rebase
branch
from
77c67a0 to
4164039
Compare
WalkthroughWalkthroughThe recent updates to the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant Main as slither/__main__.py
participant Slither as slither/slither.py
participant Graph as inheritance_graph.py
participant Test as test_printers.py
User->>Main: Run with --include-interfaces
Main->>Slither: Initialize with include_interfaces=True
Slither->>Graph: Generate inheritance graph
Graph->>Graph: Filter based on include_interfaces
Graph->>User: Return filtered inheritance graph
User->>Test: Run tests for interface inclusion
Test->>Graph: Validate output with interfaces
Graph->>Test: Return validation result
Test->>User: Test results
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
This PR nows :
I think it is ready for review 🚀 Some picture before / after
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- slither/main.py (1 hunks)
- slither/printers/inheritance/inheritance_graph.py (3 hunks)
- slither/slither.py (1 hunks)
- tests/e2e/printers/test_data/test_contract_names/C.sol (1 hunks)
- tests/e2e/printers/test_printers.py (1 hunks)
Files skipped from review due to trivial changes (1)
- slither/slither.py
Additional comments not posted (5)
tests/e2e/printers/test_data/test_contract_names/C.sol (2)
3-7: InterfaceMyInterfaceXis well-defined with appropriate function signatures.
9-20: ContractCcorrectly implementsMyInterfaceX. Please confirm if the empty implementation ofincrement()is intentional.slither/printers/inheritance/inheritance_graph.py (2)
101-115: The modifications to the inheritance list comprehension correctly implement the filtering of 'mock' contracts and conditional inclusion of interfaces based on theinclude_interfacesflag.
208-213: The changes to the output function to conditionally skip certain contracts are correctly implemented and align with the PR's objectives.slither/__main__.py (1)
351-357: The addition of the--include-interfacescommand-line argument is correctly implemented and well-documented. This new feature allows users to include interfaces in the inheritance graph as intended.
|
|
||
| # Lets also test the include/exclude interface behavior | ||
| # Check that the interface is not included | ||
| assert "MyInterfaceX" not in content | ||
|
|
||
| slither.include_interfaces = True | ||
| output = printer.output("test_printer.dot") | ||
| content = output.elements[0]["name"]["content"] | ||
| assert "MyInterfaceX" in content | ||
|
|
||
| # Remove test generated files | ||
| Path("test_printer.dot").unlink(missing_ok=True) |
There was a problem hiding this comment.
The test structure for checking the inheritance printer's behavior with and without interfaces is well-implemented. Consider adding more comprehensive tests to cover additional edge cases.
Would you like assistance in drafting additional test cases?
This PR nows :
include-interfacescommand line switch if the user wants to include interface in the inheritance graph (default isFalse).Summary by CodeRabbit
New Features
--include-interfacesargument to include interfaces in the inheritance-graph printer.Bug Fixes
--include-interfacesis set toFalse.Tests