Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ see what a refactor actually did.
## What it approximates, and how

Every measurement here is either exact or documented as an approximation. The
four that are worth knowing about before you act on them:
five that are worth knowing about before you act on them:

- **Cyclomatic complexity** counts branches, not paths: `if`, `match` arms,
loops, `&&`, `||`, `?`. A `match` whose arms all name constants is a lookup
Expand All @@ -263,6 +263,11 @@ four that are worth knowing about before you act on them:
with the same name vouch for each other, so it under-reports rather than
over-reports — the right direction for a list somebody is going to act on.
Read `crates/tinyanalyzer-core/src/dead_code/mod.rs` before deleting anything.
- **Orphaned files** are `.rs` files under a crate's `src/` that no `mod`
declaration reachable from a target root loads. A package whose tree cannot
be seen whole — a computed `include!`, an unreadable file, a macro that may
declare the module — is not searched. Following the same declarations is how
a file reached only through `#[cfg(test)]` is recognized as test code.
- **Unused dependencies** are dependencies no source file *names*. A crate
reached through a macro expansion, a build script, or a linker side effect has
no `use` naming it. Remove and build; if the build fails, add it to
Expand Down Expand Up @@ -301,6 +306,7 @@ crates/
│ ├── walk/ # which files are in scope
│ ├── loc/ # code, comment, and blank lines
│ ├── rust_source/ # syn-based items, complexity, cost signals
│ ├── module_tree/ # mod declarations: orphaned and test-only files
│ ├── deps/ # the resolved dependency graph and its real cost
│ ├── dead_code/ # the workspace-wide identifier census
│ ├── clones/ # tree-sitter clone detection and the symbol index
Expand Down
6 changes: 5 additions & 1 deletion crates/tinyanalyzer-core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ conversion at every call site that nothing checks.
| `walk` | Which files are in scope? |
| `loc` | How much of each file is code, comment, and blank? |
| `rust_source` | What does each Rust file define, and how tangled is it? |
| `module_tree` | Which files does no crate root reach, and which only tests? |
| `deps` | What does the dependency graph actually cost? |
| `dead_code` | What does nothing reference? |
| `findings` | What should somebody do about all this? |
Expand All @@ -55,7 +56,10 @@ name resolution. That is what lets it see through macro invocations, which an AS
walk cannot. The cost is that two unrelated items sharing a name vouch for each
other, so it under-reports rather than over-reports — the right direction for a
list a human will act on. Public items come back at medium confidence because
their callers may not be in this workspace at all.
their callers may not be in this workspace at all. A file no `mod` declaration
reaches from a target root (`module_tree`) is reported whole, as one `file`
candidate and an `orphan_file` finding, and only when the package's tree could
be followed completely.

**Unused dependencies** (`deps`) are dependencies no source file *names*. A crate
reached only through a derive macro or a linker side effect has no `use` naming
Expand Down
3 changes: 3 additions & 0 deletions crates/tinyanalyzer-core/src/config/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ impl Default for ScanConfig {
"**/test.rs".to_owned(),
"**/tests.rs".to_owned(),
"**/*_test.rs".to_owned(),
// The sibling-file convention:
// `#[cfg(test)] #[path = "foo_tests.rs"] mod tests;`.
"**/*_tests.rs".to_owned(),
"**/benches/**".to_owned(),
],
respect_gitignore: true,
Expand Down
49 changes: 47 additions & 2 deletions crates/tinyanalyzer-core/src/dead_code/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,17 @@
//! namespace, not a symbol: it compiles its contents whether or not anything
//! names the module itself, so "unreferenced module" would be true of almost
//! every module in a well-organized crate and would mean nothing.
//!
//! What *is* reported is a file no `mod` declaration reaches at all: it is not
//! compiled, so everything in it is dead. [`crate::report`] adds each one as a
//! single [`DefinitionKind::File`] candidate rather than one per item inside.

mod types;

pub use types::{Confidence, DeadCodeCandidate, DeadCodeInput};

use crate::config::DeadCodeConfig;
use crate::module_tree::OrphanFile;
use crate::rust_source::DefinitionKind;
use std::collections::{BTreeMap, BTreeSet};

Expand Down Expand Up @@ -118,15 +123,55 @@ pub fn analyze(files: &[DeadCodeInput<'_>], config: &DeadCodeConfig) -> Vec<Dead
}
}

rank(&mut candidates);
candidates
}

/// Adds one candidate per orphaned file to `candidates`, keeping the ranking.
///
/// An orphan is reported as a whole, at [`Confidence::High`]: no `mod`
/// declaration reachable from a crate root loads it, so nothing compiles it.
/// The caller leaves orphaned files out of the census, so their items neither
/// vouch for anything nor appear one by one.
pub(crate) fn add_orphans(
candidates: &mut Vec<DeadCodeCandidate>,
orphans: &[OrphanFile],
is_test: impl Fn(&str) -> bool,
) {
for orphan in orphans {
let name = orphan
.path
.rsplit('/')
.find(|part| *part != "mod.rs")
.unwrap_or(&orphan.path)
.trim_end_matches(".rs")
.to_owned();
candidates.push(DeadCodeCandidate {
reason: format!(
"no `mod` declaration reachable from a target root of `{}` loads this file, so it is never compiled",
orphan.crate_name
),
name,
kind: DefinitionKind::File,
file: orphan.path.clone(),
line: 1,
is_public: false,
is_test: is_test(&orphan.path),
confidence: Confidence::High,
});
}
rank(candidates);
}

