You run a SonarQube scan. It passes. Your linter shows green. Your CI pipeline gives you a thumbs-up. Yet, your team's velocity is dropping. Every new feature requires untangling a knot of dependencies. Bug fixes in one module inexplicably break another. The code is "clean" by every automated metric you have, but it feels wrong.
You're not imagining it. Most off-the-shelf static analysis tools are optimized to catch syntax errors, security vulnerabilities, and style guide violations. They are terrible at identifying the deeper, structural design flaws—the real code smells—that make software expensive to maintain. They measure the width of the tires while ignoring the cracked engine block.
"Tools measure what is easy to measure, not what is important. A method with 50 lines is not a problem. A method that secretly orchestrates 15 other objects is a time bomb." – Senior Staff Engineer, Large FinTech
This tutorial is for engineering leads and senior developers who are tired of superficial metrics. We will define five high-impact structural code smells, write detectors for them using concrete code examples, and build a workflow to surface these issues before they metastasize. We'll use Python for examples, but the concepts are language-agnostic.
The Five Smells That Actually Matter
Forget "too many parameters" or "long method" for a moment. Those are symptoms, not diseases. We're hunting for the design patterns that create systemic fragility.
- Inappropriate Intimacy: One class uses the internal fields and methods of another class, bypassing its public interface. This creates a tight, hidden coupling.
- Shotgun Surgery: A single change requires making many small edits to many different classes. The responsibility is scattered.
- Feature Envy: A method seems more interested in the data of another class than the data of its own class. It calls numerous getters on a foreign object.
- Message Chains: Client code navigates a long series of object connections (e.g.,
user.getAccount().getSettings().getEmail().getDomain()). This binds the client to the entire structure. - Cyclic Dependency: Two or more modules depend on each other directly or indirectly, making them impossible to understand, test, or deploy in isolation.
Step 1: Detecting Inappropriate Intimacy with AST Parsing
Linters check if a variable is private (_prefix in Python). They don't check who is accessing it. To find true intimacy, we need to see which external classes are reaching into another class's non-public members.
We'll use Python's ast module to build a simple but powerful detector.
Example of the Smell
# File: order.py
class Order:
def __init__(self):
self._total = 0 # Protected member
self.items = []
def calculate_total(self):
self._total = sum(item.price for item in self.items)
return self._total
# File: invoice.py
class InvoicePrinter:
def print_invoice(self, order):
# VIOLATION: Directly accessing a protected member of Order
tax = order._total * 0.08 # Should use order.calculate_total()
print(f"Total: {order._total}, Tax: {tax}")
The Detection Script
Create a file called detect_intimacy.py.
import ast
import os
from collections import defaultdict
class IntimacyDetector(ast.NodeVisitor):
def __init__(self):
self.current_class = None
self.class_members = defaultdict(set) # class_name -> {members}
self.violations = [] # (line, col, message)
def visit_ClassDef(self, node):
self.current_class = node.name
self.gather_class_members(node)
self.generic_visit(node)
self.current_class = None
def gather_class_members(self, node):
for item in node.body:
if isinstance(item, ast.FunctionDef):
self.class_members[self.current_class].add(item.name)
elif isinstance(item, ast.Assign):
for target in item.targets:
if isinstance(target, ast.Name):
self.class_members[self.current_class].add(target.id)
def visit_Attribute(self, node):
# Check if node.attr starts with '_' (protected/private)
if isinstance(node.attr, str) and node.attr.startswith('_'):
# Try to get the variable name being accessed (e.g., 'order' in 'order._total')
if isinstance(node.value, ast.Name):
accessed_class_var = node.value.id
# If we're in a different class and accessing another class's private member
if (self.current_class and
accessed_class_var in self.class_members and
accessed_class_var != self.current_class):
self.violations.append(
(node.lineno, node.col_offset,
f"Class '{self.current_class}' inappropriately accesses "
f"'{accessed_class_var}.{node.attr}'")
)
self.generic_visit(node)
def scan_file(filepath):
with open(filepath, 'r') as f:
tree = ast.parse(f.read(), filename=filepath)
detector = IntimacyDetector()
detector.visit(tree)
return detector.violations
# Example usage
if __name__ == "__main__":
violations = scan_file("invoice.py")
for line, col, msg in violations:
print(f"Line {line}:{col} - {msg}")
Running this on invoice.py would output: Line X:Y - Class 'InvoicePrinter' inappropriately accesses 'order._total'. This is a genuine, tool-missed design flaw.
Step 2: Catching Shotgun Surgery with Dependency Graph Analysis
This smell requires a project-wide view. When you change the EmailService, how many files have you touched over the last six months? We can approximate this by looking at co-change frequency from version control, but for a static check, we can look for feature-oriented cohesion.
A heuristic: If many classes in different directories all import a single common utility class (e.g., StringFormatter), that's fine. If many classes in different directories all import each other in a tangled web related to a single feature (e.g., "user notification"), that's likely Shotgun Surgery.
We can use pydeps to create a visual graph, but for automation, we need a metric.
# Example of scattered feature logic
# File: notifications/email_sender.py
from user.profile import UserProfile # Import from different domain
from content.models import Post # Import from another domain
class EmailSender:
def send_welcome(self, user_profile: UserProfile): ...
def send_post_alert(self, post: Post): ...
# File: user/profile_manager.py
from notifications.email_sender import EmailSender # Import back
class ProfileManager:
def create_user(self):
...
EmailSender().send_welcome(new_user) # Feature logic here?
The "user notification" feature is now split across notifications, user, and content packages. A change to the welcome email logic might require edits in all three.
Detection Strategy
1. Build an import graph for your project.
2. Cluster classes/modules using a community detection algorithm (like Louvain).
3. Flag clusters where the nodes (classes) are spread across many different high-level directories/packages. This indicates a feature is architecturally scattered.
This is more advanced but can be scripted using networkx. The key insight is to move beyond single-file analysis.
Step 3: Building a Custom CI Pipeline Gate
Finding smells is useless if you don't stop the rot. Integrate your custom detectors into your review process.
GitHub Actions Workflow Example
# .github/workflows/code-smell-check.yml
name: Deep Code Smell Analysis
on: [pull_request]
jobs:
analyze:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.10'
- name: Install dependencies
run: pip install astunparse networkx
- name: Run Standard Linter
run: pylint --fail-under=7.0 $(git ls-files '*.py')
continue-on-error: true # Basic lint is advisory
- name: Run Structural Smell Detector
run: python scripts/detect_structural_smells.py
# This step MUST fail the build on critical findings
- name: Upload Report
uses: actions/upload-artifact@v3
if: always()
with:
name: structural-smell-report
path: reports/smell_report.md
Your detect_structural_smells.py script should exit with a non-zero code if it finds any "High Severity" smells (like pervasive Cyclic Dependencies or Inappropriate Intimacy in core modules). Treat these with the same seriousness as a critical security bug.
Step 4: From Detection to Refactoring
Detection is only half the battle. You must provide a clear path to fix.
| Smell | Detection Signal | First Refactoring Step |
|---|---|---|
| Inappropriate Intimacy | Class A accesses _member of Class B. |
In Class B, create a public method that encapsulates the needed behavior. Does such a method exist? If not, why was the member accessed? The answer reveals missing abstraction. |
| Shotgun Surgery | Feature X touches N classes across M packages. | Apply the "Gather Feature" spike. Temporarily create a new directory/package named feature_x/. Over one sprint, move all related code into it. The friction you feel reveals the hidden dependencies. |
| Feature Envy | Method m() in Class A calls 5+ getters from Class B. |
Use the "Move Method" refactoring. Does the method want to be in Class B? If not, consider "Extract Method" to create a helper in Class B that m() can call with a single command. |
The Limits of Automation and the Role of Deep Scanners
The scripts above are illustrative starting points. Scaling this to a million-line codebase with multiple languages requires sophisticated tooling that builds a complete abstract syntax tree (AST) and control flow graph for the entire project. This is where platforms like Codequiry move beyond simple similarity checking and into deep code integrity analysis, mapping the actual semantic and structural relationships that define health.
However, even the best automated tool cannot replace critical human judgment. The goal is not to create a police state for code. The goal is to surface the hidden cost centers in your codebase so you can make informed decisions about where to invest refactoring effort. A 500-line "god class" that is stable, well-tested, and rarely changed is less of a priority than a 50-line class that is intimately coupled to five others and modified every week.
Start small. Pick one smell—Inappropriate Intimacy is a great candidate—and run a detector across your core module this week. The results will surprise you. They will also give you the concrete, irrefutable data you need to argue for dedicated time to fix the foundation, instead of just painting the walls.