Python OOP Content Review CLI Project

pyreview/
├── pyreview/
│   ├── __init__.py
│   ├── __main__.py
│   ├── cli.py           # CLI entry point with argparse
│   ├── analyzer.py      # Core analysis engine
│   ├── checks/
│   │   ├── __init__.py
│   │   ├── encapsulation.py
│   │   ├── inheritance.py
│   │   ├── cohesion.py
│   │   └── naming.py
│   ├── ast_helpers.py   # AST utility functions
│   └── reporter.py      # Output formatting with rich
├── tests/
│   ├── __init__.py
│   ├── conftest.py
│   ├── test_analyzer.py
│   ├── test_encapsulation.py
│   ├── test_inheritance.py
│   ├── test_cohesion.py
│   └── fixtures/
│       ├── good_class.py
│       ├── bad_encapsulation.py
│       ├── bad_inheritance.py
│       └── god_class.py
├── pyproject.toml
├── README.md
└── .gitignore

"""pyreview CLI — Audit Python OOP code for best practices."""
import argparse
import sys
from pathlib import Path

from pyreview.analyzer import Analyzer
from pyreview.reporter import format_report

def main() -> None:
    parser = argparse.ArgumentParser(
        prog="pyreview",
        description="Audit Python files for OOP design best practices.",
    )
    parser.add_argument(
        "path",
        type=Path,
        help="Python file or directory to analyze",
    )
    parser.add_argument(
        "--severity",
        choices=["info", "warning", "error"],
        default="warning",
        help="Minimum severity level to report (default: warning)",
    )
    parser.add_argument(
        "--format",
        choices=["rich", "json", "summary"],
        default="rich",
        help="Output format (default: rich)",
    )
    parser.add_argument(
        "--checks",
        nargs="+",
        choices=["encapsulation", "inheritance", "cohesion", "naming", "all"],
        default=["all"],
        help="Specific checks to run (default: all)",
    )

    args = parser.parse_args()

    if not args.path.exists():
        print(f"Error: '{args.path}' does not exist.", file=sys.stderr)
        sys.exit(1)

    analyzer = Analyzer(severity=args.severity, checks=args.checks)
    results = analyzer.analyze(args.path)
    format_report(results, fmt=args.format)

if __name__ == "__main__":
    main()

"""Core analysis engine that runs checks against Python source files."""
from __future__ import annotations

import ast
from pathlib import Path
from dataclasses import dataclass, field

from pyreview.checks.encapsulation import check_encapsulation
from pyreview.checks.inheritance import check_inheritance
from pyreview.checks.cohesion import check_cohesion
from pyreview.checks.naming import check_naming

@dataclass
class Issue:
    """A single code issue found during analysis."""
    file: str
    line: int
    check: str
    severity: str  # info, warning, error
    message: str
    suggestion: str = ""

@dataclass
class AnalysisResult:
    """Aggregated results from running checks."""
    file: str
    issues: list[Issue] = field(default_factory=list)

    @property
    def error_count(self) -> int:
        return sum(1 for i in self.issues if i.severity == "error")

    @property
    def warning_count(self) -> int:
        return sum(1 for i in self.issues if i.severity == "warning")

    @property
    def info_count(self) -> int:
        return sum(1 for i in self.issues if i.severity == "info")

class Analyzer:
    """Runs OOP best-practice checks against Python source files."""

    SEVERITY_ORDER = {"error": 0, "warning": 1, "info": 2}

    def __init__(self, severity: str = "warning", checks: list[str] | None = None) -> None:
        self.min_severity = severity
        self.active_checks = checks or ["all"]
        self._run_all = "all" in self.active_checks

    def _should_run(self, check_name: str) -> bool:
        return self._run_all or check_name in self.active_checks

    def _passes_severity(self, issue_severity: str) -> bool:
        return self.SEVERITY_ORDER[issue_severity] <= self.SEVERITY_ORDER[self.min_severity]

    def analyze(self, path: Path) -> list[AnalysisResult]:
        """Analyze a file or directory, returning results for each Python file."""
        if path.is_file():
            return [self._analyze_file(path)]
        return [self._analyze_file(f) for f in path.rglob("*.py") if f.is_file()]

    def _analyze_file(self, filepath: Path) -> AnalysisResult:
        source = filepath.read_text(encoding="utf-8")
        tree = ast.parse(source, filename=str(filepath))

        result = AnalysisResult(file=str(filepath))

        checks_to_run = [
            (check_encapsulation, "encapsulation"),
            (check_inheritance, "inheritance"),
            (check_cohesion, "cohesion"),
            (check_naming, "naming"),
        ]

        for check_fn, check_name in checks_to_run:
            if not self._should_run(check_name):
                continue
            for issue in check_fn(tree, str(filepath)):
                if self._passes_severity(issue.severity):
                    result.issues.append(issue)

        return result

