Skip to content

Commit

Permalink
Report ignored statement results
Browse files Browse the repository at this point in the history
  • Loading branch information
agu-z committed Oct 16, 2024
1 parent 5ced1b6 commit 5d9cb2d
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 18 deletions.
36 changes: 21 additions & 15 deletions crates/compiler/constrain/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3453,10 +3453,25 @@ fn constrain_stmt_def(
) -> Constraint {
let region = def.loc_expr.region;

// Try to extract the fn name and region if the stmt is a call to a named function
let (fn_name, error_region) = if let Expr::Call(boxed, _, _) = &def.loc_expr.value {
let loc_fn_expr = &boxed.1;

match loc_fn_expr.value {
Var(symbol, _) | ParamsVar { symbol, .. } => (Some(symbol), loc_fn_expr.region),
_ => (None, def.loc_expr.region),
}
} else {
(None, def.loc_expr.region)
};

// Statement expressions must return an empty record
let empty_record_index = constraints.push_type(types, Types::EMPTY_RECORD);
let expect_empty_record =
constraints.push_expected_type(ForReason(Reason::Stmt, empty_record_index, region));
let expect_empty_record = constraints.push_expected_type(ForReason(
Reason::Stmt(fn_name),
empty_record_index,
error_region,
));

let expr_con = env.with_enclosing_fx(fx_var, None, |env| {
constrain_expr(
Expand Down Expand Up @@ -3485,26 +3500,17 @@ fn constrain_stmt_def(
// Stmt expr must be effectful, otherwise it's dead code
let effectful_constraint = Constraint::EffectfulStmt(fx_var, region);

// Try to extract the fn name and region if the stmt is a direct call
let (fx_reason, call_region) = if let Expr::Call(boxed, _, _) = &def.loc_expr.value {
let loc_fn_expr = &boxed.1;

match loc_fn_expr.value {
Var(symbol, _) | ParamsVar { symbol, .. } => {
(FxReason::Call(Some(symbol)), loc_fn_expr.region)
}
_ => (FxReason::Stmt, def.loc_expr.region),
}
} else {
(FxReason::Stmt, def.loc_expr.region)
let fx_reason = match fn_name {
None => FxReason::Stmt,
Some(name) => FxReason::Call(Some(name)),
};

// We have to unify the stmt fx with the enclosing fx
// since we used the former to constrain the expr.
let enclosing_fx_constraint = constrain_call_fx(
env,
constraints,
call_region,
error_region,
fx_var,
Category::Unknown,
fx_reason,
Expand Down
32 changes: 32 additions & 0 deletions crates/compiler/load/tests/test_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14524,6 +14524,38 @@ All branches in an `if` must have the same type!
"###
);

test_report!(
ignored_result_stmt,
indoc!(
r#"
app [main!] { pf: platform "../../../../../examples/cli/effects-platform/main.roc" }
import pf.Effect
main! = \{} ->
Effect.getLine! {}
{}
"#
),
@r###"
── IGNORED RESULT in /code/proj/Main.roc ───────────────────────────────────────
The result of this call to `Effect.getLine!` is ignored:
6│ Effect.getLine! {}
^^^^^^^^^^^^^^^
Standalone statements are required to produce an empty record, but the
type of this one is:
Str
If you still want to ignore it, assign it to `_`, like this:
_ = File.delete! "data.json"
"###
);

test_report!(
function_def_leftover_bang,
indoc!(
Expand Down
2 changes: 1 addition & 1 deletion crates/compiler/lower_params/src/type_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ fn remove_for_reason(
| Reason::CrashArg
| Reason::FxInFunction(_, _)
| Reason::FxInTopLevel(_)
| Reason::Stmt
| Reason::Stmt(_)
| Reason::ImportParams(_) => {}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/compiler/types/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3447,7 +3447,7 @@ pub enum Reason {
foreign_symbol: ForeignSymbol,
arg_index: HumanIndex,
},
Stmt,
Stmt(Option<Symbol>),
FxInFunction(Option<Region>, FxReason),
FxInTopLevel(FxReason),
FloatLiteral,
Expand Down
33 changes: 32 additions & 1 deletion crates/reporting/src/error/type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1747,7 +1747,38 @@ fn to_expr_report<'b>(
severity,
}
}
Reason::Stmt => todo!("[purity-inference] Stmt"),
Reason::Stmt(opt_name) => {
let diff = to_diff(alloc, Parens::Unnecessary, found, expected_type);

let lines = [
match opt_name {
None => alloc.reflow("The result of this expression is ignored:"),
Some(fn_name) => alloc.concat([
alloc.reflow("The result of this call to "),
alloc.symbol_qualified(fn_name),
alloc.reflow(" is ignored:"),
]),
},
alloc.region(lines.convert_region(region), severity),
alloc.reflow("Standalone statements are required to produce an empty record, but the type of this one is:"),
alloc.type_block(type_with_able_vars(alloc, diff.left, diff.left_able)),
alloc.concat([
alloc.reflow("If you still want to ignore it, assign it to "),
alloc.keyword("_"),
alloc.reflow(", like this:"),
]),
alloc
.parser_suggestion("_ = File.delete! \"data.json\"")
.indent(4),
];

Report {
filename,
title: "IGNORED RESULT".to_string(),
doc: alloc.stack(lines),
severity,
}
}
},
}
}
Expand Down

0 comments on commit 5d9cb2d

Please sign in to comment.