From bb103dae27e0fed5f0e2610e74ed3bad4171ee36 Mon Sep 17 00:00:00 2001 From: Brad Byrd Date: Fri, 25 Sep 2026 02:42:51 -0500 Subject: [PATCH 1/4] test: reproduce include and assertion analyzer warnings --- tests/analyzer_include_expect_test.rs | 91 +++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 tests/analyzer_include_expect_test.rs diff --git a/tests/analyzer_include_expect_test.rs b/tests/analyzer_include_expect_test.rs new file mode 100644 index 00000000..a8a3fd8f --- /dev/null +++ b/tests/analyzer_include_expect_test.rs @@ -0,0 +1,91 @@ +use std::fs; +use std::path::Path; +use std::process::Command; +use tempfile::TempDir; + +fn analyze(path: &Path) -> String { + let output = Command::new(env!("CARGO_BIN_EXE_wfl")) + .arg("--analyze") + .arg(path) + .output() + .expect("run WFL analyzer"); + format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ) +} + +#[test] +fn transitive_literal_include_actions_do_not_warn_but_typos_do() { + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("leaf.wfl"), + "define action called greet with parameters name:\n return name\nend action\n", + ) + .unwrap(); + fs::write(dir.path().join("middle.wfl"), "include from \"leaf.wfl\"\n").unwrap(); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + "include from \"middle.wfl\"\nstore good as call greet with \"Ada\"\nstore bad as call grret with \"Ada\"\n", + ) + .unwrap(); + + let output = analyze(&main); + assert!( + !output.contains("Undefined action 'greet'"), + "included action was reported undefined: {output}" + ); + assert!( + output.contains("Undefined action 'grret'"), + "genuine typo was hidden: {output}" + ); +} + +#[test] +fn dynamic_include_keeps_unresolved_action_warning() { + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("module.wfl"), + "define action called greet:\n return \"hi\"\nend action\n", + ) + .unwrap(); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + "store module_path as \"module.wfl\"\ninclude from module_path\nstore good as call greet\n", + ) + .unwrap(); + + let output = analyze(&main); + assert!( + output.contains("Undefined action 'greet'"), + "a dynamic include cannot be resolved statically: {output}" + ); +} + +#[test] +fn expect_subject_and_expected_value_count_as_variable_uses() { + let dir = TempDir::new().expect("tempdir"); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + "store measured as 2\nstore expected_value as 2\nstore dead as 0\ndescribe \"uses\":\n test \"assert\":\n expect measured to equal expected_value\n end test\nend describe\n", + ) + .unwrap(); + + let output = analyze(&main); + assert!( + !output.contains("Unused variable 'measured'"), + "assertion subject was reported unused: {output}" + ); + assert!( + !output.contains("Unused variable 'expected_value'"), + "assertion expected value was reported unused: {output}" + ); + assert!( + output.contains("Unused variable 'dead'"), + "genuinely unused variable was hidden: {output}" + ); +} From afe471bb9afed730f495e61eb0d06d3089db2f6f Mon Sep 17 00:00:00 2001 From: Brad Byrd Date: Fri, 25 Sep 2026 02:43:10 -0500 Subject: [PATCH 2/4] fix: resolve literal includes and assertion uses in analyzer --- Docs/04-advanced-features/modules.md | 2 + ...-09-25-included-actions-and-expect-uses.md | 9 ++ src/analyzer/static_analyzer.rs | 22 ++- src/main.rs | 136 +++++++++++++++++- 4 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 History/dev-diary/2026/2026-09-25-included-actions-and-expect-uses.md diff --git a/Docs/04-advanced-features/modules.md b/Docs/04-advanced-features/modules.md index 89d14fd7..181e1344 100644 --- a/Docs/04-advanced-features/modules.md +++ b/Docs/04-advanced-features/modules.md @@ -151,6 +151,8 @@ still reported as a circular dependency (see Included files go through the same pipeline as the main program (parse, analyze, type check). Because `include from` runs the file in the parent scope — as if the code were written in the main program — type-check findings in an included file are reported the same way as in the main file: as **non-fatal warnings**. The program still runs. +For a literal `include from "path.wfl"`, the CLI checks action names in that file and its transitive literal includes before reporting an undefined-action warning. It does not infer the included action's call signature at this stage; the included file still goes through its normal checks when executed. A dynamic path, an unreadable file, or a file that cannot be parsed remains unresolved, so calls to its actions may still warn. Misspelled action names continue to warn. + ```text Type checking warnings in included file 'mod.wfl': error[ERROR]: Could not infer type for variable 'v' diff --git a/History/dev-diary/2026/2026-09-25-included-actions-and-expect-uses.md b/History/dev-diary/2026/2026-09-25-included-actions-and-expect-uses.md new file mode 100644 index 00000000..87db6d84 --- /dev/null +++ b/History/dev-diary/2026/2026-09-25-included-actions-and-expect-uses.md @@ -0,0 +1,9 @@ +# Included action and assertion warning fixes + +Logbie-web's seven WFL suites produced 316 `ANALYZE-SEMANTIC` warnings for actions supplied by `include from` and 126 `ANALYZE-UNUSED` warnings. The actions existed and the tests passed. The CLI analyzed only the entry file before executing includes, while the unused-variable visitor skipped `expect` statements entirely. + +The CLI now scans literal top-level includes transitively when undefined-action warnings exist. It parses each included file under the run's source and import limits, recognizes action definitions by name, and removes only matching warnings. Dynamic paths and unknown names retain their warnings. The unused-variable visitor now marks the assertion subject and expression-valued expected operands as uses. These changes do not execute includes during analysis or alter runtime include behavior. + +Three focused real-binary regressions were written first. Before the fix, the literal-include and assertion-use cases failed; the dynamic-include case passed. After the fix, all three pass. Against the seven Logbie-web suites, the patched WFL binary removes all 316 false undefined-action warnings and 90 false unused-variable warnings. The other 36 unused names were ignored return values in three Logbie-web test files; those bindings were replaced with direct calls, leaving zero analyzer warnings across all seven suites. The account suite's file-backed SQLite case requires a writable fixture directory outside the sandbox. + +The red test-only ancestor is commit bb103dae. diff --git a/src/analyzer/static_analyzer.rs b/src/analyzer/static_analyzer.rs index a384b161..26dcbf31 100644 --- a/src/analyzer/static_analyzer.rs +++ b/src/analyzer/static_analyzer.rs @@ -1,6 +1,6 @@ use super::Analyzer; use crate::diagnostics::{Severity, WflDiagnostic}; -use crate::parser::ast::{Expression, Program, Statement, Type}; +use crate::parser::ast::{Assertion, Expression, Program, Statement, Type}; use std::collections::{HashMap, HashSet}; #[derive(Debug, Clone)] @@ -1304,6 +1304,26 @@ impl Analyzer { self.mark_used_variables(stmt, usages); } } + Statement::ExpectStatement { + subject, assertion, .. + } => { + self.mark_used_in_expression(subject, usages); + match assertion { + Assertion::Equal(expected) + | Assertion::Be(expected) + | Assertion::GreaterThan(expected) + | Assertion::LessThan(expected) + | Assertion::Contain(expected) + | Assertion::HaveLength(expected) => { + self.mark_used_in_expression(expected, usages); + } + Assertion::BeYes + | Assertion::BeNo + | Assertion::Exist + | Assertion::BeEmpty + | Assertion::BeOfType(_) => {} + } + } // Compound-assignment / list statements read (and write) their // operand variables — count them as uses. Statement::AddToListStatement { diff --git a/src/main.rs b/src/main.rs index ad6e43a2..f6313368 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,18 +1,20 @@ +use std::collections::HashSet; use std::env; use std::fs; use std::io::{self, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process; use std::time::Instant; use wfl::Interpreter; use wfl::analyzer::{Analyzer, StaticAnalyzer}; use wfl::config; use wfl::debug_report; -use wfl::diagnostics::{DiagnosticReporter, Severity}; +use wfl::diagnostics::{DiagnosticReporter, Severity, WflDiagnostic}; use wfl::fixer::{CodeFixer, validate_source, write_fixed_file}; use wfl::lexer::lex_wfl_with_positions_checked; use wfl::linter::Linter; use wfl::parser::Parser; +use wfl::parser::ast::{Expression, Literal, Program, Statement}; use wfl::repl; use wfl::typechecker::{TypeCheckError, TypeChecker}; use wfl::wfl_config; @@ -227,6 +229,120 @@ fn read_source_bounded( }) } +/// Actions in literal `include from` files are visible at runtime, although +/// the top-level analyzer only receives the entry file's AST. Read static +/// includes conservatively so only confirmed definitions lose their warning. +/// Dynamic paths, unreadable files, and parse failures retain the warning. +fn literal_include_actions( + program: &Program, + source_file: &Path, + budget: &wfl::exec::budget::ExecutionBudget, +) -> Result, String> { + fn visit( + program: &Program, + source_file: &Path, + depth: usize, + budget: &wfl::exec::budget::ExecutionBudget, + seen: &mut HashSet, + actions: &mut HashSet, + ) -> Result<(), String> { + use std::io::Read; + + for statement in &program.statements { + let Statement::IncludeStatement { + path: Expression::Literal(Literal::String(relative), ..), + .. + } = statement + else { + continue; + }; + if budget.check_import_depth(depth).is_err() { + continue; + } + budget + .charge_operation(!budget.is_deadline_exempt()) + .map_err(|exceeded| exceeded.message())?; + let Some(base) = source_file.parent() else { + continue; + }; + let Ok(path) = fs::canonicalize(base.join(relative.as_ref())) else { + continue; + }; + if !seen.insert(path.clone()) { + continue; + } + + // Match the runtime's per-file source limit without turning a + // best-effort diagnostic scan into a new fatal failure. + let read_limit = (budget.max_source_bytes() as u64).saturating_add(1); + let Ok(file) = fs::File::open(&path) else { + continue; + }; + let mut bytes = Vec::new(); + if file.take(read_limit).read_to_end(&mut bytes).is_err() + || budget.check_source_bytes(bytes.len()).is_err() + { + continue; + } + let Ok(source) = String::from_utf8(bytes) else { + continue; + }; + let tokens = + lex_wfl_with_positions_checked(&source).map_err(|exceeded| exceeded.message())?; + let Ok(included) = Parser::new(&tokens).parse() else { + continue; + }; + + for nested in &included.statements { + if let Statement::ActionDefinition { name, .. } = nested { + actions.insert(name.clone()); + } + } + visit(&included, &path, depth + 1, budget, seen, actions)?; + } + Ok(()) + } + + let mut seen = HashSet::new(); + let mut actions = HashSet::new(); + visit(program, source_file, 0, budget, &mut seen, &mut actions)?; + Ok(actions) +} + +fn analyze_with_literal_includes( + analyzer: &mut Analyzer, + program: &Program, + file_id: usize, + source_file: &Path, + budget: &wfl::exec::budget::ExecutionBudget, +) -> Vec { + let mut diagnostics = analyzer.analyze_static(program, file_id); + if !diagnostics.iter().any(|diagnostic| { + diagnostic.code == "ANALYZE-SEMANTIC" + && diagnostic.message.starts_with("Undefined action '") + }) { + return diagnostics; + } + + let actions = match literal_include_actions(program, source_file, budget) { + Ok(actions) => actions, + Err(message) => { + diagnostics.push(WflDiagnostic::error(message)); + return diagnostics; + } + }; + diagnostics.retain(|diagnostic| { + !(diagnostic.severity == Severity::Warning + && diagnostic.code == "ANALYZE-SEMANTIC" + && diagnostic + .message + .strip_prefix("Undefined action '") + .and_then(|name| name.strip_suffix('\'')) + .is_some_and(|name| actions.contains(name))) + }); + diagnostics +} + /// Parse operation flags, validate their combination, and dispatch the requested work. async fn run() -> io::Result<()> { // Initialize dhat profiler if enabled @@ -929,7 +1045,13 @@ async fn run() -> io::Result<()> { let mut reporter = DiagnosticReporter::new(); let file_id = reporter.add_file(&file_path, &input); - let diagnostics = analyzer.analyze_static(&program, file_id); + let diagnostics = analyze_with_literal_includes( + &mut analyzer, + &program, + file_id, + Path::new(&file_path), + &budget, + ); if !diagnostics.is_empty() { eprintln!("Static analysis warnings:"); @@ -989,7 +1111,13 @@ async fn run() -> io::Result<()> { let mut analyzer = Analyzer::new(); let mut reporter = DiagnosticReporter::new(); let file_id = reporter.add_file(&file_path, &input); - let sema_diags = analyzer.analyze_static(&program, file_id); + let sema_diags = analyze_with_literal_includes( + &mut analyzer, + &program, + file_id, + Path::new(&file_path), + &budget, + ); let mut has_fatal_errors = false; if !sema_diags.is_empty() { for d in &sema_diags { From af4fbd08342141f3af96955c47ba14cdab9d3ab2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 08:12:35 +0000 Subject: [PATCH 3/4] test: reproduce include-scan ordering, budget, and member-use review findings Adds failing regressions for PR #747 review findings: - a call that runs before its literal include must keep its warning - an action body that can run before the include must keep its warning - the include scan must not spend the run's operation budget - an exhausted scan must not report a budget failure - a deadline breach during the scan must surface as a budget failure - expect subjects using property/method access count as variable uses The CLI helper now asserts exit status alongside diagnostics. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UPM3CTGyoba4ftXcCXjAu3 --- src/main.rs | 41 +++++ tests/analyzer_include_expect_test.rs | 217 ++++++++++++++++++++++++-- 2 files changed, 243 insertions(+), 15 deletions(-) diff --git a/src/main.rs b/src/main.rs index f6313368..fd4cc4db 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1401,3 +1401,44 @@ async fn run() -> io::Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use wfl::exec::budget::{BudgetExceeded, BudgetLimits, ExecutionBudget}; + + /// A deadline that expires during the optional include scan is a run-budget + /// breach like any other front-end breach, so it must reach the caller as + /// a budget failure (fatal, exit 2) rather than as an analyzer diagnostic. + #[test] + fn include_scan_deadline_breach_is_a_budget_failure() { + let dir = tempfile::TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("leaf.wfl"), + "define action called greet:\n return 1\nend action\n", + ) + .unwrap(); + let tokens = + lex_wfl_with_positions_checked("include from \"leaf.wfl\"\nstore x as call greet\n") + .unwrap(); + let program = Parser::new(&tokens).parse().expect("parse"); + let limits = BudgetLimits { + max_duration: Some(std::time::Duration::ZERO), + ..BudgetLimits::default() + }; + let budget = ExecutionBudget::new(limits); + std::thread::sleep(std::time::Duration::from_millis(2)); + + let result = analyze_with_literal_includes( + &mut Analyzer::new(), + &program, + 0, + &dir.path().join("main.wfl"), + &budget, + ); + assert!( + matches!(result, Err(BudgetExceeded::Deadline { .. })), + "expected a deadline breach, got {result:?}" + ); + } +} diff --git a/tests/analyzer_include_expect_test.rs b/tests/analyzer_include_expect_test.rs index a8a3fd8f..83d8a7ed 100644 --- a/tests/analyzer_include_expect_test.rs +++ b/tests/analyzer_include_expect_test.rs @@ -3,27 +3,35 @@ use std::path::Path; use std::process::Command; use tempfile::TempDir; -fn analyze(path: &Path) -> String { +const LEAF: &str = + "define action called greet with parameters name:\n return name\nend action\n"; + +/// Run `wfl` with `args` and return its exit code and combined output. +fn wfl(args: &[&str], path: &Path) -> (Option, String) { let output = Command::new(env!("CARGO_BIN_EXE_wfl")) - .arg("--analyze") + .args(args) .arg(path) + .env("NO_COLOR", "1") .output() - .expect("run WFL analyzer"); - format!( - "{}{}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) + .expect("run WFL"); + ( + output.status.code(), + format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ), ) } +fn analyze(path: &Path) -> (Option, String) { + wfl(&["--analyze"], path) +} + #[test] fn transitive_literal_include_actions_do_not_warn_but_typos_do() { let dir = TempDir::new().expect("tempdir"); - fs::write( - dir.path().join("leaf.wfl"), - "define action called greet with parameters name:\n return name\nend action\n", - ) - .unwrap(); + fs::write(dir.path().join("leaf.wfl"), LEAF).unwrap(); fs::write(dir.path().join("middle.wfl"), "include from \"leaf.wfl\"\n").unwrap(); let main = dir.path().join("main.wfl"); fs::write( @@ -32,7 +40,8 @@ fn transitive_literal_include_actions_do_not_warn_but_typos_do() { ) .unwrap(); - let output = analyze(&main); + let (status, output) = analyze(&main); + assert_eq!(status, Some(1), "warnings must exit 1: {output}"); assert!( !output.contains("Undefined action 'greet'"), "included action was reported undefined: {output}" @@ -58,13 +67,170 @@ fn dynamic_include_keeps_unresolved_action_warning() { ) .unwrap(); - let output = analyze(&main); + let (status, output) = analyze(&main); + assert_eq!(status, Some(1), "warnings must exit 1: {output}"); assert!( output.contains("Undefined action 'greet'"), "a dynamic include cannot be resolved statically: {output}" ); } +#[test] +fn call_before_literal_include_keeps_undefined_action_warning() { + let dir = TempDir::new().expect("tempdir"); + fs::write(dir.path().join("leaf.wfl"), LEAF).unwrap(); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + "store early as call greet with \"Ada\"\ninclude from \"leaf.wfl\"\nstore late as call greet with \"Bob\"\ndisplay early with late\n", + ) + .unwrap(); + + let (status, output) = analyze(&main); + assert_eq!(status, Some(1), "the early call must still warn: {output}"); + assert_eq!( + output.matches("Undefined action 'greet'").count(), + 1, + "only the call that runs before the include should warn: {output}" + ); +} + +#[test] +fn action_body_call_warns_only_when_the_action_can_run_before_the_include() { + let dir = TempDir::new().expect("tempdir"); + fs::write(dir.path().join("leaf.wfl"), LEAF).unwrap(); + let action = "define action called welcome:\n return call greet with \"Ada\"\nend action\n"; + + // The include runs before any statement that could invoke `welcome`. + let resolved = dir.path().join("resolved.wfl"); + fs::write( + &resolved, + format!( + "{action}include from \"leaf.wfl\"\nstore message as call welcome\ndisplay message\n" + ), + ) + .unwrap(); + let (status, output) = analyze(&resolved); + assert_eq!( + status, + Some(0), + "include precedes every invocation: {output}" + ); + assert!( + output.contains("No static analysis warnings found."), + "{output}" + ); + + // `welcome` is invoked before the include runs, so `greet` is undefined then. + let early = dir.path().join("early.wfl"); + fs::write( + &early, + format!( + "{action}store message as call welcome\ninclude from \"leaf.wfl\"\ndisplay message\n" + ), + ) + .unwrap(); + let (status, output) = analyze(&early); + assert_eq!( + status, + Some(1), + "the body can run before the include: {output}" + ); + assert!( + output.contains("Undefined action 'greet'"), + "action body invoked before the include lost its warning: {output}" + ); +} + +/// Smallest `max_operations` ceiling under which `wfl ` exits 0. +fn minimum_operations(dir: &Path, path: &Path) -> u64 { + let (mut low, mut high) = (1u64, 200_000u64); + while low < high { + let middle = (low + high) / 2; + fs::write(dir.join(".wflcfg"), format!("max_operations = {middle}\n")).unwrap(); + if wfl(&[], path).0 == Some(0) { + high = middle; + } else { + low = middle + 1; + } + } + low +} + +fn budget_fixture(dir: &Path) { + let mut library = String::from(LEAF); + for index in 0..300 { + library.push_str(&format!("store filler_{index} as {index}\n")); + } + fs::write(dir.join("library.wfl"), library).unwrap(); + // Same program, but the dynamic include is never scanned by the analyzer. + fs::write( + dir.join("dynamic.wfl"), + "store library_path as \"library.wfl\"\ninclude from library_path\nstore good as call greet with \"Ada\"\ndisplay good\n", + ) + .unwrap(); + fs::write( + dir.join("literal.wfl"), + "include from \"library.wfl\"\nstore good as call greet with \"Ada\"\ndisplay good\n", + ) + .unwrap(); +} + +#[test] +fn include_scan_does_not_spend_the_execution_operation_budget() { + let dir = TempDir::new().expect("tempdir"); + budget_fixture(dir.path()); + + // The literal program does strictly less runtime work than the dynamic one, + // so any ceiling that runs the dynamic program must run it too. + let ceiling = minimum_operations(dir.path(), &dir.path().join("dynamic.wfl")); + fs::write( + dir.path().join(".wflcfg"), + format!("max_operations = {ceiling}\n"), + ) + .unwrap(); + let (status, output) = wfl(&[], &dir.path().join("literal.wfl")); + assert_eq!( + status, + Some(0), + "the analyzer's include scan consumed the run's operation budget (ceiling {ceiling}): {output}" + ); + assert!(output.contains("Ada"), "{output}"); +} + +#[test] +fn exhausted_include_scan_keeps_warnings_instead_of_failing_analysis() { + let dir = TempDir::new().expect("tempdir"); + budget_fixture(dir.path()); + + // Sweep ceilings from "too small for the entry file" to "enough for the + // entry file but far too small to scan the 300-statement library". Once + // the entry file's own analysis completes (its warning is printed), the + // optional scan must neither report a budget failure nor drop the warning. + let mut completed = 0; + for ceiling in 1..=40 { + fs::write( + dir.path().join(".wflcfg"), + format!("max_operations = {ceiling}\n"), + ) + .unwrap(); + let (status, output) = analyze(&dir.path().join("literal.wfl")); + if !output.contains("Undefined action 'greet'") { + continue; + } + completed += 1; + assert!( + !output.contains("operation budget"), + "ceiling {ceiling}: an optional scan must not report a budget failure: {output}" + ); + assert_eq!(status, Some(1), "ceiling {ceiling}: {output}"); + } + assert!( + completed > 0, + "no ceiling let the entry file's analysis finish" + ); +} + #[test] fn expect_subject_and_expected_value_count_as_variable_uses() { let dir = TempDir::new().expect("tempdir"); @@ -75,7 +241,8 @@ fn expect_subject_and_expected_value_count_as_variable_uses() { ) .unwrap(); - let output = analyze(&main); + let (status, output) = analyze(&main); + assert_eq!(status, Some(1), "warnings must exit 1: {output}"); assert!( !output.contains("Unused variable 'measured'"), "assertion subject was reported unused: {output}" @@ -89,3 +256,23 @@ fn expect_subject_and_expected_value_count_as_variable_uses() { "genuinely unused variable was hidden: {output}" ); } + +#[test] +fn expect_property_and_method_receivers_count_as_variable_uses() { + let dir = TempDir::new().expect("tempdir"); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + "store items as [1, 2]\nstore other as [3]\nstore wanted as 1\ndescribe \"uses\":\n test \"members\":\n expect items.length to equal 2\n expect other.size(wanted) to equal 1\n end test\nend describe\n", + ) + .unwrap(); + + let (status, output) = analyze(&main); + assert_eq!(status, Some(0), "no variable is unused: {output}"); + for name in ["items", "other", "wanted"] { + assert!( + !output.contains(&format!("Unused variable '{name}'")), + "`{name}` is read by an assertion: {output}" + ); + } +} From c47b2d2f6d1103e7ce018d33f19f033eea43267e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 08:54:13 +0000 Subject: [PATCH 4/4] fix: honor include order and isolate the analyzer's include scan Addresses PR #747 review findings: - Keep an undefined-action warning unless the literal include that defines the action has run by the time the call runs. The analyzer now records the top-level statement holding each include-relaxed warning; the CLI compares statement positions. Calls in action/container/handler bodies count as run only after their definition, when a later statement can invoke them. - Give the include scan its own operation allowance (same ceiling, the run's remaining time) so it cannot spend the program's max_operations. Running out stops the scan and keeps warnings; a run deadline or cancellation during the scan exits 2 with an Error line like other front-end budget breaches. - Count property-access and method-call receivers (and method arguments) as variable uses in the unused-variable walker. - Reconcile the module guide's include-warning paragraphs and extend the dev diary entry. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UPM3CTGyoba4ftXcCXjAu3 --- Docs/04-advanced-features/modules.md | 13 +- ...-09-25-included-actions-and-expect-uses.md | 11 + src/analyzer/mod.rs | 36 +- src/analyzer/static_analyzer.rs | 10 +- src/main.rs | 320 +++++++++++++----- tests/analyzer_include_expect_test.rs | 9 +- 6 files changed, 298 insertions(+), 101 deletions(-) diff --git a/Docs/04-advanced-features/modules.md b/Docs/04-advanced-features/modules.md index 181e1344..5b548e7f 100644 --- a/Docs/04-advanced-features/modules.md +++ b/Docs/04-advanced-features/modules.md @@ -73,7 +73,16 @@ store line as banner # a bare name calls the zero-argument action display line ``` -All three forms work at the top level and inside your own action bodies. Because the analyzer does not read included files, it emits a **non-fatal** `Undefined action ''` note for a name it cannot see statically — the program still runs and the action resolves at runtime. +All three forms work at the top level and inside your own action bodies. + +Before a program runs (and with `wfl --analyze`), the CLI reads each literal `include from "path.wfl"` at the top level of the main file, plus the literal includes inside those files, to learn which actions they define. It does not run them. A call to one of those actions gets no warning when the include has already run at that point: + +- In top-level code, the include comes before the call. +- In an action, container, or event-handler body, the include runs before any later statement that could run that body. Placing includes before the code that uses them satisfies both rules. + +Otherwise the analyzer emits a **non-fatal** `Undefined action ''` warning. The warning also stays when the analyzer cannot check the name: a dynamic path (`include from module_path`), an include inside a block, or a file it cannot read or parse. The warning does not stop the program. At runtime the call works only if an include has defined the action by then; a misspelled name is still an error. + +This check has its own operation allowance, so it does not use up the program's `max_operations`. If it runs out, the remaining warnings stay. The program's time limit still applies to it. ### Including the same file more than once (diamond includes) @@ -151,8 +160,6 @@ still reported as a circular dependency (see Included files go through the same pipeline as the main program (parse, analyze, type check). Because `include from` runs the file in the parent scope — as if the code were written in the main program — type-check findings in an included file are reported the same way as in the main file: as **non-fatal warnings**. The program still runs. -For a literal `include from "path.wfl"`, the CLI checks action names in that file and its transitive literal includes before reporting an undefined-action warning. It does not infer the included action's call signature at this stage; the included file still goes through its normal checks when executed. A dynamic path, an unreadable file, or a file that cannot be parsed remains unresolved, so calls to its actions may still warn. Misspelled action names continue to warn. - ```text Type checking warnings in included file 'mod.wfl': error[ERROR]: Could not infer type for variable 'v' diff --git a/History/dev-diary/2026/2026-09-25-included-actions-and-expect-uses.md b/History/dev-diary/2026/2026-09-25-included-actions-and-expect-uses.md index 87db6d84..93c70883 100644 --- a/History/dev-diary/2026/2026-09-25-included-actions-and-expect-uses.md +++ b/History/dev-diary/2026/2026-09-25-included-actions-and-expect-uses.md @@ -7,3 +7,14 @@ The CLI now scans literal top-level includes transitively when undefined-action Three focused real-binary regressions were written first. Before the fix, the literal-include and assertion-use cases failed; the dynamic-include case passed. After the fix, all three pass. Against the seven Logbie-web suites, the patched WFL binary removes all 316 false undefined-action warnings and 90 false unused-variable warnings. The other 36 unused names were ignored return values in three Logbie-web test files; those bindings were replaced with direct calls, leaving zero analyzer warnings across all seven suites. The account suite's file-backed SQLite case requires a writable fixture directory outside the sandbox. The red test-only ancestor is commit bb103dae. + +## Review follow-up + +Review of the first version found four gaps, each reproduced by a failing test before the fix (red test-only commit af4fbd0): + +- **Ordering.** The first version dropped a warning whenever any literal include defined the name, even for a call that runs before the include. Top-level statements run in order, and WFL defines an action only when its `define` statement runs, so the CLI now keeps a warning unless the include has run by the time of the call. For a call in sequential code, the include must be an earlier top-level statement. For a call in an action, container, or handler body, the include must come before any later statement that can run code (another include counts). The analyzer records which top-level statement holds each `Undefined action` warning, and the CLI compares statement positions. It does not compare line numbers, because the parser records some blocks at their closing line. +- **Operation budget.** The scan charged the run's shared operation budget, so a program whose `max_operations` covered its own work could fail before it started. The scan now has its own allowance with the same ceiling and the run's remaining time. Running out stops the scan and keeps the remaining warnings. A run deadline or cancellation during the scan exits with status 2 and an `Error:` line, like other front-end budget breaches. Before this change it appeared as an analyzer finding. +- **Assertion operands.** `expect items.length ...` and `expect other.size(wanted) ...` now count `items`, `other`, and `wanted` as used. The shared unused-variable walker now visits property-access receivers and method-call receivers and arguments. +- **Test strength.** The CLI tests now assert the exit status along with the diagnostic text. + +Known limit: the check assumes that the included file supplying a name does not call back into a main-file action that uses the name before the file defines it. That case still loses its warning. diff --git a/src/analyzer/mod.rs b/src/analyzer/mod.rs index 064cdb16..dd3f2a83 100644 --- a/src/analyzer/mod.rs +++ b/src/analyzer/mod.rs @@ -301,6 +301,17 @@ impl Scope { } } +/// Where an include-relaxed `Undefined action` warning was raised. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UndefinedActionSite { + pub name: String, + pub line: usize, + pub column: usize, + /// Index into `Program::statements` of the top-level statement containing + /// the call, when the call was reached while analyzing one. + pub statement_index: Option, +} + #[derive(Debug, Clone)] pub struct SemanticError { pub message: String, @@ -400,6 +411,11 @@ pub struct Analyzer { /// actions/variables they expose; undefined-action errors are downgraded to /// warnings to avoid false fatal failures for include-exposed actions. has_includes: bool, + /// Index of the top-level statement being analyzed, if any. + current_statement_index: Option, + /// Every include-relaxed `Undefined action` warning, with its enclosing + /// top-level statement, so callers can tell when an include has run. + undefined_action_sites: Vec, /// Nesting depth of `try` bodies currently being analyzed. Undefined-name /// references inside a `try` body raise catchable runtime errors (documented /// behavior), so they are reported as warnings instead of fatal errors. @@ -683,6 +699,8 @@ impl Analyzer { current_container: None, current_method_is_static: None, has_includes: false, + current_statement_index: None, + undefined_action_sites: Vec::new(), try_depth: 0, active_loop_variables: Vec::new(), budget_error: None, @@ -802,6 +820,8 @@ impl Analyzer { self.next_scope_id = 1; self.errors.clear(); self.warnings.clear(); + self.undefined_action_sites.clear(); + self.current_statement_index = None; self.action_parameters.clear(); self.constant_bindings.clear(); self.containers.clear(); @@ -861,9 +881,11 @@ impl Analyzer { } // PASS 2: Analyze all statements (including action bodies) - for statement in &program.statements { + for (index, statement) in program.statements.iter().enumerate() { + self.current_statement_index = Some(index); self.analyze_statement(statement); } + self.current_statement_index = None; self.validate_container_inheritance_cycles(); self.warn_incompatible_inherited_property_overrides(); @@ -884,6 +906,12 @@ impl Analyzer { &self.warnings } + /// The include-relaxed `Undefined action` warnings from the last analysis, + /// with the top-level statement that contains each call. + pub fn undefined_action_sites(&self) -> &[UndefinedActionSite] { + &self.undefined_action_sites + } + /// Take the shared-budget breach recorded during analysis, if any. When /// `analyze` returns `Err`, a caller must consult this to tell a fatal /// deadline/cancellation/resource breach apart from ordinary semantic @@ -962,6 +990,12 @@ impl Analyzer { line, column, )); + self.undefined_action_sites.push(UndefinedActionSite { + name: name.to_string(), + line, + column, + statement_index: self.current_statement_index, + }); true } else { false diff --git a/src/analyzer/static_analyzer.rs b/src/analyzer/static_analyzer.rs index 26dcbf31..e6409987 100644 --- a/src/analyzer/static_analyzer.rs +++ b/src/analyzer/static_analyzer.rs @@ -1569,9 +1569,17 @@ impl Analyzer { } } } - Expression::MemberAccess { object, .. } => { + Expression::MemberAccess { object, .. } | Expression::PropertyAccess { object, .. } => { self.mark_used_in_expression(object, usages); } + Expression::MethodCall { + object, arguments, .. + } => { + self.mark_used_in_expression(object, usages); + for arg in arguments { + self.mark_used_in_expression(&arg.value, usages); + } + } Expression::IndexAccess { collection, index, .. } => { diff --git a/src/main.rs b/src/main.rs index fd4cc4db..b9648ba8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,15 +1,17 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::env; use std::fs; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::process; +use std::sync::Arc; use std::time::Instant; use wfl::Interpreter; use wfl::analyzer::{Analyzer, StaticAnalyzer}; use wfl::config; use wfl::debug_report; use wfl::diagnostics::{DiagnosticReporter, Severity, WflDiagnostic}; +use wfl::exec::budget::{BudgetExceeded, ExecutionBudget}; use wfl::fixer::{CodeFixer, validate_source, write_fixed_file}; use wfl::lexer::lex_wfl_with_positions_checked; use wfl::linter::Linter; @@ -229,118 +231,236 @@ fn read_source_bounded( }) } -/// Actions in literal `include from` files are visible at runtime, although -/// the top-level analyzer only receives the entry file's AST. Read static -/// includes conservatively so only confirmed definitions lose their warning. -/// Dynamic paths, unreadable files, and parse failures retain the warning. -fn literal_include_actions( - program: &Program, - source_file: &Path, - budget: &wfl::exec::budget::ExecutionBudget, -) -> Result, String> { - fn visit( - program: &Program, - source_file: &Path, +/// Walks literal `include from` files for action definitions on behalf of the +/// analyzer, which only receives the entry file's AST. +/// +/// The walk is optional diagnostics work, so it spends its own operation +/// allowance (with the run's ceiling) instead of the run's: running out stops +/// the walk and keeps the remaining warnings. The run's deadline and +/// cancellation still apply and are reported as budget failures. +struct IncludeScan<'a> { + run: &'a ExecutionBudget, + allowance: Arc, + seen: HashSet, + exhausted: bool, +} + +impl IncludeScan<'_> { + /// Fail if the run was cancelled or its deadline has passed. + fn check_run(&self) -> Result<(), BudgetExceeded> { + self.run.check_cancelled()?; + if !self.run.is_deadline_exempt() { + self.run.check_deadline()?; + } + Ok(()) + } + + /// Stop scanning after the allowance failed, unless the failure was the + /// run's own deadline or cancellation. + fn stop(&mut self) -> Result<(), BudgetExceeded> { + self.check_run()?; + self.exhausted = true; + Ok(()) + } + + /// Add the actions defined by `relative` (resolved against `from`) and by + /// its literal includes to `found`. Dynamic paths, unreadable or oversized + /// files, and parse failures are skipped, so their calls keep warning. + fn include( + &mut self, + relative: &str, + from: &Path, depth: usize, - budget: &wfl::exec::budget::ExecutionBudget, - seen: &mut HashSet, - actions: &mut HashSet, - ) -> Result<(), String> { + found: &mut HashSet, + ) -> Result<(), BudgetExceeded> { use std::io::Read; - for statement in &program.statements { - let Statement::IncludeStatement { - path: Expression::Literal(Literal::String(relative), ..), - .. - } = statement - else { - continue; - }; - if budget.check_import_depth(depth).is_err() { - continue; - } - budget - .charge_operation(!budget.is_deadline_exempt()) - .map_err(|exceeded| exceeded.message())?; - let Some(base) = source_file.parent() else { - continue; - }; - let Ok(path) = fs::canonicalize(base.join(relative.as_ref())) else { - continue; - }; - if !seen.insert(path.clone()) { - continue; - } + if self.exhausted || self.run.check_import_depth(depth).is_err() { + return Ok(()); + } + self.check_run()?; + if self.allowance.charge_operation(true).is_err() { + return self.stop(); + } + let Some(base) = from.parent() else { + return Ok(()); + }; + let Ok(path) = fs::canonicalize(base.join(relative)) else { + return Ok(()); + }; + if !self.seen.insert(path.clone()) { + return Ok(()); + } - // Match the runtime's per-file source limit without turning a - // best-effort diagnostic scan into a new fatal failure. - let read_limit = (budget.max_source_bytes() as u64).saturating_add(1); - let Ok(file) = fs::File::open(&path) else { - continue; - }; - let mut bytes = Vec::new(); - if file.take(read_limit).read_to_end(&mut bytes).is_err() - || budget.check_source_bytes(bytes.len()).is_err() - { - continue; - } - let Ok(source) = String::from_utf8(bytes) else { - continue; - }; - let tokens = - lex_wfl_with_positions_checked(&source).map_err(|exceeded| exceeded.message())?; - let Ok(included) = Parser::new(&tokens).parse() else { - continue; - }; + // Match the runtime's per-file source limit without turning a + // best-effort diagnostic scan into a new fatal failure. + let read_limit = (self.run.max_source_bytes() as u64).saturating_add(1); + let Ok(file) = fs::File::open(&path) else { + return Ok(()); + }; + let mut bytes = Vec::new(); + if file.take(read_limit).read_to_end(&mut bytes).is_err() + || self.run.check_source_bytes(bytes.len()).is_err() + { + return Ok(()); + } + let Ok(source) = String::from_utf8(bytes) else { + return Ok(()); + }; + let Ok(tokens) = lex_wfl_with_positions_checked(&source) else { + return self.stop(); + }; + let Ok(included) = Parser::new(&tokens).parse() else { + return Ok(()); + }; - for nested in &included.statements { - if let Statement::ActionDefinition { name, .. } = nested { - actions.insert(name.clone()); + for statement in &included.statements { + match statement { + Statement::ActionDefinition { name, .. } => { + found.insert(name.clone()); } + Statement::IncludeStatement { + path: Expression::Literal(Literal::String(nested), ..), + .. + } => self.include(nested, &path, depth + 1, found)?, + _ => {} } - visit(&included, &path, depth + 1, budget, seen, actions)?; } Ok(()) } +} - let mut seen = HashSet::new(); - let mut actions = HashSet::new(); - visit(program, source_file, 0, budget, &mut seen, &mut actions)?; +/// Actions exposed by the entry file's top-level literal includes, mapped to +/// the statement index of the first include that makes each one visible. +fn literal_include_actions( + program: &Program, + source_file: &Path, + budget: &ExecutionBudget, +) -> Result, BudgetExceeded> { + let mut limits = budget.limits().clone(); + limits.max_duration = limits + .max_duration + .map(|limit| limit.saturating_sub(budget.elapsed())); + let allowance = Arc::new(ExecutionBudget::new(limits)); + // The lexer and parser charge the current-thread budget; point them at the + // scan's allowance until the guard restores the run budget. + let _allowance_guard = ExecutionBudget::enter(Arc::clone(&allowance)); + let mut scan = IncludeScan { + run: budget, + allowance, + seen: HashSet::new(), + exhausted: false, + }; + + let mut actions = HashMap::new(); + for (index, statement) in program.statements.iter().enumerate() { + let Statement::IncludeStatement { + path: Expression::Literal(Literal::String(relative), ..), + .. + } = statement + else { + continue; + }; + let mut found = HashSet::new(); + scan.include(relative, source_file, 0, &mut found)?; + for name in found { + actions.entry(name).or_insert(index); + } + } + // A breach inside the last parse is reported by the parser as a parse + // failure; surface a run deadline or cancellation here instead. + scan.check_run()?; Ok(actions) } +/// The action name of an include-relaxed `Undefined action` warning. +fn undefined_action_warning(diagnostic: &WflDiagnostic) -> Option<&str> { + if diagnostic.severity != Severity::Warning || diagnostic.code != "ANALYZE-SEMANTIC" { + return None; + } + diagnostic + .message + .strip_prefix("Undefined action '")? + .strip_suffix('\'') +} + +/// Top-level statements whose bodies run later, when invoked or dispatched. +fn has_deferred_body(statement: &Statement) -> bool { + matches!( + statement, + Statement::ActionDefinition { .. } + | Statement::ContainerDefinition { .. } + | Statement::EventHandler { .. } + | Statement::WebSocketHandlerStatement { .. } + ) +} + +/// Top-level statements that only define or register something when reached, +/// so they cannot invoke a deferred body. +fn only_defines(statement: &Statement) -> bool { + has_deferred_body(statement) + || matches!( + statement, + Statement::InterfaceDefinition { .. } + | Statement::EventDefinition { .. } + | Statement::PatternDefinition { .. } + ) +} + +/// Whether the top-level include at `include` has run by the time a call in +/// the top-level statement at `call` executes. +/// +/// Top-level statements run in order, so sequential code sees the include only +/// when the include comes first. An action, container, or handler body runs +/// only after its definition, when a later statement invokes it; the include +/// has run first when it is reached before any statement after the definition +/// that can run code. Another include counts as code: the included file's +/// top-level statements run. +fn include_runs_before_call(statements: &[Statement], include: usize, call: usize) -> bool { + if include < call || !has_deferred_body(&statements[call]) { + return include < call; + } + statements[call + 1..] + .iter() + .position(|statement| !only_defines(statement)) + .is_none_or(|offset| include <= call + 1 + offset) +} + +/// Run static analysis, then drop `Undefined action` warnings for actions a +/// literal include has defined by the time the call runs. fn analyze_with_literal_includes( analyzer: &mut Analyzer, program: &Program, file_id: usize, source_file: &Path, - budget: &wfl::exec::budget::ExecutionBudget, -) -> Vec { + budget: &ExecutionBudget, +) -> Result, BudgetExceeded> { let mut diagnostics = analyzer.analyze_static(program, file_id); - if !diagnostics.iter().any(|diagnostic| { - diagnostic.code == "ANALYZE-SEMANTIC" - && diagnostic.message.starts_with("Undefined action '") - }) { - return diagnostics; + if !diagnostics + .iter() + .any(|diagnostic| undefined_action_warning(diagnostic).is_some()) + { + return Ok(diagnostics); } - let actions = match literal_include_actions(program, source_file, budget) { - Ok(actions) => actions, - Err(message) => { - diagnostics.push(WflDiagnostic::error(message)); - return diagnostics; - } - }; + let actions = literal_include_actions(program, source_file, budget)?; + let resolved: HashSet<(usize, usize)> = analyzer + .undefined_action_sites() + .iter() + .filter(|site| { + let (Some(&include), Some(call)) = (actions.get(&site.name), site.statement_index) + else { + return false; + }; + include_runs_before_call(&program.statements, include, call) + }) + .map(|site| (site.line, site.column)) + .collect(); diagnostics.retain(|diagnostic| { - !(diagnostic.severity == Severity::Warning - && diagnostic.code == "ANALYZE-SEMANTIC" - && diagnostic - .message - .strip_prefix("Undefined action '") - .and_then(|name| name.strip_suffix('\'')) - .is_some_and(|name| actions.contains(name))) + undefined_action_warning(diagnostic).is_none() + || !resolved.contains(&(diagnostic.line, diagnostic.column)) }); - diagnostics + Ok(diagnostics) } /// Parse operation flags, validate their combination, and dispatch the requested work. @@ -1045,13 +1165,19 @@ async fn run() -> io::Result<()> { let mut reporter = DiagnosticReporter::new(); let file_id = reporter.add_file(&file_path, &input); - let diagnostics = analyze_with_literal_includes( + let diagnostics = match analyze_with_literal_includes( &mut analyzer, &program, file_id, Path::new(&file_path), &budget, - ); + ) { + Ok(diagnostics) => diagnostics, + Err(exceeded) => { + eprintln!("Error: {}", exceeded.message()); + process::exit(2); + } + }; if !diagnostics.is_empty() { eprintln!("Static analysis warnings:"); @@ -1111,13 +1237,21 @@ async fn run() -> io::Result<()> { let mut analyzer = Analyzer::new(); let mut reporter = DiagnosticReporter::new(); let file_id = reporter.add_file(&file_path, &input); - let sema_diags = analyze_with_literal_includes( + // A run-budget breach during the include scan is fatal, like + // the other front-end budget breaches. + let sema_diags = match analyze_with_literal_includes( &mut analyzer, &program, file_id, Path::new(&file_path), &budget, - ); + ) { + Ok(diagnostics) => diagnostics, + Err(exceeded) => { + eprintln!("Error: {}", exceeded.message()); + process::exit(2); + } + }; let mut has_fatal_errors = false; if !sema_diags.is_empty() { for d in &sema_diags { diff --git a/tests/analyzer_include_expect_test.rs b/tests/analyzer_include_expect_test.rs index 83d8a7ed..ffa9bd0d 100644 --- a/tests/analyzer_include_expect_test.rs +++ b/tests/analyzer_include_expect_test.rs @@ -205,8 +205,9 @@ fn exhausted_include_scan_keeps_warnings_instead_of_failing_analysis() { // Sweep ceilings from "too small for the entry file" to "enough for the // entry file but far too small to scan the 300-statement library". Once - // the entry file's own analysis completes (its warning is printed), the - // optional scan must neither report a budget failure nor drop the warning. + // the entry file's own analysis completes (its warning is printed and the + // analyzer reported no budget error of its own), the optional scan must + // neither report a budget failure nor drop the warning. let mut completed = 0; for ceiling in 1..=40 { fs::write( @@ -215,7 +216,9 @@ fn exhausted_include_scan_keeps_warnings_instead_of_failing_analysis() { ) .unwrap(); let (status, output) = analyze(&dir.path().join("literal.wfl")); - if !output.contains("Undefined action 'greet'") { + if !output.contains("Undefined action 'greet'") + || output.contains("error[ANALYZE-SEMANTIC]") + { continue; } completed += 1;