"""Checks for encapsulation violations in Python classes."""
from __future__ import annotations

import ast

from pyreview.analyzer import Issue

def check_encapsulation(tree: ast.AST, filepath: str) -> list[Issue]:
    """Detect encapsulation issues: public mutable attributes, missing @property, direct _private access."""
    issues: list[Issue] = []

    for node in ast.walk(tree):
        if not isinstance(node, ast.ClassDef):
            continue

        public_attrs: list[str] = []
        has_init = False

        for item in node.body:
            # Check __init__ for public attribute assignments
            if isinstance(item, ast.FunctionDef) and item.name == "__init__":
                has_init = True
                for stmt in ast.walk(item):
                    if isinstance(stmt, ast.Assign):
                        for target in stmt.targets:
                            if isinstance(target, ast.Attribute) and isinstance(target.value, ast.Name):
                                if target.value.id == "self" and not target.attr.startswith("_"):
                                    public_attrs.append(target.attr)

        # Warn about classes with many public attributes (potential data class / god object)
        if len(public_attrs) > 5:
            issues.append(Issue(
                file=filepath,
                line=node.lineno,
                check="encapsulation",
                severity="warning",
                message=f"Class '{node.name}' has {len(public_attrs)} public attributes. "
                        f"Consider grouping related attributes into smaller classes or using @dataclass.",
                suggestion=f"Break '{node.name}' into smaller, focused classes or use composition.",
            ))

        # Detect direct access to _private attributes outside the defining class
        # (simplified: flag any _prefixed attribute access not on 'self')
        for child in ast.walk(node):
            if isinstance(child, ast.Attribute) and child.attr.startswith("_") and not child.attr.startswith("__"):
                if isinstance(child.value, ast.Name) and child.value.id != "self":
                    issues.append(Issue(
                        file=filepath,
                        line=child.lineno,
                        check="encapsulation",
                        severity="warning",
                        message=f"Accessing private attribute '{child.attr}' from outside the class. "
                                f"Use a public method or @property instead.",
                        suggestion="Add a public method or @property to expose the needed behavior.",
                    ))

    return issues

"""Checks for inheritance issues in Python class hierarchies."""
from __future__ import annotations

import ast

from pyreview.analyzer import Issue

MAX_INHERITANCE_DEPTH = 3

def check_inheritance(tree: ast.AST, filepath: str) -> list[Issue]:
    """Detect deep inheritance chains, missing abstract methods, and LSP violations."""
    issues: list[Issue] = []

    for node in ast.walk(tree):
        if not isinstance(node, ast.ClassDef):
            continue

        # Check inheritance depth
        depth = _inheritance_depth(node, tree)
        if depth > MAX_INHERITANCE_DEPTH:
            issues.append(Issue(
                file=filepath,
                line=node.lineno,
                check="inheritance",
                severity="warning",
                message=f"Class '{node.name}' has inheritance depth {depth} "
                        f"(max recommended: {MAX_INHERITANCE_DEPTH}). "
                        f"Deep hierarchies are hard to maintain — prefer composition.",
                suggestion="Refactor using composition or mixins instead of deep inheritance.",
            ))

        # Check for empty overrides (child overrides parent but just calls super())
        for item in node.body:
            if isinstance(item, ast.FunctionDef) and item.name != "__init__":
                if _is_empty_override(item):
                    issues.append(Issue(
                        file=filepath,
                        line=item.lineno,
                        check="inheritance",
                        severity="info",
                        message=f"Method '{item.name}' in '{node.name}' overrides parent "
                                f"but only calls super(). Remove the override if it adds no behavior.",
                        suggestion="Remove the empty override or add meaningful behavior.",
                    ))

        # Flag classes that inherit from multiple concrete classes
        bases = [b for b in node.bases if isinstance(b, ast.Name)]
        concrete_bases = _count_concrete_bases(bases)
        if concrete_bases > 1:
            issues.append(Issue(
                file=filepath,
                line=node.lineno,
                check="inheritance",
                severity="info",
                message=f"Class '{node.name}' inherits from {concrete_bases} concrete classes. "
                        f"Multiple inheritance can cause MRO confusion.",
                suggestion="Consider using mixins (abstract, no __init__) for additional behavior.",
            ))

    return issues