/// Sorts by confidence, then by file, line, and name.
fn rank(candidates: &mut [DeadCodeCandidate]) {
candidates.sort_by(|left, right| {
left.confidence
.cmp(&right.confidence)
.then_with(|| left.file.cmp(&right.file))
.then_with(|| left.line.cmp(&right.line))
.then_with(|| left.name.cmp(&right.name))
});

candidates
}

/// Total identifier occurrences across every counted file.
Expand Down
54 changes: 54 additions & 0 deletions crates/tinyanalyzer-core/src/dead_code/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,57 @@ fn every_confidence_level_has_a_label() {
assert_eq!(Confidence::High.label(), "high");
assert_eq!(Confidence::Medium.label(), "medium");
}

#[test]
fn an_orphaned_file_is_added_once_named_after_its_module() {
let file = parsed("fn kept() {}\nfn helper() { kept(); }\n");
let inputs = [DeadCodeInput {
path: "src/lib.rs",
rust: &file,
is_test_file: false,
}];
let mut found = analyze(&inputs, &DeadCodeConfig::default());
let orphans = [
crate::module_tree::OrphanFile {
path: "src/stale/mod.rs".to_owned(),
crate_name: "demo".to_owned(),
},
crate::module_tree::OrphanFile {
path: "src/old_tests.rs".to_owned(),
crate_name: "demo".to_owned(),
},
];

super::add_orphans(&mut found, &orphans, |path| path.ends_with("_tests.rs"));

let files: Vec<(&str, &str, bool)> = found
.iter()
.filter(|candidate| candidate.kind == crate::rust_source::DefinitionKind::File)
.map(|candidate| {
(
candidate.name.as_str(),
candidate.file.as_str(),
candidate.is_test,
)
})
.collect();
assert_eq!(
files,
[
("old_tests", "src/old_tests.rs", true),
("stale", "src/stale/mod.rs", false)
]
);
assert!(
found
.iter()
.all(|candidate| candidate.confidence == Confidence::High)
);
assert!(
found
.iter()
.any(|candidate| candidate.reason.contains("`demo`"))
);
// Ranking is kept: the item candidate in `src/lib.rs` still sorts first.
assert_eq!(found[0].name, "helper");
}
49 changes: 45 additions & 4 deletions crates/tinyanalyzer-core/src/findings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::config::Thresholds;
use crate::dead_code::{Confidence, DeadCodeCandidate};
use crate::deps::DependencyReport;
use crate::report::{DirectoryMetrics, FileMetrics, ParseFailureReport};
use crate::rust_source::{Function, RustFile};
use crate::rust_source::{DefinitionKind, Function, RustFile};

