Chore/remove unused scripts #2468
Conversation
WalkthroughWalkthroughThe recent updates focus on enhancing the detector selection mechanism in the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant CommandLine
participant Main as slither/__main__.py
participant Utils as slither/utils/command_line.py
User->>CommandLine: Execute command with --include-detectors
CommandLine->>Main: Parse arguments
Main->>Utils: Include detectors based on user input
Utils-->>Main: Return list of detectors to include
Main->>Main: Use __include_detectors to refine selection
Main->>User: Display selected detectors
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (4)
slither/utils/command_line.py (4)
Line range hint
126-126: Usenot infor membership tests to improve readability.- if filter_wiki in detector.WIKI: + if filter_wiki not in detector.WIKI:
Line range hint
157-157: Consider renaming the variablelto a more descriptive name to avoid confusion with the number1.
Line range hint
171-171: Consider renaming the variablelto a more descriptive name to avoid confusion with the number1.
Line range hint
240-240: Usenot infor membership tests to improve readability.- if classification in classification_map.items(): + if classification not in classification_map.items():
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- slither/main.py (3 hunks)
- slither/utils/command_line.py (1 hunks)
Additional context used
Ruff
slither/utils/command_line.py
126-126: Test for membership should be
not in(E713)
157-157: Ambiguous variable name:
l(E741)
171-171: Ambiguous variable name:
l(E741)
240-240: Test for membership should be
not in(E713)
Additional comments not posted (2)
slither/utils/command_line.py (1)
51-51: Ensure that the newdetectors_to_includeparameter is properly utilized in the code.Verification successful
The new
detectors_to_includeparameter is properly utilized in the code.
- Locations in the codebase:
slither/__main__.py: Multiple occurrences wheredetectors_to_includeis used.slither/utils/command_line.py: The parameter is defined.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new parameter `detectors_to_include` is utilized properly in the code. # Test: Search for the usage of `detectors_to_include`. Expect: At least one occurrence in the code. rg --type python $'detectors_to_include'Length of output: 70
Script:
#!/bin/bash # Description: Verify that the new parameter `detectors_to_include` is utilized properly in the code. # Test: Search for the usage of `detectors_to_include`. Expect: At least one occurrence in the code. rg 'detectors_to_include' --glob '*.py'Length of output: 518
slither/__main__.py (1)
14-14: Ensure that the updated import order follows PEP 8 guidelines.
| detectors_to_run = __include_detectors( | ||
| set(detectors_to_run), args.detectors_to_run, detectors | ||
| ) | ||
| return detectors_to_run | ||
|
|
||
| if args.exclude_optimization: | ||
| detectors_to_run = [ | ||
| d for d in detectors_to_run if d.IMPACT != DetectorClassification.OPTIMIZATION | ||
| ] | ||
| classification_map = { | ||
| DetectorClassification.HIGH: args.exclude_high, | ||
| DetectorClassification.MEDIUM: args.exclude_medium, | ||
| DetectorClassification.LOW: args.exclude_low, | ||
| DetectorClassification.INFORMATIONAL: args.exclude_informational, | ||
| DetectorClassification.OPTIMIZATION: args.exclude_optimization, | ||
| } | ||
| excluded_classification = [ | ||
| classification for classification, included in classification_map.items() if included | ||
| ] | ||
| detectors_to_run = [d for d in detectors_to_run if d.IMPACT not in excluded_classification] | ||
|
|
||
| if args.exclude_informational: | ||
| detectors_to_run = [ | ||
| d for d in detectors_to_run if d.IMPACT != DetectorClassification.INFORMATIONAL | ||
| ] | ||
| if args.exclude_low: | ||
| detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.LOW] | ||
| if args.exclude_medium: | ||
| detectors_to_run = [ | ||
| d for d in detectors_to_run if d.IMPACT != DetectorClassification.MEDIUM | ||
| ] | ||
| if args.exclude_high: | ||
| detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.HIGH] | ||
| if args.detectors_to_exclude: | ||
| detectors_to_run = [ | ||
| d for d in detectors_to_run if d.ARGUMENT not in args.detectors_to_exclude | ||
| ] | ||
|
|
||
| detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT) | ||
| if args.detectors_to_include: | ||
| detectors_to_run = __include_detectors( | ||
| set(detectors_to_run), args.detectors_to_include, detectors | ||
| ) |
There was a problem hiding this comment.
The changes to choose_detectors introduce new logic for including detectors based on user input. Ensure this new functionality is covered by unit tests.
Would you like me to help with writing the unit tests for this functionality?
| def __include_detectors( | ||
| detectors_to_run: Set[Type[AbstractDetector]], | ||
| detectors_to_include: str, | ||
| detectors: Dict[str, Type[AbstractDetector]], | ||
| ) -> List[Type[AbstractDetector]]: | ||
| include_detectors = detectors_to_include.split(",") | ||
|
|
||
| for detector in include_detectors: | ||
| if detector in detectors: | ||
| detectors_to_run.add(detectors[detector]) | ||
| else: | ||
| raise ValueError(f"Error: {detector} is not a detector") | ||
|
|
||
| detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT) | ||
| return detectors_to_run |
There was a problem hiding this comment.
The implementation of __include_detectors is straightforward and follows the expected logic. Consider improving error handling by providing more detailed error messages or suggestions for valid inputs when an error occurs.
- raise ValueError(f"Error: {detector} is not a detector")
+ raise ValueError(f"Error: {detector} is not a valid detector. Please check the detector name or refer to the documentation for a list of valid detectors.")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| def __include_detectors( | |
| detectors_to_run: Set[Type[AbstractDetector]], | |
| detectors_to_include: str, | |
| detectors: Dict[str, Type[AbstractDetector]], | |
| ) -> List[Type[AbstractDetector]]: | |
| include_detectors = detectors_to_include.split(",") | |
| for detector in include_detectors: | |
| if detector in detectors: | |
| detectors_to_run.add(detectors[detector]) | |
| else: | |
| raise ValueError(f"Error: {detector} is not a detector") | |
| detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT) | |
| return detectors_to_run | |
| def __include_detectors( | |
| detectors_to_run: Set[Type[AbstractDetector]], | |
| detectors_to_include: str, | |
| detectors: Dict[str, Type[AbstractDetector]], | |
| ) -> List[Type[AbstractDetector]]: | |
| include_detectors = detectors_to_include.split(",") | |
| for detector in include_detectors: | |
| if detector in detectors: | |
| detectors_to_run.add(detectors[detector]) | |
| else: | |
| raise ValueError(f"Error: {detector} is not a valid detector. Please check the detector name or refer to the documentation for a list of valid detectors.") | |
| detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT) | |
| return detectors_to_run |
No description provided.