From fdc2c9b1515d6d2216e7d25cf85f7c2c58fa9236 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sun, 27 Sep 2026 19:12:11 -0300 Subject: [PATCH 1/5] Split clones.rs into the matching core, the copies kept apart, the walk frame and its tests --- src/analysis/clones.rs | 1909 ---------------------------------- src/analysis/clones/apart.rs | 170 +++ src/analysis/clones/frame.rs | 261 +++++ src/analysis/clones/mod.rs | 838 +++++++++++++++ src/analysis/clones/tests.rs | 650 ++++++++++++ 5 files changed, 1919 insertions(+), 1909 deletions(-) delete mode 100644 src/analysis/clones.rs create mode 100644 src/analysis/clones/apart.rs create mode 100644 src/analysis/clones/frame.rs create mode 100644 src/analysis/clones/mod.rs create mode 100644 src/analysis/clones/tests.rs diff --git a/src/analysis/clones.rs b/src/analysis/clones.rs deleted file mode 100644 index 98e269e..0000000 --- a/src/analysis/clones.rs +++ /dev/null @@ -1,1909 +0,0 @@ -//! Type-2 clone candidates across the selected files and explicit context. -//! Identifiers and literals are normalized; windows start and end on whole -//! statements inside function bodies; identifiers must be renamed consistently. -use super::{ - fast_hash, is_comment, line_of, text, - units::{Kind, Unit}, -}; -use std::{ - collections::{BTreeMap, BTreeSet}, - ops::Range, - path::{Path, PathBuf}, -}; -use tree_sitter::Node; - -pub const MIN_BYTES: usize = 120; -/// Consecutive matching statements that seed a candidate window. -pub const MIN_STATEMENTS: usize = 2; -/// Statements a reported copy needs. -pub const MIN_CLONE_STATEMENTS: usize = 3; -pub const RUN_CAP: usize = 64; -pub const FILE_CAP: usize = 8; -const DIFFERENCES: usize = 12; -/// A statement pair repeated more often than this is an idiom; its extra pairs are not compared. -const SEED_OCCURRENCES: usize = 48; - -pub struct SourceFile<'a> { - pub path: &'a Path, - pub source: &'a str, - /// False for explicit context: a pair needs at least one selected site. - pub selected: bool, - pub units: &'a [Unit], - /// Lines excluded from comparison, such as test code when tests are not judged. - pub excluded: Vec>, - /// The package the file belongs to; explicit context has none. - pub package: Option<&'a crate::packages::Package>, -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Site { - pub file: usize, - pub path: PathBuf, - pub span: Range, - pub start_line: usize, - pub end_line: usize, - /// Enclosing function or method, when there is one. - pub function: Option, - pub function_source: Option, - pub quote: String, -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Difference { - pub a: String, - pub b: String, -} - -#[derive(Clone, Debug)] -pub struct Pair { - pub a: Site, - pub b: Site, - pub differences: Vec, - /// Non-whitespace bytes of the shorter site. - pub size: usize, - /// Distinct sites in this pair's clone group, including `a` and `b`. - pub occurrences: usize, - /// The group's other copies, reported with the judged pair. - pub copies: Vec, - /// Hash of the normalized statements; stable across renames and moves. - pub normalized: String, -} - -impl Pair { - pub fn rank(&self) -> usize { - self.size * self.occurrences - } -} - -#[derive(Default)] -pub struct Candidates { - pub pairs: Vec, - /// Pairs dropped by the per-run or per-file caps, by owning path. - pub omitted: BTreeMap, -} - -#[derive(Clone, Copy, PartialEq, Eq)] -enum TokenKind { - Identifier, - Literal, - Other, -} - -struct Token<'a> { - kind: TokenKind, - text: &'a str, - start: usize, - /// A call of the function the token sits in: `bound(child, …)` inside - /// `bound`. Recursion names the function itself, so two walks calling - /// themselves differ by no renamed name. - own: bool, -} - -/// Placeholders for renamed identifiers and literals. The control character -/// keeps them apart from any real token text. -const IDENTIFIER_TOKEN: &str = "\u{1}id"; -const LITERAL_TOKEN: &str = "\u{1}lit"; - -impl Token<'_> { - fn normal(&self) -> &str { - match self.kind { - TokenKind::Identifier => IDENTIFIER_TOKEN, - TokenKind::Literal => LITERAL_TOKEN, - TokenKind::Other => self.text, - } - } -} - -#[derive(Clone)] -struct Statement { - span: Range, - tokens: Range, - hash: u64, - /// Part of the frame of a walk rather than its work. - frame: Option, - /// Go's error check or deferred cleanup, which every call site repeats. - idiom: bool, -} - -/// The frame of a tree walk: the statements every walk has, whatever it -/// does at each node. -#[derive(Clone, Copy, PartialEq, Eq)] -enum Frame { - /// A branch that only leaves, as `if node.kind() == "call" { return; }`. - Exit, - /// A call of the function itself and nothing else, or a loop or branch - /// that only does that or leaves, as - /// `for child in node.named_children(&mut cursor) { bound(child, names); }`. - Recursion, -} - -struct Block { - file: usize, - statements: Vec, - /// The statements are the whole body of a function. - whole: bool, -} - -struct Parsed<'a> { - tokens: Vec>, -} - -pub fn find(files: &[SourceFile<'_>]) -> Candidates { - let (parsed, blocks) = statement_blocks(files); - let local: BTreeSet = files - .iter() - .filter_map(|f| f.package?.name.clone()) - .collect(); - let mut pairs: Vec = matching_windows(&blocks) - .into_iter() - .filter(|&((bx, _), (by, _), _)| { - let (a, b) = (&files[blocks[bx].file], &files[blocks[by].file]); - crate::packages::linked(a.package, b.package, &local) - && !separate_examples(a.path, b.path) - && !separate_tests(a, b) - }) - .filter_map(|window| pair(files, &parsed, &blocks, window)) - .filter(|p| !deprecated(files, &p.a) && !deprecated(files, &p.b)) - .filter(|p| !retired(&p.a.path) && !retired(&p.b.path)) - .collect(); - drop_nested(&mut pairs); - pairs.sort_by(by_rank); - let mut pairs = representatives(pairs); - // Groups rank by size times their number of copies. - pairs.sort_by(by_rank); - capped(one_per_function_pair(pairs)) -} - -/// Whether a copy lies in a function or type marked deprecated: it goes -/// with the next major version, so sharing its code with its replacement -/// is not worth doing. flysystem's deprecated phpseclib 2 adapter was -/// paired with its phpseclib 3 successor in 7 reviews. -fn deprecated(files: &[SourceFile<'_>], site: &Site) -> bool { - let file = &files[site.file]; - let Ok(Some(tree)) = crate::syntax::parse(file.path, file.source) else { - return false; - }; - let mut node = tree - .root_node() - .descendant_for_byte_range(site.span.start, site.span.start); - while let Some(current) = node { - let kind = current.kind(); - let declaration = kind.ends_with("_declaration") - || kind.ends_with("_definition") - || kind.ends_with("_item") - || matches!(kind, "method" | "class" | "module" | "function"); - if declaration && super::units::deprecated(current, file.source) { - return true; - } - node = current.parent(); - } - false -} - -/// Whether a file lies in a directory of retired code, such as -/// `deprecated`, `archive` or a proof of concept: like code marked -/// deprecated, it is not worth sharing code with. A Unity project's -/// `Assets/ProofOfConcept` builders, kept as a reference with no menu entry, -/// were paired with the live scene builders in six wrong reviews. `legacy` -/// is left out, since legacy code is often still served. -fn retired(path: &Path) -> bool { - path.parent().is_some_and(|dir| { - dir.iter().any(|part| { - let part = part - .to_string_lossy() - .to_ascii_lowercase() - .replace(['-', '_'], ""); - [ - "deprecated", - "archive", - "archived", - "attic", - "graveyard", - "retired", - "obsolete", - "proofofconcept", - "poc", - "pocs", - ] - .contains(&part.as_str()) - }) - }) -} - -/// A directory of example code: `examples`, `demo`, `tutorial`, or a name -/// such as `blog_examples`. -fn example_directory(part: &str) -> bool { - let part = part.to_ascii_lowercase(); - [ - "example", - "examples", - "demo", - "demos", - "tutorial", - "tutorials", - "docs_src", - ] - .contains(&part.as_str()) - || part.ends_with("_examples") - || part.ends_with("-examples") -} - -/// Two Bend 2 tests: each is a whole program pinned to the output its run -/// prints, so their copies are the point of each test. Of 4 shared-logic -/// findings between such tests on thirteen Bend 2 projects, all were wrong. -fn separate_tests(a: &SourceFile<'_>, b: &SourceFile<'_>) -> bool { - let test = |f: &SourceFile<'_>| { - crate::analysis::bend::file(f.path) - && crate::analysis::bend::expected_output(f.source).is_some() - }; - a.path != b.path && test(a) && test(b) -} - -/// Whether a file sits in a benchmark directory. -pub(crate) fn benchmark_code(path: &Path) -> bool { - path.parent().is_some_and(|dir| { - dir.iter() - .any(|part| benchmark_directory(&part.to_string_lossy())) - }) -} - -fn benchmark_directory(part: &str) -> bool { - matches!( - part.to_ascii_lowercase().as_str(), - "bench" | "benches" | "benchmark" | "benchmarks" - ) -} - -/// Whether a file is example code, written to be read beside other examples. -/// Also a top-level `samples` or `sample` directory (a Java package named -/// `samples` is source), a .NET project named like `MediatR.Examples.Autofac`, -/// and Go's `example_*_test.go` files, which show how to call a package. -/// Directories below a JVM source root (`src/main/java`) are packages, not -/// examples: Spring Initializr names a new project's package -/// `com.example.demo`, which made every finding of such a project a note. -pub(crate) fn example_code(path: &Path) -> bool { - let name = path - .file_name() - .map(|n| n.to_string_lossy().to_ascii_lowercase()) - .unwrap_or_default(); - let top = path - .iter() - .next() - .map(|p| p.to_string_lossy().to_ascii_lowercase()) - .filter(|_| path.iter().count() > 1) - .unwrap_or_default(); - let directories: Vec = path - .parent() - .map(|dir| { - dir.iter() - .map(|p| p.to_string_lossy().into_owned()) - .collect() - }) - .unwrap_or_default(); - let packages = jvm_source_root(&directories).unwrap_or(directories.len()); - (name.starts_with("example_") && name.ends_with("_test.go")) - || matches!(top.as_str(), "samples" | "sample") - || directories[..packages] - .iter() - .any(|part| example_directory(part) || part.to_ascii_lowercase().contains(".examples")) -} - -/// Where the package directories of a JVM source root begin: after -/// `src//java` (or `kotlin`, `scala`, `groovy`). -fn jvm_source_root(directories: &[String]) -> Option { - directories - .windows(3) - .position(|w| { - w[0] == "src" && matches!(w[2].as_str(), "java" | "kotlin" | "scala" | "groovy") - }) - .map(|at| at + 3) -} - -/// Whether two files are separate variants of one example, kept side by -/// side on purpose: under the same `examples` (or `demo`, `tutorial`) -/// directory, in different directories below it. django-styleguide shows a -/// Google login flow written by hand in `blog_examples/…/raw` and with the -/// SDK in `…/sdk`; their copies are the point. Benchmarks kept so are -/// separate programs too: each of bendlang/bend's `bench/runtime/*` is a -/// standalone program measured beside its C, TypeScript and Lean twins, -/// and the 6 shared-logic findings across them were labeled wrong. -fn separate_examples(a: &Path, b: &Path) -> bool { - let example = |part: &str| example_directory(part) || benchmark_directory(part); - let dirs = |p: &Path| -> Vec { - p.parent() - .map(|d| d.iter().map(|c| c.to_string_lossy().into_owned()).collect()) - .unwrap_or_default() - }; - let (a, b) = (dirs(a), dirs(b)); - let Some(root) = a.iter().zip(&b).position(|(x, y)| x == y && example(x)) else { - return false; - }; - a[..=root] == b[..=root] && a[root + 1..] != b[root + 1..] -} - -/// Two copied windows of the same two functions, split by one differing -/// statement, are one repetition: keep the higher-ranked pair only. -fn one_per_function_pair(pairs: Vec) -> Vec { - let mut seen = BTreeSet::new(); - pairs - .into_iter() - .filter(|pair| { - let (Some(a), Some(b)) = (&pair.a.function, &pair.b.function) else { - return true; - }; - let mut key = [(&pair.a.path, a), (&pair.b.path, b)]; - key.sort(); - seen.insert(key.map(|(path, name)| (path.clone(), name.clone()))) - }) - .collect() -} - -/// Tokens of every file and the statement blocks inside unit bodies. -fn statement_blocks<'a>(files: &[SourceFile<'a>]) -> (Vec>, Vec) { - let mut parsed = Vec::new(); - let mut blocks = Vec::new(); - for (index, file) in files.iter().enumerate() { - let Ok(Some(tree)) = crate::syntax::parse(file.path, file.source) else { - parsed.push(Parsed { tokens: Vec::new() }); - continue; - }; - let mut tokens = Vec::new(); - leaves(tree.root_node(), file.source, &mut tokens); - mark_recursion(&mut tokens, file.units, file.path); - let bodies: Vec> = file - .units - .iter() - .filter(|u| !u.equality) - .filter_map(|u| u.body.clone()) - .collect(); - collect_blocks(tree.root_node(), file, index, &bodies, &tokens, &mut blocks); - parsed.push(Parsed { tokens }); - } - (parsed, blocks) -} - -/// A maximal run of matching statement hashes: (block, start) twice and its length. -type Window = ((usize, usize), (usize, usize), usize); - -/// Seed on consecutive statement pairs, then extend each diagonal as far as the -/// hashes keep matching; a diagonal already covered is not reported again. -fn matching_windows(blocks: &[Block]) -> Vec { - let mut covered = BTreeSet::new(); - let mut found = Vec::new(); - for ((bx, kx), (by, ky)) in seed_pairs(blocks) { - let diagonal = (bx, by, kx as isize - ky as isize); - if covered.contains(&(diagonal, kx)) { - continue; - } - let n = extend(blocks, (bx, kx), (by, ky)); - covered.extend((0..n).map(|t| (diagonal, kx + t))); - if bx == by && one_run(&blocks[bx].statements[kx.min(ky)..kx.max(ky) + n]) { - continue; - } - found.push(((bx, kx), (by, ky), n)); - } - found -} - -/// Every two places one seed occurs, among its first `SEED_OCCURRENCES`; -/// two places in one block must be far enough apart not to overlap. -fn seed_pairs(blocks: &[Block]) -> impl Iterator { - seeds(blocks).into_values().flat_map(|mut places| { - places.truncate(SEED_OCCURRENCES); - let pairs: Vec<_> = places - .iter() - .enumerate() - .flat_map(|(x, &a)| places[x + 1..].iter().map(move |&b| (a, b))) - .filter(|&((bx, kx), (by, ky))| bx != by || ky >= kx + MIN_STATEMENTS) - .collect(); - pairs - }) -} - -/// Statements that all read alike, such as sqlite-utils' nine -/// `x = self.value_or_default("x", x)` lines or a list of lazy imports: a -/// list of one kind of statement, which matches itself shifted by one. -fn one_run(statements: &[Statement]) -> bool { - statements - .windows(2) - .all(|pair| pair[0].hash == pair[1].hash) -} - -/// Every place a pair of consecutive statement hashes occurs. -fn seeds(blocks: &[Block]) -> BTreeMap<(u64, u64), Vec<(usize, usize)>> { - let mut seeds = BTreeMap::<(u64, u64), Vec<(usize, usize)>>::new(); - for (b, block) in blocks.iter().enumerate() { - for k in 0..block.statements.len().saturating_sub(1) { - let key = (block.statements[k].hash, block.statements[k + 1].hash); - seeds.entry(key).or_default().push((b, k)); - } - } - seeds -} - -/// How many statements match from two seeds; a window never overlaps itself. -fn extend(blocks: &[Block], (bx, kx): (usize, usize), (by, ky): (usize, usize)) -> usize { - let (sx, sy) = (&blocks[bx].statements, &blocks[by].statements); - let mut n = MIN_STATEMENTS; - while kx + n < sx.len() - && ky + n < sy.len() - && sx[kx + n].hash == sy[ky + n].hash - && (bx != by || kx + n < ky) - { - n += 1; - } - n -} - -/// A candidate pair from one window, when its tokens align with consistent -/// renaming and it is large enough to report. The owner is a selected site. -fn pair( - files: &[SourceFile<'_>], - parsed: &[Parsed<'_>], - blocks: &[Block], - window: Window, -) -> Option { - let ((bx, kx), (by, ky), n) = window; - let (fx, fy) = (blocks[bx].file, blocks[by].file); - if !files[fx].selected && !files[fy].selected { - return None; - } - let x = &blocks[bx].statements[kx..kx + n]; - let y = &blocks[by].statements[ky..ky + n]; - let tx = &parsed[fx].tokens[x[0].tokens.start..x[n - 1].tokens.end]; - let ty = &parsed[fy].tokens[y[0].tokens.start..y[n - 1].tokens.end]; - let differences = align(tx, ty)?; - let span_x = x[0].span.start..x[n - 1].span.end; - let span_y = y[0].span.start..y[n - 1].span.end; - let size = - compact(&files[fx].source[span_x.clone()]).min(compact(&files[fy].source[span_y.clone()])); - // A repeated pair of statements is usually an idiom, such as a call and - // its check. - if n < MIN_CLONE_STATEMENTS - || size < MIN_BYTES - || only_frame(files, blocks, window) - || mostly_guards(files, blocks, window) - { - return None; - } - let normalized = crate::schema::hash( - tx.iter() - .map(Token::normal) - .collect::>() - .join(crate::schema::HASH_SEPARATOR) - .as_bytes(), - ); - let a = site(files, fx, span_x); - let b = site(files, fy, span_y); - // Ties keep path and line order. - let swap = !files[fx].selected - || (files[fy].selected && (&b.path, b.start_line) < (&a.path, a.start_line)); - let (a, b, differences) = if swap { - let flipped = differences - .into_iter() - .map(|d| Difference { a: d.b, b: d.a }) - .collect(); - (b, a, flipped) - } else { - (a, b, differences) - }; - Some(Pair { - a, - b, - differences, - size, - occurrences: 2, - copies: Vec::new(), - normalized, - }) -} - -/// Two parts of recursive functions that share little beyond the frame of a -/// walk: early exits and recursion into the function itself. Two walks that -/// stop at a different kind and recurse into their children share that -/// frame whatever they do at each node, so a copy of part of them needs two -/// statements beyond it, or one of half the size a copy needs. A copy of -/// the whole of both functions is a copy, however small its work. -fn only_frame(files: &[SourceFile<'_>], blocks: &[Block], window: Window) -> bool { - let ((bx, kx), (by, ky), n) = window; - let x = &blocks[bx].statements[kx..kx + n]; - let y = &blocks[by].statements[ky..ky + n]; - let recurses = |s: &[Statement]| s.iter().any(|s| s.frame == Some(Frame::Recursion)); - let whole = |b: usize, k: usize| blocks[b].whole && k == 0 && n == blocks[b].statements.len(); - if !recurses(x) || !recurses(y) || whole(bx, kx) && whole(by, ky) { - return false; - } - let work: Vec<(&Statement, &Statement)> = x - .iter() - .zip(y) - .filter(|(a, b)| a.frame.is_none() || b.frame.is_none()) - .collect(); - let bytes = |file: usize, s: &Statement| compact(&files[file].source[s.span.clone()]); - let size = work - .iter() - .map(|(a, _)| bytes(blocks[bx].file, a)) - .sum::() - .min(work.iter().map(|(_, b)| bytes(blocks[by].file, b)).sum()); - work.len() < MIN_STATEMENTS && size < MIN_BYTES / 2 -} - -/// Part of two Go functions that is mostly error checks and deferred -/// cleanups: `if err != nil { return err }` after each call and -/// `defer tx.Rollback()`. wtf's per-entity store functions shared a -/// transaction's begin, rollback and error checks around calls to their own -/// type's functions, which read as copies. A copy of part of them needs as -/// much other work as a copy of a walk's frame does; a copy of the whole of -/// both functions is still one. -fn mostly_guards(files: &[SourceFile<'_>], blocks: &[Block], window: Window) -> bool { - let ((bx, kx), (by, ky), n) = window; - let x = &blocks[bx].statements[kx..kx + n]; - let y = &blocks[by].statements[ky..ky + n]; - let whole = |b: usize, k: usize| blocks[b].whole && k == 0 && n == blocks[b].statements.len(); - if !x.iter().any(|s| s.idiom) || whole(bx, kx) && whole(by, ky) { - return false; - } - let work: Vec<(&Statement, &Statement)> = x - .iter() - .zip(y) - .filter(|(a, b)| !a.idiom || !b.idiom) - .collect(); - let bytes = |file: usize, s: &Statement| compact(&files[file].source[s.span.clone()]); - let size = work - .iter() - .map(|(a, _)| bytes(blocks[bx].file, a)) - .sum::() - .min(work.iter().map(|(_, b)| bytes(blocks[by].file, b)).sum()); - work.len() < MIN_CLONE_STATEMENTS && size < MIN_BYTES -} - -/// Go's `if err != nil { return …, err }` or a `defer` statement. -fn go_idiom(statement: Node<'_>, source: &str) -> bool { - match statement.kind() { - "defer_statement" => true, - "if_statement" => { - let checks_err = statement - .child_by_field_name("condition") - .is_some_and(|c| compact_text(&source[c.byte_range()]) == "err!=nil"); - let returns = statement - .child_by_field_name("consequence") - .is_some_and(|block| { - // Newer Go grammars wrap a block's statements in a list. - let list = block - .named_child(0) - .filter(|c| c.kind() == "statement_list") - .unwrap_or(block); - let mut cursor = list.walk(); - let body: Vec> = list.named_children(&mut cursor).collect(); - !body.is_empty() && body.iter().all(|s| s.kind() == "return_statement") - }); - checks_err && returns && statement.child_by_field_name("alternative").is_none() - } - _ => false, - } -} - -fn compact_text(text: &str) -> String { - text.chars().filter(|c| !c.is_whitespace()).collect() -} - -/// Drop pairs whose sites both lie inside a larger pair's sites. -fn drop_nested(pairs: &mut Vec) { - let snapshot = pairs.clone(); - pairs.retain(|p| { - !snapshot.iter().any(|q| { - let larger = q.a.span.len() + q.b.span.len() > p.a.span.len() + p.b.span.len(); - larger - && ((contains(&q.a, &p.a) && contains(&q.b, &p.b)) - || (contains(&q.a, &p.b) && contains(&q.b, &p.a))) - }) - }); -} - -/// Keep ranked groups within the per-run and per-file caps; count the rest. -fn capped(pairs: Vec) -> Candidates { - let mut omitted = BTreeMap::::new(); - let mut per_file = BTreeMap::::new(); - let mut kept = Vec::new(); - for pair in pairs { - let count = per_file.entry(pair.a.path.clone()).or_default(); - if kept.len() < RUN_CAP && *count < FILE_CAP { - *count += 1; - kept.push(pair); - } else { - *omitted.entry(pair.a.path.clone()).or_default() += 1; - } - } - Candidates { - pairs: kept, - omitted, - } -} - -fn by_rank(p: &Pair, q: &Pair) -> std::cmp::Ordering { - q.rank() - .cmp(&p.rank()) - .then_with(|| (&p.a.path, p.a.start_line).cmp(&(&q.a.path, q.a.start_line))) - .then_with(|| (&p.b.path, p.b.start_line).cmp(&(&q.b.path, q.b.start_line))) -} - -fn overlaps(x: &Site, y: &Site) -> bool { - x.path == y.path && x.span.start < y.span.end && y.span.start < x.span.end -} - -/// Sites that cover at least half of each other describe the same code. -fn same_code(x: &Site, y: &Site) -> bool { - if x.path != y.path { - return false; - } - let shared = x - .span - .end - .min(y.span.end) - .saturating_sub(x.span.start.max(y.span.start)); - 2 * shared >= x.span.len() && 2 * shared >= y.span.len() -} - -/// One judged pair per clone group. Pairs whose sites repeat the same code -/// (mutual half overlap) are linked; the first pair of each group in rank -/// order represents it and carries the other copies. Linking on plain overlap -/// let short idioms inside a larger copy chain unrelated code together. -fn representatives(pairs: Vec) -> Vec { - let groups = same_code_groups(&pairs); - let mut sites = BTreeMap::>::new(); - for (i, pair) in pairs.iter().enumerate() { - let group = sites.entry(groups[i]).or_default(); - for site in [&pair.a, &pair.b] { - if !group.iter().any(|known| overlaps(known, site)) { - group.push(site.clone()); - } - } - } - let mut kept = Vec::new(); - for (i, mut pair) in pairs.into_iter().enumerate() { - if groups[i] != i { - continue; - } - pair.copies = sites[&i] - .iter() - .filter(|s| !overlaps(s, &pair.a) && !overlaps(s, &pair.b)) - .cloned() - .collect(); - pair.copies - .sort_by(|x, y| (&x.path, x.start_line).cmp(&(&y.path, y.start_line))); - pair.occurrences = 2 + pair.copies.len(); - kept.push(pair); - } - kept -} - -/// For each pair, the first pair of its group: pairs whose sites repeat the -/// same code are linked, transitively (union-find). -fn same_code_groups(pairs: &[Pair]) -> Vec { - fn root(parent: &mut [usize], mut i: usize) -> usize { - while parent[i] != i { - parent[i] = parent[parent[i]]; - i = parent[i]; - } - i - } - let mut parent: Vec = (0..pairs.len()).collect(); - for i in 0..pairs.len() { - for j in i + 1..pairs.len() { - let (p, q) = (&pairs[i], &pairs[j]); - let linked = [&p.a, &p.b] - .iter() - .any(|x| same_code(x, &q.a) || same_code(x, &q.b)); - if linked { - let (ri, rj) = (root(&mut parent, i), root(&mut parent, j)); - parent[ri.max(rj)] = ri.min(rj); - } - } - } - (0..pairs.len()).map(|i| root(&mut parent, i)).collect() -} - -fn contains(outer: &Site, inner: &Site) -> bool { - outer.path == inner.path - && outer.span.start <= inner.span.start - && inner.span.end <= outer.span.end -} - -fn compact(text: &str) -> usize { - text.bytes().filter(|b| !b.is_ascii_whitespace()).count() -} - -fn site(files: &[SourceFile<'_>], index: usize, span: Range) -> Site { - let file = &files[index]; - let unit = file - .units - .iter() - .filter(|u| u.callable() && u.span.start <= span.start && span.end <= u.span.end) - .min_by_key(|u| u.span.len()); - Site { - file: index, - path: file.path.to_path_buf(), - start_line: line_of(file.source, span.start), - end_line: line_of(file.source, span.end.saturating_sub(1)), - function: unit.map(|u| u.name.clone()), - function_source: unit.map(|u| u.source(file.source).to_string()), - quote: file.source[span.clone()].to_string(), - span, - } -} - -/// Aligned tokens must match after normalization, and each identifier must map -/// to exactly one identifier on the other side. Returns renamed names and values. -fn align(x: &[Token<'_>], y: &[Token<'_>]) -> Option> { - if x.len() != y.len() { - return None; - } - let mut forward = BTreeMap::new(); - let mut backward = BTreeMap::new(); - let mut differences = Vec::new(); - for (a, b) in x.iter().zip(y) { - if a.normal() != b.normal() { - return None; - } - // Each side calling itself is the same step, not a rename. - if a.own && b.own { - continue; - } - if a.kind == TokenKind::Identifier - && (*forward.entry(a.text).or_insert(b.text) != b.text - || *backward.entry(b.text).or_insert(a.text) != a.text) - { - return None; - } - if a.kind != TokenKind::Other && a.text != b.text { - let difference = Difference { - a: a.text.to_string(), - b: b.text.to_string(), - }; - if !differences.contains(&difference) && differences.len() < DIFFERENCES { - differences.push(difference); - } - } - } - Some(differences) -} - -fn leaves<'a>(node: Node<'_>, source: &'a str, tokens: &mut Vec>) { - if is_comment(node) { - return; - } - let kind = node.kind(); - let literal = matches!( - kind, - "string_content" - | "string_fragment" - | "integer_literal" - | "float_literal" - | "char_literal" - | "decimal_integer_literal" - | "hex_integer_literal" - | "octal_integer_literal" - | "binary_integer_literal" - | "decimal_floating_point_literal" - | "hex_floating_point_literal" - | "character_literal" - | "number" - | "integer" - | "float" - | "string_literal_content" - | "raw_string_content" - | "verbatim_string_literal" - | "real_literal" - ); - if node.child_count() == 0 || literal { - let text = text(node, source); - if text.trim().is_empty() { - return; - } - let kind = if literal { - TokenKind::Literal - } else if kind.ends_with("identifier") - || matches!( - kind, - "identifier" | "constant" | "instance_variable" | "name" - ) - { - TokenKind::Identifier - } else { - TokenKind::Other - }; - tokens.push(Token { - kind, - text, - start: node.start_byte(), - own: false, - }); - return; - } - let mut cursor = node.walk(); - for child in node.children(&mut cursor) { - leaves(child, source, tokens); - } -} - -fn collect_blocks( - node: Node<'_>, - file: &SourceFile<'_>, - index: usize, - bodies: &[Range], - tokens: &[Token<'_>], - blocks: &mut Vec, -) { - if holds_statements(node) - && bodies - .iter() - .any(|b| b.start <= node.start_byte() && node.end_byte() <= b.end) - { - let all = block_statements(node, file, tokens); - // A Go body holds its statements in a `statement_list` inside the block. - let body = node - .parent() - .filter(|p| node.kind() == "statement_list" && p.kind() == "block") - .unwrap_or(node); - let whole = all.iter().all(Option::is_some) - && file - .units - .iter() - .any(|u| u.callable() && u.body == Some(body.byte_range())); - // Excluded statements break a window, so split the block there. - for statements in all.split(Option::is_none) { - let statements: Vec = statements.iter().flatten().cloned().collect(); - if !statements.is_empty() { - blocks.push(Block { - file: index, - statements, - whole, - }); - } - } - } - let mut cursor = node.walk(); - for child in node.named_children(&mut cursor) { - collect_blocks(child, file, index, bodies, tokens, blocks); - } -} - -/// A node whose named children are statements. Ruby holds statements in a -/// `body_statement` or `block_body`, and in the `then`, `else` and `do` of a -/// branch or loop; its `block` is a `{ … }` argument around a `block_body`. -/// PHP holds them in a `compound_statement`, and a Java constructor's -/// statements are in a `constructor_body`. -fn holds_statements(node: Node<'_>) -> bool { - let ruby_block = node.kind() == "block" && node.parent().is_some_and(|p| p.kind() == "call"); - matches!( - node.kind(), - "block" - | "statement_block" - | "statement_list" - | "compound_statement" - | "body_statement" - | "block_body" - | "then" - | "else" - | "do" - | "constructor_body" - ) && !ruby_block -} - -/// Objects a method calls itself on, as in `self.walk(`, `this.walk(`, -/// `Self::walk(`, `cls.walk(` or PHP's `$this->walk(` and `static::walk(`. -const RECEIVERS: [&str; 5] = ["self", "Self", "this", "cls", "static"]; - -/// Mark each call of the function it sits in: its name followed by its -/// arguments inside the body of the innermost callable of that name. A -/// method calls itself on the object itself (`self.walk(`, `this.walk(`, -/// `Self::walk(`); a bare `walk(` inside it calls a free or imported -/// function, except in Java, C# and Ruby, where a bare call reaches the -/// method through its object. A call through another path, as -/// `native::get()` inside `get`, names a different function. -fn mark_recursion(tokens: &mut [Token<'_>], units: &[Unit], path: &Path) { - let implicit = matches!( - path.extension().and_then(|x| x.to_str()), - Some("java" | "cs" | "rb") - ); - let callables: Vec<&Unit> = units.iter().filter(|u| u.callable()).collect(); - let names: BTreeSet<&str> = callables.iter().map(|u| u.short_name.as_str()).collect(); - for i in 1..tokens.len() { - let token = &tokens[i - 1]; - if token.kind != TokenKind::Identifier - || tokens[i].text != "(" - || !names.contains(token.text) - { - continue; - } - let Some(unit) = callables - .iter() - .filter(|u| u.body.as_ref().is_some_and(|b| b.contains(&token.start))) - .min_by_key(|u| u.span.len()) - .filter(|u| u.short_name == token.text) - else { - continue; - }; - let qualifier = i - .checked_sub(2) - .filter(|&q| matches!(tokens[q].text, "." | "::" | "->" | "?.")); - tokens[i - 1].own = match qualifier { - None => implicit || unit.kind == Kind::Function, - Some(q) => q - .checked_sub(1) - .is_some_and(|o| RECEIVERS.contains(&tokens[o].text)), - }; - } -} - -/// The tokens of one node. -fn tokens_of<'t, 'a>(tokens: &'t [Token<'a>], node: Node<'_>) -> &'t [Token<'a>] { - let start = tokens.partition_point(|t| t.start < node.start_byte()); - let end = tokens.partition_point(|t| t.start < node.end_byte()); - &tokens[start..end] -} - -/// Part of a walk's frame rather than its work, if it is. -fn frame(statement: Node<'_>, tokens: &[Token<'_>]) -> Option { - if exit_guard(statement, tokens) { - Some(Frame::Exit) - } else if recursion(statement, tokens) { - Some(Frame::Recursion) - } else { - None - } -} - -/// A call of the function itself and nothing else, as `bound(child, names);` -/// or `return self.walk(node.parent)`, or a loop or branch whose statements -/// only do that or leave early, with at least one call. Work anywhere else -/// in its body, as in a match arm, a conditional expression or a call that -/// wraps the recursion, makes it more than the frame. -fn recursion(statement: Node<'_>, tokens: &[Token<'_>]) -> bool { - if own_call(statement, tokens) { - return true; - } - let node = expression(statement); - let ruby_block = node.kind() == "call" && node.child_by_field_name("block").is_some(); - if !ruby_block - && !matches!( - node.kind(), - "for_expression" - | "for_statement" - | "for_in_statement" - | "enhanced_for_statement" - | "foreach_statement" - | "for" - | "while_expression" - | "while_statement" - | "while" - | "until" - | "loop_expression" - | "do_statement" - | "if_expression" - | "if_statement" - | "if" - | "unless" - ) - { - return false; - } - let mut body = Vec::new(); - branch_statements(node, &mut body); - body.iter().any(|&s| recursion(s, tokens)) - && body - .iter() - .all(|&s| exits(s, tokens) || exit_guard(s, tokens) || recursion(s, tokens)) -} - -/// A statement that is only a call of the function it sits in, as -/// `walk(child);`, `return self.walk(node.parent)`, `await this.walk(child);` -/// or `walk(child)?;`. -fn own_call(statement: Node<'_>, tokens: &[Token<'_>]) -> bool { - let mut words = tokens_of(tokens, statement); - if let [first, rest @ ..] = words - && matches!(first.text, "return" | "await") - { - words = rest; - } - while let [rest @ .., last] = words - && matches!(last.text, ";" | "?") - { - words = rest; - } - if let [receiver, separator, rest @ ..] = words - && RECEIVERS.contains(&receiver.text) - && matches!(separator.text, "." | "::" | "->" | "?.") - { - words = rest; - } - let [name, arguments @ ..] = words else { - return false; - }; - if !name.own || arguments.first().is_none_or(|t| t.text != "(") { - return false; - } - // The call's closing parenthesis ends the statement. - let mut depth = 0usize; - for (i, token) in arguments.iter().enumerate() { - match token.text { - "(" => depth += 1, - ")" => { - depth -= 1; - if depth == 0 { - return i + 1 == arguments.len(); - } - } - _ => {} - } - } - false -} - -/// A branch that only leaves, as `if node.kind() == "call" { return; }`, -/// `if (done) return;` or `return if done`. -fn exit_guard(statement: Node<'_>, tokens: &[Token<'_>]) -> bool { - let node = expression(statement); - if !matches!( - node.kind(), - "if_expression" | "if_statement" | "if" | "unless" | "if_modifier" | "unless_modifier" - ) { - return false; - } - let mut body = Vec::new(); - branch_statements(node, &mut body); - !body.is_empty() - && body - .into_iter() - .all(|s| exits(s, tokens) || exit_guard(s, tokens)) -} - -/// The expression a Rust or JavaScript statement wraps, as the `for` loop -/// of a Rust `expression_statement`. -fn expression(statement: Node<'_>) -> Node<'_> { - statement - .named_child(0) - .filter(|_| { - statement.kind() == "expression_statement" && statement.named_child_count() == 1 - }) - .unwrap_or(statement) -} - -/// The statements a loop or branch runs, in all its branches: the -/// statements of its blocks, or the one statement of a branch without -/// braces. Its header, as a condition or the collection a loop walks, is -/// not a statement. -fn branch_statements<'t>(node: Node<'t>, statements: &mut Vec>) { - let before = statements.len(); - let mut cursor = node.walk(); - for field in ["body", "consequence", "alternative", "block"] { - for part in node.children_by_field_name(field, &mut cursor) { - statements_in(part, statements); - } - } - // A JavaScript or Rust `else` holds its statement or block without a field. - if statements.len() == before && node.kind() == "else_clause" { - let mut cursor = node.walk(); - for part in node.named_children(&mut cursor) { - statements_in(part, statements); - } - } -} - -/// The statements of one part of a loop or branch. -fn statements_in<'t>(part: Node<'t>, statements: &mut Vec>) { - if is_comment(part) { - return; - } - if holds_statements(part) { - let mut cursor = part.walk(); - for child in part.named_children(&mut cursor) { - statements_in_block(child, statements); - } - } else if matches!( - part.kind(), - "else_clause" | "elif_clause" | "else_if_clause" | "elsif" | "block" | "do_block" - ) { - // Further branches, and the `{ … }` or `do … end` a Ruby call runs. - branch_statements(part, statements); - } else { - statements.push(part); - } -} - -/// One statement of a block; Go holds a block's statements in a list. -fn statements_in_block<'t>(child: Node<'t>, statements: &mut Vec>) { - if is_comment(child) { - return; - } - if holds_statements(child) { - statements_in(child, statements); - } else { - statements.push(child); - } -} - -/// `return`, `break`, `continue` or `next` with no value, or with nothing. -fn exits(statement: Node<'_>, tokens: &[Token<'_>]) -> bool { - match tokens_of(tokens, statement) { - [first, rest @ ..] => { - matches!(first.text, "return" | "break" | "continue" | "next") - && rest - .iter() - .all(|t| matches!(t.text, ";" | "None" | "nil" | "null")) - } - [] => false, - } -} - -/// A block's statements with normalized-token hashes; `None` for excluded lines. -fn block_statements( - node: Node<'_>, - file: &SourceFile<'_>, - tokens: &[Token<'_>], -) -> Vec> { - let mut statements = Vec::new(); - let mut cursor = node.walk(); - for child in node.named_children(&mut cursor) { - // A docstring documents; counted as a statement, it made one-line - // wrappers such as flask's `render_template` read as copies. - if is_comment(child) || super::comments::docstring(child).is_some() { - continue; - } - let line = line_of(file.source, child.start_byte()); - if file.excluded.iter().any(|r| r.contains(&line)) - || node.kind() == "constructor_body" && field_initializer(child) - || literal_setter(child, file.source) - { - statements.push(None); - continue; - } - let start = tokens.partition_point(|t| t.start < child.start_byte()); - let end = tokens.partition_point(|t| t.start < child.end_byte()); - let key = tokens[start..end] - .iter() - .map(Token::normal) - .collect::>() - .join(crate::schema::HASH_SEPARATOR); - statements.push(Some(Statement { - span: child.byte_range(), - tokens: start..end, - hash: fast_hash(&key), - frame: frame(child, tokens), - idiom: go_idiom(child, file.source), - })); - } - statements -} - -/// A Java constructor statement that stores a parameter, another object's -/// field or a literal in a field, as in `this.name = name;` or -/// `timeout = copy.timeout;`. A run of them is how a constructor fills its -/// fields: two constructors assigning different fields matched as copies. -fn field_initializer(statement: Node<'_>) -> bool { - let Some(assignment) = statement.named_child(0).filter(|a| { - statement.kind() == "expression_statement" && a.kind() == "assignment_expression" - }) else { - return false; - }; - let simple = |side: Option>, value: bool| { - side.is_some_and(|n| match n.kind() { - "identifier" => true, - "field_access" => n - .child_by_field_name("object") - .is_some_and(|o| matches!(o.kind(), "this" | "identifier")), - kind => value && (kind.ends_with("_literal") || matches!(kind, "true" | "false")), - }) - }; - assignment - .child_by_field_name("operator") - .is_some_and(|o| o.kind() == "=") - && simple(assignment.child_by_field_name("left"), false) - && simple(assignment.child_by_field_name("right"), true) -} - -/// A Java setter given one literal, as in `owner.setCity("Madison");`. A run -/// of them fills an object with data: a test fixture built in one test and a -/// helper building another owner matched as copies whose only differences -/// were the values. -fn literal_setter(statement: Node<'_>, source: &str) -> bool { - let Some(call) = statement - .named_child(0) - .filter(|c| statement.kind() == "expression_statement" && c.kind() == "method_invocation") - else { - return false; - }; - let setter = call.child_by_field_name("name").is_some_and(|name| { - text(name, source) - .strip_prefix("set") - .is_some_and(|rest| rest.starts_with(|c: char| c.is_ascii_uppercase())) - }); - let on_object = call - .child_by_field_name("object") - .is_some_and(|o| matches!(o.kind(), "identifier" | "this" | "field_access")); - let literal = call - .child_by_field_name("arguments") - .is_some_and(|arguments| { - arguments.named_child_count() == 1 - && arguments.named_child(0).is_some_and(|argument| { - argument.kind().ends_with("_literal") - || matches!(argument.kind(), "true" | "false") - }) - }); - setter && on_object && literal -} - -#[cfg(test)] -mod tests { - use super::*; - - /// Candidate pairs between two selected files, each a (path, source). - fn pairs_between(a: (&str, &str), b: (&str, &str)) -> usize { - run(&[(a.0, a.1, true), (b.0, b.1, true)]).pairs.len() - } - - /// The renamed names and values of the one pair between two selected files. - fn differences_between(a: (&str, &str), b: (&str, &str)) -> Vec { - let found = run(&[(a.0, a.1, true), (b.0, b.1, true)]); - assert_eq!(found.pairs.len(), 1); - found.pairs[0].differences.clone() - } - - fn run(files: &[(&str, &str, bool)]) -> Candidates { - let units: Vec<_> = files - .iter() - .map(|(path, source, _)| super::super::units::parse(Path::new(path), source).unwrap()) - .collect(); - let sources: Vec<_> = files - .iter() - .zip(&units) - .map(|((path, source, selected), units)| SourceFile { - path: Path::new(path), - source, - selected: *selected, - units: &units.units, - excluded: Vec::new(), - package: None, - }) - .collect(); - find(&sources) - } - - #[test] - fn copies_between_two_bend_tests_are_not_candidates() { - let program = |output: &str| { - format!( - "import Base\n\ndef main() -> IO(Unit):\n do IO:\n a : String <- IO.try(String, IO.get_env(\"HOME\"))\n b : String <- IO.try(String, IO.get_env(\"USER\"))\n c : String <- IO.try(String, IO.get_env(\"SHELL\"))\n IO.print(a ++ b ++ c)\n{output}" - ) - }; - let (golden, other) = (program("\n#|ok\n"), program("")); - assert_eq!( - pairs_between(("tests/io/a.bend", &golden), ("tests/io/b.bend", &golden)), - 0 - ); - assert_eq!( - pairs_between(("tests/io/a.bend", &other), ("tests/io/b.bend", &other)), - 1, - "tests that check themselves may share a helper" - ); - } - - #[test] - fn copies_in_unrelated_packages_are_not_candidates() { - let package = |dir: &str, dependencies: &[&str]| crate::packages::Package { - dir: dir.into(), - name: Some(dir.into()), - dependencies: dependencies.iter().map(|d| d.to_string()).collect(), - }; - let (a, b, shared) = ( - package("a", &["shared"]), - package("b", &["shared"]), - package("shared", &[]), - ); - let units = super::super::units::parse(Path::new("x.rs"), LOAD).unwrap(); - let file = |path: &'static str, package| SourceFile { - path: Path::new(path), - source: LOAD, - selected: true, - units: &units.units, - excluded: Vec::new(), - package, - }; - let separate = package("c", &[]); - assert!( - find(&[file("a/x.rs", Some(&a)), file("c/x.rs", Some(&separate))]) - .pairs - .is_empty() - ); - let linked = find(&[ - file("a/x.rs", Some(&a)), - file("b/x.rs", Some(&b)), - file("shared/x.rs", Some(&shared)), - ]); - assert!(!linked.pairs.is_empty()); - } - - const LOAD: &str = "fn load_user(path: &str) -> Result {\n let text = std::fs::read_to_string(path)?;\n let value: Value = serde_json::from_str(&text)?;\n let name = value[\"name\"].as_str().unwrap_or(\"anonymous\").trim().to_string();\n Ok(User { name })\n}\n"; - - #[test] - fn copies_in_deprecated_code_are_not_candidates() { - assert_eq!( - run(&[("a.rs", LOAD, true), ("b.rs", LOAD, true)]) - .pairs - .len(), - 1 - ); - for mark in [ - "#[deprecated(note = \"use load_account\")]\n", - "/// Deprecated: use load_account.\n", - "/** @deprecated use load_account */\n", - ] { - let old = format!("{mark}{LOAD}"); - assert!( - run(&[("a.rs", LOAD, true), ("b.rs", &old, true)]) - .pairs - .is_empty(), - "{mark}" - ); - } - // A method of a class whose documentation marks it deprecated. - let class = |doc: &str| { - format!( - "prefix->prefixPath($path);\n $contents = $this->connection->get($location);\n if ($contents === false) {{\n throw UnableToReadFile::fromLocation($path);\n }}\n return $contents;\n }}\n}}\n" - ) - }; - let current = class(""); - let legacy = class("/**\n * @deprecated use the V3 adapter\n */\n"); - let pairs = |b: &str| run(&[("v3/A.php", ¤t, true), ("v2/A.php", b, true)]).pairs; - assert_eq!(pairs(¤t).len(), 1); - assert!(pairs(&legacy).is_empty()); - // Only a declaration's own header and the lines above it count: - // another method's decorator, a parameter named `deprecated` or a - // mark named `deprecated_lifespan` leave the copy a candidate. - let python = |mark: &str| { - format!( - "import json\n\n\nclass Reader:\n @deprecated(\"use read\")\n def old(self):\n return None\n\n{mark} def load_user(self, path,\n deprecated: bool = False):\n text = open(path).read()\n value = json.loads(text)\n name = value[\"name\"].strip().lower().replace(\" \", \"_\")\n return User(name=name, path=path)\n" - ) - }; - let current = python(""); - let pairs = |b: &str| run(&[("a.py", ¤t, true), ("b.py", b, true)]).pairs; - assert_eq!(pairs(¤t).len(), 1); - assert_eq!(pairs(&python(" @deprecated_lifespan\n")).len(), 1); - assert!(pairs(&python(" @deprecated(\"use load\")\n")).is_empty()); - assert!(pairs(&python(" @typing_extensions.deprecated(\"x\")\n")).is_empty()); - } - - #[test] - fn copies_in_retired_directories_are_not_candidates() { - let pairs = |b: &str| run(&[("src/a.rs", LOAD, true), (b, LOAD, true)]).pairs; - assert_eq!(pairs("src/b.rs").len(), 1); - for retired in [ - "Assets/ProofOfConcept/Builder.rs", - "deprecated/b.rs", - "scripts/archive/b.rs", - "src/proof-of-concept/b.rs", - ] { - assert!(pairs(retired).is_empty(), "{retired}"); - } - assert_eq!( - pairs("src/legacy/b.rs").len(), - 1, - "legacy code is often live" - ); - } - - #[test] - fn renamed_copies_match_across_files_with_statement_aligned_quotes() { - let renamed = LOAD - .replace("load_user", "load_team") - .replace("text", "body") - .replace("value", "parsed") - .replace("\"name\"", "\"title\"") - .replace("User", "Team"); - let found = run(&[("a.rs", LOAD, true), ("b.rs", &renamed, true)]); - assert_eq!(found.pairs.len(), 1, "{:?}", found.pairs.len()); - let pair = &found.pairs[0]; - assert_eq!(pair.a.path, Path::new("a.rs")); - assert_eq!(pair.b.path, Path::new("b.rs")); - assert_eq!(pair.a.function.as_deref(), Some("load_user")); - assert_eq!(pair.b.function.as_deref(), Some("load_team")); - assert!( - pair.a - .quote - .starts_with("let text = std::fs::read_to_string") - ); - assert!(pair.a.quote.ends_with("Ok(User { name })")); - assert_eq!((pair.a.start_line, pair.a.end_line), (2, 5)); - assert!(pair.differences.contains(&Difference { - a: "text".into(), - b: "body".into() - })); - assert!(pair.differences.contains(&Difference { - a: "name".into(), - b: "title".into() - })); - assert_eq!(pair.occurrences, 2); - } - - #[test] - fn java_copies_are_candidates_but_equality_boilerplate_is_not() { - let position = "class Position {\n\tprivate final int line;\n\tprivate final int column;\n\n\t@Override\n\tpublic boolean equals(Object other) {\n\t\tif (this == other) return true;\n\t\tif (other == null || getClass() != other.getClass()) return false;\n\t\tPosition that = (Position) other;\n\t\tif (line != that.line) return false;\n\t\treturn column == that.column;\n\t}\n\n\tString describe(Map fields) {\n\t\tString text = fields.get(\"text\");\n\t\tString trimmed = text.trim();\n\t\tString lower = trimmed.toLowerCase();\n\t\tfields.put(\"text\", lower);\n\t\treturn lower + line;\n\t}\n}\n"; - let range = position - .replace("Position", "Range") - .replace("line", "start") - .replace("column", "end"); - let found = run(&[ - ("Position.java", position, true), - ("Range.java", &range, true), - ]); - let functions: Vec<_> = found - .pairs - .iter() - .map(|p| (p.a.function.as_deref(), p.b.function.as_deref())) - .collect(); - assert_eq!( - functions, - [(Some("Position::describe"), Some("Range::describe"))] - ); - } - - #[test] - fn java_constructors_filling_their_fields_are_not_copies() { - let position = "class Position {\n\tPosition(int sourceLineNumber, int sourceColumnNumber, int sourceByteOffset, int sourceCharacterOffset, int trackedPosition) {\n\t\tthis.sourceLineNumber = sourceLineNumber;\n\t\tthis.sourceColumnNumber = sourceColumnNumber;\n\t\tthis.sourceByteOffset = sourceByteOffset;\n\t\tthis.sourceCharacterOffset = sourceCharacterOffset;\n\t\tthis.trackedPosition = trackedPosition;\n\t\tthis.valid = true;\n\t}\n\n\tPosition(Position copy) {\n\t\tsourceLineNumber = copy.sourceLineNumber;\n\t\tsourceColumnNumber = copy.sourceColumnNumber;\n\t\tsourceByteOffset = copy.sourceByteOffset;\n\t\tsourceCharacterOffset = copy.sourceCharacterOffset;\n\t\ttrackedPosition = copy.trackedPosition;\n\t}\n}\n"; - let range = position - .replace("Position", "Range") - .replace("line", "start") - .replace("column", "end"); - assert_eq!( - pairs_between(("Position.java", position), ("Range.java", &range)), - 0 - ); - // Work beyond storing fields is still compared. - let worker = |name: &str| { - format!( - "class {name} {{\n\t{name}(Map fields) {{\n\t\tString text = fields.get(\"text\");\n\t\tString trimmed = text.trim();\n\t\tString lower = trimmed.toLowerCase();\n\t\tfields.put(\"text\", lower);\n\t\tfields.put(\"length\", String.valueOf(lower.length()));\n\t\tthis.fields = fields;\n\t}}\n}}\n" - ) - }; - let (a, b) = (worker("Position"), worker("Range")); - assert_eq!(pairs_between(("Position.java", &a), ("Range.java", &b)), 1); - } - - #[test] - fn java_setters_given_literals_are_data_not_copies() { - let fixture = "class OwnerTests {\n\tprivate Owner george() {\n\t\tOwner george = new Owner();\n\t\tgeorge.setFirstName(\"George\");\n\t\tgeorge.setLastName(\"Franklin\");\n\t\tgeorge.setAddress(\"110 W. Liberty St.\");\n\t\tgeorge.setCity(\"Madison\");\n\t\tgeorge.setTelephone(\"6085551023\");\n\t\treturn george;\n\t}\n}\n"; - let inline = "class ServiceTests {\n\tvoid insertsOwner() {\n\t\tOwner owner = new Owner();\n\t\towner.setFirstName(\"Sam\");\n\t\towner.setLastName(\"Schultz\");\n\t\towner.setAddress(\"4, Evans Street\");\n\t\towner.setCity(\"Wollongong\");\n\t\towner.setTelephone(\"4444444444\");\n\t\towners.save(owner);\n\t}\n}\n"; - assert_eq!( - pairs_between(("OwnerTests.java", fixture), ("ServiceTests.java", inline)), - 0 - ); - // Setters given computed values copy logic and are still compared. - let mapping = |name: &str| { - format!( - "class {name} {{\n\tOwnerDto map(Owner owner) {{\n\t\tOwnerDto dto = new OwnerDto();\n\t\tdto.setFirstName(owner.getFirstName().trim());\n\t\tdto.setLastName(owner.getLastName().trim());\n\t\tdto.setAddress(owner.getAddress().trim());\n\t\tdto.setCity(owner.getCity().toUpperCase());\n\t\tdto.setTelephone(owner.getTelephone().replace(\" \", \"\"));\n\t\treturn dto;\n\t}}\n}}\n" - ) - }; - let (a, b) = (mapping("OwnerMapper"), mapping("VetMapper")); - assert_eq!( - pairs_between(("OwnerMapper.java", &a), ("VetMapper.java", &b)), - 1 - ); - } - - /// A Ruby assignment's bound locals and a Rust `use` path's imported - /// names: two walks that share only the frame of skipping one kind and - /// recursing into their children. - const BOUND: &str = "fn bound(node: Node<'_>, source: &str, names: &mut Vec) {\n if node.kind() == \"identifier\" {\n names.push(text(node, source).to_string());\n return;\n }\n if node.kind() == \"call\" {\n return;\n }\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {\n bound(child, source, names);\n }\n}\n"; - const IMPORTS: &str = "fn imports(node: Node<'_>, source: &str, names: &mut BTreeSet) {\n if node.kind() == \"identifier\" {\n let name = text(node, source);\n if !matches!(name, \"self\" | \"super\" | \"crate\") {\n names.insert(name.to_string());\n }\n return;\n }\n if node.kind() == \"scoped_identifier\" {\n if let Some(name) = node.child_by_field_name(\"name\") {\n imports(name, source, names);\n }\n return;\n }\n if node.kind() == \"string\" {\n return;\n }\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {\n imports(child, source, names);\n }\n}\n"; - - #[test] - fn walks_sharing_only_an_early_exit_and_their_recursion_are_not_copies() { - assert_eq!( - pairs_between(("ruby.rs", BOUND), ("import_names.rs", IMPORTS)), - 0 - ); - // The same frame around a different exit, after a different first - // step, with the recursion written as a method on the walker itself, - // is still only the frame. - let method = |name: &str, first: &str, stop: &str, call: &str| { - format!( - "impl Walker {{\n fn {name}(&mut self, node: Node<'_>) {{\n {first}\n if node.is_missing() {{\n return;\n }}\n if node.kind() == \"{stop}\" {{\n return;\n }}\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {{\n if child.is_extra() {{\n continue;\n }}\n self.{call}(child);\n }}\n }}\n}}\n" - ) - }; - let (a, b) = ( - method("locals", "self.depth += 1;", "call", "locals"), - method( - "exports", - "self.seen.insert(node.id());", - "string", - "exports", - ), - ); - assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 0); - // Both calling another method is the same window, and a copy. - let (a, b) = ( - method("locals", "self.depth += 1;", "call", "visit"), - method("exports", "self.seen.insert(node.id());", "string", "visit"), - ); - assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 1); - // Exits without braces and `this.` recursion in JavaScript. - let script = |name: &str, first: &str, stop: &str| { - format!( - "class Scanner {{\n {name}(node) {{\n {first}\n if (!node) return;\n if (node.type === '{stop}') return;\n const children = node.namedChildren;\n for (const child of children) {{\n this.{name}(child);\n }}\n }}\n}}\n" - ) - }; - let (a, b) = ( - script("locals", "this.depth++;", "call_expression"), - script("exports", "this.seen.add(node.id);", "string"), - ); - assert_eq!(pairs_between(("a.js", &a), ("b.js", &b)), 0); - } - - #[test] - fn walks_copying_the_work_they_do_at_each_node_are_candidates() { - // Both walks collect the bound identifiers the same way; each calling - // itself is the same step, so their own names are no difference. - let copy = BOUND.replace("bound", "assigned"); - assert_eq!( - differences_between(("ruby.rs", BOUND), ("python.rs", ©)), - [] - ); - // A walk that does its work in the loop, around its recursion. - let visit = |name: &str, first: &str| { - format!( - "fn {name}(node: Node<'_>, source: &str, names: &mut Vec) {{\n {first}\n if node.kind() == \"call\" {{\n return;\n }}\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {{\n if child.kind() == \"identifier\" {{\n names.push(text(child, source).to_string());\n }}\n {name}(child, source, names);\n }}\n}}\n" - ) - }; - let (a, b) = (visit("locals", ""), visit("parameters", "")); - assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 1); - // Part of each, after a different first step. - let (a, b) = ( - visit("locals", "trace(node);"), - visit("parameters", "names.reserve(8);"), - ); - assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 1); - // Work in a match arm beside the recursion, after a different first - // step, so that only part of each walk is copied. - let arms = |name: &str, first: &str| { - format!( - "fn {name}(node: Node<'_>, source: &str, names: &mut Vec) {{\n {first}\n if node.is_missing() {{\n return;\n }}\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {{\n match child.kind() {{\n \"identifier\" => names.push(child.utf8_text(source.as_bytes()).unwrap().to_string()),\n \"call\" | \"string\" => {{}}\n _ => {name}(child, source, names),\n }}\n }}\n}}\n" - ) - }; - let (a, b) = ( - arms("locals", "trace(node);"), - arms("exports", "names.reserve(8);"), - ); - assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 1); - // Work in a conditional expression beside the recursion. - let ternary = |name: &str, first: &str| { - format!( - "class Walker {{\n {name}(node, names) {{\n {first}\n if (!node) return;\n if (node.type === 'comment') return;\n for (const child of node.namedChildren) child.type === 'identifier' ? names.push(child.text.trim().toLowerCase()) : this.{name}(child, names);\n }}\n}}\n" - ) - }; - let (a, b) = ( - ternary("collect", "this.depth++;"), - ternary("gather", "names.clear();"), - ); - assert_eq!(pairs_between(("a.js", &a), ("b.js", &b)), 1); - } - - #[test] - fn whole_copies_of_small_walks_and_guarded_handlers_are_candidates() { - // One step of work between an exit and the recursion, and no cursor. - let weights = |name: &str| { - format!( - "pub fn {name}(node: &Tree, scale: f64, out: &mut Vec) {{\n if node.hidden || node.children.is_empty() {{\n return;\n }}\n out.push(node.weight * scale + node.bias * node.decay.powi(2));\n for child in &node.children {{\n {name}(child, scale * node.decay, out);\n }}\n}}\n" - ) - }; - let (a, b) = (weights("total_weight"), weights("total_cost")); - assert_eq!(pairs_between(("weights.rs", &a), ("costs.rs", &b)), 1); - let labels = |name: &str| { - format!( - "def {name}(node, labels):\n if node is None or node.hidden:\n return\n labels.append(node.display_name.strip().lower().replace(\" \", \"_\"))\n for child in node.children:\n {name}(child, labels)\n" - ) - }; - let (a, b) = (labels("collect_names"), labels("gather_labels")); - assert_eq!(pairs_between(("names.py", &a), ("labels.py", &b)), 1); - // Exits alone are no walk: handlers that check and notify alike. - let handler = |name: &str, event: &str| { - format!( - "export function {name}({event}) {{\n if (!{event} || !{event}.repository) return;\n if ({event}.repository.archived || {event}.repository.disabled) return;\n if ({event}.sender && {event}.sender.type === 'Bot') return;\n notifyChannel({event}.repository.fullName, {event}.ref, {event}.headCommit);\n}}\n" - ) - }; - let (a, b) = (handler("onPush", "event"), handler("onTag", "payload")); - assert_eq!(pairs_between(("push.js", &a), ("tag.js", &b)), 1); - // Part of each, with little work after the exits. - let guarded = |name: &str, first: &str| { - format!( - "export function {name}(event) {{\n {first}\n if (!event || !event.repository) return;\n if (event.repository.archived || event.repository.disabled) return;\n if (event.sender && event.sender.type === 'Bot') return;\n notify(event);\n}}\n" - ) - }; - let (a, b) = ( - guarded("onPush", "log.debug('push');"), - guarded("onTag", "metrics.count('tag', 1);"), - ); - assert_eq!(pairs_between(("push.js", &a), ("tag.js", &b)), 1); - } - - #[test] - fn a_walk_with_one_large_step_of_work_is_a_candidate() { - let render = |name: &str, first: &str| { - format!( - "def {name}(node, out, depth=0):\n {first}\n if node is None or node.hidden:\n return\n out.write(\" \" * depth + f\"{{node.kind}} [{{node.start}}..{{node.end}}] {{node.name!r}} ({{len(node.children)}} children)\\n\")\n for child in node.children:\n {name}(child, out, depth + 1)\n" - ) - }; - // The whole of both functions. - let (a, b) = (render("render", "pass"), render("dump", "pass")); - assert_eq!(pairs_between(("render.py", &a), ("dump.py", &b)), 1); - // Part of each, after a different first step. - let (a, b) = ( - render("render", "depth = depth or 0"), - render("dump", "out.flush()"), - ); - assert_eq!(pairs_between(("render.py", &a), ("dump.py", &b)), 1); - } - - #[test] - fn bare_calls_in_methods_of_the_same_name_call_another_function() { - // `dump` inside the method `dump` is the imported `json.dump`. - let store = |owner: &str, module: &str, name: &str| { - format!( - "from {module} import {name}\n\n\nclass {owner}:\n def {name}(self, record):\n if record is None:\n return\n payload = {{\"id\": record.id, \"name\": record.name.strip(), \"tags\": sorted(record.tags), \"owner\": record.owner.email}}\n {name}(payload, self.handle, indent=2, sort_keys=True)\n" - ) - }; - let (a, b) = ( - store("JsonStore", "json", "dump"), - store("YamlStore", "yaml", "safe_dump"), - ); - let renamed = Difference { - a: "dump".into(), - b: "safe_dump".into(), - }; - assert!( - differences_between(("json_store.py", &a), ("yaml_store.py", &b)).contains(&renamed) - ); - // A Rust method calling a function of another module by its own name. - let cache = |owner: &str, name: &str| { - format!( - "impl {owner} {{\n fn {name}(&self, key: &str) -> Option {{\n let path = self.root.join(key.trim_start_matches('/'));\n let value = super::store::{name}(&path)?;\n self.hits.fetch_add(1, Ordering::Relaxed);\n Some(value.trim().to_string())\n }}\n}}\n" - ) - }; - let (a, b) = (cache("Cache", "get"), cache("Mirror", "fetch")); - let renamed = Difference { - a: "get".into(), - b: "fetch".into(), - }; - assert!(differences_between(("cache.rs", &a), ("mirror.rs", &b)).contains(&renamed)); - } - - #[test] - fn go_functions_sharing_only_error_checks_and_cleanups_are_not_copies() { - let store = |name: &str, find: &str, kind: &str, tail: &str| { - format!( - "package sqlite\n\nfunc (s *Service) {name}(ctx context.Context, id int) (*wtf.{kind}, error) {{\n\ttx, err := s.db.BeginTransactionWithOptions(ctx, nil)\n\tif err != nil {{\n\t\treturn nil, err\n\t}}\n\tdefer tx.Rollback()\n\trecord, err := {find}(ctx, tx, id)\n\tif err != nil {{\n\t\treturn nil, err\n\t}}\n\t{tail}\n\treturn record, nil\n}}\n" - ) - }; - let (a, b) = ( - store( - "FindAuthByID", - "findAuthByID", - "Auth", - "record.LastSeen = time.Now()", - ), - store( - "FindDialByID", - "findDialByID", - "Dial", - "go notify(record.ID, s.events)", - ), - ); - assert_eq!(pairs_between(("auth.go", &a), ("dial.go", &b)), 0); - // With more work between them, the error checks do not hide a copy. - let (a, b) = ( - a.replace("\trecord, err", "\tlog.Printf(\"loading one stored record by its identifier\")\n\tmetrics.Count(\"store.find\", 1)\n\trecord, err"), - b.replace("\trecord, err", "\tlog.Printf(\"loading one stored record by its identifier\")\n\tmetrics.Count(\"store.find\", 1)\n\trecord, err"), - ); - assert_eq!(pairs_between(("auth.go", &a), ("dial.go", &b)), 1); - } - - #[test] - fn a_list_of_alike_statements_is_not_a_copy_of_itself() { - let source = "def create(self, a=None, b=None, c=None, d=None, e=None, f=None):\n a = self.value_or_default(\"alpha_setting\", a)\n b = self.value_or_default(\"beta_setting\", b)\n c = self.value_or_default(\"gamma_setting\", c)\n d = self.value_or_default(\"delta_setting\", d)\n e = self.value_or_default(\"epsilon_setting\", e)\n f = self.value_or_default(\"zeta_setting\", f)\n return self.build(a, b, c, d, e, f)\n"; - assert!(run(&[("db.py", source, true)]).pairs.is_empty()); - } - - #[test] - fn variants_of_one_example_are_not_compared() { - let flow = |dir: &str| format!("examples/login/{dir}/apis.py"); - assert!(super::separate_examples( - Path::new(&flow("raw")), - Path::new(&flow("sdk")) - )); - assert!(super::separate_examples( - Path::new("app/blog_examples/raw/views.py"), - Path::new("app/blog_examples/sdk/views.py") - )); - assert!(!super::separate_examples( - Path::new("examples/login/raw/apis.py"), - Path::new("examples/login/raw/views.py") - )); - assert!(!super::separate_examples( - Path::new("src/billing/raw/apis.py"), - Path::new("src/billing/sdk/apis.py") - )); - // Benchmark programs side by side; one benchmark suite's files are one program. - assert!(super::separate_examples( - Path::new("bench/runtime/nbody/main.bend"), - Path::new("bench/runtime/mandelbrot/main.bend") - )); - assert!(!super::separate_examples( - Path::new("benchmarks/multipart_benchmark.py"), - Path::new("benchmarks/urlencoded_benchmark.py") - )); - for path in [ - "docs_src/tutorial/one/tutorial001.py", - "example/settings.py", - "examples/hello_world.rs", - ] { - assert!(super::example_code(Path::new(path)), "{path}"); - } - assert!(!super::example_code(Path::new("src/examples.rs"))); - for path in [ - "samples/MediatR.Examples/Runner.cs", - "src/MediatR.Examples.Autofac/Program.cs", - "example_authentication_middleware_test.go", - ] { - assert!(super::example_code(Path::new(path)), "{path}"); - } - assert!(!super::example_code(Path::new( - "src/main/java/org/springframework/samples/petclinic/Owner.java" - ))); - for path in [ - "src/main/java/com/example/demo/OrderController.java", - "service/src/test/kotlin/com/example/OrderTest.kt", - ] { - assert!(!super::example_code(Path::new(path)), "{path}"); - } - assert!(super::example_code(Path::new( - "examples/spring/src/main/java/com/example/demo/Main.java" - ))); - } - - #[test] - fn inconsistent_renaming_and_short_windows_are_rejected() { - // `text` becomes two different names on the other side. - let inconsistent = LOAD - .replace("let text", "let body") - .replace("from_str(&text)", "from_str(&other)"); - assert!( - run(&[("a.rs", LOAD, true), ("b.rs", &inconsistent, true)]) - .pairs - .is_empty() - ); - let short = "fn a(x: i32) -> i32 {\n let y = x + 1;\n y * 2\n}\nfn b(x: i32) -> i32 {\n let y = x + 1;\n y * 2\n}\n"; - assert!(run(&[("s.rs", short, true)]).pairs.is_empty()); - // A docstring is not a statement: two statements stay too few. - let wrappers = "def render_template(name, **context):\n \"\"\"Render a template by name with the given context and return the resulting page.\"\"\"\n app = current_app._get_current_object()\n return _render(app, app.jinja_env.get_or_select_template(name), context)\n\n\ndef stream_template(name, **context):\n \"\"\"Render a template by name with the given context as a stream of page parts.\"\"\"\n app = current_app._get_current_object()\n return _stream(app, app.jinja_env.get_or_select_template(name), context)\n"; - assert!(run(&[("templating.py", wrappers, true)]).pairs.is_empty()); - } - - #[test] - fn a_short_idiom_inside_a_larger_copy_forms_its_own_group() { - let head = " let text = std::fs::read_to_string(path).expect(\"reading the configured user file failed\");\n let value: Value = serde_json::from_str(&text).expect(\"parsing the configured user file failed\");\n let root = value.as_object().expect(\"the configured user file holds an object\");\n"; - let tail = " let name = root[\"name\"].as_str().unwrap_or(\"anonymous\").trim().to_string();\n let age = root[\"age\"].as_u64().unwrap_or(0).min(150) as u32;\n let city = root[\"city\"].as_str().unwrap_or(\"unknown\").trim().to_string();\n let email = root[\"email\"].as_str().unwrap_or(\"\").trim().to_lowercase();\n Ok(User { name, age, city, email })\n"; - let full = format!("fn load(path: &str) -> Result {{\n{head}{tail}}}\n"); - let other = full.replace("fn load", "fn again"); - let short = format!("fn count(path: &str) -> usize {{\n{head} root.len()\n}}\n"); - let found = run(&[ - ("a.rs", &full, true), - ("b.rs", &other, true), - ("c.rs", &short, true), - ]); - let largest = found.pairs.iter().max_by_key(|p| p.size).unwrap(); - assert_eq!( - (largest.a.path.as_path(), largest.b.path.as_path()), - (Path::new("a.rs"), Path::new("b.rs")) - ); - assert!(largest.copies.is_empty(), "{:?}", largest.copies); - assert_eq!(found.pairs.len(), 2); - } - - #[test] - fn context_only_pairs_are_excluded_but_selected_to_context_pairs_are_kept() { - let copy = LOAD.replace("load_user", "load_again"); - assert!( - run(&[("a.rs", LOAD, false), ("b.rs", ©, false)]) - .pairs - .is_empty() - ); - let found = run(&[("context.rs", LOAD, false), ("selected.rs", ©, true)]); - assert_eq!(found.pairs.len(), 1); - assert_eq!(found.pairs[0].a.path, Path::new("selected.rs")); - assert_eq!(found.pairs[0].b.path, Path::new("context.rs")); - } - - #[test] - fn repeated_copies_form_one_group_judged_through_one_pair() { - let three = [ - LOAD, - &LOAD.replace("load_user", "second"), - &LOAD.replace("load_user", "third"), - ] - .concat(); - let found = run(&[("three.rs", &three, true)]); - assert_eq!(found.pairs.len(), 1); - let pair = &found.pairs[0]; - assert_eq!((pair.occurrences, pair.a.start_line), (3, 2)); - assert_eq!(pair.copies.len(), 1); - let lines = [pair.b.start_line, pair.copies[0].start_line]; - assert!(lines.contains(&8) && lines.contains(&14), "{lines:?}"); - - // Copies of different lengths still share a group through overlapping sites. - let longer = LOAD.replace( - " Ok(User { name })", - " let checked = name.trim().to_string();\n Ok(User { name: checked })", - ); - let found = run(&[ - ("a.rs", LOAD, true), - ("b.rs", &LOAD.replace("load_user", "other"), true), - ("c.rs", &longer.replace("load_user", "third"), true), - ]); - assert_eq!( - found.pairs.len(), - 1, - "{:?}", - found - .pairs - .iter() - .map(|p| (&p.a.path, &p.b.path)) - .collect::>() - ); - assert_eq!(found.pairs[0].occurrences, 3); - let body = " total = decimal.Decimal(\"0\")\n for row in rows:\n total += row.amount * row.exchange_rate - row.discount_amount\n return total.quantize(decimal.Decimal(\"0.01\"), rounding=decimal.ROUND_HALF_UP)\n"; - let python = format!( - "def a(rows):\n{body}\ndef b(items):\n{}", - body.replace("row", "item") - ); - let found = run(&[("totals.py", &python, true)]); - assert_eq!(found.pairs.len(), 1); - assert_eq!(found.pairs[0].a.function.as_deref(), Some("a")); - } - - #[test] - fn windows_of_the_same_two_functions_split_by_one_statement_are_one_pair() { - let head = " parser = argparse.ArgumentParser(description=__doc__)\n parser.add_argument('--owner', default='Tech')\n parser.add_argument('--project', type=int, default=2)\n parser.add_argument('--apply', action='store_true')\n"; - let tail = " args = parser.parse_args()\n if args.apply and not args.backup:\n parser.error('--apply requires --backup')\n run(args.owner, args.project, args.apply, args.backup)\n"; - let first = format!( - "def main():\n{head} parser.add_argument('--completed', action='store_true')\n{tail}" - ); - let second = format!("def main():\n{head}{tail}"); - let found = run(&[("migrate.py", &first, true), ("retire.py", &second, true)]); - assert_eq!( - found - .pairs - .iter() - .map(|p| (p.a.start_line, p.b.start_line)) - .collect::>() - .len(), - 1 - ); - } -} diff --git a/src/analysis/clones/apart.rs b/src/analysis/clones/apart.rs new file mode 100644 index 0000000..e27135d --- /dev/null +++ b/src/analysis/clones/apart.rs @@ -0,0 +1,170 @@ +//! Copies that are not compared: in example, benchmark or retired directories, +//! in code marked deprecated, and between Bend 2 tests pinned to their output. +use super::*; + +/// Whether a copy lies in a function or type marked deprecated: it goes +/// with the next major version, so sharing its code with its replacement +/// is not worth doing. flysystem's deprecated phpseclib 2 adapter was +/// paired with its phpseclib 3 successor in 7 reviews. +pub(super) fn deprecated(files: &[SourceFile<'_>], site: &Site) -> bool { + let file = &files[site.file]; + let Ok(Some(tree)) = crate::syntax::parse(file.path, file.source) else { + return false; + }; + let mut node = tree + .root_node() + .descendant_for_byte_range(site.span.start, site.span.start); + while let Some(current) = node { + let kind = current.kind(); + let declaration = kind.ends_with("_declaration") + || kind.ends_with("_definition") + || kind.ends_with("_item") + || matches!(kind, "method" | "class" | "module" | "function"); + if declaration && crate::analysis::units::deprecated(current, file.source) { + return true; + } + node = current.parent(); + } + false +} + +/// Whether a file lies in a directory of retired code, such as +/// `deprecated`, `archive` or a proof of concept: like code marked +/// deprecated, it is not worth sharing code with. A Unity project's +/// `Assets/ProofOfConcept` builders, kept as a reference with no menu entry, +/// were paired with the live scene builders in six wrong reviews. `legacy` +/// is left out, since legacy code is often still served. +pub(super) fn retired(path: &Path) -> bool { + path.parent().is_some_and(|dir| { + dir.iter().any(|part| { + let part = part + .to_string_lossy() + .to_ascii_lowercase() + .replace(['-', '_'], ""); + [ + "deprecated", + "archive", + "archived", + "attic", + "graveyard", + "retired", + "obsolete", + "proofofconcept", + "poc", + "pocs", + ] + .contains(&part.as_str()) + }) + }) +} + +/// A directory of example code: `examples`, `demo`, `tutorial`, or a name +/// such as `blog_examples`. +pub(super) fn example_directory(part: &str) -> bool { + let part = part.to_ascii_lowercase(); + [ + "example", + "examples", + "demo", + "demos", + "tutorial", + "tutorials", + "docs_src", + ] + .contains(&part.as_str()) + || part.ends_with("_examples") + || part.ends_with("-examples") +} + +/// Two Bend 2 tests: each is a whole program pinned to the output its run +/// prints, so their copies are the point of each test. Of 4 shared-logic +/// findings between such tests on thirteen Bend 2 projects, all were wrong. +pub(super) fn separate_tests(a: &SourceFile<'_>, b: &SourceFile<'_>) -> bool { + let test = |f: &SourceFile<'_>| { + crate::analysis::bend::file(f.path) + && crate::analysis::bend::expected_output(f.source).is_some() + }; + a.path != b.path && test(a) && test(b) +} + +/// Whether a file sits in a benchmark directory. +pub(crate) fn benchmark_code(path: &Path) -> bool { + path.parent().is_some_and(|dir| { + dir.iter() + .any(|part| benchmark_directory(&part.to_string_lossy())) + }) +} + +pub(super) fn benchmark_directory(part: &str) -> bool { + matches!( + part.to_ascii_lowercase().as_str(), + "bench" | "benches" | "benchmark" | "benchmarks" + ) +} + +/// Whether a file is example code, written to be read beside other examples. +/// Also a top-level `samples` or `sample` directory (a Java package named +/// `samples` is source), a .NET project named like `MediatR.Examples.Autofac`, +/// and Go's `example_*_test.go` files, which show how to call a package. +/// Directories below a JVM source root (`src/main/java`) are packages, not +/// examples: Spring Initializr names a new project's package +/// `com.example.demo`, which made every finding of such a project a note. +pub(crate) fn example_code(path: &Path) -> bool { + let name = path + .file_name() + .map(|n| n.to_string_lossy().to_ascii_lowercase()) + .unwrap_or_default(); + let top = path + .iter() + .next() + .map(|p| p.to_string_lossy().to_ascii_lowercase()) + .filter(|_| path.iter().count() > 1) + .unwrap_or_default(); + let directories: Vec = path + .parent() + .map(|dir| { + dir.iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect() + }) + .unwrap_or_default(); + let packages = jvm_source_root(&directories).unwrap_or(directories.len()); + (name.starts_with("example_") && name.ends_with("_test.go")) + || matches!(top.as_str(), "samples" | "sample") + || directories[..packages] + .iter() + .any(|part| example_directory(part) || part.to_ascii_lowercase().contains(".examples")) +} + +/// Where the package directories of a JVM source root begin: after +/// `src//java` (or `kotlin`, `scala`, `groovy`). +pub(super) fn jvm_source_root(directories: &[String]) -> Option { + directories + .windows(3) + .position(|w| { + w[0] == "src" && matches!(w[2].as_str(), "java" | "kotlin" | "scala" | "groovy") + }) + .map(|at| at + 3) +} + +/// Whether two files are separate variants of one example, kept side by +/// side on purpose: under the same `examples` (or `demo`, `tutorial`) +/// directory, in different directories below it. django-styleguide shows a +/// Google login flow written by hand in `blog_examples/…/raw` and with the +/// SDK in `…/sdk`; their copies are the point. Benchmarks kept so are +/// separate programs too: each of bendlang/bend's `bench/runtime/*` is a +/// standalone program measured beside its C, TypeScript and Lean twins, +/// and the 6 shared-logic findings across them were labeled wrong. +pub(super) fn separate_examples(a: &Path, b: &Path) -> bool { + let example = |part: &str| example_directory(part) || benchmark_directory(part); + let dirs = |p: &Path| -> Vec { + p.parent() + .map(|d| d.iter().map(|c| c.to_string_lossy().into_owned()).collect()) + .unwrap_or_default() + }; + let (a, b) = (dirs(a), dirs(b)); + let Some(root) = a.iter().zip(&b).position(|(x, y)| x == y && example(x)) else { + return false; + }; + a[..=root] == b[..=root] && a[root + 1..] != b[root + 1..] +} diff --git a/src/analysis/clones/frame.rs b/src/analysis/clones/frame.rs new file mode 100644 index 0000000..4201b8a --- /dev/null +++ b/src/analysis/clones/frame.rs @@ -0,0 +1,261 @@ +//! The frame of a tree walk: the calls a function makes of itself and the +//! branches that only leave, which every walk repeats whatever work it does. +use super::*; + +/// The frame of a tree walk: the statements every walk has, whatever it +/// does at each node. +#[derive(Clone, Copy, PartialEq, Eq)] +pub(super) enum Frame { + /// A branch that only leaves, as `if node.kind() == "call" { return; }`. + Exit, + /// A call of the function itself and nothing else, or a loop or branch + /// that only does that or leaves, as + /// `for child in node.named_children(&mut cursor) { bound(child, names); }`. + Recursion, +} + +/// Objects a method calls itself on, as in `self.walk(`, `this.walk(`, +/// `Self::walk(`, `cls.walk(` or PHP's `$this->walk(` and `static::walk(`. +pub(super) const RECEIVERS: [&str; 5] = ["self", "Self", "this", "cls", "static"]; + +/// Mark each call of the function it sits in: its name followed by its +/// arguments inside the body of the innermost callable of that name. A +/// method calls itself on the object itself (`self.walk(`, `this.walk(`, +/// `Self::walk(`); a bare `walk(` inside it calls a free or imported +/// function, except in Java, C# and Ruby, where a bare call reaches the +/// method through its object. A call through another path, as +/// `native::get()` inside `get`, names a different function. +pub(super) fn mark_recursion(tokens: &mut [Token<'_>], units: &[Unit], path: &Path) { + let implicit = matches!( + path.extension().and_then(|x| x.to_str()), + Some("java" | "cs" | "rb") + ); + let callables: Vec<&Unit> = units.iter().filter(|u| u.callable()).collect(); + let names: BTreeSet<&str> = callables.iter().map(|u| u.short_name.as_str()).collect(); + for i in 1..tokens.len() { + let token = &tokens[i - 1]; + if token.kind != TokenKind::Identifier + || tokens[i].text != "(" + || !names.contains(token.text) + { + continue; + } + let Some(unit) = callables + .iter() + .filter(|u| u.body.as_ref().is_some_and(|b| b.contains(&token.start))) + .min_by_key(|u| u.span.len()) + .filter(|u| u.short_name == token.text) + else { + continue; + }; + let qualifier = i + .checked_sub(2) + .filter(|&q| matches!(tokens[q].text, "." | "::" | "->" | "?.")); + tokens[i - 1].own = match qualifier { + None => implicit || unit.kind == Kind::Function, + Some(q) => q + .checked_sub(1) + .is_some_and(|o| RECEIVERS.contains(&tokens[o].text)), + }; + } +} + +/// The tokens of one node. +pub(super) fn tokens_of<'t, 'a>(tokens: &'t [Token<'a>], node: Node<'_>) -> &'t [Token<'a>] { + let start = tokens.partition_point(|t| t.start < node.start_byte()); + let end = tokens.partition_point(|t| t.start < node.end_byte()); + &tokens[start..end] +} + +/// Part of a walk's frame rather than its work, if it is. +pub(super) fn frame(statement: Node<'_>, tokens: &[Token<'_>]) -> Option { + if exit_guard(statement, tokens) { + Some(Frame::Exit) + } else if recursion(statement, tokens) { + Some(Frame::Recursion) + } else { + None + } +} + +/// A call of the function itself and nothing else, as `bound(child, names);` +/// or `return self.walk(node.parent)`, or a loop or branch whose statements +/// only do that or leave early, with at least one call. Work anywhere else +/// in its body, as in a match arm, a conditional expression or a call that +/// wraps the recursion, makes it more than the frame. +pub(super) fn recursion(statement: Node<'_>, tokens: &[Token<'_>]) -> bool { + if own_call(statement, tokens) { + return true; + } + let node = expression(statement); + let ruby_block = node.kind() == "call" && node.child_by_field_name("block").is_some(); + if !ruby_block + && !matches!( + node.kind(), + "for_expression" + | "for_statement" + | "for_in_statement" + | "enhanced_for_statement" + | "foreach_statement" + | "for" + | "while_expression" + | "while_statement" + | "while" + | "until" + | "loop_expression" + | "do_statement" + | "if_expression" + | "if_statement" + | "if" + | "unless" + ) + { + return false; + } + let mut body = Vec::new(); + branch_statements(node, &mut body); + body.iter().any(|&s| recursion(s, tokens)) + && body + .iter() + .all(|&s| exits(s, tokens) || exit_guard(s, tokens) || recursion(s, tokens)) +} + +/// A statement that is only a call of the function it sits in, as +/// `walk(child);`, `return self.walk(node.parent)`, `await this.walk(child);` +/// or `walk(child)?;`. +pub(super) fn own_call(statement: Node<'_>, tokens: &[Token<'_>]) -> bool { + let mut words = tokens_of(tokens, statement); + if let [first, rest @ ..] = words + && matches!(first.text, "return" | "await") + { + words = rest; + } + while let [rest @ .., last] = words + && matches!(last.text, ";" | "?") + { + words = rest; + } + if let [receiver, separator, rest @ ..] = words + && RECEIVERS.contains(&receiver.text) + && matches!(separator.text, "." | "::" | "->" | "?.") + { + words = rest; + } + let [name, arguments @ ..] = words else { + return false; + }; + if !name.own || arguments.first().is_none_or(|t| t.text != "(") { + return false; + } + // The call's closing parenthesis ends the statement. + let mut depth = 0usize; + for (i, token) in arguments.iter().enumerate() { + match token.text { + "(" => depth += 1, + ")" => { + depth -= 1; + if depth == 0 { + return i + 1 == arguments.len(); + } + } + _ => {} + } + } + false +} + +/// A branch that only leaves, as `if node.kind() == "call" { return; }`, +/// `if (done) return;` or `return if done`. +pub(super) fn exit_guard(statement: Node<'_>, tokens: &[Token<'_>]) -> bool { + let node = expression(statement); + if !matches!( + node.kind(), + "if_expression" | "if_statement" | "if" | "unless" | "if_modifier" | "unless_modifier" + ) { + return false; + } + let mut body = Vec::new(); + branch_statements(node, &mut body); + !body.is_empty() + && body + .into_iter() + .all(|s| exits(s, tokens) || exit_guard(s, tokens)) +} + +/// The expression a Rust or JavaScript statement wraps, as the `for` loop +/// of a Rust `expression_statement`. +pub(super) fn expression(statement: Node<'_>) -> Node<'_> { + statement + .named_child(0) + .filter(|_| { + statement.kind() == "expression_statement" && statement.named_child_count() == 1 + }) + .unwrap_or(statement) +} + +/// The statements a loop or branch runs, in all its branches: the +/// statements of its blocks, or the one statement of a branch without +/// braces. Its header, as a condition or the collection a loop walks, is +/// not a statement. +pub(super) fn branch_statements<'t>(node: Node<'t>, statements: &mut Vec>) { + let before = statements.len(); + let mut cursor = node.walk(); + for field in ["body", "consequence", "alternative", "block"] { + for part in node.children_by_field_name(field, &mut cursor) { + statements_in(part, statements); + } + } + // A JavaScript or Rust `else` holds its statement or block without a field. + if statements.len() == before && node.kind() == "else_clause" { + let mut cursor = node.walk(); + for part in node.named_children(&mut cursor) { + statements_in(part, statements); + } + } +} + +/// The statements of one part of a loop or branch. +pub(super) fn statements_in<'t>(part: Node<'t>, statements: &mut Vec>) { + if is_comment(part) { + return; + } + if holds_statements(part) { + let mut cursor = part.walk(); + for child in part.named_children(&mut cursor) { + statements_in_block(child, statements); + } + } else if matches!( + part.kind(), + "else_clause" | "elif_clause" | "else_if_clause" | "elsif" | "block" | "do_block" + ) { + // Further branches, and the `{ … }` or `do … end` a Ruby call runs. + branch_statements(part, statements); + } else { + statements.push(part); + } +} + +/// One statement of a block; Go holds a block's statements in a list. +pub(super) fn statements_in_block<'t>(child: Node<'t>, statements: &mut Vec>) { + if is_comment(child) { + return; + } + if holds_statements(child) { + statements_in(child, statements); + } else { + statements.push(child); + } +} + +/// `return`, `break`, `continue` or `next` with no value, or with nothing. +pub(super) fn exits(statement: Node<'_>, tokens: &[Token<'_>]) -> bool { + match tokens_of(tokens, statement) { + [first, rest @ ..] => { + matches!(first.text, "return" | "break" | "continue" | "next") + && rest + .iter() + .all(|t| matches!(t.text, ";" | "None" | "nil" | "null")) + } + [] => false, + } +} diff --git a/src/analysis/clones/mod.rs b/src/analysis/clones/mod.rs new file mode 100644 index 0000000..714d86a --- /dev/null +++ b/src/analysis/clones/mod.rs @@ -0,0 +1,838 @@ +//! Type-2 clone candidates across the selected files and explicit context. +//! Identifiers and literals are normalized; windows start and end on whole +//! statements inside function bodies; identifiers must be renamed consistently. +//! `apart` holds the copies that are never compared and `frame` the statements +//! every tree walk repeats, which do not make a copy on their own. +use super::{ + fast_hash, is_comment, line_of, text, + units::{Kind, Unit}, +}; +use std::{ + collections::{BTreeMap, BTreeSet}, + ops::Range, + path::{Path, PathBuf}, +}; +use tree_sitter::Node; + +mod apart; +mod frame; +#[cfg(test)] +mod tests; +use apart::*; +pub(crate) use apart::{benchmark_code, example_code}; +use frame::*; + +pub const MIN_BYTES: usize = 120; +/// Consecutive matching statements that seed a candidate window. +pub const MIN_STATEMENTS: usize = 2; +/// Statements a reported copy needs. +pub const MIN_CLONE_STATEMENTS: usize = 3; +pub const RUN_CAP: usize = 64; +pub const FILE_CAP: usize = 8; +const DIFFERENCES: usize = 12; +/// A statement pair repeated more often than this is an idiom; its extra pairs are not compared. +const SEED_OCCURRENCES: usize = 48; + +pub struct SourceFile<'a> { + pub path: &'a Path, + pub source: &'a str, + /// False for explicit context: a pair needs at least one selected site. + pub selected: bool, + pub units: &'a [Unit], + /// Lines excluded from comparison, such as test code when tests are not judged. + pub excluded: Vec>, + /// The package the file belongs to; explicit context has none. + pub package: Option<&'a crate::packages::Package>, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Site { + pub file: usize, + pub path: PathBuf, + pub span: Range, + pub start_line: usize, + pub end_line: usize, + /// Enclosing function or method, when there is one. + pub function: Option, + pub function_source: Option, + pub quote: String, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Difference { + pub a: String, + pub b: String, +} + +#[derive(Clone, Debug)] +pub struct Pair { + pub a: Site, + pub b: Site, + pub differences: Vec, + /// Non-whitespace bytes of the shorter site. + pub size: usize, + /// Distinct sites in this pair's clone group, including `a` and `b`. + pub occurrences: usize, + /// The group's other copies, reported with the judged pair. + pub copies: Vec, + /// Hash of the normalized statements; stable across renames and moves. + pub normalized: String, +} + +impl Pair { + pub fn rank(&self) -> usize { + self.size * self.occurrences + } +} + +#[derive(Default)] +pub struct Candidates { + pub pairs: Vec, + /// Pairs dropped by the per-run or per-file caps, by owning path. + pub omitted: BTreeMap, +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum TokenKind { + Identifier, + Literal, + Other, +} + +struct Token<'a> { + kind: TokenKind, + text: &'a str, + start: usize, + /// A call of the function the token sits in: `bound(child, …)` inside + /// `bound`. Recursion names the function itself, so two walks calling + /// themselves differ by no renamed name. + own: bool, +} + +/// Placeholders for renamed identifiers and literals. The control character +/// keeps them apart from any real token text. +const IDENTIFIER_TOKEN: &str = "\u{1}id"; +const LITERAL_TOKEN: &str = "\u{1}lit"; + +impl Token<'_> { + fn normal(&self) -> &str { + match self.kind { + TokenKind::Identifier => IDENTIFIER_TOKEN, + TokenKind::Literal => LITERAL_TOKEN, + TokenKind::Other => self.text, + } + } +} + +#[derive(Clone)] +struct Statement { + span: Range, + tokens: Range, + hash: u64, + /// Part of the frame of a walk rather than its work. + frame: Option, + /// Go's error check or deferred cleanup, which every call site repeats. + idiom: bool, +} + +struct Block { + file: usize, + statements: Vec, + /// The statements are the whole body of a function. + whole: bool, +} + +struct Parsed<'a> { + tokens: Vec>, +} + +pub fn find(files: &[SourceFile<'_>]) -> Candidates { + let (parsed, blocks) = statement_blocks(files); + let local: BTreeSet = files + .iter() + .filter_map(|f| f.package?.name.clone()) + .collect(); + let mut pairs: Vec = matching_windows(&blocks) + .into_iter() + .filter(|&((bx, _), (by, _), _)| { + let (a, b) = (&files[blocks[bx].file], &files[blocks[by].file]); + crate::packages::linked(a.package, b.package, &local) + && !separate_examples(a.path, b.path) + && !separate_tests(a, b) + }) + .filter_map(|window| pair(files, &parsed, &blocks, window)) + .filter(|p| !deprecated(files, &p.a) && !deprecated(files, &p.b)) + .filter(|p| !retired(&p.a.path) && !retired(&p.b.path)) + .collect(); + drop_nested(&mut pairs); + pairs.sort_by(by_rank); + let mut pairs = representatives(pairs); + // Groups rank by size times their number of copies. + pairs.sort_by(by_rank); + capped(one_per_function_pair(pairs)) +} + +/// Two copied windows of the same two functions, split by one differing +/// statement, are one repetition: keep the higher-ranked pair only. +fn one_per_function_pair(pairs: Vec) -> Vec { + let mut seen = BTreeSet::new(); + pairs + .into_iter() + .filter(|pair| { + let (Some(a), Some(b)) = (&pair.a.function, &pair.b.function) else { + return true; + }; + let mut key = [(&pair.a.path, a), (&pair.b.path, b)]; + key.sort(); + seen.insert(key.map(|(path, name)| (path.clone(), name.clone()))) + }) + .collect() +} + +/// Tokens of every file and the statement blocks inside unit bodies. +fn statement_blocks<'a>(files: &[SourceFile<'a>]) -> (Vec>, Vec) { + let mut parsed = Vec::new(); + let mut blocks = Vec::new(); + for (index, file) in files.iter().enumerate() { + let Ok(Some(tree)) = crate::syntax::parse(file.path, file.source) else { + parsed.push(Parsed { tokens: Vec::new() }); + continue; + }; + let mut tokens = Vec::new(); + leaves(tree.root_node(), file.source, &mut tokens); + mark_recursion(&mut tokens, file.units, file.path); + let bodies: Vec> = file + .units + .iter() + .filter(|u| !u.equality) + .filter_map(|u| u.body.clone()) + .collect(); + collect_blocks(tree.root_node(), file, index, &bodies, &tokens, &mut blocks); + parsed.push(Parsed { tokens }); + } + (parsed, blocks) +} + +/// A maximal run of matching statement hashes: (block, start) twice and its length. +type Window = ((usize, usize), (usize, usize), usize); + +/// Seed on consecutive statement pairs, then extend each diagonal as far as the +/// hashes keep matching; a diagonal already covered is not reported again. +fn matching_windows(blocks: &[Block]) -> Vec { + let mut covered = BTreeSet::new(); + let mut found = Vec::new(); + for ((bx, kx), (by, ky)) in seed_pairs(blocks) { + let diagonal = (bx, by, kx as isize - ky as isize); + if covered.contains(&(diagonal, kx)) { + continue; + } + let n = extend(blocks, (bx, kx), (by, ky)); + covered.extend((0..n).map(|t| (diagonal, kx + t))); + if bx == by && one_run(&blocks[bx].statements[kx.min(ky)..kx.max(ky) + n]) { + continue; + } + found.push(((bx, kx), (by, ky), n)); + } + found +} + +/// Every two places one seed occurs, among its first `SEED_OCCURRENCES`; +/// two places in one block must be far enough apart not to overlap. +fn seed_pairs(blocks: &[Block]) -> impl Iterator { + seeds(blocks).into_values().flat_map(|mut places| { + places.truncate(SEED_OCCURRENCES); + let pairs: Vec<_> = places + .iter() + .enumerate() + .flat_map(|(x, &a)| places[x + 1..].iter().map(move |&b| (a, b))) + .filter(|&((bx, kx), (by, ky))| bx != by || ky >= kx + MIN_STATEMENTS) + .collect(); + pairs + }) +} + +/// Statements that all read alike, such as sqlite-utils' nine +/// `x = self.value_or_default("x", x)` lines or a list of lazy imports: a +/// list of one kind of statement, which matches itself shifted by one. +fn one_run(statements: &[Statement]) -> bool { + statements + .windows(2) + .all(|pair| pair[0].hash == pair[1].hash) +} + +/// Every place a pair of consecutive statement hashes occurs. +fn seeds(blocks: &[Block]) -> BTreeMap<(u64, u64), Vec<(usize, usize)>> { + let mut seeds = BTreeMap::<(u64, u64), Vec<(usize, usize)>>::new(); + for (b, block) in blocks.iter().enumerate() { + for k in 0..block.statements.len().saturating_sub(1) { + let key = (block.statements[k].hash, block.statements[k + 1].hash); + seeds.entry(key).or_default().push((b, k)); + } + } + seeds +} + +/// How many statements match from two seeds; a window never overlaps itself. +fn extend(blocks: &[Block], (bx, kx): (usize, usize), (by, ky): (usize, usize)) -> usize { + let (sx, sy) = (&blocks[bx].statements, &blocks[by].statements); + let mut n = MIN_STATEMENTS; + while kx + n < sx.len() + && ky + n < sy.len() + && sx[kx + n].hash == sy[ky + n].hash + && (bx != by || kx + n < ky) + { + n += 1; + } + n +} + +/// A candidate pair from one window, when its tokens align with consistent +/// renaming and it is large enough to report. The owner is a selected site. +fn pair( + files: &[SourceFile<'_>], + parsed: &[Parsed<'_>], + blocks: &[Block], + window: Window, +) -> Option { + let ((bx, kx), (by, ky), n) = window; + let (fx, fy) = (blocks[bx].file, blocks[by].file); + if !files[fx].selected && !files[fy].selected { + return None; + } + let x = &blocks[bx].statements[kx..kx + n]; + let y = &blocks[by].statements[ky..ky + n]; + let tx = &parsed[fx].tokens[x[0].tokens.start..x[n - 1].tokens.end]; + let ty = &parsed[fy].tokens[y[0].tokens.start..y[n - 1].tokens.end]; + let differences = align(tx, ty)?; + let span_x = x[0].span.start..x[n - 1].span.end; + let span_y = y[0].span.start..y[n - 1].span.end; + let size = + compact(&files[fx].source[span_x.clone()]).min(compact(&files[fy].source[span_y.clone()])); + // A repeated pair of statements is usually an idiom, such as a call and + // its check. + if n < MIN_CLONE_STATEMENTS + || size < MIN_BYTES + || only_frame(files, blocks, window) + || mostly_guards(files, blocks, window) + { + return None; + } + let normalized = crate::schema::hash( + tx.iter() + .map(Token::normal) + .collect::>() + .join(crate::schema::HASH_SEPARATOR) + .as_bytes(), + ); + let a = site(files, fx, span_x); + let b = site(files, fy, span_y); + // Ties keep path and line order. + let swap = !files[fx].selected + || (files[fy].selected && (&b.path, b.start_line) < (&a.path, a.start_line)); + let (a, b, differences) = if swap { + let flipped = differences + .into_iter() + .map(|d| Difference { a: d.b, b: d.a }) + .collect(); + (b, a, flipped) + } else { + (a, b, differences) + }; + Some(Pair { + a, + b, + differences, + size, + occurrences: 2, + copies: Vec::new(), + normalized, + }) +} + +/// Two parts of recursive functions that share little beyond the frame of a +/// walk: early exits and recursion into the function itself. Two walks that +/// stop at a different kind and recurse into their children share that +/// frame whatever they do at each node, so a copy of part of them needs two +/// statements beyond it, or one of half the size a copy needs. A copy of +/// the whole of both functions is a copy, however small its work. +fn only_frame(files: &[SourceFile<'_>], blocks: &[Block], window: Window) -> bool { + let ((bx, kx), (by, ky), n) = window; + let x = &blocks[bx].statements[kx..kx + n]; + let y = &blocks[by].statements[ky..ky + n]; + let recurses = |s: &[Statement]| s.iter().any(|s| s.frame == Some(Frame::Recursion)); + let whole = |b: usize, k: usize| blocks[b].whole && k == 0 && n == blocks[b].statements.len(); + if !recurses(x) || !recurses(y) || whole(bx, kx) && whole(by, ky) { + return false; + } + let work: Vec<(&Statement, &Statement)> = x + .iter() + .zip(y) + .filter(|(a, b)| a.frame.is_none() || b.frame.is_none()) + .collect(); + let bytes = |file: usize, s: &Statement| compact(&files[file].source[s.span.clone()]); + let size = work + .iter() + .map(|(a, _)| bytes(blocks[bx].file, a)) + .sum::() + .min(work.iter().map(|(_, b)| bytes(blocks[by].file, b)).sum()); + work.len() < MIN_STATEMENTS && size < MIN_BYTES / 2 +} + +/// Part of two Go functions that is mostly error checks and deferred +/// cleanups: `if err != nil { return err }` after each call and +/// `defer tx.Rollback()`. wtf's per-entity store functions shared a +/// transaction's begin, rollback and error checks around calls to their own +/// type's functions, which read as copies. A copy of part of them needs as +/// much other work as a copy of a walk's frame does; a copy of the whole of +/// both functions is still one. +fn mostly_guards(files: &[SourceFile<'_>], blocks: &[Block], window: Window) -> bool { + let ((bx, kx), (by, ky), n) = window; + let x = &blocks[bx].statements[kx..kx + n]; + let y = &blocks[by].statements[ky..ky + n]; + let whole = |b: usize, k: usize| blocks[b].whole && k == 0 && n == blocks[b].statements.len(); + if !x.iter().any(|s| s.idiom) || whole(bx, kx) && whole(by, ky) { + return false; + } + let work: Vec<(&Statement, &Statement)> = x + .iter() + .zip(y) + .filter(|(a, b)| !a.idiom || !b.idiom) + .collect(); + let bytes = |file: usize, s: &Statement| compact(&files[file].source[s.span.clone()]); + let size = work + .iter() + .map(|(a, _)| bytes(blocks[bx].file, a)) + .sum::() + .min(work.iter().map(|(_, b)| bytes(blocks[by].file, b)).sum()); + work.len() < MIN_CLONE_STATEMENTS && size < MIN_BYTES +} + +/// Go's `if err != nil { return …, err }` or a `defer` statement. +fn go_idiom(statement: Node<'_>, source: &str) -> bool { + match statement.kind() { + "defer_statement" => true, + "if_statement" => { + let checks_err = statement + .child_by_field_name("condition") + .is_some_and(|c| compact_text(&source[c.byte_range()]) == "err!=nil"); + let returns = statement + .child_by_field_name("consequence") + .is_some_and(|block| { + // Newer Go grammars wrap a block's statements in a list. + let list = block + .named_child(0) + .filter(|c| c.kind() == "statement_list") + .unwrap_or(block); + let mut cursor = list.walk(); + let body: Vec> = list.named_children(&mut cursor).collect(); + !body.is_empty() && body.iter().all(|s| s.kind() == "return_statement") + }); + checks_err && returns && statement.child_by_field_name("alternative").is_none() + } + _ => false, + } +} + +fn compact_text(text: &str) -> String { + text.chars().filter(|c| !c.is_whitespace()).collect() +} + +/// Drop pairs whose sites both lie inside a larger pair's sites. +fn drop_nested(pairs: &mut Vec) { + let snapshot = pairs.clone(); + pairs.retain(|p| { + !snapshot.iter().any(|q| { + let larger = q.a.span.len() + q.b.span.len() > p.a.span.len() + p.b.span.len(); + larger + && ((contains(&q.a, &p.a) && contains(&q.b, &p.b)) + || (contains(&q.a, &p.b) && contains(&q.b, &p.a))) + }) + }); +} + +/// Keep ranked groups within the per-run and per-file caps; count the rest. +fn capped(pairs: Vec) -> Candidates { + let mut omitted = BTreeMap::::new(); + let mut per_file = BTreeMap::::new(); + let mut kept = Vec::new(); + for pair in pairs { + let count = per_file.entry(pair.a.path.clone()).or_default(); + if kept.len() < RUN_CAP && *count < FILE_CAP { + *count += 1; + kept.push(pair); + } else { + *omitted.entry(pair.a.path.clone()).or_default() += 1; + } + } + Candidates { + pairs: kept, + omitted, + } +} + +fn by_rank(p: &Pair, q: &Pair) -> std::cmp::Ordering { + q.rank() + .cmp(&p.rank()) + .then_with(|| (&p.a.path, p.a.start_line).cmp(&(&q.a.path, q.a.start_line))) + .then_with(|| (&p.b.path, p.b.start_line).cmp(&(&q.b.path, q.b.start_line))) +} + +fn overlaps(x: &Site, y: &Site) -> bool { + x.path == y.path && x.span.start < y.span.end && y.span.start < x.span.end +} + +/// Sites that cover at least half of each other describe the same code. +fn same_code(x: &Site, y: &Site) -> bool { + if x.path != y.path { + return false; + } + let shared = x + .span + .end + .min(y.span.end) + .saturating_sub(x.span.start.max(y.span.start)); + 2 * shared >= x.span.len() && 2 * shared >= y.span.len() +} + +/// One judged pair per clone group. Pairs whose sites repeat the same code +/// (mutual half overlap) are linked; the first pair of each group in rank +/// order represents it and carries the other copies. Linking on plain overlap +/// let short idioms inside a larger copy chain unrelated code together. +fn representatives(pairs: Vec) -> Vec { + let groups = same_code_groups(&pairs); + let mut sites = BTreeMap::>::new(); + for (i, pair) in pairs.iter().enumerate() { + let group = sites.entry(groups[i]).or_default(); + for site in [&pair.a, &pair.b] { + if !group.iter().any(|known| overlaps(known, site)) { + group.push(site.clone()); + } + } + } + let mut kept = Vec::new(); + for (i, mut pair) in pairs.into_iter().enumerate() { + if groups[i] != i { + continue; + } + pair.copies = sites[&i] + .iter() + .filter(|s| !overlaps(s, &pair.a) && !overlaps(s, &pair.b)) + .cloned() + .collect(); + pair.copies + .sort_by(|x, y| (&x.path, x.start_line).cmp(&(&y.path, y.start_line))); + pair.occurrences = 2 + pair.copies.len(); + kept.push(pair); + } + kept +} + +/// For each pair, the first pair of its group: pairs whose sites repeat the +/// same code are linked, transitively (union-find). +fn same_code_groups(pairs: &[Pair]) -> Vec { + fn root(parent: &mut [usize], mut i: usize) -> usize { + while parent[i] != i { + parent[i] = parent[parent[i]]; + i = parent[i]; + } + i + } + let mut parent: Vec = (0..pairs.len()).collect(); + for i in 0..pairs.len() { + for j in i + 1..pairs.len() { + let (p, q) = (&pairs[i], &pairs[j]); + let linked = [&p.a, &p.b] + .iter() + .any(|x| same_code(x, &q.a) || same_code(x, &q.b)); + if linked { + let (ri, rj) = (root(&mut parent, i), root(&mut parent, j)); + parent[ri.max(rj)] = ri.min(rj); + } + } + } + (0..pairs.len()).map(|i| root(&mut parent, i)).collect() +} + +fn contains(outer: &Site, inner: &Site) -> bool { + outer.path == inner.path + && outer.span.start <= inner.span.start + && inner.span.end <= outer.span.end +} + +fn compact(text: &str) -> usize { + text.bytes().filter(|b| !b.is_ascii_whitespace()).count() +} + +fn site(files: &[SourceFile<'_>], index: usize, span: Range) -> Site { + let file = &files[index]; + let unit = file + .units + .iter() + .filter(|u| u.callable() && u.span.start <= span.start && span.end <= u.span.end) + .min_by_key(|u| u.span.len()); + Site { + file: index, + path: file.path.to_path_buf(), + start_line: line_of(file.source, span.start), + end_line: line_of(file.source, span.end.saturating_sub(1)), + function: unit.map(|u| u.name.clone()), + function_source: unit.map(|u| u.source(file.source).to_string()), + quote: file.source[span.clone()].to_string(), + span, + } +} + +/// Aligned tokens must match after normalization, and each identifier must map +/// to exactly one identifier on the other side. Returns renamed names and values. +fn align(x: &[Token<'_>], y: &[Token<'_>]) -> Option> { + if x.len() != y.len() { + return None; + } + let mut forward = BTreeMap::new(); + let mut backward = BTreeMap::new(); + let mut differences = Vec::new(); + for (a, b) in x.iter().zip(y) { + if a.normal() != b.normal() { + return None; + } + // Each side calling itself is the same step, not a rename. + if a.own && b.own { + continue; + } + if a.kind == TokenKind::Identifier + && (*forward.entry(a.text).or_insert(b.text) != b.text + || *backward.entry(b.text).or_insert(a.text) != a.text) + { + return None; + } + if a.kind != TokenKind::Other && a.text != b.text { + let difference = Difference { + a: a.text.to_string(), + b: b.text.to_string(), + }; + if !differences.contains(&difference) && differences.len() < DIFFERENCES { + differences.push(difference); + } + } + } + Some(differences) +} + +fn leaves<'a>(node: Node<'_>, source: &'a str, tokens: &mut Vec>) { + if is_comment(node) { + return; + } + let kind = node.kind(); + let literal = matches!( + kind, + "string_content" + | "string_fragment" + | "integer_literal" + | "float_literal" + | "char_literal" + | "decimal_integer_literal" + | "hex_integer_literal" + | "octal_integer_literal" + | "binary_integer_literal" + | "decimal_floating_point_literal" + | "hex_floating_point_literal" + | "character_literal" + | "number" + | "integer" + | "float" + | "string_literal_content" + | "raw_string_content" + | "verbatim_string_literal" + | "real_literal" + ); + if node.child_count() == 0 || literal { + let text = text(node, source); + if text.trim().is_empty() { + return; + } + let kind = if literal { + TokenKind::Literal + } else if kind.ends_with("identifier") + || matches!( + kind, + "identifier" | "constant" | "instance_variable" | "name" + ) + { + TokenKind::Identifier + } else { + TokenKind::Other + }; + tokens.push(Token { + kind, + text, + start: node.start_byte(), + own: false, + }); + return; + } + let mut cursor = node.walk(); + for child in node.children(&mut cursor) { + leaves(child, source, tokens); + } +} + +fn collect_blocks( + node: Node<'_>, + file: &SourceFile<'_>, + index: usize, + bodies: &[Range], + tokens: &[Token<'_>], + blocks: &mut Vec, +) { + if holds_statements(node) + && bodies + .iter() + .any(|b| b.start <= node.start_byte() && node.end_byte() <= b.end) + { + let all = block_statements(node, file, tokens); + // A Go body holds its statements in a `statement_list` inside the block. + let body = node + .parent() + .filter(|p| node.kind() == "statement_list" && p.kind() == "block") + .unwrap_or(node); + let whole = all.iter().all(Option::is_some) + && file + .units + .iter() + .any(|u| u.callable() && u.body == Some(body.byte_range())); + // Excluded statements break a window, so split the block there. + for statements in all.split(Option::is_none) { + let statements: Vec = statements.iter().flatten().cloned().collect(); + if !statements.is_empty() { + blocks.push(Block { + file: index, + statements, + whole, + }); + } + } + } + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + collect_blocks(child, file, index, bodies, tokens, blocks); + } +} + +/// A node whose named children are statements. Ruby holds statements in a +/// `body_statement` or `block_body`, and in the `then`, `else` and `do` of a +/// branch or loop; its `block` is a `{ … }` argument around a `block_body`. +/// PHP holds them in a `compound_statement`, and a Java constructor's +/// statements are in a `constructor_body`. +fn holds_statements(node: Node<'_>) -> bool { + let ruby_block = node.kind() == "block" && node.parent().is_some_and(|p| p.kind() == "call"); + matches!( + node.kind(), + "block" + | "statement_block" + | "statement_list" + | "compound_statement" + | "body_statement" + | "block_body" + | "then" + | "else" + | "do" + | "constructor_body" + ) && !ruby_block +} + +/// A block's statements with normalized-token hashes; `None` for excluded lines. +fn block_statements( + node: Node<'_>, + file: &SourceFile<'_>, + tokens: &[Token<'_>], +) -> Vec> { + let mut statements = Vec::new(); + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + // A docstring documents; counted as a statement, it made one-line + // wrappers such as flask's `render_template` read as copies. + if is_comment(child) || super::comments::docstring(child).is_some() { + continue; + } + let line = line_of(file.source, child.start_byte()); + if file.excluded.iter().any(|r| r.contains(&line)) + || node.kind() == "constructor_body" && field_initializer(child) + || literal_setter(child, file.source) + { + statements.push(None); + continue; + } + let start = tokens.partition_point(|t| t.start < child.start_byte()); + let end = tokens.partition_point(|t| t.start < child.end_byte()); + let key = tokens[start..end] + .iter() + .map(Token::normal) + .collect::>() + .join(crate::schema::HASH_SEPARATOR); + statements.push(Some(Statement { + span: child.byte_range(), + tokens: start..end, + hash: fast_hash(&key), + frame: frame(child, tokens), + idiom: go_idiom(child, file.source), + })); + } + statements +} + +/// A Java constructor statement that stores a parameter, another object's +/// field or a literal in a field, as in `this.name = name;` or +/// `timeout = copy.timeout;`. A run of them is how a constructor fills its +/// fields: two constructors assigning different fields matched as copies. +fn field_initializer(statement: Node<'_>) -> bool { + let Some(assignment) = statement.named_child(0).filter(|a| { + statement.kind() == "expression_statement" && a.kind() == "assignment_expression" + }) else { + return false; + }; + let simple = |side: Option>, value: bool| { + side.is_some_and(|n| match n.kind() { + "identifier" => true, + "field_access" => n + .child_by_field_name("object") + .is_some_and(|o| matches!(o.kind(), "this" | "identifier")), + kind => value && (kind.ends_with("_literal") || matches!(kind, "true" | "false")), + }) + }; + assignment + .child_by_field_name("operator") + .is_some_and(|o| o.kind() == "=") + && simple(assignment.child_by_field_name("left"), false) + && simple(assignment.child_by_field_name("right"), true) +} + +/// A Java setter given one literal, as in `owner.setCity("Madison");`. A run +/// of them fills an object with data: a test fixture built in one test and a +/// helper building another owner matched as copies whose only differences +/// were the values. +fn literal_setter(statement: Node<'_>, source: &str) -> bool { + let Some(call) = statement + .named_child(0) + .filter(|c| statement.kind() == "expression_statement" && c.kind() == "method_invocation") + else { + return false; + }; + let setter = call.child_by_field_name("name").is_some_and(|name| { + text(name, source) + .strip_prefix("set") + .is_some_and(|rest| rest.starts_with(|c: char| c.is_ascii_uppercase())) + }); + let on_object = call + .child_by_field_name("object") + .is_some_and(|o| matches!(o.kind(), "identifier" | "this" | "field_access")); + let literal = call + .child_by_field_name("arguments") + .is_some_and(|arguments| { + arguments.named_child_count() == 1 + && arguments.named_child(0).is_some_and(|argument| { + argument.kind().ends_with("_literal") + || matches!(argument.kind(), "true" | "false") + }) + }); + setter && on_object && literal +} diff --git a/src/analysis/clones/tests.rs b/src/analysis/clones/tests.rs new file mode 100644 index 0000000..fffb1c5 --- /dev/null +++ b/src/analysis/clones/tests.rs @@ -0,0 +1,650 @@ +use super::*; + +/// Candidate pairs between two selected files, each a (path, source). +fn pairs_between(a: (&str, &str), b: (&str, &str)) -> usize { + run(&[(a.0, a.1, true), (b.0, b.1, true)]).pairs.len() +} + +/// The renamed names and values of the one pair between two selected files. +fn differences_between(a: (&str, &str), b: (&str, &str)) -> Vec { + let found = run(&[(a.0, a.1, true), (b.0, b.1, true)]); + assert_eq!(found.pairs.len(), 1); + found.pairs[0].differences.clone() +} + +fn run(files: &[(&str, &str, bool)]) -> Candidates { + let units: Vec<_> = files + .iter() + .map(|(path, source, _)| super::super::units::parse(Path::new(path), source).unwrap()) + .collect(); + let sources: Vec<_> = files + .iter() + .zip(&units) + .map(|((path, source, selected), units)| SourceFile { + path: Path::new(path), + source, + selected: *selected, + units: &units.units, + excluded: Vec::new(), + package: None, + }) + .collect(); + find(&sources) +} + +#[test] +fn copies_between_two_bend_tests_are_not_candidates() { + let program = |output: &str| { + format!( + "import Base\n\ndef main() -> IO(Unit):\n do IO:\n a : String <- IO.try(String, IO.get_env(\"HOME\"))\n b : String <- IO.try(String, IO.get_env(\"USER\"))\n c : String <- IO.try(String, IO.get_env(\"SHELL\"))\n IO.print(a ++ b ++ c)\n{output}" + ) + }; + let (golden, other) = (program("\n#|ok\n"), program("")); + assert_eq!( + pairs_between(("tests/io/a.bend", &golden), ("tests/io/b.bend", &golden)), + 0 + ); + assert_eq!( + pairs_between(("tests/io/a.bend", &other), ("tests/io/b.bend", &other)), + 1, + "tests that check themselves may share a helper" + ); +} + +#[test] +fn copies_in_unrelated_packages_are_not_candidates() { + let package = |dir: &str, dependencies: &[&str]| crate::packages::Package { + dir: dir.into(), + name: Some(dir.into()), + dependencies: dependencies.iter().map(|d| d.to_string()).collect(), + }; + let (a, b, shared) = ( + package("a", &["shared"]), + package("b", &["shared"]), + package("shared", &[]), + ); + let units = super::super::units::parse(Path::new("x.rs"), LOAD).unwrap(); + let file = |path: &'static str, package| SourceFile { + path: Path::new(path), + source: LOAD, + selected: true, + units: &units.units, + excluded: Vec::new(), + package, + }; + let separate = package("c", &[]); + assert!( + find(&[file("a/x.rs", Some(&a)), file("c/x.rs", Some(&separate))]) + .pairs + .is_empty() + ); + let linked = find(&[ + file("a/x.rs", Some(&a)), + file("b/x.rs", Some(&b)), + file("shared/x.rs", Some(&shared)), + ]); + assert!(!linked.pairs.is_empty()); +} + +const LOAD: &str = "fn load_user(path: &str) -> Result {\n let text = std::fs::read_to_string(path)?;\n let value: Value = serde_json::from_str(&text)?;\n let name = value[\"name\"].as_str().unwrap_or(\"anonymous\").trim().to_string();\n Ok(User { name })\n}\n"; + +#[test] +fn copies_in_deprecated_code_are_not_candidates() { + assert_eq!( + run(&[("a.rs", LOAD, true), ("b.rs", LOAD, true)]) + .pairs + .len(), + 1 + ); + for mark in [ + "#[deprecated(note = \"use load_account\")]\n", + "/// Deprecated: use load_account.\n", + "/** @deprecated use load_account */\n", + ] { + let old = format!("{mark}{LOAD}"); + assert!( + run(&[("a.rs", LOAD, true), ("b.rs", &old, true)]) + .pairs + .is_empty(), + "{mark}" + ); + } + // A method of a class whose documentation marks it deprecated. + let class = |doc: &str| { + format!( + "prefix->prefixPath($path);\n $contents = $this->connection->get($location);\n if ($contents === false) {{\n throw UnableToReadFile::fromLocation($path);\n }}\n return $contents;\n }}\n}}\n" + ) + }; + let current = class(""); + let legacy = class("/**\n * @deprecated use the V3 adapter\n */\n"); + let pairs = |b: &str| run(&[("v3/A.php", ¤t, true), ("v2/A.php", b, true)]).pairs; + assert_eq!(pairs(¤t).len(), 1); + assert!(pairs(&legacy).is_empty()); + // Only a declaration's own header and the lines above it count: + // another method's decorator, a parameter named `deprecated` or a + // mark named `deprecated_lifespan` leave the copy a candidate. + let python = |mark: &str| { + format!( + "import json\n\n\nclass Reader:\n @deprecated(\"use read\")\n def old(self):\n return None\n\n{mark} def load_user(self, path,\n deprecated: bool = False):\n text = open(path).read()\n value = json.loads(text)\n name = value[\"name\"].strip().lower().replace(\" \", \"_\")\n return User(name=name, path=path)\n" + ) + }; + let current = python(""); + let pairs = |b: &str| run(&[("a.py", ¤t, true), ("b.py", b, true)]).pairs; + assert_eq!(pairs(¤t).len(), 1); + assert_eq!(pairs(&python(" @deprecated_lifespan\n")).len(), 1); + assert!(pairs(&python(" @deprecated(\"use load\")\n")).is_empty()); + assert!(pairs(&python(" @typing_extensions.deprecated(\"x\")\n")).is_empty()); +} + +#[test] +fn copies_in_retired_directories_are_not_candidates() { + let pairs = |b: &str| run(&[("src/a.rs", LOAD, true), (b, LOAD, true)]).pairs; + assert_eq!(pairs("src/b.rs").len(), 1); + for retired in [ + "Assets/ProofOfConcept/Builder.rs", + "deprecated/b.rs", + "scripts/archive/b.rs", + "src/proof-of-concept/b.rs", + ] { + assert!(pairs(retired).is_empty(), "{retired}"); + } + assert_eq!( + pairs("src/legacy/b.rs").len(), + 1, + "legacy code is often live" + ); +} + +#[test] +fn renamed_copies_match_across_files_with_statement_aligned_quotes() { + let renamed = LOAD + .replace("load_user", "load_team") + .replace("text", "body") + .replace("value", "parsed") + .replace("\"name\"", "\"title\"") + .replace("User", "Team"); + let found = run(&[("a.rs", LOAD, true), ("b.rs", &renamed, true)]); + assert_eq!(found.pairs.len(), 1, "{:?}", found.pairs.len()); + let pair = &found.pairs[0]; + assert_eq!(pair.a.path, Path::new("a.rs")); + assert_eq!(pair.b.path, Path::new("b.rs")); + assert_eq!(pair.a.function.as_deref(), Some("load_user")); + assert_eq!(pair.b.function.as_deref(), Some("load_team")); + assert!( + pair.a + .quote + .starts_with("let text = std::fs::read_to_string") + ); + assert!(pair.a.quote.ends_with("Ok(User { name })")); + assert_eq!((pair.a.start_line, pair.a.end_line), (2, 5)); + assert!(pair.differences.contains(&Difference { + a: "text".into(), + b: "body".into() + })); + assert!(pair.differences.contains(&Difference { + a: "name".into(), + b: "title".into() + })); + assert_eq!(pair.occurrences, 2); +} + +#[test] +fn java_copies_are_candidates_but_equality_boilerplate_is_not() { + let position = "class Position {\n\tprivate final int line;\n\tprivate final int column;\n\n\t@Override\n\tpublic boolean equals(Object other) {\n\t\tif (this == other) return true;\n\t\tif (other == null || getClass() != other.getClass()) return false;\n\t\tPosition that = (Position) other;\n\t\tif (line != that.line) return false;\n\t\treturn column == that.column;\n\t}\n\n\tString describe(Map fields) {\n\t\tString text = fields.get(\"text\");\n\t\tString trimmed = text.trim();\n\t\tString lower = trimmed.toLowerCase();\n\t\tfields.put(\"text\", lower);\n\t\treturn lower + line;\n\t}\n}\n"; + let range = position + .replace("Position", "Range") + .replace("line", "start") + .replace("column", "end"); + let found = run(&[ + ("Position.java", position, true), + ("Range.java", &range, true), + ]); + let functions: Vec<_> = found + .pairs + .iter() + .map(|p| (p.a.function.as_deref(), p.b.function.as_deref())) + .collect(); + assert_eq!( + functions, + [(Some("Position::describe"), Some("Range::describe"))] + ); +} + +#[test] +fn java_constructors_filling_their_fields_are_not_copies() { + let position = "class Position {\n\tPosition(int sourceLineNumber, int sourceColumnNumber, int sourceByteOffset, int sourceCharacterOffset, int trackedPosition) {\n\t\tthis.sourceLineNumber = sourceLineNumber;\n\t\tthis.sourceColumnNumber = sourceColumnNumber;\n\t\tthis.sourceByteOffset = sourceByteOffset;\n\t\tthis.sourceCharacterOffset = sourceCharacterOffset;\n\t\tthis.trackedPosition = trackedPosition;\n\t\tthis.valid = true;\n\t}\n\n\tPosition(Position copy) {\n\t\tsourceLineNumber = copy.sourceLineNumber;\n\t\tsourceColumnNumber = copy.sourceColumnNumber;\n\t\tsourceByteOffset = copy.sourceByteOffset;\n\t\tsourceCharacterOffset = copy.sourceCharacterOffset;\n\t\ttrackedPosition = copy.trackedPosition;\n\t}\n}\n"; + let range = position + .replace("Position", "Range") + .replace("line", "start") + .replace("column", "end"); + assert_eq!( + pairs_between(("Position.java", position), ("Range.java", &range)), + 0 + ); + // Work beyond storing fields is still compared. + let worker = |name: &str| { + format!( + "class {name} {{\n\t{name}(Map fields) {{\n\t\tString text = fields.get(\"text\");\n\t\tString trimmed = text.trim();\n\t\tString lower = trimmed.toLowerCase();\n\t\tfields.put(\"text\", lower);\n\t\tfields.put(\"length\", String.valueOf(lower.length()));\n\t\tthis.fields = fields;\n\t}}\n}}\n" + ) + }; + let (a, b) = (worker("Position"), worker("Range")); + assert_eq!(pairs_between(("Position.java", &a), ("Range.java", &b)), 1); +} + +#[test] +fn java_setters_given_literals_are_data_not_copies() { + let fixture = "class OwnerTests {\n\tprivate Owner george() {\n\t\tOwner george = new Owner();\n\t\tgeorge.setFirstName(\"George\");\n\t\tgeorge.setLastName(\"Franklin\");\n\t\tgeorge.setAddress(\"110 W. Liberty St.\");\n\t\tgeorge.setCity(\"Madison\");\n\t\tgeorge.setTelephone(\"6085551023\");\n\t\treturn george;\n\t}\n}\n"; + let inline = "class ServiceTests {\n\tvoid insertsOwner() {\n\t\tOwner owner = new Owner();\n\t\towner.setFirstName(\"Sam\");\n\t\towner.setLastName(\"Schultz\");\n\t\towner.setAddress(\"4, Evans Street\");\n\t\towner.setCity(\"Wollongong\");\n\t\towner.setTelephone(\"4444444444\");\n\t\towners.save(owner);\n\t}\n}\n"; + assert_eq!( + pairs_between(("OwnerTests.java", fixture), ("ServiceTests.java", inline)), + 0 + ); + // Setters given computed values copy logic and are still compared. + let mapping = |name: &str| { + format!( + "class {name} {{\n\tOwnerDto map(Owner owner) {{\n\t\tOwnerDto dto = new OwnerDto();\n\t\tdto.setFirstName(owner.getFirstName().trim());\n\t\tdto.setLastName(owner.getLastName().trim());\n\t\tdto.setAddress(owner.getAddress().trim());\n\t\tdto.setCity(owner.getCity().toUpperCase());\n\t\tdto.setTelephone(owner.getTelephone().replace(\" \", \"\"));\n\t\treturn dto;\n\t}}\n}}\n" + ) + }; + let (a, b) = (mapping("OwnerMapper"), mapping("VetMapper")); + assert_eq!( + pairs_between(("OwnerMapper.java", &a), ("VetMapper.java", &b)), + 1 + ); +} + +/// A Ruby assignment's bound locals and a Rust `use` path's imported +/// names: two walks that share only the frame of skipping one kind and +/// recursing into their children. +const BOUND: &str = "fn bound(node: Node<'_>, source: &str, names: &mut Vec) {\n if node.kind() == \"identifier\" {\n names.push(text(node, source).to_string());\n return;\n }\n if node.kind() == \"call\" {\n return;\n }\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {\n bound(child, source, names);\n }\n}\n"; +const IMPORTS: &str = "fn imports(node: Node<'_>, source: &str, names: &mut BTreeSet) {\n if node.kind() == \"identifier\" {\n let name = text(node, source);\n if !matches!(name, \"self\" | \"super\" | \"crate\") {\n names.insert(name.to_string());\n }\n return;\n }\n if node.kind() == \"scoped_identifier\" {\n if let Some(name) = node.child_by_field_name(\"name\") {\n imports(name, source, names);\n }\n return;\n }\n if node.kind() == \"string\" {\n return;\n }\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {\n imports(child, source, names);\n }\n}\n"; + +#[test] +fn walks_sharing_only_an_early_exit_and_their_recursion_are_not_copies() { + assert_eq!( + pairs_between(("ruby.rs", BOUND), ("import_names.rs", IMPORTS)), + 0 + ); + // The same frame around a different exit, after a different first + // step, with the recursion written as a method on the walker itself, + // is still only the frame. + let method = |name: &str, first: &str, stop: &str, call: &str| { + format!( + "impl Walker {{\n fn {name}(&mut self, node: Node<'_>) {{\n {first}\n if node.is_missing() {{\n return;\n }}\n if node.kind() == \"{stop}\" {{\n return;\n }}\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {{\n if child.is_extra() {{\n continue;\n }}\n self.{call}(child);\n }}\n }}\n}}\n" + ) + }; + let (a, b) = ( + method("locals", "self.depth += 1;", "call", "locals"), + method( + "exports", + "self.seen.insert(node.id());", + "string", + "exports", + ), + ); + assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 0); + // Both calling another method is the same window, and a copy. + let (a, b) = ( + method("locals", "self.depth += 1;", "call", "visit"), + method("exports", "self.seen.insert(node.id());", "string", "visit"), + ); + assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 1); + // Exits without braces and `this.` recursion in JavaScript. + let script = |name: &str, first: &str, stop: &str| { + format!( + "class Scanner {{\n {name}(node) {{\n {first}\n if (!node) return;\n if (node.type === '{stop}') return;\n const children = node.namedChildren;\n for (const child of children) {{\n this.{name}(child);\n }}\n }}\n}}\n" + ) + }; + let (a, b) = ( + script("locals", "this.depth++;", "call_expression"), + script("exports", "this.seen.add(node.id);", "string"), + ); + assert_eq!(pairs_between(("a.js", &a), ("b.js", &b)), 0); +} + +#[test] +fn walks_copying_the_work_they_do_at_each_node_are_candidates() { + // Both walks collect the bound identifiers the same way; each calling + // itself is the same step, so their own names are no difference. + let copy = BOUND.replace("bound", "assigned"); + assert_eq!( + differences_between(("ruby.rs", BOUND), ("python.rs", ©)), + [] + ); + // A walk that does its work in the loop, around its recursion. + let visit = |name: &str, first: &str| { + format!( + "fn {name}(node: Node<'_>, source: &str, names: &mut Vec) {{\n {first}\n if node.kind() == \"call\" {{\n return;\n }}\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {{\n if child.kind() == \"identifier\" {{\n names.push(text(child, source).to_string());\n }}\n {name}(child, source, names);\n }}\n}}\n" + ) + }; + let (a, b) = (visit("locals", ""), visit("parameters", "")); + assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 1); + // Part of each, after a different first step. + let (a, b) = ( + visit("locals", "trace(node);"), + visit("parameters", "names.reserve(8);"), + ); + assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 1); + // Work in a match arm beside the recursion, after a different first + // step, so that only part of each walk is copied. + let arms = |name: &str, first: &str| { + format!( + "fn {name}(node: Node<'_>, source: &str, names: &mut Vec) {{\n {first}\n if node.is_missing() {{\n return;\n }}\n let mut cursor = node.walk();\n for child in node.named_children(&mut cursor) {{\n match child.kind() {{\n \"identifier\" => names.push(child.utf8_text(source.as_bytes()).unwrap().to_string()),\n \"call\" | \"string\" => {{}}\n _ => {name}(child, source, names),\n }}\n }}\n}}\n" + ) + }; + let (a, b) = ( + arms("locals", "trace(node);"), + arms("exports", "names.reserve(8);"), + ); + assert_eq!(pairs_between(("a.rs", &a), ("b.rs", &b)), 1); + // Work in a conditional expression beside the recursion. + let ternary = |name: &str, first: &str| { + format!( + "class Walker {{\n {name}(node, names) {{\n {first}\n if (!node) return;\n if (node.type === 'comment') return;\n for (const child of node.namedChildren) child.type === 'identifier' ? names.push(child.text.trim().toLowerCase()) : this.{name}(child, names);\n }}\n}}\n" + ) + }; + let (a, b) = ( + ternary("collect", "this.depth++;"), + ternary("gather", "names.clear();"), + ); + assert_eq!(pairs_between(("a.js", &a), ("b.js", &b)), 1); +} + +#[test] +fn whole_copies_of_small_walks_and_guarded_handlers_are_candidates() { + // One step of work between an exit and the recursion, and no cursor. + let weights = |name: &str| { + format!( + "pub fn {name}(node: &Tree, scale: f64, out: &mut Vec) {{\n if node.hidden || node.children.is_empty() {{\n return;\n }}\n out.push(node.weight * scale + node.bias * node.decay.powi(2));\n for child in &node.children {{\n {name}(child, scale * node.decay, out);\n }}\n}}\n" + ) + }; + let (a, b) = (weights("total_weight"), weights("total_cost")); + assert_eq!(pairs_between(("weights.rs", &a), ("costs.rs", &b)), 1); + let labels = |name: &str| { + format!( + "def {name}(node, labels):\n if node is None or node.hidden:\n return\n labels.append(node.display_name.strip().lower().replace(\" \", \"_\"))\n for child in node.children:\n {name}(child, labels)\n" + ) + }; + let (a, b) = (labels("collect_names"), labels("gather_labels")); + assert_eq!(pairs_between(("names.py", &a), ("labels.py", &b)), 1); + // Exits alone are no walk: handlers that check and notify alike. + let handler = |name: &str, event: &str| { + format!( + "export function {name}({event}) {{\n if (!{event} || !{event}.repository) return;\n if ({event}.repository.archived || {event}.repository.disabled) return;\n if ({event}.sender && {event}.sender.type === 'Bot') return;\n notifyChannel({event}.repository.fullName, {event}.ref, {event}.headCommit);\n}}\n" + ) + }; + let (a, b) = (handler("onPush", "event"), handler("onTag", "payload")); + assert_eq!(pairs_between(("push.js", &a), ("tag.js", &b)), 1); + // Part of each, with little work after the exits. + let guarded = |name: &str, first: &str| { + format!( + "export function {name}(event) {{\n {first}\n if (!event || !event.repository) return;\n if (event.repository.archived || event.repository.disabled) return;\n if (event.sender && event.sender.type === 'Bot') return;\n notify(event);\n}}\n" + ) + }; + let (a, b) = ( + guarded("onPush", "log.debug('push');"), + guarded("onTag", "metrics.count('tag', 1);"), + ); + assert_eq!(pairs_between(("push.js", &a), ("tag.js", &b)), 1); +} + +#[test] +fn a_walk_with_one_large_step_of_work_is_a_candidate() { + let render = |name: &str, first: &str| { + format!( + "def {name}(node, out, depth=0):\n {first}\n if node is None or node.hidden:\n return\n out.write(\" \" * depth + f\"{{node.kind}} [{{node.start}}..{{node.end}}] {{node.name!r}} ({{len(node.children)}} children)\\n\")\n for child in node.children:\n {name}(child, out, depth + 1)\n" + ) + }; + // The whole of both functions. + let (a, b) = (render("render", "pass"), render("dump", "pass")); + assert_eq!(pairs_between(("render.py", &a), ("dump.py", &b)), 1); + // Part of each, after a different first step. + let (a, b) = ( + render("render", "depth = depth or 0"), + render("dump", "out.flush()"), + ); + assert_eq!(pairs_between(("render.py", &a), ("dump.py", &b)), 1); +} + +#[test] +fn bare_calls_in_methods_of_the_same_name_call_another_function() { + // `dump` inside the method `dump` is the imported `json.dump`. + let store = |owner: &str, module: &str, name: &str| { + format!( + "from {module} import {name}\n\n\nclass {owner}:\n def {name}(self, record):\n if record is None:\n return\n payload = {{\"id\": record.id, \"name\": record.name.strip(), \"tags\": sorted(record.tags), \"owner\": record.owner.email}}\n {name}(payload, self.handle, indent=2, sort_keys=True)\n" + ) + }; + let (a, b) = ( + store("JsonStore", "json", "dump"), + store("YamlStore", "yaml", "safe_dump"), + ); + let renamed = Difference { + a: "dump".into(), + b: "safe_dump".into(), + }; + assert!(differences_between(("json_store.py", &a), ("yaml_store.py", &b)).contains(&renamed)); + // A Rust method calling a function of another module by its own name. + let cache = |owner: &str, name: &str| { + format!( + "impl {owner} {{\n fn {name}(&self, key: &str) -> Option {{\n let path = self.root.join(key.trim_start_matches('/'));\n let value = super::store::{name}(&path)?;\n self.hits.fetch_add(1, Ordering::Relaxed);\n Some(value.trim().to_string())\n }}\n}}\n" + ) + }; + let (a, b) = (cache("Cache", "get"), cache("Mirror", "fetch")); + let renamed = Difference { + a: "get".into(), + b: "fetch".into(), + }; + assert!(differences_between(("cache.rs", &a), ("mirror.rs", &b)).contains(&renamed)); +} + +#[test] +fn go_functions_sharing_only_error_checks_and_cleanups_are_not_copies() { + let store = |name: &str, find: &str, kind: &str, tail: &str| { + format!( + "package sqlite\n\nfunc (s *Service) {name}(ctx context.Context, id int) (*wtf.{kind}, error) {{\n\ttx, err := s.db.BeginTransactionWithOptions(ctx, nil)\n\tif err != nil {{\n\t\treturn nil, err\n\t}}\n\tdefer tx.Rollback()\n\trecord, err := {find}(ctx, tx, id)\n\tif err != nil {{\n\t\treturn nil, err\n\t}}\n\t{tail}\n\treturn record, nil\n}}\n" + ) + }; + let (a, b) = ( + store( + "FindAuthByID", + "findAuthByID", + "Auth", + "record.LastSeen = time.Now()", + ), + store( + "FindDialByID", + "findDialByID", + "Dial", + "go notify(record.ID, s.events)", + ), + ); + assert_eq!(pairs_between(("auth.go", &a), ("dial.go", &b)), 0); + // With more work between them, the error checks do not hide a copy. + let (a, b) = ( + a.replace("\trecord, err", "\tlog.Printf(\"loading one stored record by its identifier\")\n\tmetrics.Count(\"store.find\", 1)\n\trecord, err"), + b.replace("\trecord, err", "\tlog.Printf(\"loading one stored record by its identifier\")\n\tmetrics.Count(\"store.find\", 1)\n\trecord, err"), + ); + assert_eq!(pairs_between(("auth.go", &a), ("dial.go", &b)), 1); +} + +#[test] +fn a_list_of_alike_statements_is_not_a_copy_of_itself() { + let source = "def create(self, a=None, b=None, c=None, d=None, e=None, f=None):\n a = self.value_or_default(\"alpha_setting\", a)\n b = self.value_or_default(\"beta_setting\", b)\n c = self.value_or_default(\"gamma_setting\", c)\n d = self.value_or_default(\"delta_setting\", d)\n e = self.value_or_default(\"epsilon_setting\", e)\n f = self.value_or_default(\"zeta_setting\", f)\n return self.build(a, b, c, d, e, f)\n"; + assert!(run(&[("db.py", source, true)]).pairs.is_empty()); +} + +#[test] +fn variants_of_one_example_are_not_compared() { + let flow = |dir: &str| format!("examples/login/{dir}/apis.py"); + assert!(super::separate_examples( + Path::new(&flow("raw")), + Path::new(&flow("sdk")) + )); + assert!(super::separate_examples( + Path::new("app/blog_examples/raw/views.py"), + Path::new("app/blog_examples/sdk/views.py") + )); + assert!(!super::separate_examples( + Path::new("examples/login/raw/apis.py"), + Path::new("examples/login/raw/views.py") + )); + assert!(!super::separate_examples( + Path::new("src/billing/raw/apis.py"), + Path::new("src/billing/sdk/apis.py") + )); + // Benchmark programs side by side; one benchmark suite's files are one program. + assert!(super::separate_examples( + Path::new("bench/runtime/nbody/main.bend"), + Path::new("bench/runtime/mandelbrot/main.bend") + )); + assert!(!super::separate_examples( + Path::new("benchmarks/multipart_benchmark.py"), + Path::new("benchmarks/urlencoded_benchmark.py") + )); + for path in [ + "docs_src/tutorial/one/tutorial001.py", + "example/settings.py", + "examples/hello_world.rs", + ] { + assert!(super::example_code(Path::new(path)), "{path}"); + } + assert!(!super::example_code(Path::new("src/examples.rs"))); + for path in [ + "samples/MediatR.Examples/Runner.cs", + "src/MediatR.Examples.Autofac/Program.cs", + "example_authentication_middleware_test.go", + ] { + assert!(super::example_code(Path::new(path)), "{path}"); + } + assert!(!super::example_code(Path::new( + "src/main/java/org/springframework/samples/petclinic/Owner.java" + ))); + for path in [ + "src/main/java/com/example/demo/OrderController.java", + "service/src/test/kotlin/com/example/OrderTest.kt", + ] { + assert!(!super::example_code(Path::new(path)), "{path}"); + } + assert!(super::example_code(Path::new( + "examples/spring/src/main/java/com/example/demo/Main.java" + ))); +} + +#[test] +fn inconsistent_renaming_and_short_windows_are_rejected() { + // `text` becomes two different names on the other side. + let inconsistent = LOAD + .replace("let text", "let body") + .replace("from_str(&text)", "from_str(&other)"); + assert!( + run(&[("a.rs", LOAD, true), ("b.rs", &inconsistent, true)]) + .pairs + .is_empty() + ); + let short = "fn a(x: i32) -> i32 {\n let y = x + 1;\n y * 2\n}\nfn b(x: i32) -> i32 {\n let y = x + 1;\n y * 2\n}\n"; + assert!(run(&[("s.rs", short, true)]).pairs.is_empty()); + // A docstring is not a statement: two statements stay too few. + let wrappers = "def render_template(name, **context):\n \"\"\"Render a template by name with the given context and return the resulting page.\"\"\"\n app = current_app._get_current_object()\n return _render(app, app.jinja_env.get_or_select_template(name), context)\n\n\ndef stream_template(name, **context):\n \"\"\"Render a template by name with the given context as a stream of page parts.\"\"\"\n app = current_app._get_current_object()\n return _stream(app, app.jinja_env.get_or_select_template(name), context)\n"; + assert!(run(&[("templating.py", wrappers, true)]).pairs.is_empty()); +} + +#[test] +fn a_short_idiom_inside_a_larger_copy_forms_its_own_group() { + let head = " let text = std::fs::read_to_string(path).expect(\"reading the configured user file failed\");\n let value: Value = serde_json::from_str(&text).expect(\"parsing the configured user file failed\");\n let root = value.as_object().expect(\"the configured user file holds an object\");\n"; + let tail = " let name = root[\"name\"].as_str().unwrap_or(\"anonymous\").trim().to_string();\n let age = root[\"age\"].as_u64().unwrap_or(0).min(150) as u32;\n let city = root[\"city\"].as_str().unwrap_or(\"unknown\").trim().to_string();\n let email = root[\"email\"].as_str().unwrap_or(\"\").trim().to_lowercase();\n Ok(User { name, age, city, email })\n"; + let full = format!("fn load(path: &str) -> Result {{\n{head}{tail}}}\n"); + let other = full.replace("fn load", "fn again"); + let short = format!("fn count(path: &str) -> usize {{\n{head} root.len()\n}}\n"); + let found = run(&[ + ("a.rs", &full, true), + ("b.rs", &other, true), + ("c.rs", &short, true), + ]); + let largest = found.pairs.iter().max_by_key(|p| p.size).unwrap(); + assert_eq!( + (largest.a.path.as_path(), largest.b.path.as_path()), + (Path::new("a.rs"), Path::new("b.rs")) + ); + assert!(largest.copies.is_empty(), "{:?}", largest.copies); + assert_eq!(found.pairs.len(), 2); +} + +#[test] +fn context_only_pairs_are_excluded_but_selected_to_context_pairs_are_kept() { + let copy = LOAD.replace("load_user", "load_again"); + assert!( + run(&[("a.rs", LOAD, false), ("b.rs", ©, false)]) + .pairs + .is_empty() + ); + let found = run(&[("context.rs", LOAD, false), ("selected.rs", ©, true)]); + assert_eq!(found.pairs.len(), 1); + assert_eq!(found.pairs[0].a.path, Path::new("selected.rs")); + assert_eq!(found.pairs[0].b.path, Path::new("context.rs")); +} + +#[test] +fn repeated_copies_form_one_group_judged_through_one_pair() { + let three = [ + LOAD, + &LOAD.replace("load_user", "second"), + &LOAD.replace("load_user", "third"), + ] + .concat(); + let found = run(&[("three.rs", &three, true)]); + assert_eq!(found.pairs.len(), 1); + let pair = &found.pairs[0]; + assert_eq!((pair.occurrences, pair.a.start_line), (3, 2)); + assert_eq!(pair.copies.len(), 1); + let lines = [pair.b.start_line, pair.copies[0].start_line]; + assert!(lines.contains(&8) && lines.contains(&14), "{lines:?}"); + + // Copies of different lengths still share a group through overlapping sites. + let longer = LOAD.replace( + " Ok(User { name })", + " let checked = name.trim().to_string();\n Ok(User { name: checked })", + ); + let found = run(&[ + ("a.rs", LOAD, true), + ("b.rs", &LOAD.replace("load_user", "other"), true), + ("c.rs", &longer.replace("load_user", "third"), true), + ]); + assert_eq!( + found.pairs.len(), + 1, + "{:?}", + found + .pairs + .iter() + .map(|p| (&p.a.path, &p.b.path)) + .collect::>() + ); + assert_eq!(found.pairs[0].occurrences, 3); + let body = " total = decimal.Decimal(\"0\")\n for row in rows:\n total += row.amount * row.exchange_rate - row.discount_amount\n return total.quantize(decimal.Decimal(\"0.01\"), rounding=decimal.ROUND_HALF_UP)\n"; + let python = format!( + "def a(rows):\n{body}\ndef b(items):\n{}", + body.replace("row", "item") + ); + let found = run(&[("totals.py", &python, true)]); + assert_eq!(found.pairs.len(), 1); + assert_eq!(found.pairs[0].a.function.as_deref(), Some("a")); +} + +#[test] +fn windows_of_the_same_two_functions_split_by_one_statement_are_one_pair() { + let head = " parser = argparse.ArgumentParser(description=__doc__)\n parser.add_argument('--owner', default='Tech')\n parser.add_argument('--project', type=int, default=2)\n parser.add_argument('--apply', action='store_true')\n"; + let tail = " args = parser.parse_args()\n if args.apply and not args.backup:\n parser.error('--apply requires --backup')\n run(args.owner, args.project, args.apply, args.backup)\n"; + let first = format!( + "def main():\n{head} parser.add_argument('--completed', action='store_true')\n{tail}" + ); + let second = format!("def main():\n{head}{tail}"); + let found = run(&[("migrate.py", &first, true), ("retire.py", &second, true)]); + assert_eq!( + found + .pairs + .iter() + .map(|p| (p.a.start_line, p.b.start_line)) + .collect::>() + .len(), + 1 + ); +} From 08e965dae7b94372c003ab53a1b4f28542c074d9 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sun, 27 Sep 2026 19:14:37 -0300 Subject: [PATCH 2/5] Split units/security.rs into its planning, the subjects it judges and the settle catalog --- src/units/{security.rs => security/mod.rs} | 530 +-------------------- src/units/security/settle.rs | 287 +++++++++++ src/units/security/subject.rs | 242 ++++++++++ 3 files changed, 537 insertions(+), 522 deletions(-) rename src/units/{security.rs => security/mod.rs} (55%) create mode 100644 src/units/security/settle.rs create mode 100644 src/units/security/subject.rs diff --git a/src/units/security.rs b/src/units/security/mod.rs similarity index 55% rename from src/units/security.rs rename to src/units/security/mod.rs index 2b18526..33befba 100644 --- a/src/units/security.rs +++ b/src/units/security/mod.rs @@ -4,7 +4,9 @@ //! handled), and for injection a recheck with up to three callers when the //! origin stays unclear. A file's top-level setup statements are one more //! unit for unsafe settings; a PHP file's top-level statements are a page -//! script, judged like a function by every rule. +//! script, judged like a function by every rule. `subject` builds what each +//! unit is judged on and `settle` holds the Choices that settle an undecided +//! check. use super::{ Asked, Block, Detail, FileContext, FilePlan, Planned, Presence, Questions, Settle, UnitPlan, compact, identity, pack_runs, questions, unique_ids, @@ -17,6 +19,11 @@ use crate::{ use serde_json::{Value, json}; use std::collections::BTreeMap; +mod settle; +mod subject; +pub(in crate::units) use settle::*; +pub(in crate::units) use subject::*; + /// The presence questions of each rule, in the order they are asked. pub(super) const PRESENCE: [(&str, &[&str]); 3] = [ (INJECTION, &["interpreted", "resource"]), @@ -27,244 +34,6 @@ pub(super) const PRESENCE: [(&str, &[&str]); 3] = [ /// Callers shown when the origin of an injection's values stays unclear. pub(super) const CALLERS: usize = 3; -/// A function or setup statements that the security rules judge. -pub(super) struct Subject<'a> { - pub name: String, - /// `function` or `module`: the state key and the source path. - pub kind: &'static str, - pub source: String, - pub sites: &'a [Site], - /// Errors it creates, with their message arguments. - pub errors: &'a [CreatedError], - pub lines: (usize, usize), - /// Functions that call it, as (name, source), for the injection recheck. - pub callers: Vec<(String, String)>, - /// Enums its sites name, such as `ConfigKey` in `'${ConfigKey.aiTag}'`, - /// defined in this or another selected file: fixed choices, not - /// parameters, which the trace otherwise could not tell apart. - pub enums: Vec, - /// Definitions of the project's types its parameters name, shown when a - /// path finding is confirmed: how a route parameter of that type is - /// parsed decides what it can hold. - pub types: Vec, - /// C# constants it names, as `Class.Field = value`: a key written in - /// the code or a value read from configuration. - pub constants: Vec, - /// Framework facts shown beside the source in every request of its - /// units, such as the settings modules that import a settings module - /// and assign its settings again. - pub evidence: serde_json::Map, - /// Whether it is Django code, whose questions name Django's calls and - /// settings and ask its extra checks. - pub django: bool, - /// Errors the functions it calls create, with their messages, shown in - /// its sensitive-data trace: whether an error's text that it sends is - /// the program's own depends on where the error was raised. - pub callee_errors: Vec, - /// Whether its file sits at a test path, such as a test app's settings. - pub test_path: bool, -} - -/// The evidence key of the templates a function renders that write values -/// without escaping. -pub(super) const RENDERED: &str = "templates_it_renders_that_write_values_without_escaping"; - -impl Subject<'_> { - fn code(&self) -> String { - format!("{}.source", self.kind) - } - - /// Whether it renders a template that writes values without escaping, - /// outside Django, whose questions name its templates already. - fn renders(&self) -> bool { - !self.django && self.evidence.contains_key(RENDERED) - } - - /// Its name, source and framework evidence, as sent. - fn state(&self) -> Value { - let mut state = serde_json::Map::new(); - state.insert("name".into(), json!(self.name)); - state.insert("source".into(), json!(self.source)); - state.extend(self.evidence.clone()); - Value::Object(state) - } -} - -pub(super) fn function_subject<'a>( - file: &FileContext<'_>, - unit: &'a Unit, - callers: Vec<(String, String)>, - (enums, types): (&BTreeMap, &BTreeMap), - constants: &BTreeMap>, -) -> Subject<'a> { - let source = unit.source(file.source).to_string(); - Subject { - name: unit.name.clone(), - kind: "function", - constants: named_constants(file, &source, constants), - source, - sites: &unit.sites, - errors: &unit.errors, - lines: (unit.line, unit.end_line), - callers, - enums: named_enums(&unit.sites, enums), - types: named_types(&unit.signature, types), - evidence: serde_json::Map::new(), - django: false, - callee_errors: Vec::new(), - test_path: false, - } -} - -/// Constant declarations shown with one subject, at most. -const CONSTANTS: usize = 4; - -/// The declarations of the C# constants a C# subject names. -fn named_constants( - file: &FileContext<'_>, - source: &str, - constants: &BTreeMap>, -) -> Vec { - if file.language != questions::CSHARP { - return Vec::new(); - } - let mut found = Vec::new(); - for (field, declarations) in constants { - let named = source.match_indices(field.as_str()).any(|(at, _)| { - let before = source[..at].chars().next_back(); - let after = source[at + field.len()..].chars().next(); - let word = |c: Option| c.is_some_and(|c| c.is_alphanumeric() || c == '_'); - !word(before) && !word(after) - }); - if named { - found.extend(declarations.iter().cloned()); - } - } - found.truncate(CONSTANTS); - found -} - -/// Type definitions shown with one subject, at most. -const TYPES: usize = 3; - -/// The definitions of the project's types named as words in a signature. -fn named_types(signature: &str, types: &BTreeMap) -> Vec { - let word = |c: Option| c.is_some_and(|c| c.is_alphanumeric() || c == '_'); - types - .iter() - .filter(|(name, _)| { - signature.match_indices(name.as_str()).any(|(at, _)| { - !word(signature[..at].chars().next_back()) - && !word(signature[at + name.len()..].chars().next()) - }) - }) - .map(|(_, definition)| definition.clone()) - .take(TYPES) - .collect() -} - -/// Enum definitions shown with one subject, at most. -const ENUMS: usize = 3; - -/// The definitions of enums named as `Name.member` or `Name::member` in the sites. -fn named_enums(sites: &[Site], enums: &BTreeMap) -> Vec { - let mut found: Vec = Vec::new(); - for site in sites { - for (name, definition) in enums { - let named = site.text.match_indices(name.as_str()).any(|(at, _)| { - let before = site.text[..at].chars().next_back(); - let after = &site.text[at + name.len()..]; - before.is_none_or(|c| !(c.is_alphanumeric() || c == '_')) - && (after.starts_with('.') || after.starts_with("::")) - }); - if named && found.len() < ENUMS && !found.contains(definition) { - found.push(definition.clone()); - } - } - } - found -} - -pub(super) fn setup_subject<'a>( - file: &FileContext<'_>, - setup: &'a crate::analysis::sites::Setup, - constants: &BTreeMap>, -) -> Option> { - let first = setup.statements.first()?; - let last = setup.statements.last()?; - let source: Vec = setup - .statements - .iter() - .map(|(range, ..)| setup.text(file.source, range.clone())) - .collect(); - let source = source.join("\n"); - let (name, kind) = if setup.script { - (SCRIPT, "function") - } else if setup.settings { - (SETTINGS_MODULE, "module") - } else { - (MODULE_SETUP, "module") - }; - Some(Subject { - name: name.into(), - kind, - constants: named_constants(file, &source, constants), - source, - sites: &setup.sites, - errors: &[], - lines: (first.1, last.2), - callers: Vec::new(), - enums: Vec::new(), - types: Vec::new(), - evidence: serde_json::Map::new(), - django: false, - callee_errors: Vec::new(), - test_path: false, - }) -} - -/// A server template's code that reads client data, judged like a function -/// by every security rule. -pub(super) fn template_subject<'a>( - file: &FileContext<'_>, - code: &'a crate::analysis::sites::Setup, -) -> Option> { - let first = code.statements.first()?; - let last = code.statements.last()?; - let source: Vec<&str> = code - .statements - .iter() - .map(|(range, ..)| &file.source[range.clone()]) - .collect(); - Some(Subject { - name: TEMPLATE_CODE.into(), - kind: "function", - source: source.join("\n"), - sites: &code.sites, - errors: &[], - lines: (first.1, last.2), - callers: Vec::new(), - enums: Vec::new(), - types: Vec::new(), - constants: Vec::new(), - evidence: serde_json::Map::new(), - django: false, - callee_errors: Vec::new(), - test_path: false, - }) -} - -/// The name of the unit that holds a server template's code. -pub(super) const TEMPLATE_CODE: &str = "template code"; - -/// The name of the unit that holds a file's top-level setup statements. -pub(super) const MODULE_SETUP: &str = "module setup"; -/// The name of that unit in a Django settings module, whose statements -/// assign the deployed site's settings. -pub(super) const SETTINGS_MODULE: &str = "settings module"; -/// The name of the unit that holds a PHP file's top-level statements. -pub(super) const SCRIPT: &str = "top-level code"; - /// Plan every enabled rule's units for these subjects. Functions and a PHP /// page script are packed; the module setup, when present, is judged for /// unsafe settings only. `django` marks Django code, whose presence @@ -866,289 +635,6 @@ fn confirm_logging( file.budget.fits(&request).then_some((request, asked)) } -/// A Choice asked when one of a rule's checks stays undecided after the -/// trace and recheck; it can only clear the checks it settles, so it is -/// asked apart from them. -pub(in crate::units) struct SettleKind { - pub rule: &'static str, - /// The question id of the Choice. - pub question: &'static str, - /// The checks that call for it while undecided, and that it settles. - pub checks: &'static [&'static str], - /// The options whose combined probability at the threshold clears them. - pub clears: &'static [&'static str], - /// Whether the functions that call the subject are sent with it. - callers: bool, - pub when: SettleWhen, - /// The files whose units it is planned for. - files: SettleFiles, -} - -/// The files, by language, whose units a settle Choice is planned for. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum SettleFiles { - All, - Only(&'static str), - Except(&'static str), -} - -impl SettleFiles { - fn include(self, language: &str) -> bool { - match self { - SettleFiles::All => true, - SettleFiles::Only(only) => language == only, - SettleFiles::Except(except) => language != except, - } - } -} - -/// Which units a settle Choice is asked for. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(in crate::units) enum SettleWhen { - /// An uncertain unit whose checks stay undecided. - Undecided, - /// Also a consider or note that rests on its undecided checks: a note - /// that a client component's fetch "places a parameter into a URL it - /// requests" only puzzled readers. - UndecidedOrFinding, - /// Any unit whose checks are not clear, and it clears a check that found - /// a concern too: a PHP page joins into HTML the body its included file - /// built, ids converted to numbers and database errors, and the markup - /// check found those at 0.9 while the origin question answered for the - /// request the page also reads. - NotClear, -} - -/// Every settle Choice. Where a URL comes from settles the URL check: on -/// clients of a fixed or configured service it split on a variable path or -/// query; so does code that runs only in the user's browser. Where a redirect leads, how markup is rendered and which origins -/// may send credentials settle theirs, which split on client components that -/// navigate to fixed paths or render values as attributes, and on route -/// handlers that answer preflights for any origin without credentials. What -/// its logs write settles its logging signals whenever they are not clear: -/// a logged object split on errors caught from a payment or database call, -/// and audit lines naming who signed in were logged personal data. Where a -/// function's text goes is asked whenever its error-detail signals are not -/// clear, too: a game client handing the server's error text to its own -/// window over a channel whose messages are named `Response` was fifteen -/// reviews for sending details to a remote client. Where a function's text goes settles error -/// details (see `exposure_signal`), also under a finding that claims the text -/// likely reaches a client. -/// -/// PHP pages ask what they join into HTML in place of how markup is -/// rendered, which names JSX and client components, and where the paths -/// they open or include come from: pages include their parts through a -/// directory constant and a file name a switch picks, and the path check -/// stayed near 0.25 on them. Both are asked whenever their check is not -/// clear: a page whose markup check leaned toward a concern was a note, so -/// its undecided path Choice was never asked, and once the markup Choice -/// cleared the markup it was left uncertain. What their command lines hold -/// settles the shell check the same way: a page that checks each octet of -/// an address with is_numeric was a command injection at 0.88. What code -/// does with tokens and how it handles passwords settle those checks -/// whenever they are not clear: front ends that send their own token and -/// HMAC signing split on them or were reviews. -pub(in crate::units) const SETTLES: [SettleKind; 14] = [ - SettleKind { - rule: INJECTION, - question: "url_parts", - checks: &["url"], - clears: &questions::OWN_PARTS, - callers: true, - when: SettleWhen::Undecided, - files: SettleFiles::All, - }, - SettleKind { - rule: INJECTION, - question: "runs_in", - checks: &["url"], - clears: &[questions::BROWSER], - callers: false, - when: SettleWhen::UndecidedOrFinding, - files: SettleFiles::All, - }, - SettleKind { - rule: INJECTION, - question: "redirect_target", - checks: &["redirect"], - clears: &questions::OWN_TARGETS, - callers: true, - when: SettleWhen::Undecided, - files: SettleFiles::All, - }, - SettleKind { - rule: INJECTION, - question: "markup_output", - checks: &["markup"], - clears: &questions::INERT_MARKUP, - callers: false, - when: SettleWhen::Undecided, - files: SettleFiles::Except(questions::PHP), - }, - SettleKind { - rule: INJECTION, - question: "markup_parts", - checks: &["markup"], - clears: &questions::HANDLED_MARKUP, - callers: true, - when: SettleWhen::NotClear, - files: SettleFiles::Only(questions::PHP), - }, - SettleKind { - rule: INJECTION, - question: "shell_parts", - checks: &["shell"], - clears: &questions::CHECKED_COMMANDS, - callers: false, - when: SettleWhen::NotClear, - files: SettleFiles::Only(questions::PHP), - }, - SettleKind { - rule: INJECTION, - question: "path_parts", - checks: &["path"], - clears: &questions::FIXED_PATHS, - callers: false, - when: SettleWhen::NotClear, - files: SettleFiles::Only(questions::PHP), - }, - SettleKind { - rule: INJECTION, - question: "path_source", - checks: &["path"], - clears: &questions::OWN_PATHS, - callers: true, - when: SettleWhen::Undecided, - files: SettleFiles::Except(questions::PHP), - }, - SettleKind { - rule: SENSITIVE_DATA, - question: "destination", - checks: &["error_details", "exception_to_client"], - clears: &questions::AWAY_FROM_CLIENTS, - callers: false, - when: SettleWhen::NotClear, - files: SettleFiles::All, - }, - SettleKind { - rule: SENSITIVE_DATA, - question: "logged", - checks: &["logs_object_secret", "logs_secret"], - clears: &questions::PLAIN_LOGS, - callers: false, - when: SettleWhen::NotClear, - files: SettleFiles::All, - }, - SettleKind { - rule: UNSAFE_SETTINGS, - question: "cors_origins", - checks: &["cors"], - clears: &questions::SAFE_ORIGINS, - callers: false, - when: SettleWhen::Undecided, - files: SettleFiles::All, - }, - SettleKind { - rule: UNSAFE_SETTINGS, - question: "cookie_flags", - checks: &["cookie"], - clears: &questions::FLAGGED_COOKIES, - callers: false, - when: SettleWhen::Undecided, - files: SettleFiles::All, - }, - SettleKind { - rule: UNSAFE_SETTINGS, - question: "token_use", - checks: &["token"], - clears: &questions::VERIFIED_TOKENS, - callers: false, - when: SettleWhen::NotClear, - files: SettleFiles::All, - }, - SettleKind { - rule: UNSAFE_SETTINGS, - question: "password_handling", - checks: &["hash"], - clears: &questions::HASHED_PASSWORDS, - callers: false, - when: SettleWhen::NotClear, - files: SettleFiles::All, - }, -]; - -/// The settle follow-ups of one unit, one per Choice of its rule, each sent -/// only when its checks stay undecided. -fn settles( - file: &FileContext<'_>, - subject: &Subject<'_>, - rule: &'static str, - id: &str, -) -> Vec { - SETTLES - .iter() - .filter(|kind| kind.rule == rule && kind.files.include(file.language)) - .filter_map(|kind| { - let request = settle(file, subject, kind, id); - file.budget.fits(&request.0).then_some(Settle { - question: kind.question, - request: request.into(), - }) - }) - .collect() -} - -fn settle( - file: &FileContext<'_>, - subject: &Subject<'_>, - kind: &SettleKind, - id: &str, -) -> (Value, Asked) { - let code = subject.code(); - let callers = kind.callers && !subject.callers.is_empty(); - let body = match kind.question { - "url_parts" => questions::security_url_parts(&code, callers), - "path_source" => questions::security_path_source(&code, callers), - "runs_in" => questions::security_runs_in(&code), - "redirect_target" => questions::security_redirect_target(&code, callers), - "markup_output" => { - questions::security_markup_output(&code, subject.django, subject.renders()) - } - "markup_parts" => questions::security_markup_parts(&code, callers), - "path_parts" => questions::security_path_parts(&code), - "shell_parts" => questions::security_shell_parts(&code), - "destination" => questions::security_destination(&code), - "logged" => questions::security_logged(&code), - "cookie_flags" => questions::security_cookie_flags(&code), - "token_use" => questions::security_token_use(&code), - "password_handling" => questions::security_password_handling(&code), - _ => questions::security_cors_origins(&code), - }; - let mut questions = Questions::default(); - questions.ask( - kind.question.into(), - body, - id, - kind.rule, - kind.question, - Pass::Settle, - ); - let mut state = json!({ - "file": file.file_state(), - subject.kind: subject.state(), - }); - if callers { - state["callers"] = json!( - subject - .callers - .iter() - .map(|(name, source)| json!({"name": name, "source": source})) - .collect::>() - ); - } - file.request("settle", state, questions) -} - fn sites(file: &FileContext<'_>, subject: &Subject<'_>) -> Vec { subject .sites diff --git a/src/units/security/settle.rs b/src/units/security/settle.rs new file mode 100644 index 0000000..73a5bfd --- /dev/null +++ b/src/units/security/settle.rs @@ -0,0 +1,287 @@ +//! The settle Choices: for each kind of check that can stay undecided after the +//! trace and recheck, the one question that settles it, and the units it is +//! asked for. +use super::*; + +/// A Choice asked when one of a rule's checks stays undecided after the +/// trace and recheck; it can only clear the checks it settles, so it is +/// asked apart from them. +pub(in crate::units) struct SettleKind { + pub rule: &'static str, + /// The question id of the Choice. + pub question: &'static str, + /// The checks that call for it while undecided, and that it settles. + pub checks: &'static [&'static str], + /// The options whose combined probability at the threshold clears them. + pub clears: &'static [&'static str], + /// Whether the functions that call the subject are sent with it. + callers: bool, + pub when: SettleWhen, + /// The files whose units it is planned for. + files: SettleFiles, +} + +/// The files, by language, whose units a settle Choice is planned for. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum SettleFiles { + All, + Only(&'static str), + Except(&'static str), +} + +impl SettleFiles { + fn include(self, language: &str) -> bool { + match self { + SettleFiles::All => true, + SettleFiles::Only(only) => language == only, + SettleFiles::Except(except) => language != except, + } + } +} + +/// Which units a settle Choice is asked for. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(in crate::units) enum SettleWhen { + /// An uncertain unit whose checks stay undecided. + Undecided, + /// Also a consider or note that rests on its undecided checks: a note + /// that a client component's fetch "places a parameter into a URL it + /// requests" only puzzled readers. + UndecidedOrFinding, + /// Any unit whose checks are not clear, and it clears a check that found + /// a concern too: a PHP page joins into HTML the body its included file + /// built, ids converted to numbers and database errors, and the markup + /// check found those at 0.9 while the origin question answered for the + /// request the page also reads. + NotClear, +} + +/// Every settle Choice. Where a URL comes from settles the URL check: on +/// clients of a fixed or configured service it split on a variable path or +/// query; so does code that runs only in the user's browser. Where a redirect leads, how markup is rendered and which origins +/// may send credentials settle theirs, which split on client components that +/// navigate to fixed paths or render values as attributes, and on route +/// handlers that answer preflights for any origin without credentials. What +/// its logs write settles its logging signals whenever they are not clear: +/// a logged object split on errors caught from a payment or database call, +/// and audit lines naming who signed in were logged personal data. Where a +/// function's text goes is asked whenever its error-detail signals are not +/// clear, too: a game client handing the server's error text to its own +/// window over a channel whose messages are named `Response` was fifteen +/// reviews for sending details to a remote client. Where a function's text goes settles error +/// details (see `exposure_signal`), also under a finding that claims the text +/// likely reaches a client. +/// +/// PHP pages ask what they join into HTML in place of how markup is +/// rendered, which names JSX and client components, and where the paths +/// they open or include come from: pages include their parts through a +/// directory constant and a file name a switch picks, and the path check +/// stayed near 0.25 on them. Both are asked whenever their check is not +/// clear: a page whose markup check leaned toward a concern was a note, so +/// its undecided path Choice was never asked, and once the markup Choice +/// cleared the markup it was left uncertain. What their command lines hold +/// settles the shell check the same way: a page that checks each octet of +/// an address with is_numeric was a command injection at 0.88. What code +/// does with tokens and how it handles passwords settle those checks +/// whenever they are not clear: front ends that send their own token and +/// HMAC signing split on them or were reviews. +pub(in crate::units) const SETTLES: [SettleKind; 14] = [ + SettleKind { + rule: INJECTION, + question: "url_parts", + checks: &["url"], + clears: &questions::OWN_PARTS, + callers: true, + when: SettleWhen::Undecided, + files: SettleFiles::All, + }, + SettleKind { + rule: INJECTION, + question: "runs_in", + checks: &["url"], + clears: &[questions::BROWSER], + callers: false, + when: SettleWhen::UndecidedOrFinding, + files: SettleFiles::All, + }, + SettleKind { + rule: INJECTION, + question: "redirect_target", + checks: &["redirect"], + clears: &questions::OWN_TARGETS, + callers: true, + when: SettleWhen::Undecided, + files: SettleFiles::All, + }, + SettleKind { + rule: INJECTION, + question: "markup_output", + checks: &["markup"], + clears: &questions::INERT_MARKUP, + callers: false, + when: SettleWhen::Undecided, + files: SettleFiles::Except(questions::PHP), + }, + SettleKind { + rule: INJECTION, + question: "markup_parts", + checks: &["markup"], + clears: &questions::HANDLED_MARKUP, + callers: true, + when: SettleWhen::NotClear, + files: SettleFiles::Only(questions::PHP), + }, + SettleKind { + rule: INJECTION, + question: "shell_parts", + checks: &["shell"], + clears: &questions::CHECKED_COMMANDS, + callers: false, + when: SettleWhen::NotClear, + files: SettleFiles::Only(questions::PHP), + }, + SettleKind { + rule: INJECTION, + question: "path_parts", + checks: &["path"], + clears: &questions::FIXED_PATHS, + callers: false, + when: SettleWhen::NotClear, + files: SettleFiles::Only(questions::PHP), + }, + SettleKind { + rule: INJECTION, + question: "path_source", + checks: &["path"], + clears: &questions::OWN_PATHS, + callers: true, + when: SettleWhen::Undecided, + files: SettleFiles::Except(questions::PHP), + }, + SettleKind { + rule: SENSITIVE_DATA, + question: "destination", + checks: &["error_details", "exception_to_client"], + clears: &questions::AWAY_FROM_CLIENTS, + callers: false, + when: SettleWhen::NotClear, + files: SettleFiles::All, + }, + SettleKind { + rule: SENSITIVE_DATA, + question: "logged", + checks: &["logs_object_secret", "logs_secret"], + clears: &questions::PLAIN_LOGS, + callers: false, + when: SettleWhen::NotClear, + files: SettleFiles::All, + }, + SettleKind { + rule: UNSAFE_SETTINGS, + question: "cors_origins", + checks: &["cors"], + clears: &questions::SAFE_ORIGINS, + callers: false, + when: SettleWhen::Undecided, + files: SettleFiles::All, + }, + SettleKind { + rule: UNSAFE_SETTINGS, + question: "cookie_flags", + checks: &["cookie"], + clears: &questions::FLAGGED_COOKIES, + callers: false, + when: SettleWhen::Undecided, + files: SettleFiles::All, + }, + SettleKind { + rule: UNSAFE_SETTINGS, + question: "token_use", + checks: &["token"], + clears: &questions::VERIFIED_TOKENS, + callers: false, + when: SettleWhen::NotClear, + files: SettleFiles::All, + }, + SettleKind { + rule: UNSAFE_SETTINGS, + question: "password_handling", + checks: &["hash"], + clears: &questions::HASHED_PASSWORDS, + callers: false, + when: SettleWhen::NotClear, + files: SettleFiles::All, + }, +]; + +/// The settle follow-ups of one unit, one per Choice of its rule, each sent +/// only when its checks stay undecided. +pub(super) fn settles( + file: &FileContext<'_>, + subject: &Subject<'_>, + rule: &'static str, + id: &str, +) -> Vec { + SETTLES + .iter() + .filter(|kind| kind.rule == rule && kind.files.include(file.language)) + .filter_map(|kind| { + let request = settle(file, subject, kind, id); + file.budget.fits(&request.0).then_some(Settle { + question: kind.question, + request: request.into(), + }) + }) + .collect() +} + +pub(super) fn settle( + file: &FileContext<'_>, + subject: &Subject<'_>, + kind: &SettleKind, + id: &str, +) -> (Value, Asked) { + let code = subject.code(); + let callers = kind.callers && !subject.callers.is_empty(); + let body = match kind.question { + "url_parts" => questions::security_url_parts(&code, callers), + "path_source" => questions::security_path_source(&code, callers), + "runs_in" => questions::security_runs_in(&code), + "redirect_target" => questions::security_redirect_target(&code, callers), + "markup_output" => { + questions::security_markup_output(&code, subject.django, subject.renders()) + } + "markup_parts" => questions::security_markup_parts(&code, callers), + "path_parts" => questions::security_path_parts(&code), + "shell_parts" => questions::security_shell_parts(&code), + "destination" => questions::security_destination(&code), + "logged" => questions::security_logged(&code), + "cookie_flags" => questions::security_cookie_flags(&code), + "token_use" => questions::security_token_use(&code), + "password_handling" => questions::security_password_handling(&code), + _ => questions::security_cors_origins(&code), + }; + let mut questions = Questions::default(); + questions.ask( + kind.question.into(), + body, + id, + kind.rule, + kind.question, + Pass::Settle, + ); + let mut state = json!({ + "file": file.file_state(), + subject.kind: subject.state(), + }); + if callers { + state["callers"] = json!( + subject + .callers + .iter() + .map(|(name, source)| json!({"name": name, "source": source})) + .collect::>() + ); + } + file.request("settle", state, questions) +} diff --git a/src/units/security/subject.rs b/src/units/security/subject.rs new file mode 100644 index 0000000..3547439 --- /dev/null +++ b/src/units/security/subject.rs @@ -0,0 +1,242 @@ +//! What a security unit is judged on: a function, a file's module-level setup or +//! its template code, with the constants, types and enums its code names. +use super::*; + +/// A function or setup statements that the security rules judge. +pub(in crate::units) struct Subject<'a> { + pub name: String, + /// `function` or `module`: the state key and the source path. + pub kind: &'static str, + pub source: String, + pub sites: &'a [Site], + /// Errors it creates, with their message arguments. + pub errors: &'a [CreatedError], + pub lines: (usize, usize), + /// Functions that call it, as (name, source), for the injection recheck. + pub callers: Vec<(String, String)>, + /// Enums its sites name, such as `ConfigKey` in `'${ConfigKey.aiTag}'`, + /// defined in this or another selected file: fixed choices, not + /// parameters, which the trace otherwise could not tell apart. + pub enums: Vec, + /// Definitions of the project's types its parameters name, shown when a + /// path finding is confirmed: how a route parameter of that type is + /// parsed decides what it can hold. + pub types: Vec, + /// C# constants it names, as `Class.Field = value`: a key written in + /// the code or a value read from configuration. + pub constants: Vec, + /// Framework facts shown beside the source in every request of its + /// units, such as the settings modules that import a settings module + /// and assign its settings again. + pub evidence: serde_json::Map, + /// Whether it is Django code, whose questions name Django's calls and + /// settings and ask its extra checks. + pub django: bool, + /// Errors the functions it calls create, with their messages, shown in + /// its sensitive-data trace: whether an error's text that it sends is + /// the program's own depends on where the error was raised. + pub callee_errors: Vec, + /// Whether its file sits at a test path, such as a test app's settings. + pub test_path: bool, +} + +/// The evidence key of the templates a function renders that write values +/// without escaping. +pub(in crate::units) const RENDERED: &str = + "templates_it_renders_that_write_values_without_escaping"; + +impl Subject<'_> { + pub(super) fn code(&self) -> String { + format!("{}.source", self.kind) + } + + /// Whether it renders a template that writes values without escaping, + /// outside Django, whose questions name its templates already. + pub(super) fn renders(&self) -> bool { + !self.django && self.evidence.contains_key(RENDERED) + } + + /// Its name, source and framework evidence, as sent. + pub(super) fn state(&self) -> Value { + let mut state = serde_json::Map::new(); + state.insert("name".into(), json!(self.name)); + state.insert("source".into(), json!(self.source)); + state.extend(self.evidence.clone()); + Value::Object(state) + } +} + +pub(in crate::units) fn function_subject<'a>( + file: &FileContext<'_>, + unit: &'a Unit, + callers: Vec<(String, String)>, + (enums, types): (&BTreeMap, &BTreeMap), + constants: &BTreeMap>, +) -> Subject<'a> { + let source = unit.source(file.source).to_string(); + Subject { + name: unit.name.clone(), + kind: "function", + constants: named_constants(file, &source, constants), + source, + sites: &unit.sites, + errors: &unit.errors, + lines: (unit.line, unit.end_line), + callers, + enums: named_enums(&unit.sites, enums), + types: named_types(&unit.signature, types), + evidence: serde_json::Map::new(), + django: false, + callee_errors: Vec::new(), + test_path: false, + } +} + +/// Constant declarations shown with one subject, at most. +pub(super) const CONSTANTS: usize = 4; + +/// The declarations of the C# constants a C# subject names. +pub(super) fn named_constants( + file: &FileContext<'_>, + source: &str, + constants: &BTreeMap>, +) -> Vec { + if file.language != questions::CSHARP { + return Vec::new(); + } + let mut found = Vec::new(); + for (field, declarations) in constants { + let named = source.match_indices(field.as_str()).any(|(at, _)| { + let before = source[..at].chars().next_back(); + let after = source[at + field.len()..].chars().next(); + let word = |c: Option| c.is_some_and(|c| c.is_alphanumeric() || c == '_'); + !word(before) && !word(after) + }); + if named { + found.extend(declarations.iter().cloned()); + } + } + found.truncate(CONSTANTS); + found +} + +/// Type definitions shown with one subject, at most. +pub(super) const TYPES: usize = 3; + +/// The definitions of the project's types named as words in a signature. +pub(super) fn named_types(signature: &str, types: &BTreeMap) -> Vec { + let word = |c: Option| c.is_some_and(|c| c.is_alphanumeric() || c == '_'); + types + .iter() + .filter(|(name, _)| { + signature.match_indices(name.as_str()).any(|(at, _)| { + !word(signature[..at].chars().next_back()) + && !word(signature[at + name.len()..].chars().next()) + }) + }) + .map(|(_, definition)| definition.clone()) + .take(TYPES) + .collect() +} + +/// Enum definitions shown with one subject, at most. +pub(super) const ENUMS: usize = 3; + +/// The definitions of enums named as `Name.member` or `Name::member` in the sites. +pub(super) fn named_enums(sites: &[Site], enums: &BTreeMap) -> Vec { + let mut found: Vec = Vec::new(); + for site in sites { + for (name, definition) in enums { + let named = site.text.match_indices(name.as_str()).any(|(at, _)| { + let before = site.text[..at].chars().next_back(); + let after = &site.text[at + name.len()..]; + before.is_none_or(|c| !(c.is_alphanumeric() || c == '_')) + && (after.starts_with('.') || after.starts_with("::")) + }); + if named && found.len() < ENUMS && !found.contains(definition) { + found.push(definition.clone()); + } + } + } + found +} + +pub(in crate::units) fn setup_subject<'a>( + file: &FileContext<'_>, + setup: &'a crate::analysis::sites::Setup, + constants: &BTreeMap>, +) -> Option> { + let first = setup.statements.first()?; + let last = setup.statements.last()?; + let source: Vec = setup + .statements + .iter() + .map(|(range, ..)| setup.text(file.source, range.clone())) + .collect(); + let source = source.join("\n"); + let (name, kind) = if setup.script { + (SCRIPT, "function") + } else if setup.settings { + (SETTINGS_MODULE, "module") + } else { + (MODULE_SETUP, "module") + }; + Some(Subject { + name: name.into(), + kind, + constants: named_constants(file, &source, constants), + source, + sites: &setup.sites, + errors: &[], + lines: (first.1, last.2), + callers: Vec::new(), + enums: Vec::new(), + types: Vec::new(), + evidence: serde_json::Map::new(), + django: false, + callee_errors: Vec::new(), + test_path: false, + }) +} + +/// A server template's code that reads client data, judged like a function +/// by every security rule. +pub(in crate::units) fn template_subject<'a>( + file: &FileContext<'_>, + code: &'a crate::analysis::sites::Setup, +) -> Option> { + let first = code.statements.first()?; + let last = code.statements.last()?; + let source: Vec<&str> = code + .statements + .iter() + .map(|(range, ..)| &file.source[range.clone()]) + .collect(); + Some(Subject { + name: TEMPLATE_CODE.into(), + kind: "function", + source: source.join("\n"), + sites: &code.sites, + errors: &[], + lines: (first.1, last.2), + callers: Vec::new(), + enums: Vec::new(), + types: Vec::new(), + constants: Vec::new(), + evidence: serde_json::Map::new(), + django: false, + callee_errors: Vec::new(), + test_path: false, + }) +} + +/// The name of the unit that holds a server template's code. +pub(in crate::units) const TEMPLATE_CODE: &str = "template code"; + +/// The name of the unit that holds a file's top-level setup statements. +pub(in crate::units) const MODULE_SETUP: &str = "module setup"; +/// The name of that unit in a Django settings module, whose statements +/// assign the deployed site's settings. +pub(in crate::units) const SETTINGS_MODULE: &str = "settings module"; +/// The name of the unit that holds a PHP file's top-level statements. +pub(in crate::units) const SCRIPT: &str = "top-level code"; From 8dbcc7c10b6c8891739f744192a1e0ad45358621 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sun, 27 Sep 2026 19:16:26 -0300 Subject: [PATCH 3/5] Split questions/security.rs into the first-pass questions, the confirm Choices and the Django checks --- src/units/questions/security/confirm.rs | 176 ++++++++++ src/units/questions/security/django.rs | 149 ++++++++ .../{security.rs => security/mod.rs} | 327 +----------------- 3 files changed, 333 insertions(+), 319 deletions(-) create mode 100644 src/units/questions/security/confirm.rs create mode 100644 src/units/questions/security/django.rs rename src/units/questions/{security.rs => security/mod.rs} (63%) diff --git a/src/units/questions/security/confirm.rs b/src/units/questions/security/confirm.rs new file mode 100644 index 0000000..03fb336 --- /dev/null +++ b/src/units/questions/security/confirm.rs @@ -0,0 +1,176 @@ +//! The Choices a security finding is asked after it is raised: what the values an +//! injection places can hold, what its paths can hold, what its markup holds +//! and where its redirects lead, and when a log line runs. +use super::*; + +/// What the values of an injection consider resting on the function's +/// parameters can hold, asked only for such a finding, with its callers. +/// Labeled by hand, those considers were right 30 times in 81: most wrong +/// ones placed text every caller passes as a literal, such as a Rust +/// helper's SQL fragments, or a command-line tool's own arguments, while +/// right ones placed names from a database others write, fetched page +/// titles or model output. Asked where the values come from, the recheck +/// answered "the function's parameters" at 0.9 even when its callers passed +/// literals. +pub fn injection_values(code: &str, callers: bool) -> Value { + let (fixed, note) = if callers { + ( + "Only text the program fixes: literals and constants, written in this code or passed by every caller in `callers`; numbers, dates or other typed values that cannot hold markup or syntax; or names chosen from a fixed list.", + format!("{CALLERS} {EVIDENCE}"), + ) + } else { + ( + "Only text the program fixes: literals and constants written in this code; numbers, dates or other typed values that cannot hold markup or syntax; or names chosen from a fixed list.", + EVIDENCE.to_string(), + ) + }; + json!({ + "type": "choice", + "instructions": { + "question": format!("What can the values that `{code}` places into a query, command, code or markup without binding or escaping them hold?"), + "note": note, + }, + "criteria": { + "fixed": fixed, + "own": "Values the program creates or keeps for itself, such as ids it generates, the names of its own tables, files or settings, or text it wrote itself.", + "local": "The arguments of a command-line program, build script or code generator, typed by the person who runs it on their own machine, or text that person runs on purpose, such as a query they typed.", + "outside": "Text another party can set: a network request, message or uploaded file, a page or feed fetched from the network, a language model's output, or records and names other users can write, such as rows of a shared database.", + "unknown": "Values from parameters or calls whose origin is not shown, which may hold any of these.", + }, + }) +} + +/// The options of `injection_values` that hold only the program's own values. +pub const PROGRAM_VALUES: [&str; 3] = ["fixed", "own", "local"]; + +/// What the variable parts of a path finding's paths can hold, asked only +/// for an injection finding whose check found a path, with its callers and +/// the definitions of the project's types its parameters name. On +/// vaultwarden, Rocket route parameters typed `PathBuf` (which Rocket parses +/// so they cannot climb above where they are joined) and id types whose +/// parsing accepts only a UUID were four wrong path reviews: the path check +/// reads a variable joined to a directory, whatever the variable can hold. +pub fn injection_paths(code: &str, callers: bool, types: bool) -> Value { + let types_note = if types { + " `types_named_in_parameters` holds the definitions of the project's types that its parameters name, with their attributes." + } else { + "" + }; + let lead = if callers { + format!("{CALLERS}{types_note}") + } else { + types_note.trim_start().to_string() + }; + let note = if lead.is_empty() { + EVIDENCE.to_string() + } else { + format!("{lead} {EVIDENCE}") + }; + json!({ + "type": "choice", + "instructions": { + "question": format!("What can the variable parts of the file paths that `{code}` opens, writes or deletes hold?"), + "note": note, + }, + "criteria": { + "confined": "Only names that cannot leave the directory they are joined to: numbers, UUIDs or ids that a type or the web framework parses before the function runs, names reduced to a base name or checked against a pattern, or a path parameter the framework parses so it cannot climb above where it is joined, such as a Rocket `PathBuf` route segment, which rejects hidden and encoded-slash segments and drops `..` at its start.", + "own": "Names the program chooses or keeps for itself, or reads from its configuration.", + "local": "The command line, settings or files of the person running a local program or script.", + "outside": "A name or path another party sets that can hold `..`, a slash or an absolute path, such as a request parameter or field read as text, an uploaded file's name or an archive entry.", + "unknown": "Values from parameters or calls whose origin is not shown, which may hold any of these.", + }, + }) +} + +/// The options of `injection_paths` that keep a path inside its directory. +pub const CONFINED_PATHS: [&str; 3] = ["confined", "own", "local"]; + +/// What a markup finding's values hold where they enter the markup, asked +/// only after a finding whose one concern is markup. vaultwarden's +/// `hibp_breach` percent-encodes the username before it builds the link, +/// oak's examples write a URL object whose serialization percent-encodes +/// `<` and `>`, and a JSP page runs its own `esc()` first: the markup check +/// reads a variable joined into HTML, whatever it was turned into before. +pub fn markup_values(code: &str, callers: bool) -> Value { + let (by_callers, note) = if callers { + ( + " or by the functions in `callers`", + format!("{CALLERS} {EVIDENCE}"), + ) + } else { + ("", EVIDENCE.to_string()) + }; + json!({ + "type": "choice", + "instructions": { + "question": format!("What do the values that `{code}` places into HTML or SVG markup hold where they enter it?"), + "note": note, + }, + "criteria": { + "encoded": format!("Text already escaped for HTML, percent-encoded or serialized as a URL before it enters the markup, in this code{by_callers}, so it cannot hold `<`, `>`, `&` or quotes."), + "typed": "Numbers, dates, booleans or ids, or names chosen from a fixed list.", + "own": "Text the program writes itself or reads from its configuration.", + "raw": "Text as another party or a caller wrote it, which can hold `<`, `>`, `&` or quotes.", + "unknown": "Values whose origin or handling is not shown.", + }, + }) +} + +/// The options of `markup_values` that cannot open a tag or attribute. +pub const HARMLESS_MARKUP: [&str; 3] = ["encoded", "typed", "own"]; + +/// Where a redirect finding's targets can lead, asked only after a finding +/// whose one concern is a redirect. vaultwarden's admin login redirects to +/// its admin path followed by the form's value, and shiori's to its login +/// page with the current path as a query value: a fixed path before the +/// variable keeps the target on the site, which the redirect check does not +/// ask. Offered "its own origin and a slash" without the form written out, +/// chatbot-ui's `requestUrl.origin + next` read as staying on the site at +/// 0.63, though `next=@evil.com` leaves it. +pub fn redirect_reach(code: &str, callers: bool) -> Value { + let note = if callers { + format!("{CALLERS} {EVIDENCE}") + } else { + EVIDENCE.to_string() + }; + json!({ + "type": "choice", + "instructions": { + "question": format!("Where can the targets that `{code}` redirects clients to lead?"), + "note": note, + }, + "criteria": { + "own_site": "Only to the program's own site: every target starts with a fixed path written in the code, such as `/admin` or `/login?next=`, so it begins with one slash and a path; or with the program's own origin followed by a slash written in the code; variables only follow that fixed part or fill its query string.", + "checked": "Only where a check allows: the target is compared with an allowed list of hosts or checked to be a path on the site before the redirect.", + "anywhere": "Anywhere a variable says: a variable starts the target, or directly follows the program's own origin or a host with no slash written between them, as in `origin + next`, where `@evil.com` or `.evil.com` in the variable names another host.", + "none": "It does not redirect.", + }, + }) +} + +/// The options of `redirect_reach` that keep a redirect on the site. +pub const OWN_SITE: [&str; 3] = ["own_site", "checked", "none"]; + +/// When a logging finding's log line runs, asked only for a sensitive-data +/// finding raised by its log checks. vaultwarden logs SSO tokens inside +/// `if CONFIG.sso_debug_tokens()`, a setting off by default and documented +/// for logging them while troubleshooting: logging an identifier instead, +/// as the finding says, would remove the feature. +pub fn logged_when(code: &str) -> Value { + json!({ + "type": "choice", + "instructions": { + "question": format!("When does `{code}` write the secret or personal value to a log?"), + "note": EVIDENCE, + }, + "criteria": { + "always": "Whenever that code runs, at a level the program logs at in normal operation, such as info, warning or error.", + "debug": "Only at debug or trace level, which an operator may turn on to troubleshoot.", + "opt_in": "Only when an operator turns on a setting, off by default, whose purpose is to log these values for troubleshooting, such as an option named for logging tokens or request bodies.", + "none": "It writes no secret or personal value to a log.", + }, + }) +} + +/// The option of `logged_when` for a setting whose purpose is the logging. +pub const OPT_IN_LOGGING: &str = "opt_in"; diff --git a/src/units/questions/security/django.rs b/src/units/questions/security/django.rs new file mode 100644 index 0000000..61d2365 --- /dev/null +++ b/src/units/questions/security/django.rs @@ -0,0 +1,149 @@ +//! The checks Django code is asked in its own words: its ORM, templates and +//! settings, and the decorators and middleware that decide them. +use super::*; + +pub(super) const DJANGO_PATH: Check = Check { + id: "path", + question: "Does `{code}` open, write or delete a file at a path built from a variable without checking that it stays inside a directory?", + yes: "A path is built from a variable that can hold a name or path from outside the program, such as a request, upload, archive entry or user input, and is used without reducing it to a base name, rejecting parent-directory parts, or checking that the resolved path stays under a base directory.", + no: "Such paths are checked; are built from the program's own directories, such as its project root, data or cache directory, joined with names the program chooses; come from the program's configuration or the command line of the person running it; or it uses no such path.", + no_examples: &[ + "A file saved through Django's storage API, such as a file field's save or default_storage.save, which keeps names inside the storage's root", + "Files listed from one of the program's own directories, such as its fixtures", + ], +}; + +pub(super) const DJANGO_SQL: Check = Check { + id: "sql", + question: "Does `{code}` put a variable into the text of an SQL query instead of passing it as a bound parameter?", + yes: "A variable is joined, formatted or interpolated into SQL text that is then run, such as with %, + or an f-string passed to execute, raw, extra or RawSQL.", + no: "Values are passed as bound parameters, placeholders or the params argument, or through the ORM's filters; identifiers such as table and column names come from a fixed list or the database schema, or are quoted by a function that wraps them in double quotes and doubles any double quote inside; or it runs no SQL.", + no_examples: &[], +}; + +pub(super) const DJANGO_MARKUP: Check = Check { + id: "markup", + question: "Does `{code}` put a variable into HTML or SVG markup without escaping it, itself or through a template it renders?", + yes: "A variable is joined into HTML or SVG text, marked as safe markup with mark_safe or SafeString, or passed to a template that writes it with a safe filter or with autoescaping off, without an escaping function.", + no: "Values go through an escaping function or a template that escapes them, or it builds no markup.", + no_examples: &[ + "A template rendered with the variable in its context, when the template writes that value without a safe filter, which Django escapes", + "format_html or format_html_join with the variables passed as its arguments, which escapes them", + ], +}; + +pub(super) const DJANGO_HASH: Check = Check { + id: "hash", + question: "Does `{code}` hash passwords or derive keys from them with a fast or broken hash, or with few iterations?", + yes: "It hashes passwords or derives keys from them with MD5, SHA-1, a single round of SHA-256, or a key derivation function with few iterations, or lists such a hasher first in PASSWORD_HASHERS.", + no: "It uses bcrypt, scrypt, Argon2 or a key derivation function with many iterations; it hashes through Django's set_password, make_password or a form's save, whose hasher the settings choose; or it does not handle passwords.", + no_examples: &[], +}; + +pub(super) const DJANGO_TLS: Check = Check { + id: "tls", + question: "Does `{code}` turn off certificate, host name or signature verification?", + yes: "It turns off certificate or host name checks, accepts invalid certificates or host names, or decodes a signed token such as a JWT without verifying its signature.", + no: "It keeps verification on, or makes no TLS connection and reads no signed token.", + no_examples: &[], +}; + +pub(super) const DJANGO_CORS: Check = Check { + id: "cors", + question: "Does `{code}` set cross-origin rules that let pages from origins it does not fully check read the deployed site's responses with credentials?", + yes: "It allows any origin, reflects the request's origin, or matches origins loosely, such as by suffix or substring, while allowing credentials, in code or settings that the deployed site uses.", + no: concat!( + "It allows only listed origins by exact match or allows no credentials; it sets no cross-origin rules itself, ", + "whatever the site's settings choose; or a settings module for production that imports these settings sets it again." + ), + no_examples: &[], +}; + +pub(super) const DJANGO_COOKIE: Check = Check { + id: "cookie", + question: "Does `{code}` set or configure a session or authentication cookie of the deployed site without the Secure or HttpOnly flag?", + yes: "A cookie that holds a session or token is set or configured without Secure or without HttpOnly, in code or settings that the deployed site uses.", + no: concat!( + "Such cookies have both flags, the cookie holds no session or token, or the code sets no cookie; ", + "or a settings module for production that imports these settings sets it again." + ), + no_examples: &[], +}; + +pub(super) const DEBUG: Check = Check { + id: "debug", + question: "Does `{code}` turn on a web framework's debug mode or detailed error pages for the deployed site?", + yes: "It turns debug mode on, such as DEBUG = True, in code or settings that the deployed site uses.", + no: "Debug mode is off, is read from the environment with off as the default, is turned on only in settings for tests or local development, or a settings module for production that imports these settings sets it again.", + no_examples: &[], +}; + +pub(super) const CSRF: Check = Check { + id: "csrf", + question: "Does `{code}` turn off protection against cross-site request forgery for requests that change data?", + yes: "A view or route that changes data as the user its session cookie signs in, such as their profile, password or records, is exempted from the CSRF check, such as with csrf_exempt, or the CSRF middleware or check is removed.", + no: "CSRF protection stays on; the exempted endpoint authenticates each request itself rather than with the session cookie, such as a webhook that verifies a signature, an API that reads a token from a header, or a form for visitors who are not signed in that asks for a password reset email or checks a reset token it is sent; it only reads data; or it sets nothing about CSRF.", + no_examples: &[], +}; + +pub(super) const SECRET: Check = Check { + id: "literal_secret", + question: "Does `{code}` set a signing key, password or token that the deployed site uses to a literal written in the code?", + yes: "A secret key, password, token or API key that the deployed program uses is a literal in the code, including one shown as a redacted literal.", + no: "Secrets are read from the environment, a file or a secret store; the literal is empty or only a placeholder; it is used only in tests or local development; or a settings module for production that imports these settings sets it again.", + no_examples: &[], +}; + +pub(super) const DJANGO_EXCEPTION_TO_CLIENT: Check = Check { + id: "exception_to_client", + question: "Does `{code}` send an exception's message, stack trace or a database error to a remote client in a response?", + yes: "The text of an exception it did not raise itself to explain bad input, or a stack trace, is put into the response to a request.", + no: "Responses carry fixed messages or codes, or only messages written to explain invalid input, such as those of Django's or a form's ValidationError; exceptions it does not catch go to the framework's error handling; details stay in server logs.", + no_examples: &[], +}; + +pub(super) const ENVIRONMENT_TO_CLIENT: Check = Check { + id: "environment_to_client", + question: "Does `{code}` send the server's environment variables, settings or whole request metadata to a remote client?", + yes: "It puts the process environment, the application's settings, or a whole request metadata object such as Django's request.META, which holds the server's environment, into a response or a page it renders.", + no: "It sends only chosen fields meant for the client, such as the user's own name or a public setting, or sends no such data.", + no_examples: &[], +}; + +pub(super) const DJANGO_REDIRECT: Check = Check { + id: "redirect", + question: "Does `{code}` redirect the client to a URL or path taken from a variable without checking where it leads?", + yes: "A URL or path that a request carries, such as a query parameter, form field, header or cookie, is passed to redirect(), HttpResponseRedirect or a Location header without checking that it is a path on the program's own site or that its host is on an allowed list.", + no: "The target is fixed, is a route name or built with reverse(), is one of the program's own paths with only ids or names from variables in it, is checked such as with url_has_allowed_host_and_scheme, comes from the program's configuration, or is read from a stored record rather than the request; or it does not redirect.", + no_examples: &[], +}; + +/// Checks asked of Django code in place of the common check with the same +/// id: they name Django's raw queries (`raw`, `extra`, `RawSQL`), safe +/// markup and templates, storage API, redirects, password hashers and +/// validation errors, and ask cookie, CORS and verification settings about +/// the deployed site, since settings modules that production imports and +/// overrides were flagged when asked about the module alone. +pub const DJANGO_VARIANTS: [Check; 9] = [ + DJANGO_SQL, + DJANGO_MARKUP, + DJANGO_PATH, + DJANGO_REDIRECT, + DJANGO_TLS, + DJANGO_HASH, + DJANGO_CORS, + DJANGO_COOKIE, + DJANGO_EXCEPTION_TO_CLIENT, +]; + +/// The injection check Django code adds: request data given to a +/// deserializer that can build any object (`pickle.loads(request.body)`). +pub const DJANGO_UNHANDLED: [Check; 1] = [DESERIALIZE]; + +/// The weak settings Django code adds, which its settings modules and view +/// decorators decide: debug mode, CSRF protection and a literal secret key. +pub const DJANGO_SETTINGS: [Check; 3] = [DEBUG, CSRF, SECRET]; + +/// The exposure Django code adds: `request.META` or the settings, which +/// hold the server's environment, sent to a client. +pub const DJANGO_EXPOSURES: [Check; 1] = [ENVIRONMENT_TO_CLIENT]; diff --git a/src/units/questions/security.rs b/src/units/questions/security/mod.rs similarity index 63% rename from src/units/questions/security.rs rename to src/units/questions/security/mod.rs index 125cd41..43409a2 100644 --- a/src/units/questions/security.rs +++ b/src/units/questions/security/mod.rs @@ -1,9 +1,16 @@ //! Questions about security in application code: whether a function places //! variables into interpreted text, logs secrets or weakens a setting, and the -//! literal checks per kind that the trace and caller rechecks ask. +//! literal checks per kind that the trace and caller rechecks ask. `confirm` +//! holds the Choices a finding is asked after it is raised, and `django` the +//! checks Django code is asked in its own words. use super::{EVIDENCE, choose_id, deserializers::DESERIALIZE, noul, score}; use serde_json::{Value, json}; +mod confirm; +mod django; +pub use confirm::*; +pub use django::*; + /// Whether a function places a variable into text another program runs or /// renders. Presence only: the trace questions decide whether it is a concern. /// In Django code it also names markup marked safe and deserializers, since @@ -185,178 +192,6 @@ pub fn security_origin(code: &str, callers: bool, django: bool) -> Value { ) } -/// What the values of an injection consider resting on the function's -/// parameters can hold, asked only for such a finding, with its callers. -/// Labeled by hand, those considers were right 30 times in 81: most wrong -/// ones placed text every caller passes as a literal, such as a Rust -/// helper's SQL fragments, or a command-line tool's own arguments, while -/// right ones placed names from a database others write, fetched page -/// titles or model output. Asked where the values come from, the recheck -/// answered "the function's parameters" at 0.9 even when its callers passed -/// literals. -pub fn injection_values(code: &str, callers: bool) -> Value { - let (fixed, note) = if callers { - ( - "Only text the program fixes: literals and constants, written in this code or passed by every caller in `callers`; numbers, dates or other typed values that cannot hold markup or syntax; or names chosen from a fixed list.", - format!("{CALLERS} {EVIDENCE}"), - ) - } else { - ( - "Only text the program fixes: literals and constants written in this code; numbers, dates or other typed values that cannot hold markup or syntax; or names chosen from a fixed list.", - EVIDENCE.to_string(), - ) - }; - json!({ - "type": "choice", - "instructions": { - "question": format!("What can the values that `{code}` places into a query, command, code or markup without binding or escaping them hold?"), - "note": note, - }, - "criteria": { - "fixed": fixed, - "own": "Values the program creates or keeps for itself, such as ids it generates, the names of its own tables, files or settings, or text it wrote itself.", - "local": "The arguments of a command-line program, build script or code generator, typed by the person who runs it on their own machine, or text that person runs on purpose, such as a query they typed.", - "outside": "Text another party can set: a network request, message or uploaded file, a page or feed fetched from the network, a language model's output, or records and names other users can write, such as rows of a shared database.", - "unknown": "Values from parameters or calls whose origin is not shown, which may hold any of these.", - }, - }) -} - -/// The options of `injection_values` that hold only the program's own values. -pub const PROGRAM_VALUES: [&str; 3] = ["fixed", "own", "local"]; - -/// What the variable parts of a path finding's paths can hold, asked only -/// for an injection finding whose check found a path, with its callers and -/// the definitions of the project's types its parameters name. On -/// vaultwarden, Rocket route parameters typed `PathBuf` (which Rocket parses -/// so they cannot climb above where they are joined) and id types whose -/// parsing accepts only a UUID were four wrong path reviews: the path check -/// reads a variable joined to a directory, whatever the variable can hold. -pub fn injection_paths(code: &str, callers: bool, types: bool) -> Value { - let types_note = if types { - " `types_named_in_parameters` holds the definitions of the project's types that its parameters name, with their attributes." - } else { - "" - }; - let lead = if callers { - format!("{CALLERS}{types_note}") - } else { - types_note.trim_start().to_string() - }; - let note = if lead.is_empty() { - EVIDENCE.to_string() - } else { - format!("{lead} {EVIDENCE}") - }; - json!({ - "type": "choice", - "instructions": { - "question": format!("What can the variable parts of the file paths that `{code}` opens, writes or deletes hold?"), - "note": note, - }, - "criteria": { - "confined": "Only names that cannot leave the directory they are joined to: numbers, UUIDs or ids that a type or the web framework parses before the function runs, names reduced to a base name or checked against a pattern, or a path parameter the framework parses so it cannot climb above where it is joined, such as a Rocket `PathBuf` route segment, which rejects hidden and encoded-slash segments and drops `..` at its start.", - "own": "Names the program chooses or keeps for itself, or reads from its configuration.", - "local": "The command line, settings or files of the person running a local program or script.", - "outside": "A name or path another party sets that can hold `..`, a slash or an absolute path, such as a request parameter or field read as text, an uploaded file's name or an archive entry.", - "unknown": "Values from parameters or calls whose origin is not shown, which may hold any of these.", - }, - }) -} - -/// The options of `injection_paths` that keep a path inside its directory. -pub const CONFINED_PATHS: [&str; 3] = ["confined", "own", "local"]; - -/// What a markup finding's values hold where they enter the markup, asked -/// only after a finding whose one concern is markup. vaultwarden's -/// `hibp_breach` percent-encodes the username before it builds the link, -/// oak's examples write a URL object whose serialization percent-encodes -/// `<` and `>`, and a JSP page runs its own `esc()` first: the markup check -/// reads a variable joined into HTML, whatever it was turned into before. -pub fn markup_values(code: &str, callers: bool) -> Value { - let (by_callers, note) = if callers { - ( - " or by the functions in `callers`", - format!("{CALLERS} {EVIDENCE}"), - ) - } else { - ("", EVIDENCE.to_string()) - }; - json!({ - "type": "choice", - "instructions": { - "question": format!("What do the values that `{code}` places into HTML or SVG markup hold where they enter it?"), - "note": note, - }, - "criteria": { - "encoded": format!("Text already escaped for HTML, percent-encoded or serialized as a URL before it enters the markup, in this code{by_callers}, so it cannot hold `<`, `>`, `&` or quotes."), - "typed": "Numbers, dates, booleans or ids, or names chosen from a fixed list.", - "own": "Text the program writes itself or reads from its configuration.", - "raw": "Text as another party or a caller wrote it, which can hold `<`, `>`, `&` or quotes.", - "unknown": "Values whose origin or handling is not shown.", - }, - }) -} - -/// The options of `markup_values` that cannot open a tag or attribute. -pub const HARMLESS_MARKUP: [&str; 3] = ["encoded", "typed", "own"]; - -/// Where a redirect finding's targets can lead, asked only after a finding -/// whose one concern is a redirect. vaultwarden's admin login redirects to -/// its admin path followed by the form's value, and shiori's to its login -/// page with the current path as a query value: a fixed path before the -/// variable keeps the target on the site, which the redirect check does not -/// ask. Offered "its own origin and a slash" without the form written out, -/// chatbot-ui's `requestUrl.origin + next` read as staying on the site at -/// 0.63, though `next=@evil.com` leaves it. -pub fn redirect_reach(code: &str, callers: bool) -> Value { - let note = if callers { - format!("{CALLERS} {EVIDENCE}") - } else { - EVIDENCE.to_string() - }; - json!({ - "type": "choice", - "instructions": { - "question": format!("Where can the targets that `{code}` redirects clients to lead?"), - "note": note, - }, - "criteria": { - "own_site": "Only to the program's own site: every target starts with a fixed path written in the code, such as `/admin` or `/login?next=`, so it begins with one slash and a path; or with the program's own origin followed by a slash written in the code; variables only follow that fixed part or fill its query string.", - "checked": "Only where a check allows: the target is compared with an allowed list of hosts or checked to be a path on the site before the redirect.", - "anywhere": "Anywhere a variable says: a variable starts the target, or directly follows the program's own origin or a host with no slash written between them, as in `origin + next`, where `@evil.com` or `.evil.com` in the variable names another host.", - "none": "It does not redirect.", - }, - }) -} - -/// The options of `redirect_reach` that keep a redirect on the site. -pub const OWN_SITE: [&str; 3] = ["own_site", "checked", "none"]; - -/// When a logging finding's log line runs, asked only for a sensitive-data -/// finding raised by its log checks. vaultwarden logs SSO tokens inside -/// `if CONFIG.sso_debug_tokens()`, a setting off by default and documented -/// for logging them while troubleshooting: logging an identifier instead, -/// as the finding says, would remove the feature. -pub fn logged_when(code: &str) -> Value { - json!({ - "type": "choice", - "instructions": { - "question": format!("When does `{code}` write the secret or personal value to a log?"), - "note": EVIDENCE, - }, - "criteria": { - "always": "Whenever that code runs, at a level the program logs at in normal operation, such as info, warning or error.", - "debug": "Only at debug or trace level, which an operator may turn on to troubleshoot.", - "opt_in": "Only when an operator turns on a setting, off by default, whose purpose is to log these values for troubleshooting, such as an option named for logging tokens or request bodies.", - "none": "It writes no secret or personal value to a log.", - }, - }) -} - -/// The option of `logged_when` for a setting whose purpose is the logging. -pub const OPT_IN_LOGGING: &str = "opt_in"; - /// Asked in the sensitive-data trace: whether every error message is the /// program's own. It can only clear the error-detail signals; functions that /// throw the program's typed errors otherwise stayed undecided, since the @@ -563,36 +398,6 @@ pub const UNHANDLED: [Check; 7] = [ }, ]; -const DJANGO_PATH: Check = Check { - id: "path", - question: "Does `{code}` open, write or delete a file at a path built from a variable without checking that it stays inside a directory?", - yes: "A path is built from a variable that can hold a name or path from outside the program, such as a request, upload, archive entry or user input, and is used without reducing it to a base name, rejecting parent-directory parts, or checking that the resolved path stays under a base directory.", - no: "Such paths are checked; are built from the program's own directories, such as its project root, data or cache directory, joined with names the program chooses; come from the program's configuration or the command line of the person running it; or it uses no such path.", - no_examples: &[ - "A file saved through Django's storage API, such as a file field's save or default_storage.save, which keeps names inside the storage's root", - "Files listed from one of the program's own directories, such as its fixtures", - ], -}; - -const DJANGO_SQL: Check = Check { - id: "sql", - question: "Does `{code}` put a variable into the text of an SQL query instead of passing it as a bound parameter?", - yes: "A variable is joined, formatted or interpolated into SQL text that is then run, such as with %, + or an f-string passed to execute, raw, extra or RawSQL.", - no: "Values are passed as bound parameters, placeholders or the params argument, or through the ORM's filters; identifiers such as table and column names come from a fixed list or the database schema, or are quoted by a function that wraps them in double quotes and doubles any double quote inside; or it runs no SQL.", - no_examples: &[], -}; - -const DJANGO_MARKUP: Check = Check { - id: "markup", - question: "Does `{code}` put a variable into HTML or SVG markup without escaping it, itself or through a template it renders?", - yes: "A variable is joined into HTML or SVG text, marked as safe markup with mark_safe or SafeString, or passed to a template that writes it with a safe filter or with autoescaping off, without an escaping function.", - no: "Values go through an escaping function or a template that escapes them, or it builds no markup.", - no_examples: &[ - "A template rendered with the variable in its context, when the template writes that value without a safe filter, which Django escapes", - "format_html or format_html_join with the variables passed as its arguments, which escapes them", - ], -}; - /// The markup check of a function outside Django that renders a template /// writing values without escaping: DVNA's product search hands the /// request's search term to `views/app/products.ejs`, which writes it with @@ -704,68 +509,6 @@ const TOKEN: Check = Check { no_examples: &[], }; -const DJANGO_HASH: Check = Check { - id: "hash", - question: "Does `{code}` hash passwords or derive keys from them with a fast or broken hash, or with few iterations?", - yes: "It hashes passwords or derives keys from them with MD5, SHA-1, a single round of SHA-256, or a key derivation function with few iterations, or lists such a hasher first in PASSWORD_HASHERS.", - no: "It uses bcrypt, scrypt, Argon2 or a key derivation function with many iterations; it hashes through Django's set_password, make_password or a form's save, whose hasher the settings choose; or it does not handle passwords.", - no_examples: &[], -}; - -const DJANGO_TLS: Check = Check { - id: "tls", - question: "Does `{code}` turn off certificate, host name or signature verification?", - yes: "It turns off certificate or host name checks, accepts invalid certificates or host names, or decodes a signed token such as a JWT without verifying its signature.", - no: "It keeps verification on, or makes no TLS connection and reads no signed token.", - no_examples: &[], -}; - -const DJANGO_CORS: Check = Check { - id: "cors", - question: "Does `{code}` set cross-origin rules that let pages from origins it does not fully check read the deployed site's responses with credentials?", - yes: "It allows any origin, reflects the request's origin, or matches origins loosely, such as by suffix or substring, while allowing credentials, in code or settings that the deployed site uses.", - no: concat!( - "It allows only listed origins by exact match or allows no credentials; it sets no cross-origin rules itself, ", - "whatever the site's settings choose; or a settings module for production that imports these settings sets it again." - ), - no_examples: &[], -}; - -const DJANGO_COOKIE: Check = Check { - id: "cookie", - question: "Does `{code}` set or configure a session or authentication cookie of the deployed site without the Secure or HttpOnly flag?", - yes: "A cookie that holds a session or token is set or configured without Secure or without HttpOnly, in code or settings that the deployed site uses.", - no: concat!( - "Such cookies have both flags, the cookie holds no session or token, or the code sets no cookie; ", - "or a settings module for production that imports these settings sets it again." - ), - no_examples: &[], -}; - -const DEBUG: Check = Check { - id: "debug", - question: "Does `{code}` turn on a web framework's debug mode or detailed error pages for the deployed site?", - yes: "It turns debug mode on, such as DEBUG = True, in code or settings that the deployed site uses.", - no: "Debug mode is off, is read from the environment with off as the default, is turned on only in settings for tests or local development, or a settings module for production that imports these settings sets it again.", - no_examples: &[], -}; - -const CSRF: Check = Check { - id: "csrf", - question: "Does `{code}` turn off protection against cross-site request forgery for requests that change data?", - yes: "A view or route that changes data as the user its session cookie signs in, such as their profile, password or records, is exempted from the CSRF check, such as with csrf_exempt, or the CSRF middleware or check is removed.", - no: "CSRF protection stays on; the exempted endpoint authenticates each request itself rather than with the session cookie, such as a webhook that verifies a signature, an API that reads a token from a header, or a form for visitors who are not signed in that asks for a password reset email or checks a reset token it is sent; it only reads data; or it sets nothing about CSRF.", - no_examples: &[], -}; - -const SECRET: Check = Check { - id: "literal_secret", - question: "Does `{code}` set a signing key, password or token that the deployed site uses to a literal written in the code?", - yes: "A secret key, password, token or API key that the deployed program uses is a literal in the code, including one shown as a redacted literal.", - no: "Secrets are read from the environment, a file or a secret store; the literal is empty or only a placeholder; it is used only in tests or local development; or a settings module for production that imports these settings sets it again.", - no_examples: &[], -}; - /// Specific exposures, asked when the broad presence questions are not clear: /// a logged configuration or argument list that holds a password, and an /// exception's own text in a response, were left undecided by them. @@ -801,57 +544,3 @@ pub const EXCEPTION_TO_CLIENT_FROM_CALLEES: Check = Check { no: "Responses carry fixed messages or codes, or messages written to explain invalid input or a missing record to the client, whether the program's own, such as the errors in `errors_created_by_functions_it_calls`, or a framework's validation and bad-request errors; details stay in server logs.", no_examples: &[], }; - -const DJANGO_EXCEPTION_TO_CLIENT: Check = Check { - id: "exception_to_client", - question: "Does `{code}` send an exception's message, stack trace or a database error to a remote client in a response?", - yes: "The text of an exception it did not raise itself to explain bad input, or a stack trace, is put into the response to a request.", - no: "Responses carry fixed messages or codes, or only messages written to explain invalid input, such as those of Django's or a form's ValidationError; exceptions it does not catch go to the framework's error handling; details stay in server logs.", - no_examples: &[], -}; - -const ENVIRONMENT_TO_CLIENT: Check = Check { - id: "environment_to_client", - question: "Does `{code}` send the server's environment variables, settings or whole request metadata to a remote client?", - yes: "It puts the process environment, the application's settings, or a whole request metadata object such as Django's request.META, which holds the server's environment, into a response or a page it renders.", - no: "It sends only chosen fields meant for the client, such as the user's own name or a public setting, or sends no such data.", - no_examples: &[], -}; - -const DJANGO_REDIRECT: Check = Check { - id: "redirect", - question: "Does `{code}` redirect the client to a URL or path taken from a variable without checking where it leads?", - yes: "A URL or path that a request carries, such as a query parameter, form field, header or cookie, is passed to redirect(), HttpResponseRedirect or a Location header without checking that it is a path on the program's own site or that its host is on an allowed list.", - no: "The target is fixed, is a route name or built with reverse(), is one of the program's own paths with only ids or names from variables in it, is checked such as with url_has_allowed_host_and_scheme, comes from the program's configuration, or is read from a stored record rather than the request; or it does not redirect.", - no_examples: &[], -}; - -/// Checks asked of Django code in place of the common check with the same -/// id: they name Django's raw queries (`raw`, `extra`, `RawSQL`), safe -/// markup and templates, storage API, redirects, password hashers and -/// validation errors, and ask cookie, CORS and verification settings about -/// the deployed site, since settings modules that production imports and -/// overrides were flagged when asked about the module alone. -pub const DJANGO_VARIANTS: [Check; 9] = [ - DJANGO_SQL, - DJANGO_MARKUP, - DJANGO_PATH, - DJANGO_REDIRECT, - DJANGO_TLS, - DJANGO_HASH, - DJANGO_CORS, - DJANGO_COOKIE, - DJANGO_EXCEPTION_TO_CLIENT, -]; - -/// The injection check Django code adds: request data given to a -/// deserializer that can build any object (`pickle.loads(request.body)`). -pub const DJANGO_UNHANDLED: [Check; 1] = [DESERIALIZE]; - -/// The weak settings Django code adds, which its settings modules and view -/// decorators decide: debug mode, CSRF protection and a literal secret key. -pub const DJANGO_SETTINGS: [Check; 3] = [DEBUG, CSRF, SECRET]; - -/// The exposure Django code adds: `request.META` or the settings, which -/// hold the server's environment, sent to a client. -pub const DJANGO_EXPOSURES: [Check; 1] = [ENVIRONMENT_TO_CLIENT]; From 7816593e1c2b00f9dcc394201bd050abfa900e19 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sun, 27 Sep 2026 19:18:32 -0300 Subject: [PATCH 4/5] Split test_units.rs into value tests, the setup a test runs and redundant pairs --- src/units/test_units.rs | 818 ---------------------------------- src/units/test_units/mod.rs | 375 ++++++++++++++++ src/units/test_units/pairs.rs | 178 ++++++++ src/units/test_units/setup.rs | 282 ++++++++++++ 4 files changed, 835 insertions(+), 818 deletions(-) delete mode 100644 src/units/test_units.rs create mode 100644 src/units/test_units/mod.rs create mode 100644 src/units/test_units/pairs.rs create mode 100644 src/units/test_units/setup.rs diff --git a/src/units/test_units.rs b/src/units/test_units.rs deleted file mode 100644 index 4b989d1..0000000 --- a/src/units/test_units.rs +++ /dev/null @@ -1,818 +0,0 @@ -//! Test quality: one request per test for value checks and one per candidate -//! redundant pair. A test whose value stays undecided is asked again with the -//! bodies of the functions it calls and its file's imports, mocks and setup; -//! an undecided pair, with the body of the function both tests call. -use super::{ - Asked, Detail, FileContext, FilePlan, Planned, Presence, Questions, TEST_PACK_ITEMS, UnitPlan, - compact, identity, pack, questions, unique_ids, -}; -use crate::{ - analysis::test_map::{self, TestCase}, - catalog::{TEST_REDUNDANCY, TEST_VALUE}, - schema::Pass, - units::questions::TestEvidence, -}; -use serde_json::{Value, json}; -use std::{ - collections::BTreeMap, - ops::Range, - path::{Path, PathBuf}, -}; - -const SUBJECTS: usize = 16; -/// Subjects whose bodies the recheck shows, in the order the test calls them. -const SOURCED_SUBJECTS: usize = 4; -/// A longer body is left out rather than cut; its signature stays. -const SUBJECT_SOURCE_BYTES: usize = 4000; -/// Setup text before the first test, and each setup hook, above these sizes -/// is left out rather than cut. -const SETUP_BYTES: usize = 4000; -const HOOK_BYTES: usize = 1500; - -/// A callable's file and full source, for the recheck. -pub(super) struct SubjectSource { - pub path: PathBuf, - pub source: String, - /// Whether other files can call it: false for a Ruby helper defined in a - /// file of test cases. - pub shared: bool, -} - -/// What the scope knows about the functions tests call. -pub(super) struct Subjects<'a> { - pub signatures: &'a BTreeMap, - pub sources: &'a BTreeMap, - /// Ruby test helpers by short name, for the recheck's setup. - pub helpers: &'a BTreeMap>, - /// Source hashes of selected and context files, for freshness checks. - pub hashes: &'a BTreeMap, - /// Controller methods by full name, with the route that reaches each, - /// such as `GET /owners/{ownerId}`. - pub routes: &'a BTreeMap, -} - -fn subject_state(names: &[&String], subjects: &BTreeMap) -> Vec { - let mut seen = Vec::new(); - for name in names { - if seen.len() < SUBJECTS && !seen.contains(name) { - seen.push(*name); - } - } - seen.iter() - .map(|name| json!({"name": name, "signature": subjects.get(*name).cloned().unwrap_or_default()})) - .collect() -} - -pub(super) fn plan_values( - file: &FileContext<'_>, - cases: &[TestCase], - subjects: &Subjects<'_>, - test_lines: &[Range], - out: &mut FilePlan, - requests: &mut Vec, -) { - let ids = unique_ids("test", cases.iter().map(|c| c.name.as_str())); - let (setup, head) = cases - .first() - .map(|first| { - let region = test_lines - .iter() - .find(|r| r.contains(&first.line)) - .map_or(1, |r| r.start); - let lines: Vec<&str> = file.source.lines().collect(); - let start = region.saturating_sub(1).min(lines.len()); - ( - file_setup(file.source, region, first.line), - setup_head(&lines, start, first.line), - ) - }) - .unwrap_or_default(); - let ruby = file.path.extension().is_some_and(|e| e == "rb"); - let mut items = Vec::new(); - for (case, id) in cases.iter().zip(ids) { - let source = case.source(file.source); - // A Ruby case gets the setup its groups declare for it, not every - // hook of the file (an RSpec file's groups often set up differently), - // and the test helpers it and its hooks call. - let (own, helper_paths) = if ruby { - ruby_setup(file, case, head.clone(), subjects.helpers) - } else { - (setup.clone(), Vec::new()) - }; - let evidence = value_evidence(file, case, subjects, &own, &helper_paths); - let recheck = value_recheck(file, &id, &evidence); - let confirm = (!reaches_past_visibility(source)) - .then(|| value_confirm(file, &id, &evidence)) - .flatten(); - out.units.push(UnitPlan { - rule: TEST_VALUE, - id: id.clone(), - name: case.name.clone(), - presence: Presence::Judged, - locations: vec![file.location(case.line, case.end_line, Some(&case.name))], - quote: None, - lines: case.end_line + 1 - case.line, - identity: identity(&[&case.name, &compact(source)]), - detail: Detail::Test { - confirm: confirm.map(Into::into), - }, - recheck: recheck.map(Into::into), - }); - items.push((out.units.len() - 1, id, case, test_item(case, source, ruby))); - } - for group in pack(items, TEST_PACK_ITEMS, |(_, _, _, item)| item) { - let (request, asked) = value_request(file, &group, subjects.signatures); - if file.budget.fits(&request) { - requests.push(Planned { - owner: file.owner, - request, - asked, - }); - } else { - for (unit, ..) in group { - out.units[unit].presence = Presence::NeedsContext; - out.units[unit].recheck = None; - out.units[unit].detail = Detail::Test { confirm: None }; - } - } - } -} - -/// One test with the bodies of the functions it calls and its file's setup, -/// and the files they come from: the evidence of its recheck and confirm. -struct Evidence { - state: Value, - sources: Vec<(PathBuf, String)>, - /// Whether it adds a body or setup to what the first pass showed. - adds: bool, - ruby: bool, -} - -impl Evidence { - fn request(&self, file: &FileContext<'_>, stage: &str, questions: Questions) -> (Value, Asked) { - let paths: Vec<(&Path, &str)> = self - .sources - .iter() - .map(|(path, hash)| (path.as_path(), hash.as_str())) - .collect(); - super::request(file.model, stage, &paths, self.state.clone(), questions) - } -} - -fn value_evidence( - file: &FileContext<'_>, - case: &TestCase, - subjects: &Subjects<'_>, - setup: &str, - setup_paths: &[PathBuf], -) -> Evidence { - let mut sources = vec![(file.path.to_path_buf(), file.source_hash.to_string())]; - for path in setup_paths { - if let Some(hash) = subjects.hashes.get(path) - && !sources.iter().any(|(known, _)| known == path) - { - sources.push((path.clone(), hash.clone())); - } - } - let listed = sourced_subjects(case, subjects, &mut sources); - let sourced = listed.iter().any(|s| s.get("source").is_some()); - let ruby = file.path.extension().is_some_and(|e| e == "rb"); - let state = json!({ - "file": file.plain_state(), - "tests": [test_item(case, case.source(file.source), ruby)], - "subjects": listed, - "setup": setup, - }); - Evidence { - state, - sources, - adds: sourced || !setup.is_empty(), - ruby, - } -} - -/// The hollow-test questions again for one test, with the bodies of the -/// functions it calls and its file's setup; none when there is nothing to add. -fn value_recheck(file: &FileContext<'_>, id: &str, evidence: &Evidence) -> Option<(Value, Asked)> { - if !evidence.adds { - return None; - } - let (request, asked) = evidence.request(file, "recheck", recheck_questions(id, evidence.ruby)); - file.budget.fits(&request).then_some((request, asked)) -} - -/// Calls that reach past a language's visibility: reflection, a cast to -/// `any`, Ruby's `send(:…)` and `instance_variable_get`. -const BYPASSES: [&str; 12] = [ - "ReflectionClass", - "ReflectionProperty", - "ReflectionMethod", - "setAccessible(", - "getDeclaredField(", - "getDeclaredMethod(", - "BindingFlags.NonPublic", - "Whitebox.", - "ReflectionTestUtils.", - "as any)", - "instance_variable_get", - ".send(:", -]; - -/// Whether a test reads or calls members past its language's visibility. It -/// reads internals by the language's own definition, so an internal-details -/// consider on it is not asked what its assertions read: 4 of the 6 labeled -/// tests that did so were right, and the question read two reflected private -/// properties and two `(service as any)` fields as results or state. -fn reaches_past_visibility(source: &str) -> bool { - BYPASSES.iter().any(|b| source.contains(b)) -} - -/// What the test's assertions read, with the same evidence as its recheck. -fn value_confirm(file: &FileContext<'_>, id: &str, evidence: &Evidence) -> Option<(Value, Asked)> { - let mut questions = Questions::default(); - let kind = if evidence.ruby { - TestEvidence::RecheckGroups - } else { - TestEvidence::Recheck - }; - questions.ask( - "reads".into(), - questions::test_reads("tests[0].source", kind), - id, - TEST_VALUE, - "reads", - Pass::Locate, - ); - let (request, asked) = evidence.request(file, "locate", questions); - file.budget.fits(&request).then_some((request, asked)) -} - -/// The functions a test calls, with the route of a controller method it -/// reaches through a request and the bodies of the first few; each body's -/// file joins `sources`. -fn sourced_subjects( - case: &TestCase, - subjects: &Subjects<'_>, - sources: &mut Vec<(PathBuf, String)>, -) -> Vec { - let names: Vec<&String> = case.subjects.iter().collect(); - let mut listed = subject_state(&names, subjects.signatures); - // A controller method the test reaches through a request, not a call. - for subject in &mut listed { - if let Some(route) = subject["name"] - .as_str() - .and_then(|name| subjects.routes.get(name)) - { - subject["route"] = json!(route); - } - } - for subject in listed.iter_mut().take(SOURCED_SUBJECTS) { - let Some(found) = subject["name"] - .as_str() - .and_then(|name| subjects.sources.get(name)) - .filter(|found| found.source.len() <= SUBJECT_SOURCE_BYTES) - else { - continue; - }; - let Some(hash) = subjects.hashes.get(&found.path) else { - continue; - }; - subject["source"] = json!(found.source); - if !sources.iter().any(|(path, _)| *path == found.path) { - sources.push((found.path.clone(), hash.clone())); - } - } - listed -} - -/// The hollow-test questions of a recheck; a Ruby test's name the setup its -/// groups declare for it. -fn recheck_questions(id: &str, ruby: bool) -> Questions { - let evidence = if ruby { - TestEvidence::RecheckGroups - } else { - TestEvidence::Recheck - }; - let mut questions = Questions::default(); - let path = "tests[0].source"; - for (question, body) in [ - ("own_logic", questions::test_own_logic(path, evidence)), - ("mock_only", questions::test_mock_only(path, evidence)), - ] { - questions.ask( - question.into(), - body, - id, - TEST_VALUE, - question, - Pass::Recheck, - ); - } - questions -} - -/// A test as sent: its name and source, and for Ruby the groups it is -/// declared in. An RSpec example reads as a sentence that continues its -/// groups (`describe Registry` … `it "finds a registered object"`), and the -/// outer group often names the class under test. -fn test_item(case: &TestCase, source: &str, ruby: bool) -> Value { - let mut item = json!({"name": case.name, "source": source}); - if ruby && !case.suite.is_empty() { - item["suite"] = json!(case.suite.join(" > ")); - } - item -} - -/// Test helpers shown with one Ruby case, at most. -const HELPERS: usize = 4; - -/// A Ruby case's setup: the file's head, the hooks its groups declare, then -/// the test helpers the case and its hooks call, and the helpers those call. -/// Also the other files the helpers come from. -fn ruby_setup( - file: &FileContext<'_>, - case: &TestCase, - head: Option, - helpers: &BTreeMap>, -) -> (String, Vec) { - let setup = case_setup(file.source, head, &case.hooks); - let mut names: Vec = case.calls.iter().chain(&case.hook_calls).cloned().collect(); - let mut shown: Vec<&SubjectSource> = Vec::new(); - let mut next = 0; - while next < names.len() && shown.len() < HELPERS { - let name = names[next].clone(); - next += 1; - let Some(defined) = helpers.get(&name) else { - continue; - }; - let found = nearest(file.path, defined); - let Some(helper) = found.filter(|h| { - h.source.len() <= HOOK_BYTES - && !setup.contains(h.source.as_str()) - && !shown.iter().any(|s| s.source == h.source) - }) else { - continue; - }; - shown.push(helper); - if let Some(tree) = crate::syntax::parse(&helper.path, &helper.source) - .ok() - .flatten() - { - let mut calls = Vec::new(); - crate::analysis::ruby::called_names(tree.root_node(), &helper.source, &mut calls); - names.extend(calls); - } - } - let mut parts: Vec = (!setup.is_empty()).then_some(setup).into_iter().collect(); - parts.extend(shown.iter().map(|h| h.source.clone())); - let paths = shown - .iter() - .filter(|h| h.path != file.path) - .map(|h| h.path.clone()) - .collect(); - (parts.join("\n\n"), paths) -} - -/// The one definition of a helper nearest the test: in its own file, else -/// in the support file that shares the most directories with it, at least -/// one, as `test/test_helper.rb` does with `test/routing_test.rb`. None when -/// two definitions are equally near. -pub(super) fn nearest<'a>(test: &Path, defined: &'a [SubjectSource]) -> Option<&'a SubjectSource> { - let folders = |path: &Path| -> Vec { - path.parent() - .into_iter() - .flat_map(Path::components) - .map(|c| c.as_os_str().to_string_lossy().into_owned()) - .collect() - }; - let own = folders(test); - let shared = |helper: &SubjectSource| { - if helper.path == test { - return usize::MAX; - } - own.iter() - .zip(folders(&helper.path)) - .take_while(|(a, b)| **a == *b) - .count() - }; - // A support file in another tree, sharing no directory with the test, - // serves other tests: `test/test_helper.rb` is not a spec's helper. - let callable = || { - defined - .iter() - .filter(|h| h.path == test || h.shared && shared(h) > 0) - }; - let best = callable().map(shared).max()?; - let mut nearest = callable().filter(|h| shared(h) == best); - let first = nearest.next(); - nearest.next().is_none().then_some(first).flatten() -} - -/// One case's setup: the file's head, then the hooks its groups declare. A -/// hook larger than its limit is left out rather than cut, and so are the -/// hooks when together they are too long. -fn case_setup(source: &str, head: Option, hooks: &[Range]) -> String { - let kept = usize::from(head.is_some()); - let mut parts: Vec = head.into_iter().collect(); - parts.extend( - hooks - .iter() - .map(|hook| source[hook.clone()].to_string()) - .filter(|text| text.len() <= HOOK_BYTES), - ); - let setup = parts.join("\n\n"); - if setup.len() <= SETUP_BYTES + HOOK_BYTES { - setup - } else { - parts.truncate(kept); - parts.join("") - } -} - -/// The hooks a case's groups declare, as sent beside a pair of tests. -fn hook_text(source: &str, case: &TestCase) -> String { - case_setup(source, None, &case.hooks) -} - -/// A test file's shared setup: the text of its test region before the first -/// test or suite (imports, mocks, fixtures), then each setup hook. A part -/// larger than its limit is left out rather than cut. -pub(super) fn file_setup(source: &str, region_start: usize, first_case: usize) -> String { - let lines: Vec<&str> = source.lines().collect(); - let start = region_start.saturating_sub(1).min(lines.len()); - let mut parts: Vec = setup_head(&lines, start, first_case).into_iter().collect(); - parts.extend(setup_hooks(&lines[start..].join("\n"))); - let setup = parts.join("\n\n"); - if setup.len() <= SETUP_BYTES + HOOK_BYTES { - setup - } else { - parts.truncate(1); - parts.join("") - } -} - -/// The lines from `start` up to the first suite, test, test module or Java -/// setup method, when they are short enough to send. A Java test class's -/// fields, such as its mocks, are part of the head. -fn setup_head(lines: &[&str], start: usize, first_case: usize) -> Option { - const OPENERS: &[&str] = &[ - "describe(", - "describe.", - "suite(", - "context(", - "test(", - "test.", - "it(", - "it.", - "def test", - "class ", - "mod tests", - "#[test]", - // Ruby: RSpec groups and examples, and Rails `test "…" do`. - "describe ", - "RSpec.describe", - "context ", - "it ", - "test ", - "module ", - // Java: setup methods and `@Nested` test classes. - "@Before", - "@Nested", - ]; - let last = first_case.saturating_sub(1).min(lines.len()); - let end = (start..last) - .find(|&i| { - let line = lines[i].trim_start(); - // A Python class opens a suite; a braced class holds the fields - // the tests share. - OPENERS.iter().any(|opener| line.starts_with(opener)) - && !(line.starts_with("class ") && line.trim_end().ends_with('{')) - }) - .unwrap_or(last); - let head = lines[start..end].join("\n"); - (!head.trim().is_empty() && head.len() <= SETUP_BYTES).then(|| head.trim().to_string()) -} - -/// Every setup hook in `region` short enough to send: `beforeEach`/`beforeAll` -/// calls, Python `setUp`/`setup_method` methods and Java methods annotated -/// `@BeforeEach`, `@BeforeAll`, `@Before` or `@BeforeClass`. -fn setup_hooks(region: &str) -> Vec { - let mut hooks = Vec::new(); - for (at, _) in region.match_indices("@Before") { - let name_end = at - + region[at + 1..] - .find(|c: char| !c.is_alphanumeric()) - .map_or(region.len() - at, |i| i + 1); - if matches!( - ®ion[at..name_end], - "@BeforeEach" | "@BeforeAll" | "@Before" | "@BeforeClass" - ) { - let text = braced_method(®ion[at..]); - if text.len() <= HOOK_BYTES { - hooks.push(text.to_string()); - } - } - } - for hook in [ - "beforeEach(", - "beforeAll(", - "def setUp(", - "def setup_method(", - ] { - let mut from = 0; - while let Some(found) = region[from..].find(hook) { - let at = from + found; - let text = if hook.starts_with("def ") { - let line_start = region[..at].rfind('\n').map_or(0, |i| i + 1); - indented_block(®ion[at..], at - line_start) - } else { - balanced_call(®ion[at..]) - }; - if text.len() <= HOOK_BYTES { - hooks.push(text.to_string()); - } - from = at + hook.len(); - } - } - hooks -} - -/// A Java method from its annotation through the brace that closes its body. -fn braced_method(text: &str) -> &str { - let Some(open) = text.find('{') else { - return text; - }; - let mut depth = 0usize; - for (i, c) in text[open..].char_indices() { - match c { - '{' => depth += 1, - '}' => { - depth -= 1; - if depth == 0 { - return &text[..open + i + 1]; - } - } - _ => {} - } - } - text -} - -/// A call from its name through the parenthesis that closes it. -fn balanced_call(text: &str) -> &str { - let mut depth = 0usize; - for (i, c) in text.char_indices() { - match c { - '(' | '{' | '[' => depth += 1, - ')' | '}' | ']' => { - depth = depth.saturating_sub(1); - if depth == 0 { - return &text[..i + 1]; - } - } - _ => {} - } - } - text -} - -/// A Python definition line, indented `base` columns, and the lines indented below it. -fn indented_block(text: &str, base: usize) -> &str { - let mut lines = text.split_inclusive('\n'); - let Some(first) = lines.next() else { - return text; - }; - let mut end = first.len(); - let indent = |line: &str| line.len() - line.trim_start().len(); - for line in lines { - if !line.trim().is_empty() && indent(line) <= base { - break; - } - end += line.len(); - } - text[..end].trim_end() -} - -/// One test-value request: four questions per test, with the signatures the tests call. -fn value_request( - file: &FileContext<'_>, - group: &[(usize, String, &TestCase, Value)], - subjects: &BTreeMap, -) -> (Value, Asked) { - let mut questions = Questions::default(); - for (index, (_, id, _, _)) in group.iter().enumerate() { - let path = format!("tests[{index}].source"); - for (question, body) in [ - ("internal", questions::test_internal(&path)), - ( - "own_logic", - questions::test_own_logic(&path, TestEvidence::First), - ), - ( - "mock_only", - questions::test_mock_only(&path, TestEvidence::First), - ), - ("several", questions::test_several(&path)), - ] { - questions.ask( - format!("t{index}_{question}"), - body, - id, - TEST_VALUE, - question, - Pass::First, - ); - } - } - let names: Vec<&String> = group - .iter() - .flat_map(|(_, _, case, _)| case.subjects.iter()) - .collect(); - let state = json!({ - "file": file.plain_state(), - "tests": group.iter().map(|(_, _, _, item)| item.clone()).collect::>(), - "subjects": subject_state(&names, subjects), - }); - file.request("tests", state, questions) -} - -/// A test's source as its words: a space inside a string, such as -/// `x:=` against `x := `, can be what two tests differ in. -fn words(source: &str) -> Vec<&str> { - source.split_whitespace().collect() -} - -pub(super) fn plan_pairs( - file: &FileContext<'_>, - (cases, table): (&[TestCase], bool), - subjects: &Subjects<'_>, - out: &mut FilePlan, - requests: &mut Vec, -) { - let (pairs, omitted) = test_map::pairs(cases); - out.rules.insert(TEST_REDUNDANCY, omitted); - let ruby = file.path.extension().is_some_and(|e| e == "rb"); - for pair in pairs { - let (a, b) = (&cases[pair.a], &cases[pair.b]); - let id = format!("test-pair:{}|{}", a.name, b.name); - let subject = subject_state(&[&pair.subject], subjects.signatures).remove(0); - let state = pair_state(file, (a, b), subject, ruby); - let recheck = pair_recheck(file, &id, &state, &pair.subject, subjects, ruby); - let identical = a.suite == b.suite - && words(&a.source(file.source).replace(a.name.as_str(), "")) - == words(&b.source(file.source).replace(b.name.as_str(), "")); - // Tests that read the same apart from their names check nothing apart. - let confirm = (!ruby && !identical).then(|| { - let mut questions = Questions::default(); - questions.ask( - "distinct".into(), - questions::test_pair_distinct(), - &id, - TEST_REDUNDANCY, - "distinct", - Pass::Locate, - ); - file.request("locate", state.clone(), questions) - }); - let (request, asked) = file.request("test-pair", state, pair_questions(&id, ruby)); - let fits = file.budget.fits(&request); - out.units.push(UnitPlan { - rule: TEST_REDUNDANCY, - id, - name: format!("`{}` and `{}`", a.name, b.name), - presence: if fits { - Presence::Judged - } else { - Presence::NeedsContext - }, - locations: vec![ - file.location(a.line, a.end_line, Some(&a.name)), - file.location(b.line, b.end_line, Some(&b.name)), - ], - quote: None, - lines: a.end_line + 1 - a.line + b.end_line + 1 - b.line, - identity: identity(&[ - &a.name, - &b.name, - &compact(a.source(file.source)), - &compact(b.source(file.source)), - ]), - detail: Detail::TestPair { - names: [a.name.clone(), b.name.clone()], - subject: pair.subject.clone(), - table, - unseen_setup: !ruby && a.suite != b.suite, - identical, - confirm: confirm - .filter(|(request, _)| fits && file.budget.fits(request)) - .map(Into::into), - }, - recheck: recheck.filter(|_| fits).map(Into::into), - }); - if fits { - requests.push(Planned { - owner: file.owner, - request, - asked, - }); - } - } -} - -/// The first-pass questions of a test pair; Ruby pairs are also asked -/// whether each test checks something the other does not. -fn pair_questions(id: &str, ruby: bool) -> Questions { - let mut questions = Questions::default(); - let distinct = ruby.then(|| ("distinct", questions::test_pair_distinct())); - for (question, body) in [ - ("overlap", questions::test_pair_overlap(ruby)), - ("same_input", questions::test_pair_same_input()), - ("same_outcome", questions::test_pair_same_outcome()), - ] - .into_iter() - .chain(distinct) - { - questions.ask( - question.into(), - body, - id, - TEST_REDUNDANCY, - question, - Pass::First, - ); - } - questions -} - -/// Both tests of a pair with their subject. -fn pair_state( - file: &FileContext<'_>, - (a, b): (&TestCase, &TestCase), - subject: Value, - ruby: bool, -) -> Value { - let mut state = json!({ - "test_a": {"name": a.name, "source": a.source(file.source)}, - "test_b": {"name": b.name, "source": b.source(file.source)}, - "subject": subject, - }); - // Tests in different groups can run on different setup: two RSpec - // examples that read alike may build different records first. - if ruby && a.suite != b.suite { - for (key, case) in [("test_a", a), ("test_b", b)] { - if !case.suite.is_empty() { - state[key]["suite"] = json!(case.suite.join(" > ")); - } - } - let (setup_a, setup_b) = (hook_text(file.source, a), hook_text(file.source, b)); - if setup_a != setup_b { - state["test_a"]["setup"] = json!(setup_a); - state["test_b"]["setup"] = json!(setup_b); - } - } - state -} - -/// The overlap question again for an undecided pair, with the body of the -/// function both tests call: whether a call throws before the rest of a test -/// runs, or which inputs it tells apart, is in that body. None when the body -/// is unknown or too long. -fn pair_recheck( - file: &FileContext<'_>, - id: &str, - state: &Value, - subject: &str, - subjects: &Subjects<'_>, - ruby: bool, -) -> Option<(Value, Asked)> { - let found = subjects - .sources - .get(subject) - .filter(|found| found.source.len() <= SUBJECT_SOURCE_BYTES)?; - let hash = subjects.hashes.get(&found.path)?; - let mut state = state.clone(); - state["subject"]["source"] = json!(found.source); - let mut questions = Questions::default(); - // A decisive recheck replaces the first answers, so a Ruby pair is asked - // again whether each test checks something the other does not. - let distinct = ruby.then(|| ("distinct", questions::test_pair_distinct())); - for (question, body) in [("overlap", questions::test_pair_overlap_recheck(ruby))] - .into_iter() - .chain(distinct) - { - questions.ask( - question.into(), - body, - id, - TEST_REDUNDANCY, - question, - Pass::Recheck, - ); - } - let mut paths = vec![(file.path, file.source_hash)]; - if found.path != file.path { - paths.push((found.path.as_path(), hash.as_str())); - } - let (request, asked) = super::request(file.model, "recheck", &paths, state, questions); - file.budget.fits(&request).then_some((request, asked)) -} diff --git a/src/units/test_units/mod.rs b/src/units/test_units/mod.rs new file mode 100644 index 0000000..fc6704b --- /dev/null +++ b/src/units/test_units/mod.rs @@ -0,0 +1,375 @@ +//! Test quality: one request per test for value checks and one per candidate +//! redundant pair. A test whose value stays undecided is asked again with the +//! bodies of the functions it calls and its file's imports, mocks and setup; +//! an undecided pair, with the body of the function both tests call. `setup` +//! reads what runs before a test from its file's text, and `pairs` plans the +//! redundant pairs. +use super::{ + Asked, Detail, FileContext, FilePlan, Planned, Presence, Questions, TEST_PACK_ITEMS, UnitPlan, + compact, identity, pack, questions, unique_ids, +}; +use crate::{ + analysis::test_map::{self, TestCase}, + catalog::{TEST_REDUNDANCY, TEST_VALUE}, + schema::Pass, + units::questions::TestEvidence, +}; +use serde_json::{Value, json}; +use std::{ + collections::BTreeMap, + ops::Range, + path::{Path, PathBuf}, +}; + +mod pairs; +mod setup; +pub(in crate::units) use pairs::*; +pub(in crate::units) use setup::*; + +const SUBJECTS: usize = 16; +/// Subjects whose bodies the recheck shows, in the order the test calls them. +const SOURCED_SUBJECTS: usize = 4; +/// A longer body is left out rather than cut; its signature stays. +const SUBJECT_SOURCE_BYTES: usize = 4000; +/// A callable's file and full source, for the recheck. +pub(super) struct SubjectSource { + pub path: PathBuf, + pub source: String, + /// Whether other files can call it: false for a Ruby helper defined in a + /// file of test cases. + pub shared: bool, +} + +/// What the scope knows about the functions tests call. +pub(super) struct Subjects<'a> { + pub signatures: &'a BTreeMap, + pub sources: &'a BTreeMap, + /// Ruby test helpers by short name, for the recheck's setup. + pub helpers: &'a BTreeMap>, + /// Source hashes of selected and context files, for freshness checks. + pub hashes: &'a BTreeMap, + /// Controller methods by full name, with the route that reaches each, + /// such as `GET /owners/{ownerId}`. + pub routes: &'a BTreeMap, +} + +fn subject_state(names: &[&String], subjects: &BTreeMap) -> Vec { + let mut seen = Vec::new(); + for name in names { + if seen.len() < SUBJECTS && !seen.contains(name) { + seen.push(*name); + } + } + seen.iter() + .map(|name| json!({"name": name, "signature": subjects.get(*name).cloned().unwrap_or_default()})) + .collect() +} + +pub(super) fn plan_values( + file: &FileContext<'_>, + cases: &[TestCase], + subjects: &Subjects<'_>, + test_lines: &[Range], + out: &mut FilePlan, + requests: &mut Vec, +) { + let ids = unique_ids("test", cases.iter().map(|c| c.name.as_str())); + let (setup, head) = cases + .first() + .map(|first| { + let region = test_lines + .iter() + .find(|r| r.contains(&first.line)) + .map_or(1, |r| r.start); + let lines: Vec<&str> = file.source.lines().collect(); + let start = region.saturating_sub(1).min(lines.len()); + ( + file_setup(file.source, region, first.line), + setup_head(&lines, start, first.line), + ) + }) + .unwrap_or_default(); + let ruby = file.path.extension().is_some_and(|e| e == "rb"); + let mut items = Vec::new(); + for (case, id) in cases.iter().zip(ids) { + let source = case.source(file.source); + // A Ruby case gets the setup its groups declare for it, not every + // hook of the file (an RSpec file's groups often set up differently), + // and the test helpers it and its hooks call. + let (own, helper_paths) = if ruby { + ruby_setup(file, case, head.clone(), subjects.helpers) + } else { + (setup.clone(), Vec::new()) + }; + let evidence = value_evidence(file, case, subjects, &own, &helper_paths); + let recheck = value_recheck(file, &id, &evidence); + let confirm = (!reaches_past_visibility(source)) + .then(|| value_confirm(file, &id, &evidence)) + .flatten(); + out.units.push(UnitPlan { + rule: TEST_VALUE, + id: id.clone(), + name: case.name.clone(), + presence: Presence::Judged, + locations: vec![file.location(case.line, case.end_line, Some(&case.name))], + quote: None, + lines: case.end_line + 1 - case.line, + identity: identity(&[&case.name, &compact(source)]), + detail: Detail::Test { + confirm: confirm.map(Into::into), + }, + recheck: recheck.map(Into::into), + }); + items.push((out.units.len() - 1, id, case, test_item(case, source, ruby))); + } + for group in pack(items, TEST_PACK_ITEMS, |(_, _, _, item)| item) { + let (request, asked) = value_request(file, &group, subjects.signatures); + if file.budget.fits(&request) { + requests.push(Planned { + owner: file.owner, + request, + asked, + }); + } else { + for (unit, ..) in group { + out.units[unit].presence = Presence::NeedsContext; + out.units[unit].recheck = None; + out.units[unit].detail = Detail::Test { confirm: None }; + } + } + } +} + +/// One test with the bodies of the functions it calls and its file's setup, +/// and the files they come from: the evidence of its recheck and confirm. +struct Evidence { + state: Value, + sources: Vec<(PathBuf, String)>, + /// Whether it adds a body or setup to what the first pass showed. + adds: bool, + ruby: bool, +} + +impl Evidence { + fn request(&self, file: &FileContext<'_>, stage: &str, questions: Questions) -> (Value, Asked) { + let paths: Vec<(&Path, &str)> = self + .sources + .iter() + .map(|(path, hash)| (path.as_path(), hash.as_str())) + .collect(); + super::request(file.model, stage, &paths, self.state.clone(), questions) + } +} + +fn value_evidence( + file: &FileContext<'_>, + case: &TestCase, + subjects: &Subjects<'_>, + setup: &str, + setup_paths: &[PathBuf], +) -> Evidence { + let mut sources = vec![(file.path.to_path_buf(), file.source_hash.to_string())]; + for path in setup_paths { + if let Some(hash) = subjects.hashes.get(path) + && !sources.iter().any(|(known, _)| known == path) + { + sources.push((path.clone(), hash.clone())); + } + } + let listed = sourced_subjects(case, subjects, &mut sources); + let sourced = listed.iter().any(|s| s.get("source").is_some()); + let ruby = file.path.extension().is_some_and(|e| e == "rb"); + let state = json!({ + "file": file.plain_state(), + "tests": [test_item(case, case.source(file.source), ruby)], + "subjects": listed, + "setup": setup, + }); + Evidence { + state, + sources, + adds: sourced || !setup.is_empty(), + ruby, + } +} + +/// The hollow-test questions again for one test, with the bodies of the +/// functions it calls and its file's setup; none when there is nothing to add. +fn value_recheck(file: &FileContext<'_>, id: &str, evidence: &Evidence) -> Option<(Value, Asked)> { + if !evidence.adds { + return None; + } + let (request, asked) = evidence.request(file, "recheck", recheck_questions(id, evidence.ruby)); + file.budget.fits(&request).then_some((request, asked)) +} + +/// Calls that reach past a language's visibility: reflection, a cast to +/// `any`, Ruby's `send(:…)` and `instance_variable_get`. +const BYPASSES: [&str; 12] = [ + "ReflectionClass", + "ReflectionProperty", + "ReflectionMethod", + "setAccessible(", + "getDeclaredField(", + "getDeclaredMethod(", + "BindingFlags.NonPublic", + "Whitebox.", + "ReflectionTestUtils.", + "as any)", + "instance_variable_get", + ".send(:", +]; + +/// Whether a test reads or calls members past its language's visibility. It +/// reads internals by the language's own definition, so an internal-details +/// consider on it is not asked what its assertions read: 4 of the 6 labeled +/// tests that did so were right, and the question read two reflected private +/// properties and two `(service as any)` fields as results or state. +fn reaches_past_visibility(source: &str) -> bool { + BYPASSES.iter().any(|b| source.contains(b)) +} + +/// What the test's assertions read, with the same evidence as its recheck. +fn value_confirm(file: &FileContext<'_>, id: &str, evidence: &Evidence) -> Option<(Value, Asked)> { + let mut questions = Questions::default(); + let kind = if evidence.ruby { + TestEvidence::RecheckGroups + } else { + TestEvidence::Recheck + }; + questions.ask( + "reads".into(), + questions::test_reads("tests[0].source", kind), + id, + TEST_VALUE, + "reads", + Pass::Locate, + ); + let (request, asked) = evidence.request(file, "locate", questions); + file.budget.fits(&request).then_some((request, asked)) +} + +/// The functions a test calls, with the route of a controller method it +/// reaches through a request and the bodies of the first few; each body's +/// file joins `sources`. +fn sourced_subjects( + case: &TestCase, + subjects: &Subjects<'_>, + sources: &mut Vec<(PathBuf, String)>, +) -> Vec { + let names: Vec<&String> = case.subjects.iter().collect(); + let mut listed = subject_state(&names, subjects.signatures); + // A controller method the test reaches through a request, not a call. + for subject in &mut listed { + if let Some(route) = subject["name"] + .as_str() + .and_then(|name| subjects.routes.get(name)) + { + subject["route"] = json!(route); + } + } + for subject in listed.iter_mut().take(SOURCED_SUBJECTS) { + let Some(found) = subject["name"] + .as_str() + .and_then(|name| subjects.sources.get(name)) + .filter(|found| found.source.len() <= SUBJECT_SOURCE_BYTES) + else { + continue; + }; + let Some(hash) = subjects.hashes.get(&found.path) else { + continue; + }; + subject["source"] = json!(found.source); + if !sources.iter().any(|(path, _)| *path == found.path) { + sources.push((found.path.clone(), hash.clone())); + } + } + listed +} + +/// The hollow-test questions of a recheck; a Ruby test's name the setup its +/// groups declare for it. +fn recheck_questions(id: &str, ruby: bool) -> Questions { + let evidence = if ruby { + TestEvidence::RecheckGroups + } else { + TestEvidence::Recheck + }; + let mut questions = Questions::default(); + let path = "tests[0].source"; + for (question, body) in [ + ("own_logic", questions::test_own_logic(path, evidence)), + ("mock_only", questions::test_mock_only(path, evidence)), + ] { + questions.ask( + question.into(), + body, + id, + TEST_VALUE, + question, + Pass::Recheck, + ); + } + questions +} + +/// A test as sent: its name and source, and for Ruby the groups it is +/// declared in. An RSpec example reads as a sentence that continues its +/// groups (`describe Registry` … `it "finds a registered object"`), and the +/// outer group often names the class under test. +fn test_item(case: &TestCase, source: &str, ruby: bool) -> Value { + let mut item = json!({"name": case.name, "source": source}); + if ruby && !case.suite.is_empty() { + item["suite"] = json!(case.suite.join(" > ")); + } + item +} + +/// One test-value request: four questions per test, with the signatures the tests call. +fn value_request( + file: &FileContext<'_>, + group: &[(usize, String, &TestCase, Value)], + subjects: &BTreeMap, +) -> (Value, Asked) { + let mut questions = Questions::default(); + for (index, (_, id, _, _)) in group.iter().enumerate() { + let path = format!("tests[{index}].source"); + for (question, body) in [ + ("internal", questions::test_internal(&path)), + ( + "own_logic", + questions::test_own_logic(&path, TestEvidence::First), + ), + ( + "mock_only", + questions::test_mock_only(&path, TestEvidence::First), + ), + ("several", questions::test_several(&path)), + ] { + questions.ask( + format!("t{index}_{question}"), + body, + id, + TEST_VALUE, + question, + Pass::First, + ); + } + } + let names: Vec<&String> = group + .iter() + .flat_map(|(_, _, case, _)| case.subjects.iter()) + .collect(); + let state = json!({ + "file": file.plain_state(), + "tests": group.iter().map(|(_, _, _, item)| item.clone()).collect::>(), + "subjects": subject_state(&names, subjects), + }); + file.request("tests", state, questions) +} + +/// A test's source as its words: a space inside a string, such as +/// `x:=` against `x := `, can be what two tests differ in. +fn words(source: &str) -> Vec<&str> { + source.split_whitespace().collect() +} diff --git a/src/units/test_units/pairs.rs b/src/units/test_units/pairs.rs new file mode 100644 index 0000000..45a95a1 --- /dev/null +++ b/src/units/test_units/pairs.rs @@ -0,0 +1,178 @@ +//! Redundant test pairs: one request per candidate pair, and the recheck of a +//! pair whose overlap stays undecided, with the body of the function both call. +use super::*; + +pub(in crate::units) fn plan_pairs( + file: &FileContext<'_>, + (cases, table): (&[TestCase], bool), + subjects: &Subjects<'_>, + out: &mut FilePlan, + requests: &mut Vec, +) { + let (pairs, omitted) = test_map::pairs(cases); + out.rules.insert(TEST_REDUNDANCY, omitted); + let ruby = file.path.extension().is_some_and(|e| e == "rb"); + for pair in pairs { + let (a, b) = (&cases[pair.a], &cases[pair.b]); + let id = format!("test-pair:{}|{}", a.name, b.name); + let subject = subject_state(&[&pair.subject], subjects.signatures).remove(0); + let state = pair_state(file, (a, b), subject, ruby); + let recheck = pair_recheck(file, &id, &state, &pair.subject, subjects, ruby); + let identical = a.suite == b.suite + && words(&a.source(file.source).replace(a.name.as_str(), "")) + == words(&b.source(file.source).replace(b.name.as_str(), "")); + // Tests that read the same apart from their names check nothing apart. + let confirm = (!ruby && !identical).then(|| { + let mut questions = Questions::default(); + questions.ask( + "distinct".into(), + questions::test_pair_distinct(), + &id, + TEST_REDUNDANCY, + "distinct", + Pass::Locate, + ); + file.request("locate", state.clone(), questions) + }); + let (request, asked) = file.request("test-pair", state, pair_questions(&id, ruby)); + let fits = file.budget.fits(&request); + out.units.push(UnitPlan { + rule: TEST_REDUNDANCY, + id, + name: format!("`{}` and `{}`", a.name, b.name), + presence: if fits { + Presence::Judged + } else { + Presence::NeedsContext + }, + locations: vec![ + file.location(a.line, a.end_line, Some(&a.name)), + file.location(b.line, b.end_line, Some(&b.name)), + ], + quote: None, + lines: a.end_line + 1 - a.line + b.end_line + 1 - b.line, + identity: identity(&[ + &a.name, + &b.name, + &compact(a.source(file.source)), + &compact(b.source(file.source)), + ]), + detail: Detail::TestPair { + names: [a.name.clone(), b.name.clone()], + subject: pair.subject.clone(), + table, + unseen_setup: !ruby && a.suite != b.suite, + identical, + confirm: confirm + .filter(|(request, _)| fits && file.budget.fits(request)) + .map(Into::into), + }, + recheck: recheck.filter(|_| fits).map(Into::into), + }); + if fits { + requests.push(Planned { + owner: file.owner, + request, + asked, + }); + } + } +} + +/// The first-pass questions of a test pair; Ruby pairs are also asked +/// whether each test checks something the other does not. +pub(super) fn pair_questions(id: &str, ruby: bool) -> Questions { + let mut questions = Questions::default(); + let distinct = ruby.then(|| ("distinct", questions::test_pair_distinct())); + for (question, body) in [ + ("overlap", questions::test_pair_overlap(ruby)), + ("same_input", questions::test_pair_same_input()), + ("same_outcome", questions::test_pair_same_outcome()), + ] + .into_iter() + .chain(distinct) + { + questions.ask( + question.into(), + body, + id, + TEST_REDUNDANCY, + question, + Pass::First, + ); + } + questions +} + +/// Both tests of a pair with their subject. +pub(super) fn pair_state( + file: &FileContext<'_>, + (a, b): (&TestCase, &TestCase), + subject: Value, + ruby: bool, +) -> Value { + let mut state = json!({ + "test_a": {"name": a.name, "source": a.source(file.source)}, + "test_b": {"name": b.name, "source": b.source(file.source)}, + "subject": subject, + }); + // Tests in different groups can run on different setup: two RSpec + // examples that read alike may build different records first. + if ruby && a.suite != b.suite { + for (key, case) in [("test_a", a), ("test_b", b)] { + if !case.suite.is_empty() { + state[key]["suite"] = json!(case.suite.join(" > ")); + } + } + let (setup_a, setup_b) = (hook_text(file.source, a), hook_text(file.source, b)); + if setup_a != setup_b { + state["test_a"]["setup"] = json!(setup_a); + state["test_b"]["setup"] = json!(setup_b); + } + } + state +} + +/// The overlap question again for an undecided pair, with the body of the +/// function both tests call: whether a call throws before the rest of a test +/// runs, or which inputs it tells apart, is in that body. None when the body +/// is unknown or too long. +pub(super) fn pair_recheck( + file: &FileContext<'_>, + id: &str, + state: &Value, + subject: &str, + subjects: &Subjects<'_>, + ruby: bool, +) -> Option<(Value, Asked)> { + let found = subjects + .sources + .get(subject) + .filter(|found| found.source.len() <= SUBJECT_SOURCE_BYTES)?; + let hash = subjects.hashes.get(&found.path)?; + let mut state = state.clone(); + state["subject"]["source"] = json!(found.source); + let mut questions = Questions::default(); + // A decisive recheck replaces the first answers, so a Ruby pair is asked + // again whether each test checks something the other does not. + let distinct = ruby.then(|| ("distinct", questions::test_pair_distinct())); + for (question, body) in [("overlap", questions::test_pair_overlap_recheck(ruby))] + .into_iter() + .chain(distinct) + { + questions.ask( + question.into(), + body, + id, + TEST_REDUNDANCY, + question, + Pass::Recheck, + ); + } + let mut paths = vec![(file.path, file.source_hash)]; + if found.path != file.path { + paths.push((found.path.as_path(), hash.as_str())); + } + let (request, asked) = crate::units::request(file.model, "recheck", &paths, state, questions); + file.budget.fits(&request).then_some((request, asked)) +} diff --git a/src/units/test_units/setup.rs b/src/units/test_units/setup.rs new file mode 100644 index 0000000..77cdf18 --- /dev/null +++ b/src/units/test_units/setup.rs @@ -0,0 +1,282 @@ +//! What runs before a test, read from its file's text: the setup head and hooks +//! of a test file, a Ruby test's groups, `let` and hooks, and the support file +//! nearest a test. +use super::*; + +/// Setup text before the first test, and each setup hook, above these sizes +/// is left out rather than cut. +pub(super) const SETUP_BYTES: usize = 4000; +pub(super) const HOOK_BYTES: usize = 1500; + +/// Test helpers shown with one Ruby case, at most. +pub(super) const HELPERS: usize = 4; + +/// A Ruby case's setup: the file's head, the hooks its groups declare, then +/// the test helpers the case and its hooks call, and the helpers those call. +/// Also the other files the helpers come from. +pub(super) fn ruby_setup( + file: &FileContext<'_>, + case: &TestCase, + head: Option, + helpers: &BTreeMap>, +) -> (String, Vec) { + let setup = case_setup(file.source, head, &case.hooks); + let mut names: Vec = case.calls.iter().chain(&case.hook_calls).cloned().collect(); + let mut shown: Vec<&SubjectSource> = Vec::new(); + let mut next = 0; + while next < names.len() && shown.len() < HELPERS { + let name = names[next].clone(); + next += 1; + let Some(defined) = helpers.get(&name) else { + continue; + }; + let found = nearest(file.path, defined); + let Some(helper) = found.filter(|h| { + h.source.len() <= HOOK_BYTES + && !setup.contains(h.source.as_str()) + && !shown.iter().any(|s| s.source == h.source) + }) else { + continue; + }; + shown.push(helper); + if let Some(tree) = crate::syntax::parse(&helper.path, &helper.source) + .ok() + .flatten() + { + let mut calls = Vec::new(); + crate::analysis::ruby::called_names(tree.root_node(), &helper.source, &mut calls); + names.extend(calls); + } + } + let mut parts: Vec = (!setup.is_empty()).then_some(setup).into_iter().collect(); + parts.extend(shown.iter().map(|h| h.source.clone())); + let paths = shown + .iter() + .filter(|h| h.path != file.path) + .map(|h| h.path.clone()) + .collect(); + (parts.join("\n\n"), paths) +} + +/// The one definition of a helper nearest the test: in its own file, else +/// in the support file that shares the most directories with it, at least +/// one, as `test/test_helper.rb` does with `test/routing_test.rb`. None when +/// two definitions are equally near. +pub(in crate::units) fn nearest<'a>( + test: &Path, + defined: &'a [SubjectSource], +) -> Option<&'a SubjectSource> { + let folders = |path: &Path| -> Vec { + path.parent() + .into_iter() + .flat_map(Path::components) + .map(|c| c.as_os_str().to_string_lossy().into_owned()) + .collect() + }; + let own = folders(test); + let shared = |helper: &SubjectSource| { + if helper.path == test { + return usize::MAX; + } + own.iter() + .zip(folders(&helper.path)) + .take_while(|(a, b)| **a == *b) + .count() + }; + // A support file in another tree, sharing no directory with the test, + // serves other tests: `test/test_helper.rb` is not a spec's helper. + let callable = || { + defined + .iter() + .filter(|h| h.path == test || h.shared && shared(h) > 0) + }; + let best = callable().map(shared).max()?; + let mut nearest = callable().filter(|h| shared(h) == best); + let first = nearest.next(); + nearest.next().is_none().then_some(first).flatten() +} + +/// One case's setup: the file's head, then the hooks its groups declare. A +/// hook larger than its limit is left out rather than cut, and so are the +/// hooks when together they are too long. +pub(super) fn case_setup(source: &str, head: Option, hooks: &[Range]) -> String { + let kept = usize::from(head.is_some()); + let mut parts: Vec = head.into_iter().collect(); + parts.extend( + hooks + .iter() + .map(|hook| source[hook.clone()].to_string()) + .filter(|text| text.len() <= HOOK_BYTES), + ); + let setup = parts.join("\n\n"); + if setup.len() <= SETUP_BYTES + HOOK_BYTES { + setup + } else { + parts.truncate(kept); + parts.join("") + } +} + +/// The hooks a case's groups declare, as sent beside a pair of tests. +pub(super) fn hook_text(source: &str, case: &TestCase) -> String { + case_setup(source, None, &case.hooks) +} + +/// A test file's shared setup: the text of its test region before the first +/// test or suite (imports, mocks, fixtures), then each setup hook. A part +/// larger than its limit is left out rather than cut. +pub(in crate::units) fn file_setup(source: &str, region_start: usize, first_case: usize) -> String { + let lines: Vec<&str> = source.lines().collect(); + let start = region_start.saturating_sub(1).min(lines.len()); + let mut parts: Vec = setup_head(&lines, start, first_case).into_iter().collect(); + parts.extend(setup_hooks(&lines[start..].join("\n"))); + let setup = parts.join("\n\n"); + if setup.len() <= SETUP_BYTES + HOOK_BYTES { + setup + } else { + parts.truncate(1); + parts.join("") + } +} + +/// The lines from `start` up to the first suite, test, test module or Java +/// setup method, when they are short enough to send. A Java test class's +/// fields, such as its mocks, are part of the head. +pub(super) fn setup_head(lines: &[&str], start: usize, first_case: usize) -> Option { + const OPENERS: &[&str] = &[ + "describe(", + "describe.", + "suite(", + "context(", + "test(", + "test.", + "it(", + "it.", + "def test", + "class ", + "mod tests", + "#[test]", + // Ruby: RSpec groups and examples, and Rails `test "…" do`. + "describe ", + "RSpec.describe", + "context ", + "it ", + "test ", + "module ", + // Java: setup methods and `@Nested` test classes. + "@Before", + "@Nested", + ]; + let last = first_case.saturating_sub(1).min(lines.len()); + let end = (start..last) + .find(|&i| { + let line = lines[i].trim_start(); + // A Python class opens a suite; a braced class holds the fields + // the tests share. + OPENERS.iter().any(|opener| line.starts_with(opener)) + && !(line.starts_with("class ") && line.trim_end().ends_with('{')) + }) + .unwrap_or(last); + let head = lines[start..end].join("\n"); + (!head.trim().is_empty() && head.len() <= SETUP_BYTES).then(|| head.trim().to_string()) +} + +/// Every setup hook in `region` short enough to send: `beforeEach`/`beforeAll` +/// calls, Python `setUp`/`setup_method` methods and Java methods annotated +/// `@BeforeEach`, `@BeforeAll`, `@Before` or `@BeforeClass`. +pub(super) fn setup_hooks(region: &str) -> Vec { + let mut hooks = Vec::new(); + for (at, _) in region.match_indices("@Before") { + let name_end = at + + region[at + 1..] + .find(|c: char| !c.is_alphanumeric()) + .map_or(region.len() - at, |i| i + 1); + if matches!( + ®ion[at..name_end], + "@BeforeEach" | "@BeforeAll" | "@Before" | "@BeforeClass" + ) { + let text = braced_method(®ion[at..]); + if text.len() <= HOOK_BYTES { + hooks.push(text.to_string()); + } + } + } + for hook in [ + "beforeEach(", + "beforeAll(", + "def setUp(", + "def setup_method(", + ] { + let mut from = 0; + while let Some(found) = region[from..].find(hook) { + let at = from + found; + let text = if hook.starts_with("def ") { + let line_start = region[..at].rfind('\n').map_or(0, |i| i + 1); + indented_block(®ion[at..], at - line_start) + } else { + balanced_call(®ion[at..]) + }; + if text.len() <= HOOK_BYTES { + hooks.push(text.to_string()); + } + from = at + hook.len(); + } + } + hooks +} + +/// A Java method from its annotation through the brace that closes its body. +pub(super) fn braced_method(text: &str) -> &str { + let Some(open) = text.find('{') else { + return text; + }; + let mut depth = 0usize; + for (i, c) in text[open..].char_indices() { + match c { + '{' => depth += 1, + '}' => { + depth -= 1; + if depth == 0 { + return &text[..open + i + 1]; + } + } + _ => {} + } + } + text +} + +/// A call from its name through the parenthesis that closes it. +pub(super) fn balanced_call(text: &str) -> &str { + let mut depth = 0usize; + for (i, c) in text.char_indices() { + match c { + '(' | '{' | '[' => depth += 1, + ')' | '}' | ']' => { + depth = depth.saturating_sub(1); + if depth == 0 { + return &text[..i + 1]; + } + } + _ => {} + } + } + text +} + +/// A Python definition line, indented `base` columns, and the lines indented below it. +pub(super) fn indented_block(text: &str, base: usize) -> &str { + let mut lines = text.split_inclusive('\n'); + let Some(first) = lines.next() else { + return text; + }; + let mut end = first.len(); + let indent = |line: &str| line.len() - line.trim_start().len(); + for line in lines { + if !line.trim().is_empty() && indent(line) <= base { + break; + } + end += line.len(); + } + text[..end].trim_end() +} From 7dc6ae591d64292eb9e8ea3e745f98c756d1cbcd Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sun, 27 Sep 2026 19:22:53 -0300 Subject: [PATCH 5/5] Split the security tests by what they test: PHP pages, checked kinds, exposure, settings, templates and confirms --- src/units/tests/security.rs | 1811 ------------------------- src/units/tests/security/confirms.rs | 181 +++ src/units/tests/security/exposure.rs | 266 ++++ src/units/tests/security/kinds.rs | 216 +++ src/units/tests/security/mod.rs | 539 ++++++++ src/units/tests/security/php.rs | 271 ++++ src/units/tests/security/settings.rs | 256 ++++ src/units/tests/security/templates.rs | 113 ++ 8 files changed, 1842 insertions(+), 1811 deletions(-) delete mode 100644 src/units/tests/security.rs create mode 100644 src/units/tests/security/confirms.rs create mode 100644 src/units/tests/security/exposure.rs create mode 100644 src/units/tests/security/kinds.rs create mode 100644 src/units/tests/security/mod.rs create mode 100644 src/units/tests/security/php.rs create mode 100644 src/units/tests/security/settings.rs create mode 100644 src/units/tests/security/templates.rs diff --git a/src/units/tests/security.rs b/src/units/tests/security.rs deleted file mode 100644 index 11ca6c2..0000000 --- a/src/units/tests/security.rs +++ /dev/null @@ -1,1811 +0,0 @@ -//! Injection, sensitive data and unsafe settings: traces, callers, settle -//! Choices and PHP pages. -use super::*; - -const QUERY: &str = "fn find(conn: &Connection, name: &str) -> Result {\n let sql = format!(\"SELECT id FROM users WHERE name = '{name}'\");\n conn.query_row(&sql, [], Row::from)\n}\n\nfn total(a: i32, b: i32) -> i32 {\n a + b\n}\n"; - -fn security_project(source: &str) -> (Project, CheckArgs) { - project_with(&[("lib.rs", source)], &catalog::SECURITY) -} - -/// A certain choice of `id` among the two sites of `find` in `QUERY`. -fn site(id: &str) -> Value { - let probabilities: serde_json::Map = ["S1", "S2", "none"] - .iter() - .map(|option| { - ( - option.to_string(), - json!(if *option == id { 1.0 } else { 0.0 }), - ) - }) - .collect(); - json!({"type":"choice","choice":id,"confidence":1.0,"probabilities":probabilities}) -} - -#[test] -fn only_functions_with_calls_built_text_or_field_assignments_are_sent_for_security() { - let (project, options) = security_project(QUERY); - let (_, plan) = planned(&project, &options); - let sent: Vec<&str> = plan - .requests - .iter() - .flat_map(|p| p.request["state"]["functions"].as_array().unwrap()) - .filter_map(|f| f["name"].as_str()) - .collect(); - assert_eq!( - sent, - ["find"], - "`total` has no call, built text or field assignment" - ); -} - -#[test] -fn clear_presence_needs_no_trace_and_clears_every_security_rule() { - let (project, options) = security_project(QUERY); - let mut eval = scripted(0); - let report = run(&project, &options, &mut eval); - assert_eq!(eval.stages, ["first"]); - for rule in catalog::SECURITY { - assert_eq!(report.files[0].dimensions[rule].status, Status::Clear); - } -} - -#[test] -fn an_unhandled_value_from_another_party_is_a_located_injection_review() { - let (project, options) = security_project(QUERY); - let mut eval = scripted(0); - eval.overrides = vec![ - ("interpreted", noul_at(0.95)), - ("sql", noul_at(0.95)), - ("origin", spread(0.0, 0.1, 0.9)), - ("site", site("S1")), - ]; - let report = run(&project, &options, &mut eval); - assert_eq!(eval.stages, ["first", "first"], "one trace, no recheck"); - let finding = &report.files[0].findings[0]; - assert_eq!(finding.rule, "security/injection"); - assert_eq!(finding.strength, Strength::Review); - assert_eq!(finding.category.as_deref(), Some("CWE-89 SQL injection")); - assert_eq!(finding.line, 2, "located at the chosen site"); - assert!(finding.action.contains("bound query parameters")); -} - -/// The options of the Choice on what an injection consider's values can hold. -const VALUES: [&str; 5] = ["fixed", "local", "outside", "own", "unknown"]; - -#[test] -fn a_parameter_origin_is_a_consider_that_callers_can_settle() { - let caller = format!( - "{QUERY}\nfn handler(conn: &Connection) -> Result {{\n find(conn, \"admin\")\n}}\n" - ); - let overrides = || { - vec![ - ("interpreted", noul_at(0.95)), - ("sql", noul_at(0.95)), - ("origin", spread(0.0, 0.9, 0.1)), - ("values", choice_of("unknown", &VALUES)), - ] - }; - let (project, options) = security_project(QUERY); - let mut eval = scripted(0); - eval.overrides = overrides(); - let report = run(&project, &options, &mut eval); - assert_eq!( - report.files[0].dimensions[catalog::INJECTION].status, - Status::Consider, - "no caller is known, so the parameter stays a concern" - ); - let (project, options) = security_project(&caller); - let mut eval = scripted(0); - eval.overrides = overrides(); - eval.recheck_level = Some(0); - let report = run(&project, &options, &mut eval); - assert!(eval.stages.contains(&"recheck".to_string())); - let find = |report: &Report| { - report.files[0] - .findings - .iter() - .any(|f| f.rule == "security/injection" && f.symbol.as_deref() == Some("find")) - }; - assert!(!find(&report), "the caller passes a fixed value"); -} - -#[test] -fn a_parameter_consider_is_a_note_when_its_values_are_the_programs_own() { - let caller = format!( - "{QUERY}\nfn handler(conn: &Connection) -> Result {{\n find(conn, \"admin\")\n}}\n" - ); - let (project, mut options) = security_project(&caller); - let mut judged = |values: Value| { - let mut eval = scripted(0); - eval.overrides = vec![ - ("interpreted", noul_at(0.95)), - ("sql", noul_at(0.95)), - ("origin", spread(0.0, 0.9, 0.1)), - ]; - // The recheck with callers keeps the parameters as the origin. - eval.recheck_overrides = vec![ - ("sql", noul_at(0.95)), - ("origin", spread(0.0, 0.9, 0.1)), - ("values", values), - ]; - let report = run(&project, &options, &mut eval); - options.refresh = true; - report.files[0] - .findings - .iter() - .find(|f| f.rule == "security/injection" && f.symbol.as_deref() == Some("find")) - .map(|f| f.strength) - }; - assert_eq!( - judged(choice_of("fixed", &VALUES)), - Some(Strength::Note), - "the caller passes a literal" - ); - assert_eq!( - judged(choice_of("outside", &VALUES)), - Some(Strength::Consider) - ); - let mut split: serde_json::Map = - VALUES.iter().map(|k| (k.to_string(), json!(0.0))).collect(); - split.insert("own".into(), json!(0.3)); - split.insert("local".into(), json!(0.25)); - split.insert("unknown".into(), json!(0.45)); - let leaning = - json!({"type":"choice","choice":"unknown","confidence":0.4,"probabilities":split}); - assert_eq!( - judged(leaning), - Some(Strength::Note), - "the program's own options together lead" - ); -} - -#[test] -fn a_check_left_undecided_is_decided_again_with_callers() { - let caller = format!( - "{QUERY}\nfn handler(conn: &Connection, request: &Request) -> Result {{\n find(conn, &request.query[\"name\"])\n}}\n" - ); - let (project, options) = security_project(&caller); - let mut eval = scripted(0); - eval.overrides = vec![ - ("resource", noul_at(0.95)), - ("path", noul_at(0.5)), - ("origin", spread(0.0, 0.9, 0.1)), - ]; - eval.recheck_level = Some(2); - let report = run(&project, &options, &mut eval); - let finding = report.files[0] - .findings - .iter() - .find(|f| f.symbol.as_deref() == Some("find") && f.rule == "security/injection") - .expect("the recheck decides the path check and the origin"); - assert_eq!( - finding.strength, - Strength::Review, - "undecided without the recheck" - ); - assert!(finding.category.is_some()); -} - -#[test] -fn a_parameter_in_a_path_or_url_is_a_note_until_callers_show_another_party() { - let (project, options) = security_project(QUERY); - let mut eval = scripted(0); - eval.overrides = vec![ - ("resource", noul_at(0.95)), - ("url", noul_at(0.95)), - ("origin", spread(0.0, 0.9, 0.1)), - ]; - let finding = &first_finding(&project, &options, &mut eval); - assert_eq!(finding.strength, Strength::Note); - assert_eq!( - finding.category.as_deref(), - Some("CWE-918 server-side request forgery") - ); -} - -const FETCH_QUOTE: &str = "fn quote(client: &Client, base: &Url, symbol: &str) -> String {\n let url = base.join(&format!(\"quotes/{symbol}\")).unwrap();\n client.get(url).send().unwrap().text().unwrap()\n}\n"; - -const URL_PARTS: [&str; 5] = ["own", "forwards", "given", "outside", "none"]; - -const PATH_SOURCE: [&str; 5] = ["own", "local", "given", "outside", "none"]; - -const RUNS_IN: [&str; 3] = ["browser", "server", "either"]; - -/// Injection status and settle requests with the URL (or path) check at -/// `check` undecided and the settle Choice answering `parts`. -fn settled_injection( - project: &Project, - options: &CheckArgs, - check: &'static str, - parts: &str, -) -> (Status, u64) { - let mut eval = scripted(0); - eval.overrides = vec![ - ("resource", noul_at(0.95)), - (check, noul_at(0.4)), - ("origin", spread(0.0, 0.9, 0.1)), - ("url_parts", choice_of(parts, &URL_PARTS)), - ("path_source", choice_of(parts, &PATH_SOURCE)), - ("runs_in", choice_of("server", &RUNS_IN)), - ]; - let report = run(project, options, &mut eval); - ( - report.files[0].dimensions[catalog::INJECTION] - .status - .clone(), - report - .stages - .get("settle") - .map_or(0, |stage| stage.successful_requests), - ) -} - -#[test] -fn an_undecided_url_is_settled_only_by_a_host_of_the_programs_own() { - let (project, mut options) = security_project(FETCH_QUOTE); - assert_eq!( - settled_injection(&project, &options, "url", "own"), - (Status::Clear, 2), - "where its URLs come from and where it runs" - ); - // A URL its caller gives or forwards is a note, as a found one would be; - // one from another party stays open. - for (parts, status) in [ - ("forwards", Status::Note), - ("given", Status::Note), - ("outside", Status::Uncertain), - ] { - options.refresh = true; - assert_eq!( - settled_injection(&project, &options, "url", parts).0, - status, - "{parts}" - ); - } -} - -#[test] -fn an_undecided_path_is_settled_by_the_programs_own_or_its_local_users_paths() { - let (project, mut options) = security_project(FETCH_QUOTE); - for (parts, status) in [ - ("own", Status::Clear), - ("local", Status::Clear), - ("given", Status::Note), - ("outside", Status::Uncertain), - ] { - assert_eq!( - settled_injection(&project, &options, "path", parts), - (status, 1), - "{parts}" - ); - options.refresh = true; - } - // The note names the path it left undecided. - let mut eval = scripted(0); - eval.overrides = vec![ - ("resource", noul_at(0.95)), - ("path", noul_at(0.4)), - ("origin", spread(0.0, 0.9, 0.1)), - ("path_source", choice_of("given", &PATH_SOURCE)), - ]; - let report = run(&project, &options, &mut eval); - let note = &report.files[0].findings[0]; - assert!( - note.message.contains("places a parameter into a file path"), - "{}", - note.message - ); -} - -#[test] -fn error_details_are_settled_by_where_the_text_goes() { - let (project, mut options) = security_project(QUERY); - // `own` 0.1 names a foreign message, which with a leaning exception would - // otherwise raise a consider. - let status = |destination: &str, (exception, own): (f64, f64), options: &CheckArgs| { - let mut eval = scripted(0); - eval.overrides = vec![ - ("error_details", noul_at(0.3)), - ("exception_to_client", noul_at(exception)), - ("own_messages", noul_at(own)), - ( - "destination", - choice_of( - destination, - &["client", "local", "logs", "caller", "stored"], - ), - ), - ]; - run(&project, options, &mut eval).files[0].dimensions[catalog::SENSITIVE_DATA] - .status - .clone() - }; - assert_eq!(status("local", (0.3, 0.5), &options), Status::Clear); - options.refresh = true; - assert_eq!(status("client", (0.3, 0.5), &options), Status::Uncertain); - assert_eq!( - status("local", (0.6, 0.1), &options), - Status::Clear, - "a foreign message that stays on the local terminal" - ); -} - -#[test] -fn checks_that_all_clear_rule_out_an_uncertain_presence() { - let (project, options) = security_project(QUERY); - let mut eval = scripted(0); - eval.overrides = vec![ - ("interpreted", noul_at(0.5)), - ("origin", spread(0.0, 1.0, 0.0)), - ]; - let report = run(&project, &options, &mut eval); - assert_eq!( - report.files[0].dimensions[catalog::INJECTION].status, - Status::Clear - ); -} - -#[test] -fn development_only_exposure_is_one_level_lower_and_names_its_weakness() { - let (project, options) = security_project(QUERY); - let report = run_with_nouls( - &project, - &options, - &[("logs_secret", 0.95), ("dev_only", 0.95)], - ); - let finding = &report.files[0].findings[0]; - assert_eq!(finding.rule, "security/sensitive-data"); - assert_eq!(finding.strength, Strength::Consider); - assert_eq!( - finding.category.as_deref(), - Some("CWE-532 sensitive data in logs") - ); - assert!(finding.message.contains("runs only in development")); -} - -#[test] -fn top_level_setup_is_one_unit_for_unsafe_settings() { - let (project, options) = project_with( - &[( - "server.ts", - "const app = express()\napp.use(cors({ origin: true, credentials: true }))\n", - )], - &[catalog::UNSAFE_SETTINGS], - ); - let (_, plan) = planned(&project, &options); - let request = &plan.requests[0].request; - assert!( - request["state"]["module"]["source"] - .as_str() - .unwrap() - .contains("app.use(cors(") - ); - let report = run_with_nouls(&project, &options, &[("weakened", 0.95), ("cors", 0.95)]); - let finding = &report.files[0].findings[0]; - assert_eq!(finding.category.as_deref(), Some("CWE-942 permissive CORS")); - assert!(finding.message.starts_with("Module setup")); -} - -const PAGE: &str = "' . $_GET['name'] . '

';\n}\n?>\n
\n"; - -#[test] -fn a_php_page_script_is_one_unit_judged_by_every_security_rule() { - let (project, options) = project_with(&[("page.php", PAGE)], &catalog::SECURITY); - let (_, plan) = planned(&project, &options); - let request = &plan.requests[0].request; - let functions = request["state"]["functions"].as_array().unwrap(); - let names: Vec<&str> = functions - .iter() - .filter_map(|f| f["name"].as_str()) - .collect(); - assert_eq!(names, ["escape_html", "top-level code"]); - let script = functions[1]["source"].as_str().unwrap(); - assert!(script.starts_with("require_once 'lib.php';\nif (isset($_GET['id']))")); - assert!(!script.contains("function escape_html") && !script.contains("