/// Block nesting at or beyond which a function is worth flattening.
///
Expand Down Expand Up @@ -69,6 +69,7 @@ pub fn analyze(inputs: FindingInputs<'_>, thresholds: &Thresholds) -> Vec<Findin
directories(inputs.directories, thresholds, &mut findings);
dependencies(inputs.dependencies, thresholds, &mut findings);
dead_code(inputs.dead_code, &mut findings);
orphan_files(inputs.dead_code, inputs.files, &mut findings);
parse_failures(inputs.parse_failures, &mut findings);
duplicate_code(inputs.clones, thresholds, &mut findings);

Expand Down Expand Up @@ -450,8 +451,15 @@ fn dependencies(report: &DependencyReport, thresholds: &Thresholds, out: &mut Ve
/// One finding for the whole list rather than one per item: a hundred separate
/// findings would bury every other rule, and the action — read the list, delete
/// what is genuinely dead — is the same for all of them.
///
/// Orphaned files are left to [`orphan_files`], which reports each one: a
/// whole file nothing compiles is a different action from a stray function.
fn dead_code(candidates: &[DeadCodeCandidate], out: &mut Vec<Finding>) {
let certain = candidates
let items: Vec<&DeadCodeCandidate> = candidates
.iter()
.filter(|candidate| candidate.kind != DefinitionKind::File)
.collect();
let certain = items
.iter()
.filter(|candidate| candidate.confidence == Confidence::High)
.count();
Expand All @@ -460,7 +468,7 @@ fn dead_code(candidates: &[DeadCodeCandidate], out: &mut Vec<Finding>) {
return;
}

let example = candidates
let example = items
.iter()
.find(|candidate| candidate.confidence == Confidence::High);

Expand All @@ -470,14 +478,47 @@ fn dead_code(candidates: &[DeadCodeCandidate], out: &mut Vec<Finding>) {
format!("{certain} items are referenced by nothing"),
format!(
"{certain} private items have no reference anywhere in the workspace, out of {} candidates in total.",
candidates.len()
items.len()
),
"Delete them. Anything worth keeping for later is in the history; anything reached only through a macro belongs in the dead-code `ignore` list so the report stays worth reading.".to_owned(),
example.map(|candidate| at_line(&candidate.file, candidate.line)),
metric(certain),
));
}

/// Flags every source file no crate root reaches.
///
/// One finding per file, unlike [`dead_code`]: orphans are rare, each one is a
/// whole file, and the remedy names the file and the module it would be.
fn orphan_files(candidates: &[DeadCodeCandidate], files: &[FileMetrics], out: &mut Vec<Finding>) {
for candidate in candidates
.iter()
.filter(|candidate| candidate.kind == DefinitionKind::File)
{
let lines = files
.iter()
.find(|file| file.path == candidate.file)
.map_or(0, |file| file.lines.code);

out.push(finding(
Rule::OrphanFile,
Severity::High,
format!("{} is compiled by nothing", candidate.file),
format!(
"{lines} line{} of code that no `mod` declaration reachable from a target root loads, so the compiler never builds, tests, or lints {}.",
plural(lines),
if lines == 1 { "it" } else { "them" }
),
format!(
"Delete it if it is left over from a move or a rename; if it is meant to be live, declare it with `mod {};` in its parent module so the compiler checks it again.",
candidate.name
),
Some(at_file(&candidate.file)),
metric(lines),
));
}
}

/// Flags files the parser refused.
///
/// A parse failure is reported rather than swallowed because it silently
Expand Down
46 changes: 46 additions & 0 deletions crates/tinyanalyzer-core/src/findings/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,8 @@ fn every_rule_has_an_identifier_and_a_description() {
Rule::PanicPath,
Rule::UnfinishedWork,
Rule::ParseFailure,
Rule::DuplicateCode,
Rule::OrphanFile,
] {
assert_ne!(rule.id().len(), 0);
assert_ne!(rule.description().len(), 0);
Expand Down Expand Up @@ -615,3 +617,47 @@ fn test_only_or_read_only_duplicates_are_not_findings() {
0
);
}

#[test]
fn each_orphaned_file_is_its_own_finding_and_not_part_of_the_dead_code_count() {
let orphan = |path: &str| DeadCodeCandidate {
name: "stale".to_owned(),
kind: DefinitionKind::File,
file: path.to_owned(),
line: 1,
is_public: false,
is_test: false,
confidence: Confidence::High,
reason: "nothing loads it".to_owned(),
};
let candidates = [orphan("src/stale.rs"), orphan("src/one.rs")];
let files = [
file("src/stale.rs", lines(12, 0), None),
file("src/one.rs", lines(1, 0), None),
];

let findings = analyze(
FindingInputs {
files: &files,
directories: &[],
dependencies: &DependencyReport::default(),
dead_code: &candidates,
parse_failures: &[],
clones: &[],
},
&Thresholds::default(),
);

assert!(!rules(&findings).contains(&Rule::DeadCode));
let orphans: Vec<_> = findings
.iter()
.filter(|finding| finding.rule == Rule::OrphanFile)
.collect();
assert_eq!(orphans.len(), 2);
assert_eq!(orphans[0].severity, Severity::High);
assert_eq!(orphans[0].title, "src/stale.rs is compiled by nothing");
assert!(orphans[0].detail.starts_with("12 lines of code"));
assert!(orphans[0].suggestion.contains("`mod stale;`"));
assert!(orphans[1].detail.starts_with("1 line of code"));
assert!(orphans[1].detail.ends_with("lints it."));
}
6 changes: 6 additions & 0 deletions crates/tinyanalyzer-core/src/findings/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ pub enum Rule {
ParseFailure,
/// Code written more than once.
DuplicateCode,
/// A source file no crate root reaches, so it is never compiled.
OrphanFile,
}

impl Rule {
Expand All @@ -128,6 +130,7 @@ impl Rule {
Self::UnfinishedWork => "unfinished_work",
Self::ParseFailure => "parse_failure",
Self::DuplicateCode => "duplicate_code",
Self::OrphanFile => "orphan_file",
}
}

Expand All @@ -152,6 +155,9 @@ impl Rule {
Self::UnfinishedWork => "unfinished-work markers left in comments",
Self::ParseFailure => "files the Rust parser refused",
Self::DuplicateCode => "code written more than once that one helper could replace",
Self::OrphanFile => {
"source files no `mod` declaration reaches, so nothing compiles them"
}
}
}
}
2 changes: 2 additions & 0 deletions crates/tinyanalyzer-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
//! | [`loc`] | How much of each file is code, comment, and blank? |
//! | [`rust_source`] | What does each Rust file define, and how tangled is it? |
//! | [`deps`] | What does the dependency graph actually cost? |
//! | `module_tree` | Which files does no crate root reach, and which only tests? |
//! | [`dead_code`] | What does nothing reference? |
//! | [`clones`] | What is written more than once, and what would replace it? |
//! | [`findings`] | What should somebody do about all this? |
Expand Down Expand Up @@ -60,6 +61,7 @@ pub mod deps;
pub mod error;
pub mod findings;
pub mod loc;
mod module_tree;
pub mod report;
pub mod rust_source;
pub mod walk;
Expand Down
Loading
Loading