Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Options:
--dump Dump sqlite database contents to the console
--dump-candidate-counts Dump number of removal candidates in each file and exit
--dump-candidates Dump removal candidates and exit (for debugging)
--filtered Restrict --dump to entries that are still removal candidates and whose recorded text still matches what Necessist read at their span; the candidates are the ones the framework finds in the paths given on the command line, or under the project root when none is given, by the same file selection and ignore rules a normal run uses, so naming fewer paths can hide more entries
--find-skill Check whether the `necessist-audit` skill is installed in a well-known directory, and whether it is the current version or later, exiting with code 1 if it is not; add --write to update an outdated skill
--framework <FRAMEWORK> Assume testing framework is <FRAMEWORK> [possible values: anchor, auto, foundry, go, hardhat, php, python, rust, vitest]
--no-lines-or-columns Do not output line or column information (experimental)
Expand Down
11 changes: 11 additions & 0 deletions core/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ pub struct Opts<Identifier: Clone + Send + Sync + ValueEnum + 'static> {
dump_candidate_counts: bool,
#[clap(long, help = "Dump removal candidates and exit (for debugging)")]
dump_candidates: bool,
#[clap(
long,
help = "Restrict --dump to entries that are still removal candidates and whose recorded \
text still matches what Necessist read at their span; the candidates are the ones \
the framework finds in the paths given on the command line, or under the project \
root when none is given, by the same file selection and ignore rules a normal run \
uses, so naming fewer paths can hide more entries"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uses, so naming fewer paths can hide more entries"
help = "Restrict --dump to entries that are still removal candidates according to the current configuration file"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. This was meant to be part of a larger review. I did plan to request this change, though.

)]
filtered: bool,
#[clap(
long,
help = "Check whether the `necessist-audit` skill is installed in a well-known directory, \
Expand Down Expand Up @@ -100,6 +109,7 @@ impl<Identifier: Clone + Send + Sync + ValueEnum> From<Opts<Identifier>>
dump,
dump_candidate_counts,
dump_candidates,
filtered,
find_skill,
framework,
no_lines_or_columns,
Expand Down Expand Up @@ -127,6 +137,7 @@ impl<Identifier: Clone + Send + Sync + ValueEnum> From<Opts<Identifier>>
dump,
dump_candidate_counts,
dump_candidates,
filtered,
find_skill,
no_lines_or_columns,
no_local_functions,
Expand Down
91 changes: 90 additions & 1 deletion core/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ pub struct Necessist {
pub dump: bool,
pub dump_candidate_counts: bool,
pub dump_candidates: bool,
pub filtered: bool,
pub find_skill: bool,
pub no_lines_or_columns: bool,
pub no_local_functions: bool,
Expand Down Expand Up @@ -220,7 +221,11 @@ fn prepare<Identifier: Applicable + Display + IntoEnumIterator + ToImplementatio

let config = config::Toml::read(context, context.root)?;

if context.opts.dump {
// An unfiltered `--dump` returns here, before a backend is resolved and before any source is
// parsed, so it still works on a project whose framework cannot be determined or whose sources
// no longer parse. `--dump --filtered` needs the candidates those steps produce, so it falls
// through to the branch below.
if context.opts.dump && !context.opts.filtered {
let past_removals = past_removals_init_lazy(context)?;
dump(context, &past_removals);
return Ok(None);
Expand Down Expand Up @@ -262,6 +267,12 @@ fn prepare<Identifier: Applicable + Display + IntoEnumIterator + ToImplementatio
return Ok(None);
}

if context.opts.dump && context.opts.filtered {
let past_removals = past_removals_init_lazy(context)?;
dump_filtered(context, &past_removals, &source_file_span_test_map);
return Ok(None);
}

// smoelius: Curious. The code used to look like:
// ```
// (context.println)({
Expand Down Expand Up @@ -505,6 +516,13 @@ fn process_options(opts: &Necessist) -> Result<()> {
// smoelius: `--quiet` and `--verbose` are contradictory.
incompatible!(opts, quiet, verbose);

// `--filtered` modifies `--dump`; on its own it has nothing to act on. Stating this as a
// requirement rather than as pairwise incompatibilities also gives `--filtered` every
// restriction `--dump` has, without repeating any of them.
if opts.filtered && !opts.dump {
bail!("--filtered can be used only with --dump");
}

if opts.write && opts.check_skill.is_none() && !opts.find_skill {
bail!("--write can be used only with --check-skill <PATH> or --find-skill");
}
Expand Down Expand Up @@ -554,6 +572,77 @@ fn dump(context: &LightContext, removals: &[Removal]) {
}
}

fn dump_filtered(
context: &LightContext,
removals: &[Removal],
source_file_span_test_map: &SourceFileSpanTestMap,
) {
let mut other_than_passed = false;
let mut n_hidden = 0;
for removal in removals {
let would_print = context.opts.verbose || removal.outcome == Outcome::Passed;
if is_current_candidate(source_file_span_test_map, removal) {
emit_to_console(context, removal);
other_than_passed |= removal.outcome != Outcome::Passed;
} else if would_print {
n_hidden += 1;
}
}

note(
context,
"an entry is shown when its span is still a removal candidate and the text Necessist read \
at that span matches the text recorded with the entry; the outcome shown is the recorded \
one, not one re-established against the current source",
);

if n_hidden > 0 {
note(
context,
&format!(
"{n_hidden} entr{} hidden by filtering",
if n_hidden == 1 { "y was" } else { "ies were" }
),
);
}

if !context.opts.verbose && other_than_passed {
note(context, "more output would be produced with --verbose");
}
}

/// Returns true if `removal`'s span is a removal candidate in `source_file_span_test_map` and the
/// source text at that span still matches `removal`'s stored text
///
/// Being a candidate does not put a span within the contents `SourceFile` cached: those are read
/// once per path, while a backend rereads the file when it parses it. `Span::source_text` reports
/// some such spans as an error and others as an `Ok` holding the wrong text, an overlong column
/// being clamped rather than rejected; an `Ok` is no evidence that the span is current, and the
/// comparison below decides either way.
///
/// The candidate check stays first for two reasons besides cost. It keeps `source_text` from being
/// called on a span this parse did not produce. And computing a span's offsets asserts that if the
/// text up to its end is all ASCII, so is the text up to its start -- automatic when the end
/// follows the start, but an entry read back from the database can hold a span where it is not.
fn is_current_candidate(
source_file_span_test_map: &SourceFileSpanTestMap,
removal: &Removal,
) -> bool {
let Removal { span, text, .. } = removal;

let Some(span_test_maps) = source_file_span_test_map.get(&span.source_file) else {
return false;
};

if !span_test_maps.statement.contains_key(span)
&& !span_test_maps.method_call.contains_key(span)
{
return false;
}

span.source_text().is_ok_and(|current| current == *text)
}

fn backend_for_framework<Identifier: Applicable + Display + IntoEnumIterator + ToImplementation>(
context: &LightContext,
identifier: framework::Auto<Identifier>,
Expand Down
1 change: 1 addition & 0 deletions fixtures/basic_appended/.hidden/hidden.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This deliberately invalid Rust file verifies that hidden paths are not walked.
1 change: 1 addition & 0 deletions fixtures/basic_appended/.ignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ignored/
7 changes: 7 additions & 0 deletions fixtures/basic_appended/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "basic_appended"
version = "0.1.0"
edition = "2024"
publish = false

[workspace]
1 change: 1 addition & 0 deletions fixtures/basic_appended/ignored/ignored.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This deliberately invalid Rust file verifies that `.ignore` files are respected.
37 changes: 37 additions & 0 deletions fixtures/basic_appended/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#[test]
fn passed() {
let mut n = 0;
n += 1;
noop();
}

fn noop() {}

#[test]
fn timed_out() {
let mut n = 0;
while n < 1 {
n += 1;
}
}

#[test]
fn failed() {
let mut n = 0;
n += 1;
assert!(n >= 1);
}

#[test]
fn nonbuildable() {
let _ = |xs: &[&str]| -> String {
return xs.join("");
};
}

#[test]
fn appended() {
let mut s = String::new();
s.push_str("x");
assert!(!s.is_empty());
}
1 change: 1 addition & 0 deletions fixtures/basic_no_candidates/.hidden/hidden.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This deliberately invalid Rust file verifies that hidden paths are not walked.
1 change: 1 addition & 0 deletions fixtures/basic_no_candidates/.ignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ignored/
7 changes: 7 additions & 0 deletions fixtures/basic_no_candidates/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "basic_no_candidates"
version = "0.1.0"
edition = "2024"
publish = false

[workspace]
1 change: 1 addition & 0 deletions fixtures/basic_no_candidates/ignored/ignored.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This deliberately invalid Rust file verifies that `.ignore` files are respected.
1 change: 1 addition & 0 deletions fixtures/basic_no_candidates/necessist.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ignored_tests = ["passed", "timed_out", "failed", "nonbuildable"]
30 changes: 30 additions & 0 deletions fixtures/basic_no_candidates/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#[test]
fn passed() {
let mut n = 0;
n += 1;
noop();
}

fn noop() {}

#[test]
fn timed_out() {
let mut n = 0;
while n < 1 {
n += 1;
}
}

#[test]
fn failed() {
let mut n = 0;
n += 1;
assert!(n >= 1);
}

#[test]
fn nonbuildable() {
let _ = |xs: &[&str]| -> String {
return xs.join("");
};
}
7 changes: 7 additions & 0 deletions fixtures/case/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "case"
version = "0.1.0"
edition = "2024"
publish = false

[workspace]
7 changes: 7 additions & 0 deletions fixtures/case/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#[test]
fn cased() {
let mut s = String::new();
s.push_str("ab");
s.push_str("cd");
assert!(!s.is_empty());
}
7 changes: 7 additions & 0 deletions fixtures/dry_run_failure_method_call/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dry_run_failure_method_call"
version = "0.1.0"
edition = "2024"
publish = false

[workspace]
1 change: 1 addition & 0 deletions fixtures/dry_run_failure_method_call/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

7 changes: 7 additions & 0 deletions fixtures/dry_run_failure_method_call/tests/a.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#[test]
fn dry_run_failed() {
let mut n = 0;
n += 1;
let _ = ["x"].join("");
assert!(n >= 2);
}
8 changes: 8 additions & 0 deletions fixtures/dry_run_failure_method_call/tests/b.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#[test]
fn passed() {
let mut n = 0;
n += 1;
noop();
}

fn noop() {}
7 changes: 7 additions & 0 deletions fixtures/ignored_function/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "ignored_function"
version = "0.1.0"
edition = "2024"
publish = false

[workspace]
13 changes: 13 additions & 0 deletions fixtures/ignored_function/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn ignored_function<T>(_: T) {}

fn foo() -> u32 {
1
}

#[test]
fn test() {
let mut n = 0;
n += 1;
ignored_function(foo());
assert!(n > 0);
}
7 changes: 7 additions & 0 deletions fixtures/marker/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "marker"
version = "0.1.0"
edition = "2024"
publish = false

[workspace]
1 change: 1 addition & 0 deletions fixtures/marker/sentinel
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
alive
8 changes: 8 additions & 0 deletions fixtures/marker/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// The test deletes `sentinel`, so the file survives only if the test never runs.
#[test]
fn marks() {
let _ = std::fs::remove_file("sentinel");
let mut n = 0;
n += 1;
assert!(n > 0);
}
7 changes: 7 additions & 0 deletions fixtures/whitespace/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "whitespace"
version = "0.1.0"
edition = "2024"
publish = false

[workspace]
11 changes: 11 additions & 0 deletions fixtures/whitespace/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#[test]
fn multiline() {
let mut v = Vec::new();
v.extend(
[1, 2],
);
v.push(
3,
);
assert_eq!(3, v.len());
}
Loading
Loading