Check record field suffixes in annotations

crates/compiler/can/src/annotation.rs

@@ -2,12 +2,12 @@ use crate::env::Env;
 use crate::procedure::{QualifiedReference, References};
 use crate::scope::{PendingAbilitiesInScope, Scope, SymbolLookup};
 use roc_collections::{ImMap, MutSet, SendMap, VecMap, VecSet};
-use roc_module::ident::{Ident, Lowercase, TagName};
+use roc_module::ident::{Ident, IdentSuffix, Lowercase, TagName};
 use roc_module::symbol::Symbol;
 use roc_parse::ast::{
     AssignedField, ExtractSpaces, FunctionArrow, Pattern, Tag, TypeAnnotation, TypeHeader,
 };
-use roc_problem::can::ShadowKind;
+use roc_problem::can::{Problem, ShadowKind};
 use roc_region::all::{Loc, Region};
 use roc_types::subs::{VarStore, Variable};
 use roc_types::types::{
@@ -1378,6 +1378,8 @@ fn can_assigned_fields<'a>(
                     );
 
                     let label = Lowercase::from(field_name.value);
+                    check_record_field_suffix(env, label.suffix(), &field_type, &loc_field.region);
+
                     field_types.insert(label.clone(), RigidRequired(field_type));
 
                     break 'inner label;
@@ -1396,6 +1398,8 @@ fn can_assigned_fields<'a>(
                     );
 
                     let label = Lowercase::from(field_name.value);
+                    check_record_field_suffix(env, label.suffix(), &field_type, &loc_field.region);
+
                     field_types.insert(label.clone(), RigidOptional(field_type));
 
                     break 'inner label;
@@ -1450,6 +1454,23 @@ fn can_assigned_fields<'a>(
     field_types
 }
 
