Added PreSlitherValidator for contract syntax validation and fixes #106
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new PreSlitherValidator module that reads a Solidity file, runs a pipeline of fixers (SPDX, abstract/virtual adjustments, Changes
Sequence Diagram(s)sequenceDiagram
actor Caller
participant Validator as PreSlitherValidator
participant FS as Filesystem
Caller->>Validator: validate_and_fix_contract(file_path)
Validator->>FS: Read file (UTF-8)
Validator->>Validator: _apply_all_fixes(content, file_path)
alt Changes detected
Validator->>FS: Create backup (<path>.pre_slither_backup)
Validator->>FS: Write updated content
Validator-->>Caller: (True, updated_content, fixes)
else No changes
Validator-->>Caller: (True, original_content, fixes)
end
opt Error
Validator-->>Caller: (False, error_message, fixes)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (9)
Static_agent/Slither_agent/pre_slither_validator.py (9)
11-16: Prune unused imports.
os,subprocess, andDictare unused. Keepdatetime(used after backup change below).-import os -import re -import shutil -import subprocess -from datetime import datetime -from typing import Tuple, List, Dict +import re +import shutil +from datetime import datetime +from typing import Tuple, List
1-1: Shebang without execute bit.Either make the file executable or drop the shebang since this is a module.
-#!/usr/bin/env python3
60-63: Anchor SPDX check to file start and accept block comments.Current regex matches anywhere and only the
//form. Solidity accepts//or/* */, and the identifier should be at the file start.- if not re.search(r'//\s*SPDX-License-Identifier:', content): + if not re.match(r'^\s*(?://|/\*)\s*SPDX-License-Identifier:', content): content = '// SPDX-License-Identifier: MIT\n' + content fixes.append("Added SPDX license identifier")
41-49: Avoid overwriting backups across runs; add a timestamp suffix.Current backup path is constant and will be overwritten on repeated validations.
- if content != original_content: - backup_path = f"{file_path}.pre_slither_backup" + if content != original_content: + ts = datetime.utcnow().strftime("%Y%m%dT%H%M%SZ") + backup_path = f"{file_path}.pre_slither_backup.{ts}" shutil.copy2(file_path, backup_path)
52-53: Avoid blindexcept Exception; narrow to expected errors and improve message.Catching all exceptions hides programmer errors in fixers.
- except Exception as e: - return False, str(e), fixes + except (OSError, UnicodeError, ValueError) as e: + return False, f"{e.__class__.__name__}: {e}", fixes
50-53: Optional: move success return to atry/elsefor clarity (ruff TRY300).Not functional, just style.
- return True, content, fixes - - except (OSError, UnicodeError, ValueError) as e: - return False, f"{e.__class__.__name__}: {e}", fixes + except (OSError, UnicodeError, ValueError) as e: + return False, f"{e.__class__.__name__}: {e}", fixes + else: + return True, content, fixes
55-55: Silence ARG002 by marking unused parameter.
file_pathis not used inside_apply_all_fixes. Keep the arg for future use but mark it unused.- def _apply_all_fixes(self, content: str, file_path: str) -> Tuple[str, List[str]]: + def _apply_all_fixes(self, content: str, _file_path: str) -> Tuple[str, List[str]]:
18-21:self.fixes_appliedis never used; either remove it or aggregate per-run fixes.If you want cumulative visibility across files, extend it.
def __init__(self): - self.fixes_applied = [] + self.fixes_applied = [] @@ - content, file_fixes = self._apply_all_fixes(content, file_path) - fixes.extend(file_fixes) + content, file_fixes = self._apply_all_fixes(content, file_path) + fixes.extend(file_fixes) + self.fixes_applied.extend(file_fixes)Also applies to: 36-39
31-33: Optional: handle UTF‑8 BOM on read.Some Solidity files include a BOM; using
utf-8-sigavoids injecting the SPDX line ahead of stray\ufeff.- with open(file_path, 'r', encoding='utf-8') as f: + with open(file_path, 'r', encoding='utf-8-sig') as f: content = f.read()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Static_agent/Slither_agent/pre_slither_validator.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
Static_agent/Slither_agent/pre_slither_validator.py
1-1: Shebang is present but file is not executable
(EXE001)
50-50: Consider moving this statement to an else block
(TRY300)
52-52: Do not catch blind exception: Exception
(BLE001)
55-55: Unused method argument: file_path
(ARG002)
suratkhan
force-pushed
the
feature/stag-slither-validator-start
branch
from
eed7784 to
5e8e6be
Compare
Summary by CodeRabbit