def _inheritance_depth(cls: ast.ClassDef, tree: ast.AST, current_depth: int = 0) -> int:
    """Recursively compute inheritance depth."""
    max_depth = current_depth
    for base in cls.bases:
        if isinstance(base, ast.Name):
            parent = _find_class(tree, base.id)
            if parent:
                depth = _inheritance_depth(parent, tree, current_depth + 1)
                max_depth = max(max_depth, depth)
    return max_depth

def _find_class(tree: ast.AST, name: str) -> ast.ClassDef | None:
    for node in ast.walk(tree):
        if isinstance(node, ast.ClassDef) and node.name == name:
            return node
    return None

def _is_empty_override(func: ast.FunctionDef) -> bool:
    """Check if a method body only calls super()."""
    body = func.body
    if len(body) == 1 and isinstance(body[0], ast.Expr):
        call = body[0].value
        if isinstance(call, ast.Call) and isinstance(call.func, ast.Attribute):
            if isinstance(call.func.value, ast.Call) and isinstance(call.func.value.func, ast.Name):
                return call.func.value.func.id == "super"
    return False

def _count_concrete_bases(bases: list[ast.Name]) -> int:
    """Count bases that look like concrete classes (not ABC, Protocol, etc.)."""
    abstract_names = {"ABC", "Protocol", "object", "Exception", "BaseException"}
    return sum(1 for b in bases if b.id not in abstract_names)

"""Checks for class cohesion — god classes, unrelated methods, etc."""
from __future__ import annotations

import ast

from pyreview.analyzer import Issue

MAX_METHODS_PER_CLASS = 15
MAX_PUBLIC_METHODS = 10

def check_cohesion(tree: ast.AST, filepath: str) -> list[Issue]:
    """Detect god classes, low cohesion, and classes that do too much."""
    issues: list[Issue] = []

    for node in ast.walk(tree):
        if not isinstance(node, ast.ClassDef):
            continue

        methods = [item for item in node.body if isinstance(item, ast.FunctionDef)]
        public_methods = [m for m in methods if not m.name.startswith("_")]
        total_lines = sum(
            m.end_lineno - m.lineno + 1
            for m in methods
            if m.end_lineno is not None
        )

        # God class: too many methods or too many lines
        if len(methods) > MAX_METHODS_PER_CLASS:
            issues.append(Issue(
                file=filepath,
                line=node.lineno,
                check="cohesion",
                severity="warning",
                message=f"Class '{node.name}' has {len(methods)} methods "
                        f"(max recommended: {MAX_METHODS_PER_CLASS}). "
                        f"This may be a 'God Class' with too many responsibilities.",
                suggestion="Split the class into smaller, single-responsibility classes.",
            ))

        # Too many public methods
        if len(public_methods) > MAX_PUBLIC_METHODS:
            issues.append(Issue(
                file=filepath,
                line=node.lineno,
                check="cohesion",
                severity="info",
                message=f"Class '{node.name}' has {len(public_methods)} public methods. "
                        f"Consider if some can be made private or extracted.",
                suggestion="Make internal helpers private (_method) or extract to a utility module.",
            ))

        # Very large class by line count
        if total_lines > 300:
            issues.append(Issue(
                file=filepath,
                line=node.lineno,
                check="cohesion",
                severity="warning",
                message=f"Class '{node.name}' is {total_lines} lines long. "
                        f"Large classes are hard to test and maintain.",
                suggestion="Extract related behavior into separate classes using composition.",
            ))

    return issues

"""Checks for naming convention violations in OOP code."""
from __future__ import annotations

import ast
import re

from pyreview.analyzer import Issue