+fn check_record_field_suffix(
+    env: &mut Env,
+    suffix: IdentSuffix,
+    field_type: &Type,
+    region: &Region,
+) {
+    match (suffix, field_type) {
+        (IdentSuffix::None, Type::Function(_, _, _, fx)) if **fx == Type::Effectful => env
+            .problems
+            .push(Problem::UnsuffixedEffectfulRecordField(*region)),
+        (IdentSuffix::Bang, Type::Function(_, _, _, fx)) if **fx == Type::Pure => {
+            env.problems.push(Problem::SuffixedPureRecordField(*region))
+        }
+        _ => {}
+    }
+}
+
 // TODO trim down these arguments!
 #[allow(clippy::too_many_arguments)]
 fn can_assigned_tuple_elems(

crates/compiler/load/tests/test_reporting.rs

@@ -101,7 +101,7 @@ mod test_reporting {
         use std::fs::File;
         use std::io::Write;
 
-        let module_src = if src.starts_with("app") {
+        let module_src = if src.starts_with("app") || src.starts_with("module") {
             maybe_save_parse_test_case(subdir, src, false);
             // this is already a module
             src.to_string()
@@ -15006,6 +15006,60 @@ All branches in an `if` must have the same type!
     "###
     );
 
+    test_report!(
+        unsuffixed_fx_in_record_annotation,
+        indoc!(
+            r#"
+            module [Fx]
+
+            Fx : {
+                getLine: {} => Str
+            }
+            "#
+        ),
+        @r"
+    ── MISSING EXCLAMATION in /code/proj/Main.roc ──────────────────────────────────
+
+    The type of this record field is an effectful function, but its name
+    does not indicate so:
+
+    4│      getLine: {} => Str
+            ^^^^^^^^^^^^^^^^^^
+
+    Add an exclamation mark at the end, like:
+
+        { readFile!: Str => Str }
+
+    This will help readers identify it as a source of effects.
+    "
+    );
+
+    test_report!(
+        suffixed_pure_fn_in_record_annotation,
+        indoc!(
+            r#"
+            module [Fx]
+
+            Fx : {
+                getLine!: {} -> Str
+            }
+            "#
+        ),
+        @r"
+    ── UNNECESSARY EXCLAMATION in /code/proj/Main.roc ──────────────────────────────
+
+    The type of this record field is a pure function, but its name
+    suggests otherwise:
+
+    4│      getLine!: {} -> Str
+            ^^^^^^^^^^^^^^^^^^^
+
+    The exclamation mark at the end is reserved for effectful functions.
+
+    Hint: Did you mean to use `=>` instead of `->`?
+    "
+    );
+
     test_report!(
         unsuffixed_fx_arg,
         indoc!(

crates/compiler/problem/src/can.rs

@@ -252,6 +252,8 @@ pub enum Problem {
         region: Region,
     },
     StmtAfterExpr(Region),
+    UnsuffixedEffectfulRecordField(Region),
+    SuffixedPureRecordField(Region),
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -337,6 +339,9 @@ impl Problem {
             Problem::StatementsAfterReturn { .. } => Warning,
             Problem::ReturnAtEndOfFunction { .. } => Warning,
             Problem::StmtAfterExpr(_) => Fatal,
+            Problem::UnsuffixedEffectfulRecordField(_) | Problem::SuffixedPureRecordField(..) => {
+                Warning
+            }
         }
     }
 
@@ -502,11 +507,14 @@ impl Problem {
             | Problem::DefsOnlyUsedInRecursion(_, region)
             | Problem::ReturnOutsideOfFunction { region }
             | Problem::StatementsAfterReturn { region }
-            | Problem::ReturnAtEndOfFunction { region } => Some(*region),
+            | Problem::ReturnAtEndOfFunction { region }
+            | Problem::UnsuffixedEffectfulRecordField(region)
+            | Problem::SuffixedPureRecordField(region) => Some(*region),
             Problem::RuntimeError(RuntimeError::CircularDef(cycle_entries))
             | Problem::BadRecursion(cycle_entries) => {
                 cycle_entries.first().map(|entry| entry.expr_region)
             }
+
             Problem::StmtAfterExpr(region) => Some(*region),
             Problem::RuntimeError(RuntimeError::UnresolvedTypeVar)
             | Problem::RuntimeError(RuntimeError::ErroneousType)

crates/reporting/src/error/canonicalize.rs

@@ -65,6 +65,8 @@ const DUPLICATE_IMPLEMENTATION: &str = "DUPLICATE IMPLEMENTATION";
 const UNNECESSARY_IMPLEMENTATIONS: &str = "UNNECESSARY IMPLEMENTATIONS";
 const INCOMPLETE_ABILITY_IMPLEMENTATION: &str = "INCOMPLETE ABILITY IMPLEMENTATION";
 const STATEMENT_AFTER_EXPRESSION: &str = "STATEMENT AFTER EXPRESSION";
+const MISSING_EXCLAMATION: &str = "MISSING EXCLAMATION";
+const UNNECESSARY_EXCLAMATION: &str = "UNNECESSARY EXCLAMATION";
 
 pub fn can_problem<'b>(
     alloc: &'b RocDocAllocator<'b>,
@@ -1427,6 +1429,42 @@ pub fn can_problem<'b>(
 
             title = STATEMENT_AFTER_EXPRESSION.to_string();
         }
+
+        Problem::UnsuffixedEffectfulRecordField(region) => {
+            doc = alloc.stack([
+                alloc.reflow(
+                    "The type of this record field 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, like:"),
+                alloc
+                    .parser_suggestion("{ readFile!: Str => Str }")
+                    .indent(4),
+                alloc.reflow("This will help readers identify it as a source of effects."),
+            ]);
+
+            title = MISSING_EXCLAMATION.to_string();
+        }
+
+        Problem::SuffixedPureRecordField(region) => {
+            doc = alloc.stack([
+                alloc.reflow(
+                    "The type of this record field is a pure function, but its name suggests otherwise:",
+                ),
+                alloc.region(lines.convert_region(region), severity),
+                alloc
+                    .reflow("The exclamation mark at the end is reserved for effectful functions."),
+                alloc.concat([
+                    alloc.hint("Did you mean to use "),
+                    alloc.keyword("=>"),
+                    alloc.text(" instead of "),
+                    alloc.keyword("->"),
+                    alloc.text("?"),
+                ]),
+            ]);
+
+            title = UNNECESSARY_EXCLAMATION.to_string();
+        }
     };
 
     Report {