Check suffixes of all pattern identifiers

crates/compiler/can/src/constraint.rs

@@ -30,6 +30,7 @@ pub struct Constraints {
     pub pattern_eq: Vec<PatternEq>,
     pub cycles: Vec<Cycle>,
     pub fx_call_constraints: Vec<FxCallConstraint>,
+    pub fx_suffix_constraints: Vec<FxSuffixConstraint>,
 }
 
 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(
+    pub fn fx_pattern_suffix(
-        &self,
+        &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<FxCallConstraint>),
+    /// Require idents to be accurately suffixed
+    FxSuffix(Index<FxSuffixConstraint>),
     /// 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(),

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);

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 {

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
+    The exclamation mark at the end is reserved for effectful functions.
-    behavior.
+
+    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
+    The exclamation mark at the end is reserved for effectful functions.
-    behavior.
+
+    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
+    The exclamation mark at the end is reserved for effectful functions.
-    behavior.
+
+    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?
     "###
     );
 

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<TypeError>,
-    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)

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<Region>),
     FxInTopLevel(Region, FxCallKind),
     PureStmt(Region),
-    UnsuffixedEffectfulFunction(Region, SuffixErrorKind),
+    UnsuffixedEffectfulFunction(Region, FxSuffixKind),
-    SuffixedPureFunction(Region, SuffixErrorKind),
+    SuffixedPureFunction(Region, FxSuffixKind),
-}
-
-#[derive(Debug, Clone)]
-pub enum SuffixErrorKind {
-    Let(Symbol),
-    RecordField,
 }
 
 impl TypeError {

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 => {
+                    FxSuffixKind::Pattern(_) => alloc
-                        alloc.reflow("This field's value is a pure function, but its name suggests otherwise:")
+                        .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(
+                alloc
-                    "Remove the exclamation mark to give an accurate impression of its behavior.",
+                    .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 {