Remove deprecated flags and their migration. #2410
Conversation
DarkaMaul
requested review from
montyly,
0xalpharush and
smonicas
as code owners
WalkthroughThe recent modifications streamline the Slither tool's configuration handling by removing outdated options and simplifying the process of reading config files. Additionally, there's a tweak in the tool's behavior towards failure conditions and path filtering in tests, making it more flexible in handling specific issues and file paths. Changes
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: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- slither/utils/command_line.py (3 hunks)
- tests/e2e/config/test_path_filtering/slither.config.json (1 hunks)
Additional comments not posted (4)
tests/e2e/config/test_path_filtering/slither.config.json (2)
5-5: Change in failure condition setting to not fail on any specific condition enhances configuration flexibility.
6-6: Adjustment in path filtering criteria likely improves test efficiency or relevance.slither/utils/command_line.py (2)
Line range hint
1-1: Ensure all references to removed deprecated flags and themigrate_config_optionsfunction have been cleaned up.Verification successful
The executed script searched the codebase for any remaining references to
deprecated_flagsor themigrate_config_optionsfunction and did not produce any output. This indicates that these references have likely been successfully removed or are not present in the codebase, aligning with the intended changes mentioned in the review comment.Given this information, it appears that the removal of deprecated flags and the migration handler function has been handled correctly, with no lingering references found in the Python files of the project.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to deprecated flags or the migrate_config_options function. rg --type py 'deprecated_flags|migrate_config_options'Length of output: 56
Line range hint
1-1: Review the impact of removing deprecated functionality on the overall configuration and testing process.
| @@ -2,6 +2,6 @@ | |||
| "detectors_to_run": "all", | |||
| "exclude_informational": true, | |||
| "exclude_low": true, | |||
| "fail_pedantic": false, | |||
| "fail_on": "none", | |||
There was a problem hiding this comment.
I think this is different than the current default and may mess up CI usage
cc @elopez
There was a problem hiding this comment.
this is config for a test though, right? it's not a default
There was a problem hiding this comment.
if key.startswith("fail_") and getattr(args, "fail_on") == defaults_flag_in_config["fail_on"]:
if key == "fail_pedantic":
pedantic_setting = elem
fail_on = FailOnLevel.PEDANTIC if pedantic_setting else FailOnLevel.NONE
setattr(args, "fail_on", fail_on)
logger.info(f"Migrating fail_pedantic: {pedantic_setting} as fail_on: {fail_on.value}")The original migration was like this so I think fail_pedantic: False was migrated to fail_on: None
This PR solves #2351
It removes the deprecated flags, the migration handler and their usage in some test file.
Summary by CodeRabbit