use crate::code_lint::ast::{self, CallClusterRule, OrderingReason, 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, Reference,
RuleDoc, RuleOptions, Topic,
};
use std::path::Path;
const TEMPLATE: ViolationTemplate = violation_template! {
summary: "Private `{function}` is defined before the higher-level private `{caller}`: {relation}.",
rationale: "Defining lower-abstraction private declarations above `{caller}` inverts the top-down reading flow.",
suggestion: "Move `{function}` below `{caller}`, for example below `{anchor}` (ends on line {line}).",
};
pub const RULE: CodeRule = CodeRule {
declaration: Declaration {
name: RuleName("callee-before-caller"),
template: &TEMPLATE,
languages: &[Language::Python, Language::Rust],
options: RuleOptions::code_rule(()),
classification: Classification {
topics: &[Topic::DECLARATION_ORDER],
precision: Precision::Exact,
consensus: Consensus::Opinionated,
impacted_quality: ImpactedQuality::Maintainability,
},
doc: RuleDoc {
summary: "Flags private functions and methods defined before their private callers.",
what_it_does: indoc::indoc! {r"
Flags a private function or method declared above a private function that calls
it, in a module, Python `class` or Rust inherent `impl` block: `_parse` above
`_load` when `_load` calls `_parse`. Private types follow the same top-down order:
a type comes after the function that uses it only inside its body, and before
every function that names it in its parameter or return types. Calls between
public functions and recursive calls are not constrained, and a function already
flagged by `private-before-public` or `uncolocated-helper` is not reported again."},
why_is_this_bad: indoc::indoc! {r"
Unlike public entrypoints, a private helper exists solely as an internal
decomposition step of its callers. Defining private callees above their private
callers forces readers to encounter low-level leaf utilities before the
higher-level helper logic that gives them context.
Order private helpers from higher-level callers down to lower-level callees."},
known_problems: Some(indoc::indoc! {r"
- Constants (`const`, `static`, `UPPER_CASE = ...`) and type aliases are not ordered.
- In Rust macro arguments, a pattern binding or named argument that shares a sibling
function's name counts as a call, which can create a false ordering constraint."}),
references: &[
Reference {
title: "Robert C. Martin: Clean Code — Chapter 5: Formatting (The Stepdown Rule)",
url: "https://www.oreilly.com/library/view/clean-code-a/9780136083238/",
},
Reference {
title: "Soloway & Ehrlich (1984): Empirical Studies of Programming Knowledge",
url: "https://ieeexplore.ieee.org/document/5010283",
},
],
examples: &[
Example {
language: Language::Python,
flagged: indoc::indoc! {r"
def parse_document(raw: str) -> list[str]:
return _split_lines(raw)
def _trim_line(line: str) -> str:
return line.strip()
def _split_lines(raw: str) -> list[str]:
return [_trim_line(line) for line in raw.splitlines()]
"},
flagged_span: "_trim_line",
fixed: indoc::indoc! {r"
def parse_document(raw: str) -> list[str]:
return _split_lines(raw)
def _split_lines(raw: str) -> list[str]:
return [_trim_line(line) for line in raw.splitlines()]
def _trim_line(line: str) -> str:
return line.strip()
"},
},
Example {
language: Language::Rust,
flagged: indoc::indoc! {r"
pub fn parse_config(raw: &str) -> Vec<&str> {
parse_lines(raw)
}
fn strip_line(line: &str) -> &str {
line.trim()
}
fn parse_lines(raw: &str) -> Vec<&str> {
raw.lines().map(strip_line).collect()
}
"},
flagged_span: "strip_line",
fixed: indoc::indoc! {r"
pub fn parse_config(raw: &str) -> Vec<&str> {
parse_lines(raw)
}
fn parse_lines(raw: &str) -> Vec<&str> {
raw.lines().map(strip_line).collect()
}
fn strip_line(line: &str) -> &str {
line.trim()
}
"},
},
],
},
},
target: RuleTarget::SourceOnly,
check: check_file,
};
fn check_file(rule: &CodeRule, path: &Path, file: &ParsedFile, (): ()) -> Vec<Diagnostic> {
ast::collect_call_cluster_findings(file, CallClusterRule::CalleeBeforeCaller)
.iter()
.map(|finding| {
let line = finding.anchor.line.to_string();
let relation = relation(finding.reason, &finding.related);
rule.diagnostic_at_node(
path,
&finding.name_node,
&[
("function", &finding.item),
("caller", &finding.related),
("relation", &relation),
("anchor", &finding.anchor.name),
("line", &line),
],
)
})
.collect()
}
fn relation(reason: Option<OrderingReason>, caller: &str) -> String {
match reason {
Some(OrderingReason::Call) | None => format!("`{caller}` calls it"),
Some(OrderingReason::Containment) => format!("`{caller}` contains it in its definition"),
Some(OrderingReason::ImplBehavior) => format!("the methods of `{caller}` use it"),
Some(OrderingReason::Encapsulation) => format!("`{caller}` uses it only inside its body"),
Some(OrderingReason::SignatureContract) => format!("its signature names `{caller}`"),
}
}
#[cfg(test)]
crate::test_utils::rule_test!(
RULE,
{
Python => {
pass: [
top_down_caller_before_callee_across_tiers => r#"
def compile_all(raw: str) -> str:
return compile_single(raw)
def compile_single(raw: str) -> str:
return _parse_tokens(raw)
def _parse_tokens(raw: str) -> str:
return _strip_whitespace(raw)
def _strip_whitespace(raw: str) -> str:
return raw.strip()
"#,
public_callee_before_public_caller_exempt => r#"
def parse_item(raw: str) -> str:
return raw.strip()
def parse_batch(items: list[str]) -> list[str]:
return [parse_item(item) for item in items]
"#,
mutual_recursion_exempt => r#"
def _even(number: int) -> bool:
return True if number == 0 else _odd(number - 1)
def _odd(number: int) -> bool:
return False if number == 0 else _even(number - 1)
"#,
function_flagged_by_earlier_stage_not_reported_again => r#"
class Pipeline:
def _flagged_by_stage_one(self, raw: str) -> str:
return raw.strip()
def run(self, raw: str) -> str:
return self._caller_below(raw)
def _caller_below(self, raw: str) -> str:
return self._flagged_by_stage_one(raw)
"#,
local_binding_shadowing_exempt => r#"
def evaluate(number: int) -> bool:
return _check(number) and _validate(number)
def _check(number: int) -> bool:
return number > 0
def _validate(number: int) -> bool:
_check = lambda value: value < 10
return _check(number)
"#,
],
fail: [
exclusive_helper_callee_before_caller => r#"
def run_pipeline(raw: str) -> str:
return _step_one(raw)
def _step_two(raw: str) -> str:
return raw.strip()
def _step_one(raw: str) -> str:
return _step_two(raw)
"# => "_step_two",
class_private_callee_before_private_caller => r#"
class Decoder:
def decode(self, raw: str) -> str:
return self._decode_frame(raw)
def _parse_byte(self, raw: str) -> str:
return raw.strip()
def _decode_frame(self, raw: str) -> str:
return self._parse_byte(raw)
"# => "_parse_byte",
],
},
Rust => {
pass: [
top_down_impl_order => r#"
pub struct Compiler;
impl Compiler {
pub fn new() -> Self {
Self
}
pub fn rebuild(&self) -> Self {
Self::new()
}
pub fn compile(&self, src: &str) -> usize {
self.lower_ast(src)
}
fn lower_ast(&self, src: &str) -> usize {
Self::count_bytes(src)
}
fn count_bytes(src: &str) -> usize {
src.len()
}
}
"#,
mutual_recursion_exempt => r#"
pub fn evaluate(n: usize) -> bool {
is_even(n)
}
fn is_even(n: usize) -> bool {
if n == 0 { true } else { is_odd(n - 1) }
}
fn is_odd(n: usize) -> bool {
if n == 0 { false } else { is_even(n - 1) }
}
"#,
local_binding_shadowing_exempt => r#"
pub fn evaluate(n: usize) -> bool {
check(n) && validate(n)
}
fn check(n: usize) -> bool {
n > 0
}
fn validate(n: usize) -> bool {
let check = |value: usize| value < 10;
check(n)
}
"#,
trait_impl_exempt => r#"
pub trait Codec {
fn finish(&self) -> usize;
fn encode(&self) -> usize;
}
pub struct Frame;
impl Codec for Frame {
fn finish(&self) -> usize {
0
}
fn encode(&self) -> usize {
self.finish()
}
}
"#,
macro_field_access_is_not_a_method_call => r#"
pub struct Buffer {
len: usize,
}
impl Buffer {
pub fn describe(&self) -> String {
self.render() + &self.len().to_string()
}
fn len(&self) -> usize {
self.len
}
fn render(&self) -> String {
format!("{}", self.len)
}
}
"#,
signature_closure_type_not_encapsulated_exempt => r#"
pub fn parse_config(raw: &str) -> usize {
parse_frame(raw).header.len
}
struct Frame {
header: Header,
}
struct Header {
len: usize,
}
fn parse_frame(raw: &str) -> Frame {
Frame { header: Header { len: raw.len() } }
}
"#,
],
fail: [
private_callee_before_private_caller_in_module => r#"
fn leaf_helper(input: &str) -> &str {
input.trim()
}
fn middle_helper(input: &str) -> usize {
leaf_helper(input).len()
}
"# => "leaf_helper",
call_inside_macro_counts => r#"
fn is_blank(input: &str) -> bool {
input.trim().is_empty()
}
fn validate(input: &str) {
assert!(!is_blank(input));
}
"# => "is_blank",
impl_private_callee_before_private_caller => r#"
pub struct Lexer;
impl Lexer {
pub fn tokenize(&self, input: &str) -> usize {
self.scan_tokens(input)
}
fn is_delimiter(ch: char) -> bool {
ch.is_whitespace()
}
fn scan_tokens(&self, input: &str) -> usize {
input.chars().filter(|&ch| Self::is_delimiter(ch)).count()
}
}
"# => "is_delimiter",
private_helper_type_before_private_caller => r#"
pub fn parse_config(raw: &str) -> usize {
decode_section(raw)
}
struct Frame {
len: usize,
}
fn decode_section(raw: &str) -> usize {
parse_frame(raw).len
}
fn parse_frame(raw: &str) -> Frame {
Frame { len: raw.len() }
}
"# => "Frame",
private_fn_before_its_return_type => r#"
pub fn parse_config(raw: &str) -> usize {
decode_section(raw)
}
fn decode_section(raw: &str) -> usize {
parse_frame(raw).len
}
fn parse_frame(raw: &str) -> Frame {
Frame { len: raw.len() }
}
struct Frame {
len: usize,
}
"# => "parse_frame",
private_fn_before_its_parameter_type => r#"
pub fn parse_config(raw: &str) -> usize {
measure(raw)
}
fn measure(raw: &str) -> usize {
let frame = Frame { len: raw.len() };
frame_len(&frame)
}
fn frame_len(frame: &Frame) -> usize {
frame.len
}
struct Frame {
len: usize,
}
"# => "frame_len",
cycle_through_public_entrypoint_not_exempt => r#"
pub fn evaluate(depth: usize) -> usize {
if depth == 0 { 0 } else { step_one(depth) }
}
fn step_two(depth: usize) -> usize {
evaluate(depth - 1)
}
fn step_one(depth: usize) -> usize {
step_two(depth)
}
"# => "step_two",
],
},
}
);