1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
//! Refused Bequest detector - identifies improper inheritance
//!
//! Graph-aware detection of classes that inherit but don't use parent functionality.
//! A "refused bequest" occurs when a child class overrides parent methods
//! without calling super() or using parent functionality.
//!
//! Enhanced with graph analysis:
//! - Check if child is used polymorphically (through parent type) - if so, higher severity
//! - Trace inheritance depth - deep hierarchies with refused bequest are worse
//! - Analyze if overridden methods are actually called differently by callers
//!
//! Example:
//! ```text
//! class Bird {
//! fn fly(&self) { ... }
//! fn eat(&self) { ... }
//! }
//! class Penguin extends Bird {
//! fn fly(&self) { panic!("Penguins can't fly!") } // Refused bequest
//! fn eat(&self) { ... }
//! }
//! ```
//! Penguin shouldn't inherit from Bird if it can't fly.
use crate::detectors::base::{Detector, DetectorConfig};
use crate::graph::GraphQueryExt;
use crate::models::{Finding, Severity};
use anyhow::Result;
use std::collections::HashSet;
use std::path::PathBuf;
use tracing::{debug, info};
/// Thresholds for refused bequest detection
#[allow(dead_code)] // Config struct for refused bequest thresholds
#[derive(Debug, Clone)]
pub struct RefusedBequestThresholds {
/// Minimum overrides to consider
pub min_overrides: usize,
/// Flag if less than this ratio call parent
pub max_parent_call_ratio: f64,
}
impl Default for RefusedBequestThresholds {
fn default() -> Self {
Self {
min_overrides: 2,
max_parent_call_ratio: 0.3,
}
}
}
/// Patterns to exclude from detection (abstract base classes)
#[allow(dead_code)] // Used by is_abstract_parent
static EXCLUDE_PARENT_PATTERNS: &[&str] =
&["ABC", "Abstract", "Interface", "Base", "Mixin", "Protocol"];
/// Detects classes that inherit but don't use parent functionality
pub struct RefusedBequestDetector {
#[allow(dead_code)] // Stored for future config access
config: DetectorConfig,
#[allow(dead_code)] // Config field
thresholds: RefusedBequestThresholds,
}
impl RefusedBequestDetector {
/// Create a new detector with default thresholds
pub fn new() -> Self {
Self::with_thresholds(RefusedBequestThresholds::default())
}
/// Create with custom thresholds
pub fn with_thresholds(thresholds: RefusedBequestThresholds) -> Self {
Self {
config: DetectorConfig::new(),
thresholds,
}
}
/// Create with custom config
#[allow(dead_code)] // Builder pattern method
pub fn with_config(config: DetectorConfig) -> Self {
let thresholds = RefusedBequestThresholds {
min_overrides: config.get_option_or("min_overrides", 2),
max_parent_call_ratio: config.get_option_or("max_parent_call_ratio", 0.3),
};
Self { config, thresholds }
}
/// Check if parent is an abstract class
#[allow(dead_code)] // Helper for graph-based detection
fn is_abstract_parent(&self, parent_name: &str) -> bool {
if parent_name.is_empty() {
return false;
}
let parent_lower = parent_name.to_lowercase();
EXCLUDE_PARENT_PATTERNS
.iter()
.any(|pattern| parent_lower.contains(&pattern.to_lowercase()))
}
/// Calculate severity based on parent call ratio
#[allow(dead_code)] // Helper for graph-based detection
fn calculate_severity(&self, ratio: f64) -> Severity {
if ratio == 0.0 {
Severity::High
} else if ratio < 0.2 {
Severity::Medium
} else {
Severity::Low
}
}
/// Estimate effort based on severity
#[allow(dead_code)] // Helper for graph-based detection
fn estimate_effort(&self, severity: Severity) -> String {
match severity {
Severity::Critical | Severity::High => "Medium (2-4 hours)".to_string(),
Severity::Medium => "Medium (1-2 hours)".to_string(),
Severity::Low | Severity::Info => "Small (30-60 minutes)".to_string(),
}
}
/// Check if child class is used polymorphically (through parent type)
fn check_polymorphic_usage(
&self,
graph: &dyn crate::graph::GraphQuery,
child_qn: &str,
parent_qn: &str,
) -> bool {
let i = graph.interner();
// Look for functions that reference the parent type and might receive child instances
// This is heuristic - check if parent is used as parameter/return type in callers of child methods
let child_methods: Vec<_> = graph
.get_functions()
.into_iter()
.filter(|f| f.qn(i).starts_with(child_qn))
.collect();
for method in &child_methods {
let callers = graph.get_callers(method.qn(i));
for caller in &callers {
// If caller also calls parent methods, it might be polymorphic usage
let caller_callees = graph.get_callees(caller.qn(i));
for callee in &caller_callees {
if callee.qn(i).starts_with(parent_qn) {
debug!(
"Polymorphic usage: {} calls both {} and parent {}",
caller.node_name(i),
method.node_name(i),
parent_qn
);
return true;
}
}
}
}
false
}
/// Get the depth of the inheritance hierarchy
fn get_inheritance_depth(&self, graph: &dyn crate::graph::GraphQuery, class_qn: &str) -> usize {
let i = graph.interner();
let mut depth = 0;
let mut current = class_qn.to_string();
let mut seen = HashSet::new();
while let Some((_, parent)) = graph
.get_inheritance()
.into_iter()
.find(|(child, _)| i.resolve(*child) == current)
{
let parent_str = i.resolve(parent).to_string();
if seen.contains(&parent_str) {
break; // Avoid cycles
}
seen.insert(parent_str.clone());
current = parent_str;
depth += 1;
if depth > 10 {
break; // Safety limit
}
}
depth
}
/// Check if child class methods are called differently than parent
fn check_divergent_callers(
&self,
graph: &dyn crate::graph::GraphQuery,
child_methods: &[crate::graph::CodeNode],
parent_qn: &str,
) -> bool {
let i = graph.interner();
// Get parent methods
let parent_methods: Vec<_> = graph
.get_functions()
.into_iter()
.filter(|f| f.qn(i).starts_with(parent_qn))
.collect();
// For each child method, check if its callers are different from parent's callers
for child_method in child_methods {
let method_name = child_method.name;
let child_callers: HashSet<_> = graph
.get_callers(child_method.qn(i))
.into_iter()
.map(|c| c.qualified_name)
.collect();
// Find corresponding parent method
if let Some(parent_method) = parent_methods.iter().find(|m| m.name == method_name) {
let parent_callers: HashSet<_> = graph
.get_callers(parent_method.qn(i))
.into_iter()
.map(|c| c.qualified_name)
.collect();
// If there are callers unique to child, behavior might diverge
let unique_child_callers: HashSet<_> =
child_callers.difference(&parent_callers).collect();
if unique_child_callers.len() >= 2 {
return true;
}
}
}
false
}
}
impl Default for RefusedBequestDetector {
fn default() -> Self {
Self::new()
}
}
impl Detector for RefusedBequestDetector {
fn name(&self) -> &'static str {
"RefusedBequestDetector"
}
fn description(&self) -> &'static str {
"Detects classes that inherit but don't use parent functionality"
}
fn category(&self) -> &'static str {
"design"
}
fn config(&self) -> Option<&DetectorConfig> {
Some(&self.config)
}
fn detect(
&self,
ctx: &crate::detectors::analysis_context::AnalysisContext,
) -> Result<Vec<Finding>> {
let graph = ctx.graph;
let i = graph.interner();
let mut findings = Vec::new();
for (child_qn_key, parent_qn_key) in graph.get_inheritance() {
let child_qn = i.resolve(child_qn_key);
let parent_qn = i.resolve(parent_qn_key);
// Skip common patterns
if parent_qn.contains("Base")
|| parent_qn.contains("Abstract")
|| parent_qn.contains("Mixin")
{
continue;
}
if let Some(child) = graph.get_node(child_qn) {
// Check if child overrides many methods without calling super
let child_methods: Vec<_> = graph
.get_functions()
.into_iter()
.filter(|f| f.qn(i).starts_with(child_qn))
.collect();
if child_methods.len() >= 3 {
// Count methods that might be refusing bequest (low complexity overrides)
let potential_refusals: Vec<_> = child_methods
.iter()
.filter(|m| m.complexity_opt().unwrap_or(1) <= 2 && m.loc() <= 5)
.collect();
if potential_refusals.len() >= 2 {
// === Graph-enhanced analysis ===
// Check if child is used polymorphically (someone calls parent type methods on it)
let is_polymorphic =
self.check_polymorphic_usage(graph, child_qn, parent_qn);
// Check inheritance depth (deeper = worse)
let inheritance_depth = self.get_inheritance_depth(graph, parent_qn);
// Calculate if child methods are called differently from parent
let has_divergent_callers =
self.check_divergent_callers(graph, &child_methods, parent_qn);
// Determine severity based on graph analysis
let severity = if is_polymorphic && has_divergent_callers {
// Used polymorphically but behaves differently - LSP violation!
Severity::High
} else if is_polymorphic || inheritance_depth >= 3 {
// Either polymorphic usage or deep hierarchy
Severity::Medium
} else {
Severity::Low
};
// Build enhanced description
let mut notes = Vec::new();
if is_polymorphic {
notes.push("⚠️ Used polymorphically (through parent type)".to_string());
}
if has_divergent_callers {
notes.push("📞 Callers use it differently than parent".to_string());
}
if inheritance_depth >= 2 {
notes.push(format!("📊 Inheritance depth: {}", inheritance_depth));
}
let graph_notes = if notes.is_empty() {
String::new()
} else {
format!("\n\n**Graph analysis:**\n{}", notes.join("\n"))
};
findings.push(Finding {
id: String::new(),
detector: "RefusedBequestDetector".to_string(),
severity,
title: format!("Refused Bequest: {}", child.node_name(i)),
description: format!(
"Class '{}' inherits from '{}' but may not use inherited behavior properly.\n\n\
{} of {} methods appear to override without using parent.{}",
child.node_name(i),
parent_qn.rsplit("::").next().unwrap_or(parent_qn),
potential_refusals.len(),
child_methods.len(),
graph_notes
),
affected_files: vec![child.path(i).to_string().into()],
line_start: Some(child.line_start),
line_end: Some(child.line_end),
suggested_fix: Some(
if is_polymorphic {
format!(
"Since '{}' is used polymorphically, consider:\n\
1. Fix the overrides to properly extend parent behavior\n\
2. Extract a new interface that both classes implement\n\
3. Use the Strategy pattern if behavior varies",
child.node_name(i)
)
} else {
"Consider composition over inheritance if not using parent behavior".to_string()
}
),
estimated_effort: Some(if severity == Severity::High {
"Large (2-4 hours)".to_string()
} else {
"Medium (1-2 hours)".to_string()
}),
category: Some("structure".to_string()),
cwe_id: None,
why_it_matters: Some(
"Refused bequest indicates improper use of inheritance and may violate \
the Liskov Substitution Principle. This makes code harder to reason about \
and can lead to subtle bugs when the child is used in place of the parent."
.to_string()
),
..Default::default()
});
}
}
}
}
info!(
"RefusedBequestDetector found {} findings (graph-aware)",
findings.len()
);
Ok(findings)
}
}
impl crate::detectors::RegisteredDetector for RefusedBequestDetector {
fn create(_init: &crate::detectors::DetectorInit) -> std::sync::Arc<dyn Detector> {
std::sync::Arc::new(Self::new())
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_default_thresholds() {
let detector = RefusedBequestDetector::new();
assert_eq!(detector.thresholds.min_overrides, 2);
assert!((detector.thresholds.max_parent_call_ratio - 0.3).abs() < f64::EPSILON);
}
#[test]
fn test_is_abstract_parent() {
let detector = RefusedBequestDetector::new();
assert!(detector.is_abstract_parent("ABC"));
assert!(detector.is_abstract_parent("AbstractBase"));
assert!(detector.is_abstract_parent("BaseClass"));
assert!(detector.is_abstract_parent("UserInterface"));
assert!(detector.is_abstract_parent("MyMixin"));
assert!(!detector.is_abstract_parent("User"));
assert!(!detector.is_abstract_parent("OrderService"));
}
#[test]
fn test_severity_calculation() {
let detector = RefusedBequestDetector::new();
assert_eq!(detector.calculate_severity(0.0), Severity::High);
assert_eq!(detector.calculate_severity(0.1), Severity::Medium);
assert_eq!(detector.calculate_severity(0.2), Severity::Low);
assert_eq!(detector.calculate_severity(0.5), Severity::Low);
}
}