diff --git a/crates/compiler/can/src/constraint.rs b/crates/compiler/can/src/constraint.rs index d08a86f8d1..8ac14f35fd 100644 --- a/crates/compiler/can/src/constraint.rs +++ b/crates/compiler/can/src/constraint.rs @@ -30,6 +30,7 @@ pub struct Constraints { pub pattern_eq: Vec, pub cycles: Vec, pub fx_call_constraints: Vec, + pub fx_suffix_constraints: Vec, } impl std::fmt::Debug for Constraints { @@ -84,6 +85,7 @@ impl Constraints { let pattern_eq = Vec::new(); let cycles = Vec::new(); let fx_call_constraints = Vec::with_capacity(16); + let fx_suffix_constraints = Vec::new(); categories.extend([ Category::Record, @@ -134,6 +136,7 @@ impl Constraints { pattern_eq, cycles, fx_call_constraints, + fx_suffix_constraints, } } @@ -597,13 +600,39 @@ impl Constraints { Constraint::FxCall(constraint_index) } - pub fn check_record_field_fx( - &self, + pub fn fx_pattern_suffix( + &mut self, + symbol: Symbol, + type_index: TypeOrVar, + region: Region, + ) -> Constraint { + let constraint = FxSuffixConstraint { + kind: FxSuffixKind::Pattern(symbol), + type_index, + region, + }; + + let constraint_index = index_push_new(&mut self.fx_suffix_constraints, constraint); + + Constraint::FxSuffix(constraint_index) + } + + pub fn fx_record_field_suffix( + &mut self, suffix: IdentSuffix, variable: Variable, region: Region, ) -> Constraint { - Constraint::CheckRecordFieldFx(suffix, variable, region) + let type_index = Self::push_type_variable(variable); + let constraint = FxSuffixConstraint { + kind: FxSuffixKind::RecordField(suffix), + type_index, + region, + }; + + let constraint_index = index_push_new(&mut self.fx_suffix_constraints, constraint); + + Constraint::FxSuffix(constraint_index) } pub fn contains_save_the_environment(&self, constraint: &Constraint) -> bool { @@ -632,7 +661,7 @@ impl Constraints { | Constraint::Pattern(..) | Constraint::EffectfulStmt(..) | Constraint::FxCall(_) - | Constraint::CheckRecordFieldFx(_, _, _) + | Constraint::FxSuffix(_) | Constraint::FlexToPure(_) | Constraint::True | Constraint::IsOpenType(_) @@ -808,12 +837,12 @@ pub enum Constraint { ), /// Check call fx against enclosing function fx FxCall(Index), + /// Require idents to be accurately suffixed + FxSuffix(Index), /// Set an fx var as pure if flex (no effectful functions were called) FlexToPure(Variable), /// Expect statement to be effectful EffectfulStmt(Variable, Region), - /// Require field name to be accurately suffixed - CheckRecordFieldFx(IdentSuffix, Variable, Region), /// Used for things that always unify, e.g. blanks and runtime errors True, SaveTheEnvironment, @@ -906,6 +935,29 @@ pub enum FxCallKind { Stmt, } +#[derive(Debug, Clone, Copy)] +pub struct FxSuffixConstraint { + pub type_index: TypeOrVar, + pub kind: FxSuffixKind, + pub region: Region, +} + +#[derive(Debug, Clone, Copy)] +pub enum FxSuffixKind { + Let(Symbol), + Pattern(Symbol), + RecordField(IdentSuffix), +} + +impl FxSuffixKind { + pub fn suffix(&self) -> IdentSuffix { + match self { + Self::Let(symbol) | Self::Pattern(symbol) => symbol.suffix(), + Self::RecordField(suffix) => *suffix, + } + } +} + /// Custom impl to limit vertical space used by the debug output impl std::fmt::Debug for Constraint { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -923,7 +975,10 @@ impl std::fmt::Debug for Constraint { write!(f, "Pattern({arg0:?}, {arg1:?}, {arg2:?}, {arg3:?})") } Self::FxCall(arg0) => { - write!(f, "CallFx({arg0:?})") + write!(f, "FxCall({arg0:?})") + } + Self::FxSuffix(arg0) => { + write!(f, "FxSuffix({arg0:?})") } Self::EffectfulStmt(arg0, arg1) => { write!(f, "EffectfulStmt({arg0:?}, {arg1:?})") @@ -931,9 +986,6 @@ impl std::fmt::Debug for Constraint { Self::FlexToPure(arg0) => { write!(f, "FlexToPure({arg0:?})") } - Self::CheckRecordFieldFx(arg0, arg1, arg2) => { - write!(f, "CheckRecordFieldFx({arg0:?}, {arg1:?}, {arg2:?})") - } Self::True => write!(f, "True"), Self::SaveTheEnvironment => write!(f, "SaveTheEnvironment"), Self::Let(arg0, arg1) => f.debug_tuple("Let").field(arg0).field(arg1).finish(), diff --git a/crates/compiler/constrain/src/expr.rs b/crates/compiler/constrain/src/expr.rs index 18ca1e0236..8c41c9c779 100644 --- a/crates/compiler/constrain/src/expr.rs +++ b/crates/compiler/constrain/src/expr.rs @@ -284,7 +284,7 @@ pub fn constrain_expr( constrain_field(types, constraints, env, field_var, loc_field_expr); let check_field_con = - constraints.check_record_field_fx(label.suffix(), field_var, field.region); + constraints.fx_record_field_suffix(label.suffix(), field_var, field.region); let field_con = constraints.and_constraint([field_con, check_field_con]); field_vars.push(field_var); diff --git a/crates/compiler/constrain/src/pattern.rs b/crates/compiler/constrain/src/pattern.rs index f1d686993d..bc02e188b1 100644 --- a/crates/compiler/constrain/src/pattern.rs +++ b/crates/compiler/constrain/src/pattern.rs @@ -276,6 +276,10 @@ pub fn constrain_pattern( .push(constraints.is_open_type(type_index)); } + state + .constraints + .push(constraints.fx_pattern_suffix(*symbol, type_index, region)); + state.headers.insert( *symbol, Loc { diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index d5220a8878..a8792e2a10 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -14770,8 +14770,9 @@ All branches in an `if` must have the same type! 5│ main! = \{} -> ^^^^^ - Remove the exclamation mark to give an accurate impression of its - behavior. + The exclamation mark at the end is reserved for effectful functions. + + Hint: Did you forget to run an effect? Is the type annotation wrong? "### ); @@ -14798,7 +14799,7 @@ All branches in an `if` must have the same type! 6│ printHello = \{} -> ^^^^^^^^^^ - Add an exclamation mark at the end of its name, like: + Add an exclamation mark at the end, like: printHello! @@ -14861,8 +14862,9 @@ All branches in an `if` must have the same type! 8│ hello! = \{} -> ^^^^^^ - Remove the exclamation mark to give an accurate impression of its - behavior. + The exclamation mark at the end is reserved for effectful functions. + + Hint: Did you forget to run an effect? Is the type annotation wrong? "### ); @@ -14926,7 +14928,7 @@ All branches in an `if` must have the same type! 8│ printLn = Effect.putLine! ^^^^^^^ - Add an exclamation mark at the end of its name, like: + Add an exclamation mark at the end, like: printLn! @@ -14958,7 +14960,7 @@ All branches in an `if` must have the same type! 7│ putLine: Effect.putLine! ^^^^^^^^^^^^^^^^^^^^^^^^ - Add an exclamation mark at the end of its name, like: + Add an exclamation mark at the end, like: { readFile! : File.read! } @@ -14990,8 +14992,81 @@ All branches in an `if` must have the same type! 7│ trim!: Str.trim ^^^^^^^^^^^^^^^ - Remove the exclamation mark to give an accurate impression of its - behavior. + The exclamation mark at the end is reserved for effectful functions. + + Hint: Did you forget to run an effect? Is the type annotation wrong? + "### + ); + + test_report!( + unsuffixed_fx_arg, + indoc!( + r#" + app [main!] { pf: platform "../../../../../examples/cli/effects-platform/main.roc" } + + import pf.Effect + + main! = \{} -> + ["Hello", "world!"] + |> forEach! Effect.putLine! + + forEach! : List a, (a => {}) => {} + forEach! = \l, f -> + when l is + [] -> {} + [x, .. as xs] -> + f x + forEach! xs f + "# + ), + @r###" + ── MISSING EXCLAMATION in /code/proj/Main.roc ────────────────────────────────── + + This matches an effectful function, but its name does not indicate so: + + 10│ forEach! = \l, f -> + ^ + + Add an exclamation mark at the end, like: + + f! + + This will help readers identify it as a source of effects. + "### + ); + + test_report!( + suffixed_pure_arg, + indoc!( + r#" + app [main!] { pf: platform "../../../../../examples/cli/effects-platform/main.roc" } + + import pf.Effect + + main! = \{} -> + Ok " hi " + |> mapOk Str.trim + |> Result.withDefault "" + |> Effect.putLine! + + mapOk : Result a err, (a -> b) -> Result b err + mapOk = \result, fn! -> + when result is + Ok x -> Ok (fn! x) + Err e -> Err e + "# + ), + @r###" + ── UNNECESSARY EXCLAMATION in /code/proj/Main.roc ────────────────────────────── + + This matches a pure function, but the name suggests otherwise: + + 12│ mapOk = \result, fn! -> + ^^^ + + The exclamation mark at the end is reserved for effectful functions. + + Hint: Did you forget to run an effect? Is the type annotation wrong? "### ); diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs index 33ecef708a..d42bcb9208 100644 --- a/crates/compiler/solve/src/solve.rs +++ b/crates/compiler/solve/src/solve.rs @@ -14,7 +14,9 @@ use crate::Aliases; use bumpalo::Bump; use roc_can::abilities::{AbilitiesStore, MemberSpecializationInfo}; use roc_can::constraint::Constraint::{self, *}; -use roc_can::constraint::{Cycle, FxCallConstraint, LetConstraint, OpportunisticResolve}; +use roc_can::constraint::{ + Cycle, FxCallConstraint, FxSuffixConstraint, FxSuffixKind, LetConstraint, OpportunisticResolve, +}; use roc_can::expected::{Expected, PExpected}; use roc_can::module::ModuleParams; use roc_collections::VecMap; @@ -26,7 +28,7 @@ use roc_module::ident::IdentSuffix; use roc_module::symbol::{ModuleId, Symbol}; use roc_problem::can::CycleEntry; use roc_region::all::{Loc, Region}; -use roc_solve_problem::{SuffixErrorKind, TypeError}; +use roc_solve_problem::TypeError; use roc_solve_schema::UnificationMode; use roc_types::subs::{ self, Content, FlatType, GetSubsSlice, Mark, OptVariable, Rank, Subs, TagExt, UlsOfVar, @@ -453,10 +455,9 @@ fn solve( check_ident_suffix( env, problems, - symbol.suffix(), + FxSuffixKind::Let(*symbol), loc_var.value, &loc_var.region, - SuffixErrorKind::Let(*symbol), ); } @@ -834,6 +835,27 @@ fn solve( } } } + FxSuffix(constraint_index) => { + let FxSuffixConstraint { + type_index, + kind, + region, + } = &env.constraints.fx_suffix_constraints[constraint_index.index()]; + + let actual = either_type_index_to_var( + env, + rank, + problems, + abilities_store, + obligation_cache, + &mut can_types, + aliases, + *type_index, + ); + + check_ident_suffix(env, problems, *kind, actual, region); + state + } EffectfulStmt(variable, region) => { let content = env.subs.get_content_without_compacting(*variable); @@ -882,17 +904,6 @@ fn solve( } } } - CheckRecordFieldFx(suffix, field_var, region) => { - check_ident_suffix( - env, - problems, - *suffix, - *field_var, - region, - SuffixErrorKind::RecordField, - ); - state - } Let(index, pool_slice) => { let let_con = &env.constraints.let_constraints[index.index()]; @@ -1614,12 +1625,11 @@ fn solve( fn check_ident_suffix( env: &mut InferenceEnv<'_>, problems: &mut Vec, - suffix: IdentSuffix, + kind: FxSuffixKind, variable: Variable, region: &Region, - kind: SuffixErrorKind, ) { - match suffix { + match kind.suffix() { IdentSuffix::None => { if let Content::Structure(FlatType::Func(_, _, _, fx)) = env.subs.get_content_without_compacting(variable) diff --git a/crates/compiler/solve_problem/src/lib.rs b/crates/compiler/solve_problem/src/lib.rs index bd4918f7fd..e5b3f88d9f 100644 --- a/crates/compiler/solve_problem/src/lib.rs +++ b/crates/compiler/solve_problem/src/lib.rs @@ -1,6 +1,7 @@ //! Provides types to describe problems that can occur during solving. use std::{path::PathBuf, str::Utf8Error}; +use roc_can::constraint::FxSuffixKind; use roc_can::{ constraint::FxCallKind, expected::{Expected, PExpected}, @@ -45,14 +46,8 @@ pub enum TypeError { FxInPureFunction(Region, FxCallKind, Option), FxInTopLevel(Region, FxCallKind), PureStmt(Region), - UnsuffixedEffectfulFunction(Region, SuffixErrorKind), - SuffixedPureFunction(Region, SuffixErrorKind), -} - -#[derive(Debug, Clone)] -pub enum SuffixErrorKind { - Let(Symbol), - RecordField, + UnsuffixedEffectfulFunction(Region, FxSuffixKind), + SuffixedPureFunction(Region, FxSuffixKind), } impl TypeError { diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index 4ed3c5703a..d890bc7f34 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -4,7 +4,7 @@ use crate::error::canonicalize::{to_circular_def_doc, CIRCULAR_DEF}; use crate::report::{Annotation, Report, RocDocAllocator, RocDocBuilder}; use itertools::EitherOrBoth; use itertools::Itertools; -use roc_can::constraint::FxCallKind; +use roc_can::constraint::{FxCallKind, FxSuffixKind}; use roc_can::expected::{Expected, PExpected}; use roc_collections::all::{HumanIndex, MutSet, SendMap}; use roc_collections::VecMap; @@ -16,7 +16,7 @@ use roc_module::symbol::Symbol; use roc_problem::Severity; use roc_region::all::{LineInfo, Region}; use roc_solve_problem::{ - NotDerivableContext, NotDerivableEq, SuffixErrorKind, TypeError, UnderivableReason, Unfulfilled, + NotDerivableContext, NotDerivableEq, TypeError, UnderivableReason, Unfulfilled, }; use roc_std::RocDec; use roc_types::pretty_print::{Parens, WILDCARD}; @@ -384,11 +384,23 @@ pub fn type_problem<'b>( severity, }) } - UnsuffixedEffectfulFunction(region, SuffixErrorKind::Let(symbol)) => { + UnsuffixedEffectfulFunction( + region, + kind @ (FxSuffixKind::Let(symbol) | FxSuffixKind::Pattern(symbol)), + ) => { let stack = [ - alloc.reflow("This function is effectful, but its name does not indicate so:"), + match kind { + FxSuffixKind::Let(_) => alloc + .reflow("This function is effectful, but its name does not indicate so:"), + FxSuffixKind::Pattern(_) => alloc.reflow( + "This matches an effectful function, but its name does not indicate so:", + ), + FxSuffixKind::RecordField(_) => { + unreachable!() + } + }, alloc.region(lines.convert_region(region), severity), - alloc.reflow("Add an exclamation mark at the end of its name, like:"), + alloc.reflow("Add an exclamation mark at the end, like:"), alloc .string(format!("{}!", symbol.as_str(alloc.interns))) .indent(4), @@ -401,13 +413,13 @@ pub fn type_problem<'b>( severity, }) } - UnsuffixedEffectfulFunction(region, SuffixErrorKind::RecordField) => { + UnsuffixedEffectfulFunction(region, FxSuffixKind::RecordField(_)) => { let stack = [ alloc.reflow( "This field's value is an effectful function, but its name does not indicate so:", ), alloc.region(lines.convert_region(region), severity), - alloc.reflow("Add an exclamation mark at the end of its name, like:"), + alloc.reflow("Add an exclamation mark at the end, like:"), alloc .parser_suggestion("{ readFile! : File.read! }") .indent(4), @@ -423,17 +435,19 @@ pub fn type_problem<'b>( SuffixedPureFunction(region, kind) => { let stack = [ match kind { - SuffixErrorKind::Let(_) => { + FxSuffixKind::Let(_) => { alloc.reflow("This function is pure, but its name suggests otherwise:") } - SuffixErrorKind::RecordField => { - alloc.reflow("This field's value is a pure function, but its name suggests otherwise:") - } + FxSuffixKind::Pattern(_) => alloc + .reflow("This matches a pure function, but the name suggests otherwise:"), + FxSuffixKind::RecordField(_) => alloc.reflow( + "This field's value is a pure function, but its name suggests otherwise:", + ), }, alloc.region(lines.convert_region(region), severity), - alloc.reflow( - "Remove the exclamation mark to give an accurate impression of its behavior.", - ), + alloc + .reflow("The exclamation mark at the end is reserved for effectful functions."), + alloc.hint("Did you forget to run an effect? Is the type annotation wrong?"), ]; Some(Report {