Skip to content
Merged
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
4 changes: 4 additions & 0 deletions crates/beamtalk-cli/src/commands/test_stdlib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,10 @@ fn compile_fixture(
allow_primitives: false,
workspace_mode: false,
suppress_warnings,
// BT-979: Bootstrap-test fixtures use top-level expressions as test assertions
// paired with `// =>` comments. Skip the module-expression lint to avoid
// false positives on intentional assertion expressions.
skip_module_expression_lint: true,
};

crate::beam_compiler::compile_source(fixture_path, &module_name, &core_file, &options)
Expand Down
1 change: 1 addition & 0 deletions crates/beamtalk-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ fn run() -> Result<()> {
allow_primitives,
workspace_mode: false,
suppress_warnings: no_warnings,
..Default::default()
};
commands::build::build(&path, &options)
}
Expand Down
3 changes: 3 additions & 0 deletions crates/beamtalk-compiler-port/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,8 @@ fn handle_compile_expression(request: &Map) -> Term {
d.severity,
beamtalk_core::source_analysis::Severity::Warning
| beamtalk_core::source_analysis::Severity::Hint
// BT-979: Include lint warnings so REPL users see effect-free statement hints.
| beamtalk_core::source_analysis::Severity::Lint
)
})
.map(|d| d.message.to_string())
Expand Down Expand Up @@ -556,6 +558,7 @@ fn handle_compile(request: &Map) -> Term {
allow_primitives: false,
workspace_mode,
suppress_warnings: false,
..Default::default()
};
let primitive_diags =
beamtalk_core::semantic_analysis::primitive_validator::validate_primitives(
Expand Down
8 changes: 8 additions & 0 deletions crates/beamtalk-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,12 @@ pub struct CompilerOptions {
/// When true, suppress warning diagnostics during compilation.
/// Useful for test fixtures that intentionally trigger warnings.
pub suppress_warnings: bool,

/// BT-979: When true, skip the effect-free lint check on `module.expressions`.
///
/// Set this for bootstrap-test compilation, where top-level expressions are
/// intentional test assertions (paired with `// =>` comments) rather than
/// accidentally discarded values. Defaults to false so the REPL and normal
/// `beamtalk build` / `beamtalk lint` paths all get the check.
pub skip_module_expression_lint: bool,
}
40 changes: 39 additions & 1 deletion crates/beamtalk-core/src/lint/effect_free_statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ pub(crate) struct EffectFreeStatementPass;

impl LintPass for EffectFreeStatementPass {
fn check(&self, module: &Module, diagnostics: &mut Vec<Diagnostic>) {
crate::semantic_analysis::validators::check_effect_free_statements(module, diagnostics);
// BT-979: lint always checks module.expressions (skip_module_expression_lint = false)
crate::semantic_analysis::validators::check_effect_free_statements(
module,
diagnostics,
false,
);
}
}

Expand Down Expand Up @@ -103,4 +108,37 @@ mod tests {
"Expected no effect-free lints, got: {effect_free:?}"
);
}

// BT-979: Module-level expression linting

#[test]
fn discarded_module_level_map_literal_surfaced() {
// Two module-level expressions: the first (map literal) is discarded.
// This matches the REPL scenario: `#{#x => 1} #{#y => 2}`
let diags = lint("#{#x => 1}\n#{#y => 2}");
let effect_free: Vec<_> = diags
.iter()
.filter(|d| d.message.contains("no effect"))
.collect();
assert_eq!(
effect_free.len(),
1,
"Expected 1 effect-free lint for discarded module-level map literal, got: {effect_free:?}"
);
assert!(effect_free[0].message.contains("map literal"));
}

