diff --git a/desloppify/app/commands/show/formatting.py b/desloppify/app/commands/show/formatting.py index cc159add..500fb2f9 100644 --- a/desloppify/app/commands/show/formatting.py +++ b/desloppify/app/commands/show/formatting.py @@ -31,8 +31,13 @@ ] -def format_detail(detail: dict) -> list[str]: - """Build display parts from a finding's detail dict.""" +def format_detail(detail: object) -> list[str]: + """Build display parts from a finding's detail payload.""" + if isinstance(detail, str): + return [f"detail: {detail}"] if detail else [] + if not isinstance(detail, dict): + return [] + parts = [] for key, label, formatter in DETAIL_DISPLAY: value = detail.get(key) @@ -60,8 +65,6 @@ def format_detail(detail: dict) -> list[str]: def suppressed_match_estimate(pattern: str, hidden_by_detector: dict[str, int]) -> int: """Estimate hidden-match count for a show pattern using detector-level noise totals.""" - if not isinstance(pattern, str) or not isinstance(hidden_by_detector, dict): - return 0 detector = pattern.split("::", 1)[0] return int(hidden_by_detector.get(detector, 0)) diff --git a/desloppify/app/commands/show/render.py b/desloppify/app/commands/show/render.py index 3d7ebe8d..03fcb8a9 100644 --- a/desloppify/app/commands/show/render.py +++ b/desloppify/app/commands/show/render.py @@ -60,7 +60,8 @@ def _print_single_finding(finding: dict, *, show_code: bool) -> None: if detail_parts: print(colorize(f" {' · '.join(detail_parts)}", "dim")) if show_code: - detail = finding.get("detail", {}) + detail_raw = finding.get("detail", {}) + detail = detail_raw if isinstance(detail_raw, dict) else {} target_line = ( detail.get("line") or (detail.get("lines", [None]) or [None])[0] ) diff --git a/desloppify/languages/_framework/treesitter/phases.py b/desloppify/languages/_framework/treesitter/phases.py index ef4d9267..2cec2f80 100644 --- a/desloppify/languages/_framework/treesitter/phases.py +++ b/desloppify/languages/_framework/treesitter/phases.py @@ -80,7 +80,11 @@ def run(path, lang): f"{e['component_count']} disconnected function clusters " f"({e['function_count']} functions) — likely mixed responsibilities" ), - detail=f"Clusters: {families}", + detail={ + "cluster_count": e["component_count"], + "family": families, + "families": e["families"], + }, )) if entries: potentials["responsibility_cohesion"] = len(entries) diff --git a/desloppify/languages/go/__init__.py b/desloppify/languages/go/__init__.py index 04b6c014..0fab0dd3 100644 --- a/desloppify/languages/go/__init__.py +++ b/desloppify/languages/go/__init__.py @@ -7,7 +7,7 @@ from __future__ import annotations from desloppify.core._internal.text_utils import get_area -from desloppify.engine.policy.zones import COMMON_ZONE_RULES, Zone, ZoneRule +from desloppify.engine.policy.zones import COMMON_ZONE_RULES, FileZoneMap, Zone, ZoneRule from desloppify.hook_registry import register_lang_hooks from desloppify.languages import register_lang from desloppify.languages._framework.base.phase_builders import ( @@ -22,12 +22,18 @@ from desloppify.languages.go import test_coverage as go_test_coverage_hooks from desloppify.languages.go.commands import get_detect_commands from desloppify.languages.go.detectors.deps import build_dep_graph as build_go_dep_graph +from desloppify.languages.go.detectors.security import detect_go_security from desloppify.languages.go.extractors import ( GO_FILE_EXCLUSIONS, extract_functions, find_go_files, ) -from desloppify.languages.go.phases import _phase_structural +from desloppify.languages.go.phases import ( + _phase_coupling, + _phase_smells, + _phase_structural, + _phase_unused, +) from desloppify.languages.go.review import ( HOLISTIC_REVIEW_DIMENSIONS, LOW_VALUE_PATTERN, @@ -62,6 +68,9 @@ def __init__(self): barrel_names=set(), phases=[ DetectorPhase("Structural analysis", _phase_structural), + DetectorPhase("Unused symbols", _phase_unused), + DetectorPhase("Code smells", _phase_smells), + DetectorPhase("Coupling + cycles + orphaned", _phase_coupling), make_tool_phase( "golangci-lint", "golangci-lint run --out-format=json", @@ -72,7 +81,11 @@ def __init__(self): make_tool_phase( "go vet", "go vet ./...", "gnu", "vet_error", tier=3 ), - *all_treesitter_phases("go"), + # Exclude tree-sitter unused imports: Go compiler already + # enforces unused imports (compile error), and the generic + # name extractor mishandles Go versioned paths (e.g. v2). + *[p for p in all_treesitter_phases("go") + if p.label != "Unused imports"], detector_phase_signature(), detector_phase_test_coverage(), detector_phase_security(), @@ -100,3 +113,9 @@ def __init__(self): extract_functions=extract_functions, zone_rules=GO_ZONE_RULES, ) + + def detect_lang_security( + self, files: list[str], zone_map: FileZoneMap | None + ) -> tuple[list[dict], int]: + """Go-specific security checks.""" + return detect_go_security(files, zone_map) diff --git a/desloppify/languages/go/commands.py b/desloppify/languages/go/commands.py index b2148dbd..9ef334fa 100644 --- a/desloppify/languages/go/commands.py +++ b/desloppify/languages/go/commands.py @@ -6,17 +6,26 @@ from __future__ import annotations import argparse +from pathlib import Path +from desloppify.engine.detectors import gods as gods_detector_mod from desloppify.languages._framework.commands_base import ( - build_standard_detect_registry, make_cmd_complexity, make_cmd_cycles, make_cmd_deps, make_cmd_dupes, make_cmd_large, + make_cmd_naming, make_cmd_orphaned, + make_cmd_single_use, + make_cmd_smells, ) +from desloppify.core.discovery_api import rel +from desloppify.core.output_api import display_entries from desloppify.languages.go.detectors.deps import build_dep_graph +from desloppify.languages.go.detectors.gods import GO_GOD_RULES, extract_go_structs +from desloppify.languages.go.detectors.smells import detect_smells +from desloppify.languages.go.detectors.unused import detect_unused from desloppify.languages.go.extractors import extract_functions, find_go_files from desloppify.languages.go.phases import GO_COMPLEXITY_SIGNALS @@ -38,6 +47,16 @@ extra_barrel_names=set(), ) _cmd_dupes_impl = make_cmd_dupes(extract_functions_fn=extract_functions) +_cmd_single_use_impl = make_cmd_single_use( + build_dep_graph=build_dep_graph, + barrel_names=set(), +) +_cmd_smells_impl = make_cmd_smells(detect_smells) +_cmd_naming_impl = make_cmd_naming( + find_go_files, + skip_names={"main.go", "doc.go"}, + skip_dirs={"vendor"}, +) def cmd_large(args: argparse.Namespace) -> None: @@ -64,13 +83,67 @@ def cmd_dupes(args: argparse.Namespace) -> None: _cmd_dupes_impl(args) -def get_detect_commands() -> dict[str, object]: - """Return the standard detect command registry for Go.""" - return build_standard_detect_registry( - cmd_deps=cmd_deps, - cmd_cycles=cmd_cycles, - cmd_orphaned=cmd_orphaned, - cmd_dupes=cmd_dupes, - cmd_large=cmd_large, - cmd_complexity=cmd_complexity, +def cmd_single_use(args: argparse.Namespace) -> None: + _cmd_single_use_impl(args) + + +def cmd_smells(args: argparse.Namespace) -> None: + _cmd_smells_impl(args) + + +def cmd_naming(args: argparse.Namespace) -> None: + _cmd_naming_impl(args) + + +def cmd_unused(args: argparse.Namespace) -> None: + entries, total, available = detect_unused(Path(args.path)) + if not available: + empty = "staticcheck is not installed — install it for unused symbol detection." + else: + empty = "No unused symbols found." + display_entries( + args, + entries, + label=f"Unused symbols ({total} files checked)", + empty_msg=empty, + columns=["File", "Line", "Name", "Category"], + widths=[55, 6, 30, 10], + row_fn=lambda e: [rel(e["file"]), str(e["line"]), e["name"], e["category"]], + ) + + +def cmd_gods(args: argparse.Namespace) -> None: + entries, _ = gods_detector_mod.detect_gods( + extract_go_structs(Path(args.path)), GO_GOD_RULES ) + display_entries( + args, + entries, + label="God structs", + empty_msg="No god structs found.", + columns=["File", "Struct", "LOC", "Why"], + widths=[50, 20, 6, 45], + row_fn=lambda e: [ + rel(e["file"]), + e["name"], + str(e["loc"]), + ", ".join(e["reasons"]), + ], + ) + + +def get_detect_commands() -> dict[str, object]: + """Return the detect command registry for Go.""" + return { + "deps": cmd_deps, + "cycles": cmd_cycles, + "orphaned": cmd_orphaned, + "dupes": cmd_dupes, + "large": cmd_large, + "complexity": cmd_complexity, + "single_use": cmd_single_use, + "smells": cmd_smells, + "naming": cmd_naming, + "unused": cmd_unused, + "gods": cmd_gods, + } diff --git a/desloppify/languages/go/detectors/deps.py b/desloppify/languages/go/detectors/deps.py index 869b5d0d..8a36aa22 100644 --- a/desloppify/languages/go/detectors/deps.py +++ b/desloppify/languages/go/detectors/deps.py @@ -1,19 +1,135 @@ -"""Go dependency graph builder (stub). - -Originally contributed by tinker495 (KyuSeok Jung) in PR #128. -Go import resolution is not yet implemented — returns an empty graph. -""" +"""Go dependency graph builder — regex-based import parsing.""" from __future__ import annotations +import os +import re +from collections import defaultdict from pathlib import Path from typing import Any +from desloppify.engine.detectors.graph import finalize_graph +from desloppify.languages._framework.treesitter._imports import resolve_go_import +from desloppify.languages.go.extractors import find_go_files + +# Match single import: import "path" or import alias "path" +_SINGLE_IMPORT_RE = re.compile( + r'^\s*import\s+(?:\w+\s+)?"([^"]+)"' +) + +# Match start of grouped import block: import ( +_GROUP_IMPORT_START_RE = re.compile(r'^\s*import\s*\(') + +# Match import line inside group: "path" or alias "path" or . "path" +_GROUP_IMPORT_LINE_RE = re.compile( + r'^\s*(?:\w+\s+|\.\s+)?"([^"]+)"' +) + + +def _extract_imports(content: str) -> list[str]: + """Extract all import paths from Go source content.""" + imports: list[str] = [] + lines = content.splitlines() + i = 0 + while i < len(lines): + line = lines[i] + + # Check for grouped import + if _GROUP_IMPORT_START_RE.match(line): + i += 1 + while i < len(lines): + group_line = lines[i].strip() + if group_line == ')': + break + m = _GROUP_IMPORT_LINE_RE.match(lines[i]) + if m: + imports.append(m.group(1)) + i += 1 + i += 1 + continue + + # Check for single import + m = _SINGLE_IMPORT_RE.match(line) + if m: + imports.append(m.group(1)) + + i += 1 + return imports + def build_dep_graph( path: Path, roslyn_cmd: str | None = None, ) -> dict[str, dict[str, Any]]: - """Build Go dependency graph — stub returning empty dict.""" - del path, roslyn_cmd - return {} + """Build Go dependency graph from import declarations. + + Parses Go import blocks (single and grouped), resolves local imports + via go.mod module path, and returns the standard graph shape. + """ + del roslyn_cmd # Not used for Go + + scan_path = str(path.resolve()) + files = find_go_files(path) + + # Normalize all file paths to absolute for consistent matching. + # find_go_files may return relative paths while resolve_go_import + # returns absolute paths — both must use the same form. + abs_files = [ + os.path.normpath(os.path.join(scan_path, f)) + if not os.path.isabs(f) else os.path.normpath(f) + for f in files + ] + file_set = set(abs_files) + + # Initialize graph with all files + graph: dict[str, dict[str, Any]] = {} + for f in abs_files: + graph[f] = {"imports": set(), "importers": set()} + + # Track package declarations per file for same-package edge linking + dir_pkg: dict[tuple[str, str], list[str]] = defaultdict(list) + + for filepath in abs_files: + try: + content = Path(filepath).read_text(errors="replace") + except OSError: + continue + + # Collect package name for same-package linking (single read) + pkg_match = _PACKAGE_RE.search(content) + if pkg_match: + key = (str(Path(filepath).parent), pkg_match.group(1)) + dir_pkg[key].append(filepath) + + import_paths = _extract_imports(content) + for import_path in import_paths: + resolved = resolve_go_import(import_path, filepath, scan_path) + if resolved is None: + continue + + resolved = os.path.normpath(resolved) + + # Only track edges within the scanned file set + if resolved not in file_set: + continue + + graph[filepath]["imports"].add(resolved) + if resolved in graph: + graph[resolved]["importers"].add(filepath) + + # Add implicit same-package edges: Go files in the same directory + # sharing the same `package` declaration are implicitly linked. + # Without these edges, every file that isn't cross-package imported + # would appear "orphaned." + for pkg_files in dir_pkg.values(): + if len(pkg_files) < 2: + continue + rep = pkg_files[0] + for other in pkg_files[1:]: + graph[rep]["importers"].add(other) + graph[other]["importers"].add(rep) + + return finalize_graph(graph) + + +_PACKAGE_RE = re.compile(r"^\s*package\s+(\w+)", re.MULTILINE) diff --git a/desloppify/languages/go/detectors/gods.py b/desloppify/languages/go/detectors/gods.py new file mode 100644 index 00000000..eb3841e2 --- /dev/null +++ b/desloppify/languages/go/detectors/gods.py @@ -0,0 +1,130 @@ +"""Go god struct detection — structs with too many fields/methods/LOC.""" + +from __future__ import annotations + +import re +from pathlib import Path + +from desloppify.engine.detectors.base import ClassInfo, FunctionInfo, GodRule +from desloppify.languages.go.extractors import find_go_files + +# ── Receiver regex: func (r *StructName) MethodName(...) +_RECEIVER_RE = re.compile( + r"^func\s+\(\s*\w+\s+\*?(\w+)(?:\[[\w,\s]+\])?\s*\)\s+(\w+)" +) + +# ── Struct declaration regex +_STRUCT_DECL_RE = re.compile( + r"^type\s+(\w+)\s+struct\s*\{" +) + +GO_GOD_RULES: list[GodRule] = [ + GodRule( + name="method_count", + description="methods", + extract=lambda cls: len(cls.methods), + threshold=10, + ), + GodRule( + name="field_count", + description="fields", + extract=lambda cls: len(cls.attributes), + threshold=15, + ), + GodRule( + name="loc", + description="LOC", + extract=lambda cls: cls.loc, + threshold=300, + ), +] + + +def extract_go_structs(path: Path) -> list[ClassInfo]: + """Extract Go struct definitions with their fields and associated methods.""" + files = find_go_files(path) + # First pass: collect all methods by receiver type across all files + methods_by_struct: dict[str, list] = {} + + for filepath in files: + try: + content = Path(filepath).read_text(errors="replace") + except OSError: + continue + for line in content.splitlines(): + m = _RECEIVER_RE.match(line) + if m: + struct_name = m.group(1) + method_name = m.group(2) + methods_by_struct.setdefault(struct_name, []).append( + (method_name, filepath) + ) + + # Second pass: extract struct definitions with fields + structs: list[ClassInfo] = [] + for filepath in files: + try: + content = Path(filepath).read_text(errors="replace") + except OSError: + continue + lines = content.splitlines() + + i = 0 + while i < len(lines): + m = _STRUCT_DECL_RE.match(lines[i].strip()) + if not m: + i += 1 + continue + + struct_name = m.group(1) + start_line = i + 1 # 1-indexed + + # Find closing brace by tracking depth + depth = 1 + fields: list[str] = [] + j = i + 1 + while j < len(lines) and depth > 0: + stripped = lines[j].strip() + depth += stripped.count('{') - stripped.count('}') + if depth > 0 and stripped and not stripped.startswith("//") and not stripped.startswith("/*"): + # This is a field line + # Skip embedded structs (lines that are just a type name) + parts = stripped.split() + if len(parts) >= 2 and not stripped.startswith("}"): + fields.append(parts[0]) + j += 1 + + end_line = j # 1-indexed + loc = end_line - start_line + 1 + + # Get methods for this struct + methods = methods_by_struct.get(struct_name, []) + + total_loc = loc + + # Create FunctionInfo stubs for methods + method_infos = [ + FunctionInfo( + name=mname, file=mfile, line=0, end_line=0, + loc=0, body="", + ) + for mname, mfile in methods + ] + + structs.append(ClassInfo( + name=struct_name, + file=filepath, + line=start_line, + loc=total_loc, + methods=method_infos, + attributes=fields, + metrics={ + "method_count": len(methods), + "field_count": len(fields), + "name": struct_name, + }, + )) + + i = j + + return structs diff --git a/desloppify/languages/go/detectors/security.py b/desloppify/languages/go/detectors/security.py new file mode 100644 index 00000000..5d68b5b9 --- /dev/null +++ b/desloppify/languages/go/detectors/security.py @@ -0,0 +1,180 @@ +"""Go-specific security detectors.""" + +from __future__ import annotations + +import logging +import re +from pathlib import Path + +from desloppify.core.fallbacks import log_best_effort_failure +from desloppify.engine.detectors.security import rules as security_detector_mod +from desloppify.engine.policy.zones import FileZoneMap, Zone + +logger = logging.getLogger(__name__) + +# ── Patterns ── + +_UNSAFE_POINTER_RE = re.compile(r"\bunsafe\.Pointer\b") + +# SQL injection: string concat in Query/Exec calls +_SQL_CONCAT_RE = re.compile( + r"\b(?:Query|QueryRow|Exec|ExecContext|QueryContext)\s*\(" + r"[^)]*(?:fmt\.Sprintf|\"[^\"]*\"\s*\+|\+\s*\")" +) + +# Also catch simpler pattern: db.Query("SELECT * FROM " + var) +_SQL_CONCAT_SIMPLE_RE = re.compile( + r"\b(?:Query|QueryRow|Exec)\s*\(\s*\"[^\"]*\"\s*\+" +) + +# Weak crypto +_WEAK_HASH_USE_RE = re.compile(r"\b(?:md5|sha1)\.(?:New|Sum)") + +# Insecure random for crypto +_MATH_RAND_IMPORT_RE = re.compile(r'"math/rand"') +_CRYPTO_CONTEXT_RE = re.compile( + r"(?:token|secret|password|key|nonce|salt|cipher|crypt|auth)", re.IGNORECASE +) + +# HTTP without TLS +_HTTP_LISTEN_RE = re.compile(r"\bhttp\.ListenAndServe\s*\(") + +# Command injection +_EXEC_COMMAND_VAR_RE = re.compile(r"\bexec\.Command\s*\(\s*[a-zA-Z_]\w*") + +# Hardcoded credentials +_HARDCODED_CRED_RE = re.compile( + r'(?:password|passwd|secret|token|api_?key)\s*(?::=|=)\s*"[^"]{8,}"', + re.IGNORECASE, +) + +# InsecureSkipVerify disables TLS certificate verification +_INSECURE_SKIP_VERIFY_RE = re.compile(r"InsecureSkipVerify\s*:\s*true") + + +def _make_entry(filepath, line_num, line, *, check_id, summary, severity, confidence, remediation): + return security_detector_mod.make_security_entry( + filepath, line_num, line, + security_detector_mod.SecurityRule( + check_id=check_id, summary=summary, + severity=severity, confidence=confidence, + remediation=remediation, + ), + ) + + +def detect_go_security( + files: list[str], + zone_map: FileZoneMap | None, +) -> tuple[list[dict], int]: + """Detect Go-specific security issues.""" + entries: list[dict] = [] + scanned = 0 + + for filepath in files: + if zone_map is not None: + zone = zone_map.get(filepath) + if zone in (Zone.TEST, Zone.VENDOR, Zone.GENERATED, Zone.CONFIG): + continue + + try: + content = Path(filepath).read_text(errors="replace") + except OSError as exc: + log_best_effort_failure(logger, f"read Go security source {filepath}", exc) + continue + + scanned += 1 + lines = content.splitlines() + + for line_num, line in enumerate(lines, 1): + stripped = line.lstrip() + if stripped.startswith("//"): + continue + + # Unsafe pointer + if _UNSAFE_POINTER_RE.search(line): + entries.append(_make_entry( + filepath, line_num, line, + check_id="unsafe_pointer", + summary="unsafe.Pointer usage — bypasses Go type safety", + severity="high", confidence="medium", + remediation="Avoid unsafe.Pointer unless absolutely necessary; document why", + )) + + # SQL injection via string concatenation + if _SQL_CONCAT_RE.search(line) or _SQL_CONCAT_SIMPLE_RE.search(line): + entries.append(_make_entry( + filepath, line_num, line, + check_id="sql_injection", + summary="SQL query built with string concatenation — verify values are parameterized", + severity="critical", confidence="medium", + remediation="Use db.Query(\"SELECT ... WHERE id = $1\", id) with placeholders; if only table/column names are interpolated, add a // SECURITY: comment explaining why", + )) + + # Weak hash usage + if _WEAK_HASH_USE_RE.search(line): + entries.append(_make_entry( + filepath, line_num, line, + check_id="weak_hash", + summary="Weak hash function (MD5/SHA1) — use SHA-256 or stronger", + severity="medium", confidence="high", + remediation="Replace with crypto/sha256 or crypto/sha512", + )) + + # HTTP without TLS + if _HTTP_LISTEN_RE.search(line): + entries.append(_make_entry( + filepath, line_num, line, + check_id="http_no_tls", + summary="http.ListenAndServe without TLS — use ListenAndServeTLS", + severity="medium", confidence="medium", + remediation="Use http.ListenAndServeTLS() or put behind a TLS-terminating proxy", + )) + + # Command injection + if _EXEC_COMMAND_VAR_RE.search(line): + # Check if the argument is a variable (not a string literal) + if not re.search(r'exec\.Command\s*\(\s*"', line): + entries.append(_make_entry( + filepath, line_num, line, + check_id="command_injection", + summary="exec.Command with variable argument — potential command injection", + severity="high", confidence="medium", + remediation="Validate and sanitize command arguments; prefer allowlisted commands", + )) + + # Hardcoded credentials + if _HARDCODED_CRED_RE.search(line): + entries.append(_make_entry( + filepath, line_num, line, + check_id="hardcoded_credentials", + summary="Hardcoded credential detected", + severity="high", confidence="medium", + remediation="Use environment variables or a secrets manager", + )) + + # InsecureSkipVerify disables TLS cert verification + if _INSECURE_SKIP_VERIFY_RE.search(line): + entries.append(_make_entry( + filepath, line_num, line, + check_id="insecure_tls", + summary="InsecureSkipVerify disables TLS certificate verification", + severity="high", confidence="high", + remediation="Remove InsecureSkipVerify or use a proper CA certificate", + )) + + # File-level: check for math/rand in security context + if _MATH_RAND_IMPORT_RE.search(content) and _CRYPTO_CONTEXT_RE.search(content): + # Find the import line + for i, line in enumerate(lines, 1): + if _MATH_RAND_IMPORT_RE.search(line): + entries.append(_make_entry( + filepath, i, line, + check_id="insecure_random", + summary="math/rand used in file with crypto context — use crypto/rand", + severity="medium", confidence="medium", + remediation="Use crypto/rand for security-sensitive random number generation", + )) + break + + return entries, scanned diff --git a/desloppify/languages/go/detectors/smells.py b/desloppify/languages/go/detectors/smells.py new file mode 100644 index 00000000..52c2a57f --- /dev/null +++ b/desloppify/languages/go/detectors/smells.py @@ -0,0 +1,516 @@ +"""Go code smell detection.""" + +from __future__ import annotations + +import re +import logging +from pathlib import Path +from typing import Any + +from desloppify.core.fallbacks import log_best_effort_failure +from desloppify.languages.go.extractors import ( + find_go_files, + _FUNC_DECL_RE, + _find_body_brace, + _find_matching_brace, +) + +logger = logging.getLogger(__name__) + + +GO_SMELL_CHECKS: list[dict[str, Any]] = [ + { + "id": "ignored_error", + "label": "Ignored error value", + "pattern": r"_\s*(?:,\s*_\s*)?=\s*\w+.*(?:err|Err)", + "severity": "high", + }, + { + "id": "ignored_error", + "label": "Ignored error value", + "pattern": r"_\s*=\s*\w+\.\w+\(", + "severity": "high", + }, + { + "id": "naked_return", + "label": "Naked return (no values)", + "pattern": r"^\s*return\s*$", + "severity": "medium", + }, + { + "id": "empty_error_branch", + "label": "Empty error-handling branch", + "pattern": None, + "severity": "high", + }, + { + "id": "panic_in_lib", + "label": "panic() in library package", + "pattern": None, + "severity": "high", + }, + { + "id": "init_side_effects", + "label": "func init() with side effects", + "pattern": None, + "severity": "medium", + }, + { + "id": "global_var", + "label": "Package-level mutable var declaration", + "pattern": None, + "severity": "low", + }, + { + "id": "defer_in_loop", + "label": "defer inside a for loop", + "pattern": None, + "severity": "high", + }, + { + "id": "goroutine_closure_capture", + "label": "Goroutine captures loop variable", + "pattern": None, + "severity": "high", + }, + { + "id": "error_wrap_verb", + "label": "fmt.Errorf wraps error with %v instead of %w", + "pattern": r"""fmt\.Errorf\s*\([^)]*%v[^)]*\berr\b""", + "severity": "medium", + }, + { + "id": "json_unmarshal_interface", + "label": "json.Unmarshal into interface{}/any without struct", + "pattern": None, + "severity": "medium", + }, + { + "id": "hardcoded_url", + "label": "Hardcoded URL in source code", + "pattern": r"""(?:['\"]|`)https?://[^\s'\"` ]+(?:['\"]|`)""", + "severity": "medium", + }, + { + "id": "todo_fixme", + "label": "TODO/FIXME/HACK comments", + "pattern": r"//\s*(?:TODO|FIXME|HACK|XXX)", + "severity": "low", + }, + { + "id": "magic_number", + "label": "Magic number in logic", + "pattern": r"(?:==|!=|>=?|<=?|[+\-*/])\s*\d{4,}", + "severity": "low", + }, + { + "id": "monster_function", + "label": "Monster function (>150 LOC)", + "pattern": None, + "severity": "high", + }, + { + "id": "dead_function", + "label": "Dead function (empty body)", + "pattern": None, + "severity": "medium", + }, + { + "id": "unreachable_code", + "label": "Unreachable code after return/panic/os.Exit", + "pattern": None, + "severity": "medium", + }, +] + +# De-duplicate check IDs for smell_counts initialization — some IDs appear +# twice (ignored_error has two regex patterns). +_UNIQUE_IDS = list(dict.fromkeys(c["id"] for c in GO_SMELL_CHECKS)) + + +# --------------------------------------------------------------------------- +# Multi-line helpers +# --------------------------------------------------------------------------- + +def _detect_empty_error_branch( + filepath: str, lines: list[str], smell_counts: dict[str, list[dict]] +) -> None: + for i, line in enumerate(lines): + stripped = line.strip() + if not re.match(r"if\s+err\s*!=\s*nil\s*\{", stripped): + continue + for j in range(i + 1, min(i + 3, len(lines))): + next_stripped = lines[j].strip() + if not next_stripped: + continue + if next_stripped == "}": + smell_counts["empty_error_branch"].append( + {"file": filepath, "line": i + 1, "content": stripped[:100]} + ) + break + + +def _detect_panic_in_lib( + filepath: str, + content: str, + lines: list[str], + smell_counts: dict[str, list[dict]], +) -> None: + pkg_match = re.search(r"^package\s+(\w+)", content, re.MULTILINE) + if not pkg_match or pkg_match.group(1) == "main": + return + for i, line in enumerate(lines): + stripped = line.strip() + if stripped.startswith("//"): + continue + if re.search(r"\bpanic\s*\(", stripped): + smell_counts["panic_in_lib"].append( + {"file": filepath, "line": i + 1, "content": stripped[:100]} + ) + + +def _detect_init_side_effects( + filepath: str, + content: str, + lines: list[str], + smell_counts: dict[str, list[dict]], +) -> None: + side_effect_patterns = [ + r"\bhttp\.\w+", + r"\bos\.(Open|Create|Remove|Mkdir|Stat|ReadFile|WriteFile)", + r"\bexec\.Command", + r"\bnet\.Dial", + r"\bsql\.Open", + r"\bioutil\.Read", + ] + for match in _FUNC_DECL_RE.finditer(content): + if match.group(1) != "init": + continue + brace_pos = _find_body_brace(content, match.end()) + if brace_pos is None: + continue + end = _find_matching_brace(content, brace_pos) + if end is None: + continue + body = content[brace_pos + 1 : end] + for pat in side_effect_patterns: + if re.search(pat, body): + start_line = content.count("\n", 0, match.start()) + 1 + smell_counts["init_side_effects"].append( + {"file": filepath, "line": start_line, "content": "func init() with side effects"} + ) + break + + +def _detect_defer_in_loop( + filepath: str, lines: list[str], smell_counts: dict[str, list[dict]] +) -> None: + in_for = False + for_depth = 0 + brace_depth = 0 + for i, line in enumerate(lines): + stripped = line.strip() + if stripped.startswith("//"): + continue + if re.match(r"\bfor\b", stripped) and "{" in stripped: + if not in_for: + in_for = True + for_depth = brace_depth + brace_depth += stripped.count("{") - stripped.count("}") + if in_for and brace_depth <= for_depth: + in_for = False + if in_for and re.match(r"\s*defer\b", line): + smell_counts["defer_in_loop"].append( + {"file": filepath, "line": i + 1, "content": stripped[:100]} + ) + + +def _detect_goroutine_closure( + filepath: str, lines: list[str], smell_counts: dict[str, list[dict]] +) -> None: + """Find goroutine closures inside for loops that capture loop variables.""" + in_for = False + for_depth = 0 + brace_depth = 0 + for_vars: list[str] = [] + for i, line in enumerate(lines): + stripped = line.strip() + if stripped.startswith("//"): + continue + for_match = re.match( + r"for\s+(?:(\w+)(?:\s*,\s*(\w+))?\s*:?=|_\s*,\s*(\w+)\s*:?=)", stripped + ) + if for_match and "{" in stripped: + if not in_for: + in_for = True + for_depth = brace_depth + for_vars = [g for g in for_match.groups() if g] + brace_depth += stripped.count("{") - stripped.count("}") + if in_for and brace_depth <= for_depth: + in_for = False + for_vars = [] + if in_for and re.match(r"go\s+func\s*\(", stripped): + # Check a few lines ahead for captured loop vars + for j in range(i, min(i + 20, len(lines))): + body_line = lines[j] + for var in for_vars: + if re.search(rf"\b{re.escape(var)}\b", body_line) and j != i: + smell_counts["goroutine_closure_capture"].append( + {"file": filepath, "line": i + 1, "content": stripped[:100]} + ) + # Only report once per goroutine + for_vars = [] + break + if not for_vars: + break + + +# Idiomatic Go patterns that use package-level var — not smells. +_GLOBAL_VAR_SKIP_RE = re.compile( + r"^var\s+\w+\s*=?\s*(?:" + r"&?cobra\.Command\b" # CLI commands + r"|regexp\.MustCompile\b" # compiled regexes + r"|sync\.\w+" # sync primitives + r"|errors\.New\b" # sentinel errors + r"|fmt\.Errorf\b" # sentinel errors + r"|template\.Must\b" # parsed templates + r")" +) +# Also skip: var ( ... ) blocks that define types/consts (common Go enum pattern), +# and var declarations with explicit struct/func/interface types. +_GLOBAL_VAR_RE = re.compile(r"^var\s+\w+") + + +def _detect_global_var( + filepath: str, lines: list[str], smell_counts: dict[str, list[dict]] +) -> None: + """Flag mutable package-level var declarations, excluding idiomatic patterns. + + Only flags top-level var (no indentation) — function-local var is fine. + Skips var (...) blocks (common Go enum/const patterns) and idiomatic + patterns like cobra commands, compiled regexes, sync primitives, etc. + """ + in_var_block = False + for i, line in enumerate(lines): + stripped = line.strip() + if stripped.startswith("//"): + continue + # Only consider lines with no leading whitespace (top-level declarations) + if line != line.lstrip(): + continue + # Track var ( ... ) blocks + if re.match(r"^var\s*\(", stripped): + in_var_block = True + continue + if in_var_block: + if stripped == ")": + in_var_block = False + continue + if not _GLOBAL_VAR_RE.match(stripped): + continue + if _GLOBAL_VAR_SKIP_RE.match(stripped): + continue + smell_counts["global_var"].append( + {"file": filepath, "line": i + 1, "content": stripped[:100]} + ) + + +def _detect_monster_functions( + filepath: str, content: str, smell_counts: dict[str, list[dict]] +) -> None: + for match in _FUNC_DECL_RE.finditer(content): + brace_pos = _find_body_brace(content, match.end()) + if brace_pos is None: + continue + end = _find_matching_brace(content, brace_pos) + if end is None: + continue + start_line = content.count("\n", 0, match.start()) + 1 + end_line = content.count("\n", 0, end) + 1 + loc = end_line - start_line + 1 + if loc > 150: + smell_counts["monster_function"].append( + { + "file": filepath, + "line": start_line, + "content": f"func {match.group(1)} ({loc} LOC)", + } + ) + + +def _detect_dead_functions( + filepath: str, content: str, smell_counts: dict[str, list[dict]] +) -> None: + for match in _FUNC_DECL_RE.finditer(content): + brace_pos = _find_body_brace(content, match.end()) + if brace_pos is None: + continue + end = _find_matching_brace(content, brace_pos) + if end is None: + continue + body = content[brace_pos + 1 : end].strip() + body_clean = re.sub(r"//.*", "", body) + body_clean = re.sub(r"/\*.*?\*/", "", body_clean, flags=re.DOTALL) + if not body_clean.strip(): + start_line = content.count("\n", 0, match.start()) + 1 + smell_counts["dead_function"].append( + { + "file": filepath, + "line": start_line, + "content": f"func {match.group(1)} (empty body)", + } + ) + + +def _detect_unreachable_code( + filepath: str, lines: list[str], smell_counts: dict[str, list[dict]] +) -> None: + """Find code after return/panic/os.Exit that is not a closing brace or blank.""" + for i, line in enumerate(lines): + stripped = line.strip() + if stripped.startswith("//"): + continue + if re.match(r"(?:return\b|panic\(|os\.Exit\()", stripped): + # If the return/panic line has unbalanced braces (e.g. return &Foo{), + # the statement spans multiple lines — skip until braces balance. + depth = stripped.count("{") - stripped.count("}") + depth += stripped.count("(") - stripped.count(")") + if depth > 0: + continue + # Look at the next non-blank line + for j in range(i + 1, min(i + 5, len(lines))): + next_stripped = lines[j].strip() + if not next_stripped: + continue + if next_stripped.startswith("//"): + continue + if re.match(r"^[}\)]+[,;\s]*$", next_stripped): + break + if next_stripped.startswith("case ") or next_stripped.startswith("default:"): + break + smell_counts["unreachable_code"].append( + {"file": filepath, "line": j + 1, "content": next_stripped[:100]} + ) + break + + +_JSON_UNMARSHAL_RE = re.compile(r"json\.(?:Unmarshal|NewDecoder)") +_INTERFACE_EMPTY_RE = re.compile(r"\binterface\s*\{\}|\bany\b") + + +def _detect_json_unmarshal_interface( + filepath: str, lines: list[str], smell_counts: dict[str, list[dict]] +) -> None: + """Flag json.Unmarshal/NewDecoder targeting interface{}/any variables.""" + # Track lines that declare interface{}/any variables + iface_vars: set[str] = set() + for line in lines: + stripped = line.strip() + # var v interface{} / var v any + m = re.match(r"var\s+(\w+)\s+(?:interface\s*\{\}\s*|any\b)", stripped) + if m: + iface_vars.add(m.group(1)) + + for i, line in enumerate(lines): + stripped = line.strip() + if stripped.startswith("//"): + continue + if not _JSON_UNMARSHAL_RE.search(stripped): + continue + # Direct: json.Unmarshal(data, &v) where v is interface{}/any + for var in iface_vars: + if re.search(rf"&{re.escape(var)}\b", stripped): + smell_counts["json_unmarshal_interface"].append( + {"file": filepath, "line": i + 1, "content": stripped[:100]} + ) + break + else: + # Inline: json.Unmarshal(data, &x) where x is declared as interface{} nearby + if _INTERFACE_EMPTY_RE.search(stripped) and _JSON_UNMARSHAL_RE.search(stripped): + smell_counts["json_unmarshal_interface"].append( + {"file": filepath, "line": i + 1, "content": stripped[:100]} + ) + + +# --------------------------------------------------------------------------- +# Main entry point +# --------------------------------------------------------------------------- + +def detect_smells(path: Path) -> tuple[list[dict], int]: + """Detect Go code smell patterns across the codebase. + + Returns (entries, total_files_checked). + """ + smell_counts: dict[str, list[dict]] = {sid: [] for sid in _UNIQUE_IDS} + files = find_go_files(path) + + for filepath in files: + if "_test.go" in filepath or "/vendor/" in filepath: + continue + try: + p = Path(filepath) if Path(filepath).is_absolute() else Path(path) / filepath + content = p.read_text(errors="replace") + lines = content.splitlines() + except (OSError, UnicodeDecodeError) as exc: + log_best_effort_failure(logger, f"read Go smell candidate {filepath}", exc) + continue + + # Regex-based smells + for check in GO_SMELL_CHECKS: + if check["pattern"] is None: + continue + for i, line in enumerate(lines): + stripped = line.strip() + # Skip comment lines for most checks, but allow todo_fixme + if stripped.startswith("//") and check["id"] != "todo_fixme": + continue + m = re.search(check["pattern"], line) + if not m: + continue + # Skip URLs assigned to top-level constants + if check["id"] == "hardcoded_url" and re.match( + r"^(?:var|const)\s+[A-Za-z_]\w*\s*=", stripped + ): + continue + smell_counts[check["id"]].append( + {"file": filepath, "line": i + 1, "content": stripped[:100]} + ) + + # Multi-line smell helpers + _detect_empty_error_branch(filepath, lines, smell_counts) + _detect_panic_in_lib(filepath, content, lines, smell_counts) + _detect_init_side_effects(filepath, content, lines, smell_counts) + _detect_defer_in_loop(filepath, lines, smell_counts) + _detect_goroutine_closure(filepath, lines, smell_counts) + _detect_global_var(filepath, lines, smell_counts) + _detect_monster_functions(filepath, content, smell_counts) + _detect_dead_functions(filepath, content, smell_counts) + _detect_unreachable_code(filepath, lines, smell_counts) + _detect_json_unmarshal_interface(filepath, lines, smell_counts) + + # Build summary entries sorted by severity then count + severity_order = {"high": 0, "medium": 1, "low": 2} + # Build a label lookup from the first check entry for each ID + label_lookup = {} + severity_lookup = {} + for check in GO_SMELL_CHECKS: + if check["id"] not in label_lookup: + label_lookup[check["id"]] = check["label"] + severity_lookup[check["id"]] = check["severity"] + + entries = [] + for sid in _UNIQUE_IDS: + matches = smell_counts[sid] + if matches: + entries.append( + { + "id": sid, + "label": label_lookup[sid], + "severity": severity_lookup[sid], + "count": len(matches), + "files": len(set(m["file"] for m in matches)), + "matches": matches[:50], + } + ) + entries.sort(key=lambda e: (severity_order.get(e["severity"], 9), -e["count"])) + return entries, len(files) diff --git a/desloppify/languages/go/detectors/unused.py b/desloppify/languages/go/detectors/unused.py new file mode 100644 index 00000000..7fc6ee4d --- /dev/null +++ b/desloppify/languages/go/detectors/unused.py @@ -0,0 +1,135 @@ +"""Go unused symbol detection via staticcheck.""" + +from __future__ import annotations + +import re +import subprocess +from pathlib import Path + +from desloppify.core._internal.text_utils import PROJECT_ROOT +from desloppify.languages.go.extractors import find_go_files + +# staticcheck checks we care about: +# U1000: unused code (functions, types, constants, variables) +# SA4006: value assigned and never used + +_STATICCHECK_RE = re.compile( + r"^(.+?):(\d+):(\d+):\s+(U1000|SA4006)\s+(.+)$" +) + +_NAME_RE = re.compile(r"(?:\"([^\"]+)\"|(\S+))\s+is\s+(?:unused|assigned)") + + +def detect_unused( + path: Path, category: str = "all" +) -> tuple[list[dict], int, bool]: + """Detect unused symbols in Go code using staticcheck. + + Falls back gracefully if staticcheck is not installed. + Returns (entries, total_files_checked, staticcheck_available). + """ + total_files = len(find_go_files(path)) + + result = _try_staticcheck(path, category) + if result is not None: + entries, available = result + return entries, total_files, available + + # Graceful degradation: Go compiler already catches unused imports and locals + return [], total_files, False + + +def _try_staticcheck(path: Path, category: str) -> tuple[list[dict], bool] | None: + """Try staticcheck for unused detection. + + Returns (entries, tool_available) on success, None if staticcheck is + not installed or timed out. + """ + checks = [] + if category in ("all", "exports"): + checks.append("U1000") + if category in ("all", "vars"): + checks.append("SA4006") + if not checks: + return [], True + + cmd = [ + "staticcheck", + "-checks", + ",".join(checks), + "-f", + "text", + "./...", + ] + + try: + proc = subprocess.run( + cmd, + capture_output=True, + text=True, + cwd=str(path), + timeout=120, + ) + except (FileNotFoundError, subprocess.TimeoutExpired): + return None + except UnicodeDecodeError: + return [], True + + lines = (proc.stdout + proc.stderr).splitlines() + return _parse_staticcheck_output(lines, category, str(path.resolve())), True + + +def _parse_staticcheck_output( + lines: list[str], category: str, base_path: str +) -> list[dict]: + """Parse staticcheck text output into entry dicts.""" + entries: list[dict] = [] + for line in lines: + m = _STATICCHECK_RE.match(line) + if not m: + continue + filepath = m.group(1) + line_num = int(m.group(2)) + col = int(m.group(3)) + code = m.group(4) + message = m.group(5) + + # Make path absolute if relative + if not Path(filepath).is_absolute(): + filepath = str(Path(base_path) / filepath) + + # Skip test files + if filepath.endswith("_test.go"): + continue + + # Determine category + cat = "vars" if code == "SA4006" else "exports" + if category != "all" and cat != category: + continue + + # Extract name from message + name = _extract_name(message) + if name.startswith("_"): + continue + + entries.append({ + "file": filepath, + "line": line_num, + "col": col, + "name": name, + "category": cat, + }) + return entries + + +def _extract_name(message: str) -> str: + """Extract the symbol name from a staticcheck message.""" + m = _NAME_RE.search(message) + if m: + return m.group(1) or m.group(2) + # Fallback: grab first quoted word or first word + quote_m = re.search(r'"([^"]+)"', message) + if quote_m: + return quote_m.group(1) + parts = message.split() + return parts[0] if parts else message diff --git a/desloppify/languages/go/phases.py b/desloppify/languages/go/phases.py index 8687f0b0..b72fcf7b 100644 --- a/desloppify/languages/go/phases.py +++ b/desloppify/languages/go/phases.py @@ -8,9 +8,22 @@ from pathlib import Path from desloppify.engine.detectors.base import ComplexitySignal -from desloppify.languages._framework.base.shared_phases import run_structural_phase +from desloppify.languages._framework.base.shared_phases import ( + run_coupling_phase, + run_structural_phase, +) +from desloppify.languages._framework.finding_factories import ( + make_smell_findings, + make_unused_findings, +) from desloppify.languages._framework.runtime import LangRun +from desloppify.languages.go.detectors.deps import build_dep_graph +from desloppify.languages.go.detectors.gods import GO_GOD_RULES, extract_go_structs +from desloppify.languages.go.detectors.smells import detect_smells +from desloppify.languages.go.detectors.unused import detect_unused from desloppify.core.output_api import log +from desloppify.engine.policy.zones import adjust_potential +from desloppify.state import Finding GO_COMPLEXITY_SIGNALS = [ ComplexitySignal( @@ -59,10 +72,38 @@ def _phase_structural(path: Path, lang: LangRun) -> tuple[list[dict], dict[str, int]]: - """Run structural detectors (large/complexity/flat directories).""" + """Run structural detectors (large/complexity/flat directories/god structs).""" return run_structural_phase( path, lang, complexity_signals=GO_COMPLEXITY_SIGNALS, log_fn=log, + god_rules=GO_GOD_RULES, + god_extractor_fn=extract_go_structs, + ) + + +def _phase_smells(path: Path, lang: LangRun) -> tuple[list[Finding], dict[str, int]]: + """Run Go code smell detection.""" + entries, total_files = detect_smells(path) + return make_smell_findings(entries, log), { + "smells": adjust_potential(lang.zone_map, total_files), + } + + +def _phase_unused(path: Path, lang: LangRun) -> tuple[list[Finding], dict[str, int]]: + """Run Go unused symbol detection via staticcheck.""" + entries, total_files, _available = detect_unused(path) + return make_unused_findings(entries, log), { + "unused": adjust_potential(lang.zone_map, total_files), + } + + +def _phase_coupling(path: Path, lang: LangRun) -> tuple[list[dict], dict[str, int]]: + """Run coupling/cycles/orphaned detectors against the Go dep graph.""" + return run_coupling_phase( + path, + lang, + build_dep_graph_fn=build_dep_graph, + log_fn=log, ) diff --git a/desloppify/languages/go/tests/test_deps.py b/desloppify/languages/go/tests/test_deps.py new file mode 100644 index 00000000..979e56d5 --- /dev/null +++ b/desloppify/languages/go/tests/test_deps.py @@ -0,0 +1,204 @@ +"""Tests for desloppify.languages.go.detectors.deps — Go dependency graph builder.""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +from desloppify.languages.go.detectors.deps import ( + _extract_imports, + build_dep_graph, +) + + +# ── _extract_imports ──────────────────────────────────────── + + +def test_extract_single_import(): + content = 'package main\n\nimport "fmt"\n' + assert _extract_imports(content) == ["fmt"] + + +def test_extract_single_import_with_alias(): + content = 'package main\n\nimport f "fmt"\n' + assert _extract_imports(content) == ["fmt"] + + +def test_extract_grouped_imports(): + content = 'package main\n\nimport (\n "fmt"\n "os"\n)\n' + imports = _extract_imports(content) + assert "fmt" in imports + assert "os" in imports + assert len(imports) == 2 + + +def test_extract_grouped_imports_with_aliases(): + content = 'package main\n\nimport (\n f "fmt"\n . "os"\n "strings"\n)\n' + imports = _extract_imports(content) + assert "fmt" in imports + assert "os" in imports + assert "strings" in imports + assert len(imports) == 3 + + +def test_extract_mixed_imports(): + content = 'package main\n\nimport "log"\n\nimport (\n "fmt"\n "os"\n)\n' + imports = _extract_imports(content) + assert set(imports) == {"log", "fmt", "os"} + + +def test_extract_no_imports(): + content = "package main\n\nfunc main() {}\n" + assert _extract_imports(content) == [] + + +def test_extract_blank_lines_in_group(): + content = 'package main\n\nimport (\n "fmt"\n\n "os"\n)\n' + imports = _extract_imports(content) + assert "fmt" in imports + assert "os" in imports + + +# ── build_dep_graph ───────────────────────────────────────── + + +def _write(tmp_path: Path, name: str, content: str) -> str: + f = tmp_path / name + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(content) + return str(f) + + +def test_build_graph_empty_project(tmp_path): + with patch( + "desloppify.languages.go.detectors.deps.find_go_files", + return_value=[], + ): + graph = build_dep_graph(tmp_path) + assert graph == {} + + +def test_build_graph_no_local_imports(tmp_path): + filepath = _write( + tmp_path, + "main.go", + 'package main\n\nimport "fmt"\n\nfunc main() { fmt.Println("hi") }\n', + ) + with patch( + "desloppify.languages.go.detectors.deps.find_go_files", + return_value=[filepath], + ): + graph = build_dep_graph(tmp_path) + assert filepath in graph + assert graph[filepath]["import_count"] == 0 + + +def test_build_graph_local_import(tmp_path): + """When go.mod exists and imports resolve, edges should appear.""" + # Set up go.mod + (tmp_path / "go.mod").write_text("module example.com/myapp\n\ngo 1.21\n") + + # Create package files + pkg_dir = tmp_path / "pkg" + pkg_dir.mkdir() + pkg_file = _write(pkg_dir, "lib.go", "package pkg\n\nfunc Helper() {}\n") + main_file = _write( + tmp_path, + "main.go", + 'package main\n\nimport "example.com/myapp/pkg"\n\nfunc main() { pkg.Helper() }\n', + ) + + with patch( + "desloppify.languages.go.detectors.deps.find_go_files", + return_value=[main_file, pkg_file], + ): + graph = build_dep_graph(tmp_path) + + # main.go should import pkg/lib.go + assert graph[main_file]["import_count"] >= 1 + assert pkg_file in graph[main_file]["imports"] + # pkg/lib.go should be imported by main.go + assert main_file in graph[pkg_file]["importers"] + assert graph[pkg_file]["importer_count"] >= 1 + + +def test_build_graph_external_imports_ignored(tmp_path): + """External imports (not matching go.mod module) should not create edges.""" + (tmp_path / "go.mod").write_text("module example.com/myapp\n\ngo 1.21\n") + + main_file = _write( + tmp_path, + "main.go", + 'package main\n\nimport (\n "fmt"\n "github.com/other/lib"\n)\n\nfunc main() {}\n', + ) + + with patch( + "desloppify.languages.go.detectors.deps.find_go_files", + return_value=[main_file], + ): + graph = build_dep_graph(tmp_path) + + assert graph[main_file]["import_count"] == 0 + + +def test_build_graph_counts_correct(tmp_path): + """Verify import_count and importer_count are set by finalize_graph.""" + (tmp_path / "go.mod").write_text("module example.com/myapp\n\ngo 1.21\n") + + pkg_dir = tmp_path / "pkg" + pkg_dir.mkdir() + pkg_file = _write(pkg_dir, "lib.go", "package pkg\n\nfunc F() {}\n") + a_file = _write( + tmp_path, + "a.go", + 'package main\n\nimport "example.com/myapp/pkg"\n\nfunc a() { pkg.F() }\n', + ) + b_file = _write( + tmp_path, + "b.go", + 'package main\n\nimport "example.com/myapp/pkg"\n\nfunc b() { pkg.F() }\n', + ) + + with patch( + "desloppify.languages.go.detectors.deps.find_go_files", + return_value=[a_file, b_file, pkg_file], + ): + graph = build_dep_graph(tmp_path) + + # pkg_file should have 2 importers (cross-package) + assert graph[pkg_file]["importer_count"] == 2 + # Each main file should have 1 import + assert graph[a_file]["import_count"] == 1 + assert graph[b_file]["import_count"] == 1 + + +def test_same_package_files_not_orphaned(tmp_path): + """Files in the same package get implicit edges so they aren't orphaned.""" + lib_file = _write(tmp_path, "lib.go", "package mylib\n\nfunc Helper() {}\n") + util_file = _write(tmp_path, "util.go", "package mylib\n\nfunc Util() {}\n") + + with patch( + "desloppify.languages.go.detectors.deps.find_go_files", + return_value=[lib_file, util_file], + ): + graph = build_dep_graph(tmp_path) + + # Both files should have at least one importer (each other) + assert graph[lib_file]["importer_count"] >= 1 + assert graph[util_file]["importer_count"] >= 1 + + +def test_different_packages_same_dir_no_edges(tmp_path): + """Files with different package names in the same dir don't get linked.""" + main_file = _write(tmp_path, "main.go", "package main\n\nfunc main() {}\n") + lib_file = _write(tmp_path, "lib.go", "package lib\n\nfunc F() {}\n") + + with patch( + "desloppify.languages.go.detectors.deps.find_go_files", + return_value=[main_file, lib_file], + ): + graph = build_dep_graph(tmp_path) + + # Different packages — no implicit edges + assert graph[main_file]["importer_count"] == 0 + assert graph[lib_file]["importer_count"] == 0 diff --git a/desloppify/languages/go/tests/test_gods.py b/desloppify/languages/go/tests/test_gods.py new file mode 100644 index 00000000..98d32086 --- /dev/null +++ b/desloppify/languages/go/tests/test_gods.py @@ -0,0 +1,161 @@ +"""Tests for desloppify.languages.go.detectors.gods — Go god struct detection.""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +from desloppify.engine.detectors.gods import detect_gods +from desloppify.languages.go.detectors.gods import ( + GO_GOD_RULES, + extract_go_structs, +) + + +def _write(tmp_path: Path, name: str, content: str) -> str: + f = tmp_path / name + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(content) + return str(f) + + +def _extract(tmp_path: Path) -> list: + files = [str(f) for f in tmp_path.rglob("*.go")] + with patch( + "desloppify.languages.go.detectors.gods.find_go_files", + return_value=files, + ): + return extract_go_structs(tmp_path) + + +# ── extract_go_structs ───────────────────────────────────── + + +def test_extract_simple_struct(tmp_path): + _write( + tmp_path, + "model.go", + "package model\n\ntype User struct {\n Name string\n Age int\n}\n", + ) + structs = _extract(tmp_path) + assert len(structs) == 1 + assert structs[0].name == "User" + assert len(structs[0].attributes) == 2 + + +def test_extract_struct_with_methods(tmp_path): + _write( + tmp_path, + "model.go", + "package model\n\n" + "type User struct {\n Name string\n}\n\n" + "func (u *User) GetName() string {\n return u.Name\n}\n\n" + "func (u *User) SetName(name string) {\n u.Name = name\n}\n", + ) + structs = _extract(tmp_path) + assert len(structs) == 1 + assert structs[0].name == "User" + assert len(structs[0].methods) == 2 + + +def test_extract_methods_across_files(tmp_path): + _write( + tmp_path, + "model.go", + "package model\n\ntype User struct {\n Name string\n}\n", + ) + _write( + tmp_path, + "user_methods.go", + "package model\n\nfunc (u *User) Validate() error {\n return nil\n}\n", + ) + structs = _extract(tmp_path) + assert len(structs) == 1 + assert len(structs[0].methods) == 1 + + +def test_extract_multiple_structs(tmp_path): + _write( + tmp_path, + "model.go", + "package model\n\n" + "type User struct {\n Name string\n}\n\n" + "type Order struct {\n ID int\n Item string\n}\n", + ) + structs = _extract(tmp_path) + assert len(structs) == 2 + names = {s.name for s in structs} + assert names == {"User", "Order"} + + +def test_extract_struct_skips_comments(tmp_path): + _write( + tmp_path, + "model.go", + "package model\n\n" + "type User struct {\n" + " // Name is the user's name\n" + " Name string\n" + " Age int\n" + "}\n", + ) + structs = _extract(tmp_path) + assert len(structs) == 1 + # Comment lines should not count as fields + assert "Name" in structs[0].attributes + assert len(structs[0].attributes) == 2 + + +def test_metrics_populated(tmp_path): + _write( + tmp_path, + "model.go", + "package model\n\n" + "type User struct {\n Name string\n Age int\n}\n\n" + "func (u *User) Hello() {}\n", + ) + structs = _extract(tmp_path) + assert structs[0].metrics["field_count"] == 2 + assert structs[0].metrics["method_count"] == 1 + + +# ── God struct detection via detect_gods ──────────────────── + + +def test_god_struct_triggered(tmp_path): + """A struct with many methods AND many fields triggers god detection.""" + methods = "\n".join( + f"func (s *Big) Method{i}() {{}}" for i in range(12) + ) + fields = "\n".join(f" Field{i} int" for i in range(16)) + _write( + tmp_path, + "big.go", + f"package big\n\ntype Big struct {{\n{fields}\n}}\n\n{methods}\n", + ) + structs = _extract(tmp_path) + entries, total = detect_gods(structs, GO_GOD_RULES, min_reasons=2) + assert total == 1 + assert len(entries) == 1 + assert entries[0]["name"] == "Big" + assert len(entries[0]["reasons"]) >= 2 + + +def test_small_struct_not_flagged(tmp_path): + _write( + tmp_path, + "small.go", + "package small\n\ntype Small struct {\n X int\n}\n\n" + "func (s *Small) Get() int { return s.X }\n", + ) + structs = _extract(tmp_path) + entries, _ = detect_gods(structs, GO_GOD_RULES, min_reasons=2) + assert not entries + + +def test_god_rules_thresholds(): + """Verify the configured threshold values.""" + rules_by_name = {r.name: r for r in GO_GOD_RULES} + assert rules_by_name["method_count"].threshold == 10 + assert rules_by_name["field_count"].threshold == 15 + assert rules_by_name["loc"].threshold == 300 diff --git a/desloppify/languages/go/tests/test_security.py b/desloppify/languages/go/tests/test_security.py new file mode 100644 index 00000000..0bae73dc --- /dev/null +++ b/desloppify/languages/go/tests/test_security.py @@ -0,0 +1,193 @@ +"""Tests for desloppify.languages.go.detectors.security — Go security detection.""" + +from __future__ import annotations + +from pathlib import Path + +from desloppify.languages.go.detectors.security import detect_go_security + + +def _write(tmp_path: Path, name: str, content: str) -> str: + f = tmp_path / name + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(content) + return str(f) + + +def _detect(tmp_path: Path, filename: str, content: str) -> list[dict]: + filepath = _write(tmp_path, filename, content) + entries, _ = detect_go_security([filepath], zone_map=None) + return entries + + +# ── unsafe.Pointer ────────────────────────────────────────── + + +def test_unsafe_pointer(tmp_path): + entries = _detect( + tmp_path, + "hack.go", + 'package hack\n\nimport "unsafe"\n\nfunc f() {\n p := unsafe.Pointer(nil)\n _ = p\n}\n', + ) + ids = {e["detail"]["kind"] for e in entries} + assert "unsafe_pointer" in ids + + +# ── SQL injection ─────────────────────────────────────────── + + +def test_sql_injection_sprintf(tmp_path): + entries = _detect( + tmp_path, + "db.go", + 'package db\n\nfunc f(db *sql.DB, name string) {\n db.Query(fmt.Sprintf("SELECT * FROM users WHERE name = \'%s\'", name))\n}\n', + ) + ids = {e["detail"]["kind"] for e in entries} + assert "sql_injection" in ids + + +def test_sql_injection_concat(tmp_path): + entries = _detect( + tmp_path, + "db.go", + 'package db\n\nfunc f(db *sql.DB, name string) {\n db.Query("SELECT * FROM users WHERE name = " + name)\n}\n', + ) + ids = {e["detail"]["kind"] for e in entries} + assert "sql_injection" in ids + + +# ── Weak hash ────────────────────────────────────────────── + + +def test_weak_hash_md5(tmp_path): + entries = _detect( + tmp_path, + "hash.go", + "package hash\n\nfunc f() {\n h := md5.New()\n _ = h\n}\n", + ) + ids = {e["detail"]["kind"] for e in entries} + assert "weak_hash" in ids + + +def test_weak_hash_sha1(tmp_path): + entries = _detect( + tmp_path, + "hash.go", + "package hash\n\nfunc f() {\n h := sha1.Sum(nil)\n _ = h\n}\n", + ) + ids = {e["detail"]["kind"] for e in entries} + assert "weak_hash" in ids + + +# ── HTTP without TLS ──────────────────────────────────────── + + +def test_http_no_tls(tmp_path): + entries = _detect( + tmp_path, + "server.go", + 'package main\n\nfunc main() {\n http.ListenAndServe(":8080", nil)\n}\n', + ) + ids = {e["detail"]["kind"] for e in entries} + assert "http_no_tls" in ids + + +# ── Command injection ────────────────────────────────────── + + +def test_command_injection_variable(tmp_path): + entries = _detect( + tmp_path, + "exec.go", + "package exec\n\nfunc f(cmd string) {\n exec.Command(cmd)\n}\n", + ) + ids = {e["detail"]["kind"] for e in entries} + assert "command_injection" in ids + + +def test_command_injection_literal_not_flagged(tmp_path): + entries = _detect( + tmp_path, + "exec.go", + 'package exec\n\nfunc f() {\n exec.Command("ls", "-la")\n}\n', + ) + cmd_entries = [e for e in entries if e["detail"]["kind"] == "command_injection"] + assert not cmd_entries + + +# ── Hardcoded credentials ────────────────────────────────── + + +def test_hardcoded_credentials(tmp_path): + entries = _detect( + tmp_path, + "auth.go", + 'package auth\n\nfunc f() {\n password := "supersecretpassword123"\n _ = password\n}\n', + ) + ids = {e["detail"]["kind"] for e in entries} + assert "hardcoded_credentials" in ids + + +# ── Insecure random ──────────────────────────────────────── + + +def test_insecure_random_in_crypto_context(tmp_path): + entries = _detect( + tmp_path, + "token.go", + 'package token\n\nimport "math/rand"\n\nfunc GenerateToken() int {\n return rand.Int()\n}\n', + ) + ids = {e["detail"]["kind"] for e in entries} + assert "insecure_random" in ids + + +# ── Zone filtering ───────────────────────────────────────── + + +def test_test_files_skipped_with_zone_map(tmp_path): + from desloppify.engine.policy.zones import FileZoneMap, Zone, ZoneRule + + filepath = _write( + tmp_path, + "auth_test.go", + 'package auth\n\nfunc f() {\n password := "testpassword1234"\n _ = password\n}\n', + ) + zone_map = FileZoneMap( + [filepath], + [ZoneRule(Zone.TEST, ["_test.go"])], + rel_fn=lambda p: p, + ) + entries, scanned = detect_go_security([filepath], zone_map=zone_map) + assert scanned == 0 + assert not entries + + +# ── Clean code ───────────────────────────────────────────── + + +def test_clean_code_no_findings(tmp_path): + entries = _detect( + tmp_path, + "clean.go", + "package clean\n\nfunc Add(a, b int) int {\n return a + b\n}\n", + ) + assert not entries + + +def test_insecure_skip_verify(tmp_path): + entries = _detect( + tmp_path, + "lib.go", + 'package lib\n\nimport "crypto/tls"\n\nfunc f() *tls.Config {\n return &tls.Config{InsecureSkipVerify: true}\n}\n', + ) + ids = {e["detail"]["kind"] for e in entries} + assert "insecure_tls" in ids + + +def test_comment_lines_skipped(tmp_path): + entries = _detect( + tmp_path, + "lib.go", + "package lib\n\n// unsafe.Pointer is dangerous\nfunc f() {}\n", + ) + assert not entries diff --git a/desloppify/languages/go/tests/test_smells.py b/desloppify/languages/go/tests/test_smells.py new file mode 100644 index 00000000..04b32965 --- /dev/null +++ b/desloppify/languages/go/tests/test_smells.py @@ -0,0 +1,334 @@ +"""Tests for desloppify.languages.go.detectors.smells — Go code smell detection.""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +from desloppify.languages.go.detectors.smells import detect_smells + + +def _write(tmp_path: Path, name: str, content: str) -> str: + f = tmp_path / name + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(content) + return str(f) + + +def _detect(tmp_path: Path) -> tuple[list[dict], int]: + files = [str(f) for f in tmp_path.rglob("*.go") if "_test.go" not in f.name] + with patch( + "desloppify.languages.go.detectors.smells.find_go_files", + return_value=files, + ): + return detect_smells(tmp_path) + + +# ── Regex-based smells ────────────────────────────────────── + + +def test_ignored_error_underscore(tmp_path): + _write(tmp_path, "main.go", "package main\n\nfunc f() {\n _ = pkg.DoSomething()\n}\n") + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "ignored_error" in ids + + +def test_ignored_error_assigned_err(tmp_path): + _write(tmp_path, "main.go", "package main\n\nfunc f() {\n _ = someErr\n}\n") + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "ignored_error" in ids + + +def test_naked_return(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc f() (x int) {\n x = 1\n return\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "naked_return" in ids + + +def test_global_var(tmp_path): + _write(tmp_path, "lib.go", "package lib\n\nvar GlobalState int\n") + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "global_var" in ids + + +def test_global_var_cobra_command_not_flagged(tmp_path): + _write(tmp_path, "lib.go", 'package lib\n\nvar Cmd = &cobra.Command{}\n') + entries, _ = _detect(tmp_path) + global_entries = [e for e in entries if e["id"] == "global_var"] + assert not global_entries + + +def test_global_var_regexp_not_flagged(tmp_path): + _write(tmp_path, "lib.go", 'package lib\n\nvar re = regexp.MustCompile(`\\d+`)\n') + entries, _ = _detect(tmp_path) + global_entries = [e for e in entries if e["id"] == "global_var"] + assert not global_entries + + +def test_global_var_block_not_flagged(tmp_path): + """var ( ... ) blocks (enum patterns) should be skipped.""" + _write(tmp_path, "lib.go", "package lib\n\nvar (\n x = 1\n y = 2\n)\n") + entries, _ = _detect(tmp_path) + global_entries = [e for e in entries if e["id"] == "global_var"] + assert not global_entries + + +def test_hardcoded_url(tmp_path): + _write( + tmp_path, + "lib.go", + 'package lib\n\nfunc f() string {\n return "https://api.example.com/v1"\n}\n', + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "hardcoded_url" in ids + + +def test_hardcoded_url_constant_skipped(tmp_path): + _write( + tmp_path, + "lib.go", + 'package lib\n\nconst BaseURL = "https://api.example.com/v1"\n', + ) + entries, _ = _detect(tmp_path) + url_entries = [e for e in entries if e["id"] == "hardcoded_url"] + assert not url_entries + + +def test_todo_fixme(tmp_path): + _write(tmp_path, "lib.go", "package lib\n\nfunc f() {\n // TODO fix this\n}\n") + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "todo_fixme" in ids + + +def test_magic_number(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc f(x int) bool {\n return x >= 9999\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "magic_number" in ids + + +def test_error_wrap_verb(tmp_path): + _write( + tmp_path, + "lib.go", + 'package lib\n\nimport "fmt"\n\nfunc f(err error) error {\n return fmt.Errorf("failed: %v", err)\n}\n', + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "error_wrap_verb" in ids + + +def test_error_wrap_w_not_flagged(tmp_path): + _write( + tmp_path, + "lib.go", + 'package lib\n\nimport "fmt"\n\nfunc f(err error) error {\n return fmt.Errorf("failed: %w", err)\n}\n', + ) + entries, _ = _detect(tmp_path) + wrap_entries = [e for e in entries if e["id"] == "error_wrap_verb"] + assert not wrap_entries + + +def test_json_unmarshal_interface(tmp_path): + _write( + tmp_path, + "lib.go", + 'package lib\n\nimport "encoding/json"\n\nfunc f(data []byte) {\n var v interface{}\n json.Unmarshal(data, &v)\n}\n', + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "json_unmarshal_interface" in ids + + +# ── Multi-line smells ─────────────────────────────────────── + + +def test_empty_error_branch(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc f() {\n err := doSomething()\n if err != nil {\n }\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "empty_error_branch" in ids + + +def test_panic_in_lib(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc f() {\n panic(\"oh no\")\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "panic_in_lib" in ids + + +def test_panic_in_main_not_flagged(tmp_path): + _write( + tmp_path, + "main.go", + "package main\n\nfunc main() {\n panic(\"expected\")\n}\n", + ) + entries, _ = _detect(tmp_path) + panic_entries = [e for e in entries if e["id"] == "panic_in_lib"] + assert not panic_entries + + +def test_defer_in_loop(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc f() {\n for i := 0; i < 10; i++ {\n defer cleanup()\n }\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "defer_in_loop" in ids + + +def test_monster_function(tmp_path): + body_lines = "\n".join(f" x{i} := {i}" for i in range(155)) + _write( + tmp_path, + "lib.go", + f"package lib\n\nfunc BigFunc() {{\n{body_lines}\n}}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "monster_function" in ids + + +def test_dead_function(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc Noop() {\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "dead_function" in ids + + +def test_dead_function_with_comment_only(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc Noop() {\n // placeholder\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "dead_function" in ids + + +def test_init_side_effects(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nimport \"net/http\"\n\nfunc init() {\n http.Get(\"https://example.com\")\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "init_side_effects" in ids + + +def test_unreachable_code(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc f() int {\n return 1\n x := 2\n}\n", + ) + entries, _ = _detect(tmp_path) + ids = {e["id"] for e in entries} + assert "unreachable_code" in ids + + +def test_unreachable_code_closing_brace_not_flagged(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc f() int {\n return 1\n}\n", + ) + entries, _ = _detect(tmp_path) + unreachable = [e for e in entries if e["id"] == "unreachable_code"] + assert not unreachable + + +def test_unreachable_code_closure_brace_not_flagged(tmp_path): + """Closing }) or }), after return inside closure should not flag.""" + _write( + tmp_path, + "lib.go", + "package lib\n\n" + "func f() {\n" + " doSomething(func() error {\n" + " return nil\n" + " })\n" + "}\n", + ) + entries, _ = _detect(tmp_path) + unreachable = [e for e in entries if e["id"] == "unreachable_code"] + assert not unreachable + + +# ── Skipping / filtering ──────────────────────────────────── + + +def test_test_files_skipped(tmp_path): + _write( + tmp_path, + "lib_test.go", + "package lib\n\nfunc f() {\n panic(\"test panic\")\n}\n", + ) + # Also add a file that find_go_files would return + files = [str(tmp_path / "lib_test.go")] + with patch( + "desloppify.languages.go.detectors.smells.find_go_files", + return_value=files, + ): + entries, _ = detect_smells(tmp_path) + panic_entries = [e for e in entries if e["id"] == "panic_in_lib"] + assert not panic_entries + + +def test_clean_code_no_smells(tmp_path): + _write( + tmp_path, + "lib.go", + "package lib\n\nfunc Add(a, b int) int {\n return a + b\n}\n", + ) + entries, total = _detect(tmp_path) + assert total == 1 + assert not entries + + +def test_entries_sorted_by_severity(tmp_path): + # Write code that triggers both high and low severity smells + _write( + tmp_path, + "lib.go", + "package lib\n\n" + "// TODO fix\n" + "func f() {\n" + " _ = doSomething()\n" + "}\n", + ) + entries, _ = _detect(tmp_path) + if len(entries) >= 2: + severity_order = {"high": 0, "medium": 1, "low": 2} + severities = [severity_order.get(e["severity"], 9) for e in entries] + assert severities == sorted(severities) diff --git a/desloppify/languages/go/tests/test_unused.py b/desloppify/languages/go/tests/test_unused.py new file mode 100644 index 00000000..b0f294ee --- /dev/null +++ b/desloppify/languages/go/tests/test_unused.py @@ -0,0 +1,139 @@ +"""Tests for desloppify.languages.go.detectors.unused — staticcheck-based unused detection.""" + +from __future__ import annotations + +from unittest.mock import patch + +from desloppify.languages.go.detectors.unused import ( + _extract_name, + _parse_staticcheck_output, + detect_unused, +) + + +# ── _extract_name ─────────────────────────────────────────── + + +def test_extract_name_quoted(): + assert _extract_name('"MyFunc" is unused') == "MyFunc" + + +def test_extract_name_unquoted(): + assert _extract_name("MyFunc is unused") == "MyFunc" + + +def test_extract_name_assigned(): + assert _extract_name('"x" is assigned but never used') == "x" + + +def test_extract_name_fallback(): + assert _extract_name("something else entirely") == "something" + + +# ── _parse_staticcheck_output ─────────────────────────────── + + +def test_parse_u1000_unused(): + lines = [ + "pkg/handler.go:10:6: U1000 func handleRequest is unused", + ] + entries = _parse_staticcheck_output(lines, "all", "/project") + assert len(entries) == 1 + assert entries[0]["name"] == "handleRequest" + assert entries[0]["category"] == "exports" + assert entries[0]["line"] == 10 + assert entries[0]["file"] == "/project/pkg/handler.go" + + +def test_parse_sa4006_unused_var(): + lines = [ + 'pkg/handler.go:15:2: SA4006 "result" is assigned but never used', + ] + entries = _parse_staticcheck_output(lines, "all", "/project") + assert len(entries) == 1 + assert entries[0]["name"] == "result" + assert entries[0]["category"] == "vars" + + +def test_parse_filters_category(): + lines = [ + "pkg/handler.go:10:6: U1000 func handleRequest is unused", + 'pkg/handler.go:15:2: SA4006 "result" is assigned but never used', + ] + exports = _parse_staticcheck_output(lines, "exports", "/project") + assert len(exports) == 1 + assert exports[0]["category"] == "exports" + + vars_ = _parse_staticcheck_output(lines, "vars", "/project") + assert len(vars_) == 1 + assert vars_[0]["category"] == "vars" + + +def test_parse_skips_test_files(): + lines = [ + "pkg/handler_test.go:10:6: U1000 func testHelper is unused", + ] + entries = _parse_staticcheck_output(lines, "all", "/project") + assert not entries + + +def test_parse_skips_underscore_prefix(): + lines = [ + "pkg/handler.go:10:6: U1000 func _internalHelper is unused", + ] + entries = _parse_staticcheck_output(lines, "all", "/project") + assert not entries + + +def test_parse_absolute_paths(): + lines = [ + "/abs/path/handler.go:10:6: U1000 func Unused is unused", + ] + entries = _parse_staticcheck_output(lines, "all", "/project") + assert entries[0]["file"] == "/abs/path/handler.go" + + +def test_parse_malformed_lines_ignored(): + lines = [ + "not a valid staticcheck line", + "", + "# some comment", + ] + entries = _parse_staticcheck_output(lines, "all", "/project") + assert not entries + + +# ── detect_unused (integration with subprocess mock) ──────── + + +def test_detect_unused_staticcheck_not_installed(tmp_path): + """Graceful fallback when staticcheck is not available.""" + with patch( + "desloppify.languages.go.detectors.unused.find_go_files", + return_value=["a.go"], + ), patch( + "desloppify.languages.go.detectors.unused._try_staticcheck", + return_value=None, + ): + entries, total, available = detect_unused(tmp_path) + assert entries == [] + assert total == 1 + assert available is False + + +def test_detect_unused_returns_staticcheck_results(tmp_path): + """When staticcheck is available, returns parsed results.""" + mock_entries = [ + {"file": "a.go", "line": 5, "col": 1, "name": "unused", "category": "exports"}, + ] + with patch( + "desloppify.languages.go.detectors.unused.find_go_files", + return_value=["a.go", "b.go"], + ), patch( + "desloppify.languages.go.detectors.unused._try_staticcheck", + return_value=(mock_entries, True), + ): + entries, total, available = detect_unused(tmp_path) + assert entries == mock_entries + assert total == 2 + assert available is True diff --git a/desloppify/tests/commands/test_cmd_show.py b/desloppify/tests/commands/test_cmd_show.py index 496d7183..9705dbd9 100644 --- a/desloppify/tests/commands/test_cmd_show.py +++ b/desloppify/tests/commands/test_cmd_show.py @@ -118,6 +118,13 @@ def test_outliers_truncated(self): out_part = [p for p in parts if p.startswith("outliers:")][0] assert "f" not in out_part # Only first 5 + def test_string_detail_is_rendered(self): + parts = format_detail("Clusters: alpha, beta") + assert parts == ["detail: Clusters: alpha, beta"] + + def test_non_dict_non_string_detail_is_ignored(self): + assert format_detail(123) == [] + # --------------------------------------------------------------------------- # build_show_payload diff --git a/desloppify/tests/lang/common/test_phase_builders.py b/desloppify/tests/lang/common/test_phase_builders.py index 61e90c4d..17d4cef9 100644 --- a/desloppify/tests/lang/common/test_phase_builders.py +++ b/desloppify/tests/lang/common/test_phase_builders.py @@ -244,6 +244,8 @@ def test_make_cohesion_phase_run_with_entries(): assert potentials["responsibility_cohesion"] == 1 assert findings[0]["detector"] == "responsibility_cohesion" assert "disconnected function clusters" in findings[0]["summary"] + assert findings[0]["detail"]["cluster_count"] == 4 + assert findings[0]["detail"]["family"] == "network, database, ui, auth" def test_make_unused_imports_phase_run_with_entries(): diff --git a/pyproject.toml b/pyproject.toml index 311dda26..e9de0766 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -80,6 +80,7 @@ python_version = "3.11" warn_unused_configs = true warn_redundant_casts = true warn_unreachable = true +check_untyped_defs = true show_error_codes = true strict_optional = true ignore_missing_imports = true @@ -97,8 +98,20 @@ files = [ "desloppify/app/commands/scan/scan_reporting_presentation.py", "desloppify/app/commands/scan/scan_reporting_subjective.py", "desloppify/app/commands/scan/scan_workflow.py", + "desloppify/app/commands/show/formatting.py", + "desloppify/app/commands/show/render.py", "desloppify/app/cli_support/parser.py", "desloppify/app/cli_support/parser_groups.py", + "desloppify/engine/_state/filtering.py", "desloppify/engine/concerns.py", + "desloppify/languages/_framework/treesitter/phases.py", "desloppify/languages/_framework/generic.py", + "desloppify/languages/go/__init__.py", + "desloppify/languages/go/phases.py", + "desloppify/languages/go/commands.py", + "desloppify/languages/go/detectors/deps.py", + "desloppify/languages/go/detectors/smells.py", + "desloppify/languages/go/detectors/unused.py", + "desloppify/languages/go/detectors/security.py", + "desloppify/languages/go/detectors/gods.py", ]