implementation_smells.skill, is a zip file with a "skill"
extension. Its structure is shown in Fig 2.
implementation_smells.skill:
Systematically identify code quality issues, anti-patterns, and implementation smells across multiple programming languages to improve maintainability, readability, and robustness.
Follow this process when analyzing code for implementation smells:
scripts/smell_detector.py on individual filesreferences/smell-catalog.mdRead references/smell-catalog.md to access the comprehensive smell catalog when needed:
Analyze code in this order for comprehensive coverage:
Structure findings clearly:
=== Implementation Smells Analysis ===
HIGH SEVERITY (requires immediate attention):
- [Smell Type] at Line X: [Description]
β Impact: [Why it matters]
π‘ Suggestion: [How to fix]
MEDIUM SEVERITY (should address soon):
- [Similar format]
LOW SEVERITY (nice to have):
- [Similar format]
=== Summary ===
Total: X smell(s) detected
Prioritized recommendations: [Top 3 fixes]
For each detected smell:
Use the provided script for quick automated scanning:
python scripts/smell_detector.py <source_file> [language]
The script detects:
When analyzing language-specific code:
1 #!/usr/bin/env python3
2 """
3 Automated implementation smell detector.
4 Analyzes source code files for common code smells.
5 """
6
7 import re
8 import sys
9 from pathlib import Path
10 from typing import List, Dict, Tuple
11 from dataclasses import dataclass
12
13
14 @dataclass
15 class SmellReport:
16 """Represents a detected code smell."""
17 smell_type: str
18 severity: str # 'high', 'medium', 'low'
19 line_number: int
20 description: str
21 suggestion: str
22
23
24 class SmellDetector:
25 """Detects implementation smells in source code."""
26
27 def __init__(self, language: str):
28 self.language = language.lower()
29 self.reports: List[SmellReport] = []
30
31 def detect_long_function(self, lines: List[str], threshold: int = 50) -> List[SmellReport]:
32 """Detect functions exceeding line count threshold."""
33 smells = []
34
35 # Language-specific function patterns
36 patterns = {
37 'python': r'^\s*def\s+(\w+)',
38 'c++': r'^\s*\w+[\s\*&]+(\w+)\s*\([^)]*\)\s*{?',
39 'rust': r'^\s*fn\s+(\w+)',
40 'c#': r'^\s*\w+\s+(\w+)\s*\([^)]*\)\s*{',
41 }
42
43 pattern = patterns.get(self.language)
44 if not pattern:
45 return smells
46
47 in_function = False
48 func_start = 0
49 func_name = ""
50
51 for i, line in enumerate(lines, 1):
52 match = re.search(pattern, line)
53 if match:
54 # If we were in a function, check its length
55 if in_function and (i - func_start) > threshold:
56 smells.append(SmellReport(
57 smell_type="Long Function",
58 severity="high",
59 line_number=func_start,
60 description=f"Function '{func_name}' is {i - func_start} lines long",
61 suggestion=f"Consider breaking down '{func_name}' into smaller, focused functions"
62 ))
63
64 in_function = True
65 func_start = i
66 func_name = match.group(1)
67
68 return smells
69
70 def detect_deep_nesting(self, lines: List[str], max_depth: int = 4) -> List[SmellReport]:
71 """Detect deeply nested control structures."""
72 smells = []
73
74 for i, line in enumerate(lines, 1):
75 # Count leading whitespace
76 stripped = line.lstrip()
77 if not stripped or stripped.startswith('#') or stripped.startswith('//'):
78 continue
79
80 indent = len(line) - len(stripped)
81
82 # Assume 4 spaces per level (adjust based on detected style)
83 depth = indent // 4
84
85 if depth > max_depth:
86 smells.append(SmellReport(
87 smell_type="Deep Nesting",
88 severity="medium",
89 line_number=i,
90 description=f"Nesting level {depth} exceeds recommended maximum",
91 suggestion="Consider extracting nested logic into separate functions or use early returns"
92 ))
93
94 return smells
95
96 def detect_magic_numbers(self, lines: List[str]) -> List[SmellReport]:
97 """Detect hardcoded numeric literals (excluding 0, 1, -1)."""
98 smells = []
99
100 # Numbers to ignore (common constants)
101 ignore = {0, 1, -1, 2}
102
103 for i, line in enumerate(lines, 1):
104 # Skip comments and strings
105 if re.search(r'^\s*[#//]', line) or '"' in line or "'" in line:
106 continue
107
108 # Find numeric literals
109 numbers = re.findall(r'\b(\d+\.?\d*)\b', line)
110 for num_str in numbers:
111 try:
112 num = float(num_str)
113 if num not in ignore and num > 1:
114 smells.append(SmellReport(
115 smell_type="Magic Number",
116 severity="low",
117 line_number=i,
118 description=f"Magic number '{num_str}' used without named constant",
119 suggestion=f"Consider extracting '{num_str}' to a named constant"
120 ))
121 except ValueError:
122 pass
123
124 return smells
125
126 def detect_unclear_naming(self, lines: List[str]) -> List[SmellReport]:
127 """Detect unclear variable/function names."""
128 smells = []
129
130 # Problematic name patterns
131 unclear_patterns = [
132 r'\b(x|y|z|i|j|k)\s*=', # Single letter vars (except in loops)
133 r'\b(temp|tmp|data|info|val|var)\d*\s*=',
134 r'\b(foo|bar|baz)\s*=',
135 ]
136
137 for i, line in enumerate(lines, 1):
138 # Skip for loops (i, j, k are acceptable)
139 if 'for' in line and any(x in line for x in ['i', 'j', 'k']):
140 continue
141
142 for pattern in unclear_patterns:
143 if re.search(pattern, line):
144 smells.append(SmellReport(
145 smell_type="Unclear Naming",
146 severity="medium",
147 line_number=i,
148 description="Variable has unclear or generic name",
149 suggestion="Use descriptive names that reveal intent"
150 ))
151 break
152
153 return smells
154
155 def detect_all(self, code: str) -> List[SmellReport]:
156 """Run all smell detectors on the code."""
157 lines = code.split('\n')
158
159 all_smells = []
160 all_smells.extend(self.detect_long_function(lines))
161 all_smells.extend(self.detect_deep_nesting(lines))
162 all_smells.extend(self.detect_magic_numbers(lines))
163 all_smells.extend(self.detect_unclear_naming(lines))
164
165 # Sort by line number
166 all_smells.sort(key=lambda s: s.line_number)
167
168 return all_smells
169
170
171 def analyze_file(filepath: Path, language: str = None) -> List[SmellReport]:
172 """Analyze a source file for implementation smells."""
173
174 # Auto-detect language from extension if not provided
175 if not language:
176 ext_to_lang = {
177 '.py': 'python',
178 '.cpp': 'c++',
179 '.cc': 'c++',
180 '.cxx': 'c++',
181 '.rs': 'rust',
182 '.cs': 'c#',
183 }
184 language = ext_to_lang.get(filepath.suffix.lower(), 'unknown')
185
186 with open(filepath, 'r', encoding='utf-8') as f:
187 code = f.read()
188
189 detector = SmellDetector(language)
190 return detector.detect_all(code)
191
192
193 def format_report(smells: List[SmellReport], filepath: Path) -> str:
194 """Format smell reports for display."""
195 if not smells:
196 return f"β No implementation smells detected in {filepath.name}"
197
198 output = [f"\n=== Implementation Smells in {filepath.name} ===\n"]
199
200 # Group by severity
201 by_severity = {'high': [], 'medium': [], 'low': []}
202 for smell in smells:
203 by_severity[smell.severity].append(smell)
204
205 for severity in ['high', 'medium', 'low']:
206 items = by_severity[severity]
207 if not items:
208 continue
209
210 output.append(f"\n{severity.upper()} SEVERITY ({len(items)} found):")
211 for smell in items:
212 output.append(f"\n Line {smell.line_number}: {smell.smell_type}")
213 output.append(f" β¦ {smell.description}")
214 output.append(f" ?? {smell.suggestion}")
215
216 output.append(f"\n\nTotal: {len(smells)} smell(s) detected")
217 return '\n'.join(output)
218
219
220 if __name__ == '__main__':
221 if len(sys.argv) < 2:
222 print("Usage: python smell_detector.py <source_file> [language]")
223 sys.exit(1)
224
225 filepath = Path(sys.argv[1])
226 language = sys.argv[2] if len(sys.argv) > 2 else None
227
228 if not filepath.exists():
229 print(f"Error: File '{filepath}' not found")
230 sys.exit(1)
231
232 smells = analyze_file(filepath, language)
233 print(format_report(smells, filepath))
This document catalogs common implementation smells across multiple programming languages, organized by category.
x, temp, data, info, single letters (except loop counters)new/delete instead of smart pointersexcept: without exception typewith.clone() everywhere to satisfy borrow checker.unwrap() instead of proper error handling 1
2 ?
3 |
4 ?β? Claude Code v2.1.45
5 β?βββ?β Sonnet 4.6 Β· Claude Pro
6 ??βββββ?? C:\github\Jimfawcett\NewSite\code\AI
7 ?? ??
8
9 ? analyze code smells for /DemoAgent-Claude2/Test/RustDirNav
10
11 ? Skill(implementation-smells)
12 ? Β Successfully loaded skill
13
14 ? Searched for 1 pattern, read 3 files (ctrl+o to expand)
15
16 ? Now let me run the automated detector on both source files in parallel.
17
18 ? 2 Bash agents finished (ctrl+o to expand)
19 ββ Run smell detector on lib.rs Β· 1 tool use Β· 5.6k tokens
20 β ? Β Done
21 ββ Run smell detector on test1.rs Β· 1 tool use Β· 5.6k tokens
22 ? Β Done
23
24 ? Now I have everything needed for a thorough manual + automated analysis.
25
26 ---
27 Implementation Smells Analysis - RustDirNav
28
29 Files analyzed: src/lib.rs (259 lines), examples/test1.rs (77 lines)
30
31 ---
32 HIGH SEVERITY
33
34 1. Bitwise OR instead of logical OR - lib.rs:136
35 if self.in_patterns(&entry) | self.pats.is_empty()
36 Uses | (bitwise/non-short-circuit) instead of || (logical, short-circuit). For bool both produce the
37 same result, but | always evaluates both sides - pats.is_empty() is called even when in_patterns is
38 already true. This is a readability and correctness risk: future maintainers may read | as intentional
39 non-short-circuit and be confused, or a side-effecting predicate added later would behave unexpectedly.
40
41 Fix: if self.in_patterns(&entry) || self.pats.is_empty()
42
43 ---
44 MEDIUM SEVERITY
45
46 2. Redundant where clauses - lib.rs:65-67, lib.rs:118-119
47
48 The impl<App: DirEvent + Default> block already declares the bounds. Both new() and visit() repeat them
49 redundantly:
50 pub fn new() -> Self where App: DirEvent + Default { ... } // redundant
51 pub fn visit(&mut self, dir: &Path) -> io::Result<()> where App: DirEvent { ... } // redundant
52 Fix: Remove the where clauses on both methods.
53
54 3. #[allow(dead_code)] / #[allow(unused_imports)] suppressing warnings - lib.rs:18,47
55
56 #[allow(unused_imports)] on use std::path::{Path, PathBuf} and #[allow(dead_code)] on DirNav hide real
57 signals. PathBuf is used internally but the allow attribute suggests it was added hastily. Suppressing
58 instead of resolving makes it easy to miss genuine dead code added later.
59
60 Fix: Remove the allow attributes; verify each import/field is needed, remove what isn't.
61
62 4. Test ordering dependency - lib.rs:182-183
63 // test_setup() should run first. To ensure that:
64 // use cargo -- --test-threads=1
65 Tests requiring a specific execution order and single-threaded mode is a brittle design. test_walk
66 silently passes if test_setup hasn't run (it may find pre-existing files from a prior run), and fails
67 on a clean checkout with no indication why.
68
69 Fix: Make test_walk call setup logic directly, or use a #[test] fixture pattern (std::sync::Once +
70 OnceLock) to guarantee initialization before dependent tests.
71
72 5. num_file counts ALL files, not just matching ones - lib.rs:135
73 self.num_file += 1; // incremented for every file
74 if self.in_patterns(&entry) | ... {
75 files.push(entry.file_name()); // only matching files added
76 }
77 get_files() returns a count that includes non-matching files, while the user typically expects "files
78 found/matched." The field name and doc comment say "number of files processed" - ambiguous, and callers
79 comparing get_files() to app results will be confused.
80
81 Fix: Either rename to num_files_seen and add a separate num_files_matched, or increment only when the
82 file matches a pattern.
83
84 6. _-prefixed variables that are actively used - test1.rs:33,34
85 let _pat1: String = "rs".to_string();
86 let _pat4: String = "rlib".to_string();
87 // ...
88 dn.add_pat(&_pat1); // _pat1 IS used
89 dn.add_pat(_pat4); // _pat4 IS used
90 The _ prefix idiomatically signals "intentionally unused." These variables are clearly used. This
91 misleads readers and masks future real unused-variable warnings.
92
93 Fix: Rename to pat1, pat4.
94
95 ---
96 LOW SEVERITY
97
98 7. Explicit return Ok(()) in non-tail position - lib.rs:157
99
100 return Ok(()); // normal return
101 }
102 Err(Error::new(...))
103 Idiomatic Rust uses Ok(()) as the implicit expression of the last statement in an if branch rather than
104 an explicit return. Minor style issue but inconsistent with Rust conventions.
105
106 8. Double string conversion - lib.rs:123, lib.rs:146
107 self.replace_sep(dir).to_string_lossy().to_string()
108 replace_sep already returns an OsString, calling .to_string_lossy() yields a Cow<str>, then
109 .to_string() allocates again. This is a minor allocation smell on a hot path (called per directory).
110
111 9. /// doc-comment syntax used for regular comments - test1.rs:67-71
112 ///////////////////////////////////////////////
113 // uncomment lines below to see error return
114 The leading /// line is a doc comment. While it doesn't cause a bug here, doc comment syntax should be
115 reserved for API documentation, not inline banners/notes.
116
117 10. Manual Default impl for unit struct - test1.rs:21-25
118 impl Default for Appl {
119 fn default() -> Self { Appl }
120 }
121 Appl is a unit struct; #[derive(Default)] would express the same intent with less code.
122
123 ---
124 Automated Detector Notes
125
126 The script flagged a false positive: do_file appearing as "168 lines long" - this is a measurement
127 error (the tool measured from the trait definition line to end of file rather than the method body,
128 which is 2 lines). The deep-nesting flags at lines 132-154 are real but borderline; the 4-level nesting
129 in visit is typical for filesystem traversal code.
130
131 ---
132 Summary
133
134 ββββββββββββ¬ββββββββ¬βββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
135 β Severity β Count β Top Priority β
136 ββββββββββββΌββββββββΌβββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
137 β High β 1 β Bitwise | instead of || (line 136) β
138 ββββββββββββΌββββββββΌβββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
139 β Medium β 5 β Test ordering, file count semantics, _ variable naming β
140 ββββββββββββΌββββββββΌβββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
141 β Low β 4 β Style and minor allocation issues β
142 ββββββββββββ΄ββββββββ΄βββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
143
144 Top 3 fixes by impact:
145 1. Line 136 - Change | β¦ || (correctness risk)
146 2. num_file semantics - Clarify what the counter actually counts
147 3. Test ordering - Eliminate the --test-threads=1 requirement
148
149 ? SautΓ©ed for 2m 29s
150
151 βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
152 ?Β fix the bitwise OR smell on line 136
153 βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
154 ? for shortcuts