# Python naming conventions (PEP 8)
CLASS_NAME_RE = re.compile(r"^[A-Z][a-zA-Z0-9]*$")
METHOD_NAME_RE = re.compile(r"^[a-z_][a-z0-9_]*$")
CONSTANT_RE = re.compile(r"^[A-Z][A-Z0-9_]*$")
BOOLEAN_METHOD_PREFIXES = ("is_", "has_", "can_", "should_", "will_", "does_")

def check_naming(tree: ast.AST, filepath: str) -> list[Issue]:
    """Detect naming convention violations (PEP 8)."""
    issues: list[Issue] = []

    for node in ast.walk(tree):
        # Class names should be PascalCase
        if isinstance(node, ast.ClassDef):
            if not CLASS_NAME_RE.match(node.name):
                issues.append(Issue(
                    file=filepath,
                    line=node.lineno,
                    check="naming",
                    severity="error",
                    message=f"Class '{node.name}' does not follow PascalCase naming convention.",
                    suggestion=f"Rename to PascalCase, e.g., '{_to_pascal(node.name)}'.",
                ))

        # Method names should be snake_case
        if isinstance(node, ast.FunctionDef):
            # Skip dunder methods and test methods
            if node.name.startswith("__") and node.name.endswith("__"):
                continue
            if node.name.startswith("test_"):
                continue

            if not METHOD_NAME_RE.match(node.name):
                issues.append(Issue(
                    file=filepath,
                    line=node.lineno,
                    check="naming",
                    severity="error",
                    message=f"Method '{node.name}' does not follow snake_case naming convention.",
                    suggestion=f"Rename to snake_case, e.g., '{_to_snake(node.name)}'.",
                ))

        # Boolean-returning methods should start with is_/has_/can_
        if isinstance(node, ast.FunctionDef):
            if _returns_bool(node) and not node.name.startswith(BOOLEAN_METHOD_PREFIXES):
                issues.append(Issue(
                    file=filepath,
                    line=node.lineno,
                    check="naming",
                    severity="info",
                    message=f"Method '{node.name}' appears to return a boolean. "
                            f"Consider prefixing with is_/has_/can_ for clarity.",
                    suggestion=f"Rename to 'is_{node.name}' or 'has_{node.name}'.",
                ))

    return issues

def _returns_bool(func: ast.FunctionDef) -> bool:
    """Heuristic: check if function body contains 'return True' or 'return False'."""
    for node in ast.walk(func):
        if isinstance(node, ast.Return) and node.value is not None:
            if isinstance(node.value, ast.NameConstant):
                return isinstance(node.value.value, bool)
            if isinstance(node.value, ast.Constant):
                return isinstance(node.value.value, bool)
    return False

def _to_pascal(name: str) -> str:
    return "".join(word.capitalize() for word in re.split(r"[_\\s-]+", name) if word)

def _to_snake(name: str) -> str:
    result = re.sub(r"([A-Z])", r"_\1", name).lower().strip("_")
    return re.sub(r"_+", "_", result)

"""Output formatting for analysis results using rich."""
from __future__ import annotations

import json
from rich.console import Console
from rich.table import Table
from rich.panel import Panel
from rich.text import Text
from rich import box

from pyreview.analyzer import AnalysisResult

console = Console()

SEVERITY_COLORS = {
    "error": "bold red",
    "warning": "bold yellow",
    "info": "dim cyan",
}

SEVERITY_ICONS = {
    "error": "❌",
    "warning": "⚠️ ",
    "info": "ℹ️ ",
}

def format_report(results: list[AnalysisResult], fmt: str = "rich") -> None:
    """Format and print analysis results."""
    if fmt == "json":
        _format_json(results)
    elif fmt == "summary":
        _format_summary(results)
    else:
        _format_rich(results)