#[test]
fn single_module_level_expression_not_flagged() {
// A single module-level expression is the last (and only) expression — not flagged.
let diags = lint("1 + 2");
let effect_free: Vec<_> = diags
.iter()
.filter(|d| d.message.contains("no effect"))
.collect();
assert!(
effect_free.is_empty(),
"Single module-level expression should not be flagged: {effect_free:?}"
);
}
}
28 changes: 22 additions & 6 deletions crates/beamtalk-core/src/semantic_analysis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ pub enum MutationKind {
/// assert_eq!(result.diagnostics.len(), 0);
/// ```
pub fn analyse(module: &Module) -> AnalysisResult {
analyse_full(module, &[], false)
analyse_full(module, &[], false, false)
}

/// Analyse a module with pre-defined variables (for REPL context).
Expand All @@ -197,19 +197,29 @@ pub fn analyse(module: &Module) -> AnalysisResult {
/// services. Pre-defining known variables is essential for REPL contexts where
/// users build up state incrementally across multiple evaluations.
pub fn analyse_with_known_vars(module: &Module, known_vars: &[&str]) -> AnalysisResult {
analyse_full(module, known_vars, false)
analyse_full(module, known_vars, false, false)
}

/// Analyse a module with compiler options controlling stdlib-specific behaviour.
///
/// When `stdlib_mode` is true, built-in classes are permitted to subclass sealed
/// classes (BT-791). This should only be set when compiling stdlib sources.
pub fn analyse_with_options(module: &Module, options: &crate::CompilerOptions) -> AnalysisResult {
analyse_full(module, &[], options.stdlib_mode)
analyse_full(
module,
&[],
options.stdlib_mode,
options.skip_module_expression_lint,
)
}

/// Internal: full analysis with all knobs.
fn analyse_full(module: &Module, known_vars: &[&str], stdlib_mode: bool) -> AnalysisResult {
fn analyse_full(
module: &Module,
known_vars: &[&str],
stdlib_mode: bool,
skip_module_expression_lint: bool,
) -> AnalysisResult {
let mut result = AnalysisResult::new();

// Phase 0: Build Class Hierarchy (ADR 0006 Phase 1a)
Expand Down Expand Up @@ -280,8 +290,14 @@ fn analyse_full(module: &Module, known_vars: &[&str], stdlib_mode: bool) -> Anal
// BT-955: Warn on literal boolean conditions (always true / always false)
validators::check_literal_boolean_condition(module, &mut result.diagnostics);

// BT-951: Lint on effect-free statements (suppressed during normal compile)
validators::check_effect_free_statements(module, &mut result.diagnostics);
// BT-951/BT-979: Lint on effect-free statements (suppressed during normal compile).
// Module-level expressions are checked by default; set skip_module_expression_lint
// to opt out (bootstrap-test compilation uses this).
validators::check_effect_free_statements(
module,
&mut result.diagnostics,
skip_module_expression_lint,
);

// Phase 6: Module-level validation (BT-349)
let module_diags = module_validator::validate_single_class(module);
Expand Down
65 changes: 44 additions & 21 deletions crates/beamtalk-core/src/semantic_analysis/validators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1094,19 +1094,26 @@ fn check_seq_for_effect_free(
/// BT-951: Warn (as a lint) when a statement is an effect-free expression
/// whose value is silently discarded.
///
/// Checks method bodies and standalone method bodies only. Module-level
/// expressions (`module.expressions`) are intentionally skipped because
/// bootstrap-test files use top-level expressions as test assertions
/// (paired with `// =>` comments) and these are evaluated intentionally.
/// Checks method bodies, standalone method bodies, and (by default)
/// module-level expressions (`module.expressions`). Pass
/// `skip_module_expression_lint = true` to suppress the module-level check —
/// bootstrap-test compilation uses this because those files intentionally use
/// top-level expressions as test assertions paired with `// =>` comments.
///
/// Effect-free expressions include literals, variable references, and pure
/// binary arithmetic / comparison expressions composed from pure sub-expressions.
///
/// Uses `Severity::Lint` so the warning is suppressed during normal compilation
/// and only surfaces when running `beamtalk lint`.
pub(crate) fn check_effect_free_statements(module: &Module, diagnostics: &mut Vec<Diagnostic>) {
// Skip module.expressions — those are intentional top-level test assertions.
// Only check method bodies and standalone method definitions.
/// and only surfaces when running `beamtalk lint` or in the REPL.
pub(crate) fn check_effect_free_statements(
module: &Module,
diagnostics: &mut Vec<Diagnostic>,
skip_module_expression_lint: bool,
) {
// BT-979: Check module-level expressions unless the caller opts out.
if !skip_module_expression_lint {
check_seq_for_effect_free(&module.expressions, diagnostics);
}
for class in &module.classes {
for method in class.methods.iter().chain(class.class_methods.iter()) {
check_seq_for_effect_free(&method.body, diagnostics);
Expand Down Expand Up @@ -1389,7 +1396,7 @@ mod tests {
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, false);
assert!(
diagnostics.is_empty(),
"Expected no lint for single literal return value, got: {diagnostics:?}"
Expand All @@ -1404,7 +1411,7 @@ mod tests {
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, false);
assert_eq!(
diagnostics.len(),
1,
Expand All @@ -1430,7 +1437,7 @@ mod tests {
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, false);
assert_eq!(
diagnostics.len(),
1,
Expand All @@ -1447,7 +1454,7 @@ mod tests {
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, false);
assert_eq!(
diagnostics.len(),
1,
Expand All @@ -1469,7 +1476,7 @@ mod tests {
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, false);
assert!(
diagnostics.is_empty(),
"Expected no lint for effectful sends, got: {diagnostics:?}"
Expand All @@ -1484,7 +1491,7 @@ mod tests {
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, false);
assert_eq!(
diagnostics.len(),
1,
Expand All @@ -1501,27 +1508,43 @@ mod tests {
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, false);
assert_eq!(
diagnostics.len(),
2,
"Expected 2 lints for two discarded literals, got: {diagnostics:?}"
);
}

/// Module-level expressions: NOT linted, because bootstrap-test files use
/// top-level expressions as intentional test assertions (paired with `// =>`).
/// BT-979: Module-level expressions ARE linted by default.
#[test]
fn module_level_effect_free_linted_by_default() {
let src = "42.\nself doSomething";
let tokens = lex_with_eof(src);
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics, false);
assert_eq!(
diagnostics.len(),
1,
"Expected 1 lint for discarded module-level literal, got: {diagnostics:?}"
);
assert_eq!(diagnostics[0].severity, Severity::Lint);
}

/// BT-979: Module-level expressions NOT linted when opt-out flag is set.
#[test]
fn module_level_effect_free_not_linted() {
fn module_level_effect_free_skipped_with_flag() {
let src = "42.\nself doSomething";
let tokens = lex_with_eof(src);
let (module, parse_diags) = parse(tokens);
assert!(parse_diags.is_empty(), "Parse failed: {parse_diags:?}");
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, true);
assert!(
diagnostics.is_empty(),
"Expected no lint for module-level expressions (they are intentional test assertions), got: {diagnostics:?}"
"Expected no lint when skip_module_expression_lint is true, got: {diagnostics:?}"
);
}

Expand All @@ -1538,7 +1561,7 @@ mod tests {
"Expected 1 standalone method"
);
let mut diagnostics = Vec::new();
check_effect_free_statements(&module, &mut diagnostics);
check_effect_free_statements(&module, &mut diagnostics, false);
assert_eq!(
diagnostics.len(),
1,
Expand Down