Conversation
|
Important Auto Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 (
|
|
|
It'd be great to put together better testing infra for our CLI so we can feel confident about this and also changes like #2428 (and quickly iterate on them in the future) |
Resolve conflicts between typer migration PR and master changes: - Keep typer-based CLI implementation for all tools - Accept master's deletion of setup.py (migrated to hatchling) - Accept master's deletion of scripts/ci_test.sh (unused) - Update CI test scripts with correct paths and typer subcommand syntax - Resolve documentation README rename conflict (keep in docs/src/tools/) - Remove slither/utils/codex.py (codex functionality reorganized in PR) - Keep PR's codex detector implementation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
# Conflicts: # scripts/ci_test_etherscan.sh # scripts/ci_test_interface.sh
- Fix all standalone entry points by adding main() wrapper functions that call the Typer app (12 tools fixed) - Move --no-fail flag from global options to print command since it only applies to echidna printer - Add missing parse_target_selectors function to mutator module - Add CLI testing infrastructure with 52 tests covering: - All subcommands have working help - All standalone entry points work - Flag placement verification - Global options validation Addresses review comments from @0xalpharush on PR #2452. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add typer>=0.12.0 to project dependencies (required for CLI) - Add noqa comments for pre-existing E402 and B019 lint errors - Fix F541 f-string without placeholder errors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
pkg_resources from setuptools is deprecated and not always available. Use importlib_metadata backport for Python < 3.10 instead. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Typer uses rich formatting which includes ANSI escape codes in the output. The tests now strip these codes before checking for expected strings. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add default_map pattern in __main__.py to propagate config file values to detect and print subcommands - Add --config as alias for --config-file option - Fix fail_on string to FailOnLevel enum conversion in read_config_file - Fix CommaSeparatedValueParser to handle list inputs from default_map - Remove protected_args modification (read-only in newer Click versions) - Add explicit exit codes to interface, possible_paths, erc_conformance, and upgradeability tools - Move logging.basicConfig() before slither imports in tools to suppress CryticCompile INFO messages - Fix properties tool to use tool-specific logger instead of modifying shared Slither logger - Update CI scripts for Typer CLI argument order (options before positional) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Resolve merge conflict in slither/tools/erc_conformance/__main__.py by keeping the Typer implementation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The imports are intentionally placed after logging.basicConfig() to suppress CryticCompile INFO messages before slither modules are loaded. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Typer validates enum options against their string values, so we need to pass the string value (e.g., 'pedantic') not the enum object (e.g., FailOnLevel.PEDANTIC) in the default_map. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Configure logging to suppress CryticCompile INFO-level messages before importing crytic_compile, ensuring the "running" and compilation messages don't appear in tool output. This fixes CI e2e tests that compare output against expected files. Changes: - Move logging.basicConfig() before crytic_compile import - Set CryticCompile logger to WARNING level early - Change configure_logger() to use WARNING instead of INFO for CryticCompile - Add noqa: E402 comments for imports after logging config Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Clear all handlers from the CryticCompile logger before adding the WARNING-level handler to prevent any existing handlers from outputting INFO-level messages that would break CI tests comparing output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
d13dc72 to
e99c39b
Compare
Code Review for PR #2452 (Typer Migration)Bugs Found
CLAUDE.md Violations
🤖 Generated with Claude Code |
Resolve conflicts: - slither/__main__.py: Keep refactored detect() with helper functions, add console_handler setup from master - slither/tools/similarity/model.py: Use lowercase fasttext import with master's improved error messages - slither/utils/command_line.py: Take master's version (removed MarkdownRoot class, added output functions) - uv.lock: Take master's version Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Description
This PR replaces #2220
1. Replace
argparsewithtyperModify Slither CLI from
argparseto Typer.With this migration we gain :
slither c<TAB>)I kept the
crytic-compilemodule as if, and created a backward compatibility layer with it.Notable points :
slither) or with one of its subcommand (detect). For instance, the formatting or the output file.slither .has been added, converting it to the new format :slither detect ..SlitherStateobject is accessible. It contains every parsed argument on the upper command to be accessed by the sub commands if needed. For instance, this is where are stored thecrytic-compilearguments.2. Update tools to use sub-commands
Each
slithertool now has its own subcommand.Use like
slither mutate <>Screenshots
Remaining TODO
While I tried to test the changes I made, it is touching almost every part of the code. Moreover, it contains breaking changes. So we should try to test a bit more (and on more than a single computer) before merging.