feat(deslop): add code smell detection patterns (#106) - #115
Conversation
Add high-impact code smell detection patterns: High-Certainty Patterns (regex-based): - boolean_blindness: Functions with 3+ consecutive boolean params - message_chains_methods: Long method chains (4+ calls) - message_chains_properties: Deep property access (5+ levels) - mutable_globals_js: Mutable globals with UPPERCASE names - mutable_globals_py: Mutable global collections in Python Heuristic Patterns (may have false positives): - feature_envy: Method using another object 3+ times - speculative_generality_unused_params: Underscore-prefixed params - speculative_generality_empty_interface: Empty TypeScript interfaces Multi-Pass Analyzers: - analyzeDeadCode: Unreachable code after return/throw/break/continue - analyzeShotgunSurgery: Files frequently changing together (git analysis) Pattern skeleton added for dead_code and shotgun_surgery (requiresMultiPass).
Add tests for new code smell detection patterns:
High-Certainty Pattern Tests:
- boolean_blindness: 3+ consecutive boolean params
- message_chains_methods: 4+ method call chains
- message_chains_properties: 5+ property access chains
- mutable_globals_js: let/var UPPERCASE in JS
- mutable_globals_py: mutable collections in Python
Heuristic Pattern Tests:
- feature_envy: repeated access to another object
- speculative_generality_unused_params: underscore-prefixed params
- speculative_generality_empty_interface: empty TS interfaces
Multi-Pass Analyzer Tests:
- analyzeDeadCode: unreachable code detection (JS, Python, Go, Rust)
- analyzeShotgunSurgery: git co-change analysis
Also:
- Fixed analyzeDeadCode to handle "} else {" patterns
- Added ReDoS safety tests for all new patterns
- Added language filtering tests
…tection (#106) - Add code smell detection section to deslop-around.md - Add CHANGELOG entry for all new patterns and analyzers - Sync slop-patterns.js and slop-analyzers.js to all plugins
Updated the README.md `/deslop-around` section to include the newly added code smell detection patterns: - Boolean blindness - Message chains - Mutable globals - Dead code - Shotgun surgery This ensures the main README is consistent with the detailed documentation in CHANGELOG.md and deslop-around.md.
Summary of ChangesHello @avifenesh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b7a9bd8e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of code smell detection features, including new analyzeDeadCode and analyzeShotgunSurgery functions, along with patterns for boolean blindness, message chains, mutable globals, feature envy, and speculative generality. The changes include extensive test coverage for these new analyzers and patterns, updates to the CHANGELOG.md and README.md to document the new capabilities, and an increase in the severity of infrastructure_without_implementation violations from 'low' to 'high'. However, a review comment points out that the analyzeDeadCode function's current implementation relies on brace-counting for block scope detection, which is incorrect for Python and will lead to inaccurate dead code detection in Python files; it suggests an indentation-based approach for Python to correctly identify dead code after termination statements.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
There was a problem hiding this comment.
Pull request overview
This PR implements high-impact code smell detection patterns for the /deslop-around command as specified in issue #106. The implementation adds 7 new code smell patterns and 2 multi-pass analyzers to detect maintainability issues in codebases.
Changes:
- Added 7 new code smell patterns (boolean blindness, message chains, mutable globals, dead code, shotgun surgery, feature envy, speculative generality) with regex-based and multi-pass detection
- Implemented 2 new multi-pass analyzers:
analyzeDeadCode()for unreachable code detection andanalyzeShotgunSurgery()for git history-based coupling analysis - Updated severity of
infrastructure_without_implementationfrom 'low' to 'high' and improved INSTANTIATION_PATTERNS to support module-qualified constructors
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| lib/patterns/slop-patterns.js | Added 11 new pattern definitions for code smell detection with comprehensive documentation |
| lib/patterns/slop-analyzers.js | Implemented analyzeDeadCode and analyzeShotgunSurgery multi-pass analyzers with language-specific handling |
| plugins/*/lib/patterns/slop-patterns.js | Synced pattern definitions across all 5 plugins |
| plugins/*/lib/patterns/slop-analyzers.js | Synced analyzer implementations across all 5 plugins |
| plugins/deslop-around/commands/deslop-around.md | Updated command documentation with code smell detection section |
| tests/slop-patterns.test.js | Added 370+ test cases for new patterns with ReDoS safety tests |
| tests/slop-analyzers.test.js | Added 630+ test cases for analyzers with edge case coverage |
| README.md | Added code smells to feature list |
| CHANGELOG.md | Documented new features comprehensively |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nals Addresses review feedback from Gemini and Codex: 1. **Python indentation tracking** - Use indentation-based scope detection instead of brace-counting for Python files. Fixes false positives where next function definition was flagged as dead code. 2. **One-line conditional detection** - Skip termination statements that are part of one-line conditionals (e.g., "if (x) return;"). These don't make subsequent code unreachable. 3. **Language-aware block detection** - Maintains brace-depth tracking for JS/Go/Rust while using indentation for Python. Fixes issues reported in PR #115 review.
Fix indentation level check to allow same-level code (dead code) while
stopping only when dedented to a lesser indentation (exited block).
Previous logic stopped at same-or-less indentation, which prevented
detecting dead code at the same level as the return statement.
Now correctly detects:
def test():
return 42
print('unreachable') # Same indentation = dead code
Review Feedback AddressedI've addressed all review comments from Gemini and Codex: Fixed Issues:
Changes Made:
Test Status:
The implementation now correctly handles both brace-based languages (JS, Go, Rust) and indentation-based languages (Python) as requested in the reviews. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 44 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add bounds to regex quantifiers to prevent catastrophic backtracking: - Word boundary \b at start (prevents matching mid-string) - Identifier length limit: 100 chars max - Method argument limit: 200 chars max - Repetition bounds: 4-8 for methods, 5-10 for properties Fixes CI test failure: 'message_chains patterns should resist ReDoS'
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 43 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
- Add commitLimit validation in analyzeShotgunSurgery to prevent command injection - Sync validated changes to all plugin lib copies
Summary
Implements high-impact code smell detection patterns for the
/deslop-aroundcommand.Patterns Added:
Implementation:
lib/patterns/slop-patterns.jslib/patterns/slop-analyzers.jsTest Coverage:
slop-patterns.js: 96.0% coverageslop-analyzers.js: 84.41% coverageTest Plan
npm test- all tests pass./scripts/sync-lib.shRelated Issues
Closes #106