tldr_cli/commands/smells.rs
1//! Smells command - Detect code smells
2//!
3//! Identifies common code smells like God Class, Long Method, etc.
4//! Auto-routes through daemon when available for ~35x speedup.
5
6use std::path::PathBuf;
7
8use anyhow::Result;
9use clap::Args;
10
11use tldr_core::{
12 analyze_smells_aggregated_with_walker_opts, detect_smells_with_walker_opts, Language,
13 SmellType, SmellsReport, SmellsWalkerOpts, ThresholdPreset,
14};
15
16use crate::commands::daemon_router::{params_for_smells, try_daemon_route};
17use crate::output::{format_smells_text, OutputFormat, OutputWriter};
18
19/// Detect code smells
20#[derive(Debug, Args)]
21pub struct SmellsArgs {
22 /// Path to analyze (file or directory)
23 #[arg(default_value = ".")]
24 pub path: PathBuf,
25
26 /// Programming language to filter by (auto-detected if omitted)
27 #[arg(long, short = 'l')]
28 pub lang: Option<Language>,
29
30 /// Threshold preset
31 #[arg(long, short = 't', default_value = "default")]
32 pub threshold: ThresholdPresetArg,
33
34 /// Filter by smell type
35 #[arg(long, short = 's')]
36 pub smell_type: Option<SmellTypeArg>,
37
38 /// Include suggestions for fixing
39 #[arg(long)]
40 pub suggest: bool,
41
42 /// Deep analysis: aggregate findings from cohesion, coupling, dead code,
43 /// similarity, and cognitive complexity analyzers in addition to the
44 /// standard smell detectors
45 #[arg(long)]
46 pub deep: bool,
47
48 /// Walk vendored/build dirs (node_modules, target, dist, etc.) that would normally be skipped.
49 #[arg(long)]
50 pub no_default_ignore: bool,
51
52 /// Limit the scan to specific files (repeatable; EXACT-PATH-ONLY, no glob expansion).
53 /// Each entry is validated via `validate_file_path` (rejects path traversal /
54 /// non-existent files). When set, the path argument becomes a project-root
55 /// anchor for output ordering only and the walker is bypassed. Implies
56 /// `--include-tests` (caller picked the list).
57 #[arg(long)]
58 pub files: Vec<PathBuf>,
59
60 /// Include findings from test files. Default: test-file findings are excluded
61 /// (PR-review default). Implicit `true` when `--files` is non-empty.
62 #[arg(long)]
63 pub include_tests: bool,
64}
65
66/// CLI wrapper for threshold preset
67#[derive(Debug, Clone, Copy, Default, clap::ValueEnum)]
68pub enum ThresholdPresetArg {
69 /// Strict thresholds for high-quality codebases
70 Strict,
71 /// Default thresholds (recommended)
72 #[default]
73 Default,
74 /// Relaxed thresholds for legacy code
75 Relaxed,
76}
77
78impl From<ThresholdPresetArg> for ThresholdPreset {
79 fn from(arg: ThresholdPresetArg) -> Self {
80 match arg {
81 ThresholdPresetArg::Strict => ThresholdPreset::Strict,
82 ThresholdPresetArg::Default => ThresholdPreset::Default,
83 ThresholdPresetArg::Relaxed => ThresholdPreset::Relaxed,
84 }
85 }
86}
87
88/// CLI wrapper for smell type
89#[derive(Debug, Clone, Copy, clap::ValueEnum)]
90pub enum SmellTypeArg {
91 /// God Class (>20 methods or >500 LOC)
92 GodClass,
93 /// Long Method (>50 LOC or cyclomatic >10)
94 LongMethod,
95 /// Long Parameter List (>5 parameters)
96 LongParameterList,
97 /// Feature Envy
98 FeatureEnvy,
99 /// Data Clumps
100 DataClumps,
101 /// Low Cohesion (LCOM4 >= 2) -- requires --deep
102 LowCohesion,
103 /// Tight Coupling (score >= 0.6) -- requires --deep
104 TightCoupling,
105 /// Dead Code (unreachable functions) -- requires --deep
106 DeadCode,
107 /// Code Clone (similar functions) -- requires --deep
108 CodeClone,
109 /// High Cognitive Complexity (>= 15) -- requires --deep
110 HighCognitiveComplexity,
111 /// Deep Nesting (nesting depth >= 5)
112 DeepNesting,
113 /// Data Class (many fields, few/no methods)
114 DataClass,
115 /// Lazy Element (class with only 1 method and 0-1 fields)
116 LazyElement,
117 /// Message Chain (long method call chains > 3)
118 MessageChain,
119 /// Primitive Obsession (many primitive-typed parameters)
120 PrimitiveObsession,
121 /// Middle Man (>60% delegation) -- requires --deep
122 MiddleMan,
123 /// Refused Bequest (<33% inherited usage) -- requires --deep
124 RefusedBequest,
125 /// Inappropriate Intimacy (bidirectional coupling) -- requires --deep
126 InappropriateIntimacy,
127}
128
129impl From<SmellTypeArg> for SmellType {
130 fn from(arg: SmellTypeArg) -> Self {
131 match arg {
132 SmellTypeArg::GodClass => SmellType::GodClass,
133 SmellTypeArg::LongMethod => SmellType::LongMethod,
134 SmellTypeArg::LongParameterList => SmellType::LongParameterList,
135 SmellTypeArg::FeatureEnvy => SmellType::FeatureEnvy,
136 SmellTypeArg::DataClumps => SmellType::DataClumps,
137 SmellTypeArg::LowCohesion => SmellType::LowCohesion,
138 SmellTypeArg::TightCoupling => SmellType::TightCoupling,
139 SmellTypeArg::DeadCode => SmellType::DeadCode,
140 SmellTypeArg::CodeClone => SmellType::CodeClone,
141 SmellTypeArg::HighCognitiveComplexity => SmellType::HighCognitiveComplexity,
142 SmellTypeArg::DeepNesting => SmellType::DeepNesting,
143 SmellTypeArg::DataClass => SmellType::DataClass,
144 SmellTypeArg::LazyElement => SmellType::LazyElement,
145 SmellTypeArg::MessageChain => SmellType::MessageChain,
146 SmellTypeArg::PrimitiveObsession => SmellType::PrimitiveObsession,
147 SmellTypeArg::MiddleMan => SmellType::MiddleMan,
148 SmellTypeArg::RefusedBequest => SmellType::RefusedBequest,
149 SmellTypeArg::InappropriateIntimacy => SmellType::InappropriateIntimacy,
150 }
151 }
152}
153
154impl SmellsArgs {
155 /// Run the smells command
156 pub fn run(&self, format: OutputFormat, quiet: bool) -> Result<()> {
157 let writer = OutputWriter::new(format, quiet);
158
159 // BUG-11: validate path exists BEFORE any analysis. Without this
160 // check, a missing path silently slipped through: `is_dir()` returned
161 // false, the file branch ran with no files to scan, and the command
162 // returned exit 0 with empty results. Now: missing path => exit 1
163 // (matches `health`, `structure`, `deps`, `vuln`).
164 if !self.path.exists() {
165 anyhow::bail!("Path not found: {}", self.path.display());
166 }
167
168 // v0.2.3 (#1.D): when `--files` is non-empty, the caller explicitly named
169 // each path. Trust them and force `include_tests=true` so user-listed
170 // test files are not silently filtered.
171 let include_tests = self.include_tests || !self.files.is_empty();
172
173 // v0.2.3 (#1.D): each `--files` entry MUST go through the CORE
174 // validator (`tldr_core::validation::validate_file_path`) — same one
175 // the daemon uses. We pass the smells `path` argument as the project
176 // root so path-traversal attempts (`/etc/passwd`, `../../etc/...`)
177 // produce a hard error rather than a silent skip. Failures bubble up
178 // as a CLI error (non-zero exit), NOT a silent skip.
179 let project_root = if self.path.is_dir() {
180 // Try to canonicalize the path (so traversal checks work). Fall
181 // back to the literal path on canonicalize error (e.g. tmpdir
182 // shenanigans on macOS where /var -> /private/var).
183 dunce::canonicalize(&self.path).unwrap_or_else(|_| self.path.clone())
184 } else {
185 // For file paths, use the parent dir (or "." if none).
186 self.path
187 .parent()
188 .map(|p| dunce::canonicalize(p).unwrap_or_else(|_| p.to_path_buf()))
189 .unwrap_or_else(|| PathBuf::from("."))
190 };
191 let mut validated_files: Vec<PathBuf> = Vec::with_capacity(self.files.len());
192 for f in &self.files {
193 let f_str = f.to_str().ok_or_else(|| {
194 anyhow::anyhow!("--files entry contains non-UTF8 bytes: {:?}", f)
195 })?;
196 let canonical =
197 tldr_core::validation::validate_file_path(f_str, Some(&project_root))
198 .map_err(|e| anyhow::anyhow!("--files {}: {}", f.display(), e))?;
199 validated_files.push(canonical);
200 }
201
202 // Try daemon first for cached result
203 if let Some(report) = try_daemon_route::<SmellsReport>(
204 &self.path,
205 "smells",
206 params_for_smells(Some(&self.path), &validated_files, include_tests),
207 ) {
208 // Output based on format
209 if writer.is_text() {
210 let text = format_smells_text(&report);
211 writer.write_text(&text)?;
212 return Ok(());
213 } else {
214 writer.write(&report)?;
215 return Ok(());
216 }
217 }
218
219 // determinism-and-stderr-hygiene-v1 (BUG-18): the M14
220 // (med-cleanup-bundle-v1) deep-only-smells hint used to be
221 // unconditionally written to stderr via `eprintln!`, which
222 // broke the JSON-mode contract (`tldr smells <path> 2>err >
223 // out.json` always produced a non-empty stderr stream).
224 //
225 // Relocate the same advisory into `SmellsReport.warnings` so
226 // BOTH JSON consumers (introspectable via `report.warnings[]`)
227 // AND text consumers (rendered to stdout by the text
228 // formatter — see `format_smells_text`) still see it. Skip
229 // injection when `--quiet`, when the user asked for a single
230 // smell type via `--smell-type` (warning would be misleading),
231 // or when `--deep` is set (the analyzers ARE running).
232 let deep_only_warning: Option<String> =
233 (!self.deep && !quiet && self.smell_type.is_none()).then(|| {
234 const DEEP_ONLY_SMELLS: &[&str] = &[
235 "low_cohesion",
236 "tight_coupling",
237 "dead_code",
238 "code_clone",
239 "high_cognitive_complexity",
240 "middle_man",
241 "refused_bequest",
242 "inappropriate_intimacy",
243 ];
244 format!(
245 "Note: {} smell analyzers require --deep flag. Run with --deep for: {}",
246 DEEP_ONLY_SMELLS.len(),
247 DEEP_ONLY_SMELLS.join(", ")
248 )
249 });
250
251 // Fallback to direct compute
252 writer.progress(&format!(
253 "Scanning for code smells in {}{}...",
254 self.path.display(),
255 if self.deep { " (deep analysis)" } else { "" }
256 ));
257
258 // Detect smells - use aggregated analysis when --deep is set
259 let walker_opts = SmellsWalkerOpts {
260 no_default_ignore: self.no_default_ignore,
261 lang: self.lang,
262 files: validated_files,
263 include_tests,
264 };
265 let mut report = if self.deep {
266 analyze_smells_aggregated_with_walker_opts(
267 &self.path,
268 self.threshold.into(),
269 self.smell_type.map(|s| s.into()),
270 self.suggest,
271 walker_opts,
272 )?
273 } else {
274 detect_smells_with_walker_opts(
275 &self.path,
276 self.threshold.into(),
277 self.smell_type.map(|s| s.into()),
278 self.suggest,
279 walker_opts,
280 )?
281 };
282
283 if let Some(msg) = deep_only_warning {
284 report.warnings.push(msg);
285 }
286
287 // Output based on format
288 if writer.is_text() {
289 let text = format_smells_text(&report);
290 writer.write_text(&text)?;
291 } else {
292 writer.write(&report)?;
293 }
294
295 Ok(())
296 }
297}