def _format_rich(results: list[AnalysisResult]) -> None:
    total_errors = sum(r.error_count for r in results)
    total_warnings = sum(r.warning_count for r in results)
    total_info = sum(r.info_count for r in results)
    total_files = len(results)

    # Summary panel
    summary_text = Text()
    summary_text.append(f"{total_files} file(s) analyzed", style="bold white")
    if total_errors:
        summary_text.append(f"  |  {total_errors} errors", style="bold red")
    if total_warnings:
        summary_text.append(f"  |  {total_warnings} warnings", style="bold yellow")
    if total_info:
        summary_text.append(f"  |  {total_info} info", style="dim cyan")
    if total_errors == 0 and total_warnings == 0:
        summary_text.append("  |  ✨ All clear!", style="bold green")

    console.print(Panel(summary_text, title="pyreview", border_style="blue"))
    console.print()

    for result in results:
        if not result.issues:
            continue

        table = Table(
            title=f"{result.file}",
            box=box.ROUNDED,
            border_style="dim blue",
            show_header=True,
            header_style="bold",
        )
        table.add_column("Line", style="dim cyan", width=6)
        table.add_column("Severity", width=10)
        table.add_column("Check", style="dim", width=16)
        table.add_column("Issue", ratio=1)

        for issue in sorted(result.issues, key=lambda i: i.line):
            sev_style = SEVERITY_COLORS.get(issue.severity, "white")
            icon = SEVERITY_ICONS.get(issue.severity, "")
            table.add_row(
                str(issue.line),
                f"{icon} {issue.severity}",
                issue.check,
                issue.message,
                style=sev_style,
            )

            if issue.suggestion:
                table.add_row(
                    "",
                    "",
                    "",
                    f"  💡 {issue.suggestion}",
                    style="dim italic",
                )

        console.print(table)
        console.print()

def _format_json(results: list[AnalysisResult]) -> None:
    output = []
    for result in results:
        output.append({
            "file": result.file,
            "errors": result.error_count,
            "warnings": result.warning_count,
            "info": result.info_count,
            "issues": [
                {
                    "line": i.line,
                    "check": i.check,
                    "severity": i.severity,
                    "message": i.message,
                    "suggestion": i.suggestion,
                }
                for i in result.issues
            ],
        })
    console.print_json(data=output)

def _format_summary(results: list[AnalysisResult]) -> None:
    total_errors = sum(r.error_count for r in results)
    total_warnings = sum(r.warning_count for r in results)

    for result in results:
        icon = "✅" if result.error_count == 0 else "❌"
        console.print(
            f"  {icon} {result.file}  "
            f"[red]{result.error_count} errors[/red]  "
            f"[yellow]{result.warning_count} warnings[/yellow]"
        )

    console.print()
    if total_errors == 0 and total_warnings == 0:
        console.print("[bold green]All checks passed! No issues found.[/bold green]")
    else:
        console.print(f"[bold]Total: [red]{total_errors} errors[/red], [yellow]{total_warnings} warnings[/yellow][/bold]")

# Install in development mode
pip install -e .

# Run against a single file
pyreview my_module.py

# Run against a directory
pyreview ./src --severity info

# JSON output for CI pipelines
pyreview ./src --format json

# Only specific checks
pyreview ./src --checks encapsulation inheritance

# Summary mode
pyreview ./src --format summary

"""Shared test fixtures."""
from pathlib import Path

FIXTURES_DIR = Path(__file__).parent / "fixtures"

def load_fixture(name: str) -> str:
    return (FIXTURES_DIR / name).read_text()

class BadEncapsulation:
    def __init__(self):
        self.name = "test"        # public, mutable
        self.age = 25              # public, mutable
        self.email = "[email protected]"    # public, mutable
        self.address = "123 St"    # public, mutable
        self.phone = "555-0000"    # public, mutable
        self.active = True         # public, mutable
        self.role = "admin"        # public, mutable  (7 total = triggers warning)

"""Tests for encapsulation checks."""
import ast
from pyreview.checks.encapsulation import check_encapsulation

def test_detects_too_many_public_attributes():
    source = """
class BadEncapsulation:
    def __init__(self):
        self.name = "test"
        self.age = 25
        self.email = "[email protected]"
        self.address = "123 St"
        self.phone = "555-0000"
        self.active = True
        self.role = "admin"
"""
    tree = ast.parse(source)
    issues = check_encapsulation(tree, "test.py")
    assert len(issues) >= 1
    assert any("public attributes" in i.message for i in issues)

def test_clean_class_passes():
    source = """
class CleanClass:
    def __init__(self, name: str):
        self._name = name

    @property
    def name(self) -> str:
        return self._name
"""
    tree = ast.parse(source)
    issues = check_encapsulation(tree, "test.py")
    assert len(issues) == 0