use crate::code_lint::ast::{self, ParsedFile};
use crate::code_lint::contract::{CodeRule, RuleTarget};
use crate::diagnostic::{Diagnostic, Language, RuleName, ViolationTemplate, violation_template};
use crate::rule_declaration::{
Classification, Consensus, Declaration, Example, ImpactedQuality, Precision, RuleDoc,
RuleOptions, Topic,
};
use std::path::Path;
const TEMPLATE: ViolationTemplate = violation_template! {
summary: "Function `{function}` is defined inside another function.",
rationale: "A nested named function captures the enclosing scope implicitly and cannot be imported or tested on its own.",
suggestion: "Extract `{function}` to a module-level private function (`_{function}`), or use an inline `lambda` for a trivial callback.",
};
pub const RULE: CodeRule = CodeRule {
declaration: Declaration {
name: RuleName("nested-function"),
template: &TEMPLATE,
languages: &[Language::Python],
options: RuleOptions::code_rule(()),
classification: Classification {
topics: &[Topic::NESTING],
precision: Precision::Exact,
consensus: Consensus::Opinionated,
impacted_quality: ImpactedQuality::Maintainability,
},
doc: RuleDoc {
summary: "Flags Python functions defined inside other functions.",
what_it_does: indoc::indoc! {r"
Flags a `def` inside the body of another Python function or method. `lambda`
expressions and the methods of a class declared inside a function are not
flagged."},
why_is_this_bad: indoc::indoc! {r"
A nested function captures the enclosing function's local variables implicitly, so
its real inputs are not visible in its signature. It cannot be imported, tested or
reused on its own, and it makes the enclosing function longer and harder to follow.
Move the function to module level, conventionally with a leading underscore, and
pass what it needs as parameters. A `lambda` remains fine for a trivial callback
such as a sort key."},
known_problems: None,
references: &[],
examples: &[Example {
language: Language::Python,
flagged: indoc::indoc! {r"
def total_price(items):
def line_price(item):
return item.unit_price * item.quantity
return sum(line_price(item) for item in items)
"},
flagged_span: indoc::indoc! {r"
def line_price(item):
return item.unit_price * item.quantity
"},
fixed: indoc::indoc! {r"
def _line_price(item):
return item.unit_price * item.quantity
def total_price(items):
return sum(_line_price(item) for item in items)
"},
}],
},
},
target: RuleTarget::SourceOnly,
check: check_file,
};
fn check_file(rule: &CodeRule, path: &Path, file: &ParsedFile, (): ()) -> Vec<Diagnostic> {
ast::python::find_nested_functions(file)
.into_iter()
.map(|(func, func_name)| rule.diagnostic_at_node(path, &func, &[("function", &func_name)]))
.collect()
}
#[cfg(test)]
crate::test_utils::rule_test!(
RULE,
{
Python => {
pass: [
top_level_functions_allowed => r#"
def _compute_checksum(payload):
return len(payload)
def process(payload):
return _compute_checksum(payload)
"#,
class_methods_allowed => r#"
class Greeter:
def greet(self):
return "hello"
"#,
lambda_callbacks_allowed => r#"
def sort_items(items):
return sorted(items, key=lambda item: item.id)
"#,
method_of_local_class_not_flagged => r#"
def make_handler():
class Handler:
def handle(self, event):
return event
return Handler()
"#,
],
fail: [
nested_in_function => r#"
def outer():
def inner():
return 1
return inner()
"# => r#"
def inner():
return 1
"#,
nested_with_return_annotation => r#"
def outer() -> int:
def inner() -> int:
return 1
return inner()
"# => r#"
def inner() -> int:
return 1
"#,
nested_in_class_method => r#"
class Processor:
def run(self, data):
def transform(item):
return item * 2
return [transform(val) for val in data]
"# => r#"
def transform(item):
return item * 2
"#,
nested_in_method_of_local_class => r#"
def make_handler():
class Handler:
def handle(self, event):
def parse(raw):
return raw
return parse(event)
return Handler()
"# => r#"
def parse(raw):
return raw
"#,
],
},
}
);