Merge pull request #2012 from rtfeldman/i/1714

Take syntactic sugar into account when reporting errors

ast/src/constrain.rs

@@ -277,7 +277,7 @@ pub fn constrain_expr<'a>(
             expr_id: expr_node_id,
             closure_var,
             fn_var,
-            ..
+            called_via,
         } => {
             // The expression that evaluates to the function being called, e.g. `foo` in
             // (foo) bar baz
@@ -349,7 +349,7 @@ pub fn constrain_expr<'a>(
                 region,
             );
 
-            let category = Category::CallResult(opt_symbol);
+            let category = Category::CallResult(opt_symbol, *called_via);
 
             let mut and_constraints = BumpVec::with_capacity_in(4, arena);
 

ast/src/lang/core/expr/expr2.rs

@@ -6,8 +6,8 @@ use crate::{
     mem_pool::{pool::NodeId, pool_str::PoolStr, pool_vec::PoolVec},
 };
 use roc_can::expr::Recursive;
+use roc_module::called_via::CalledVia;
 use roc_module::low_level::LowLevel;
-use roc_module::operator::CalledVia;
 use roc_module::symbol::Symbol;
 
 use super::record_field::RecordField;

ast/src/lang/core/str.rs

@@ -1,4 +1,4 @@
-use roc_module::{operator::CalledVia, symbol::Symbol};
+use roc_module::{called_via::CalledVia, symbol::Symbol};
 use roc_parse::ast::StrLiteral;
 
 use crate::{

cli/src/repl/eval.rs

@@ -2,8 +2,8 @@ use bumpalo::collections::Vec;
 use bumpalo::Bump;
 use libloading::Library;
 use roc_gen_llvm::{run_jit_function, run_jit_function_dynamic_type};
+use roc_module::called_via::CalledVia;
 use roc_module::ident::TagName;
-use roc_module::operator::CalledVia;
 use roc_module::symbol::{Interns, ModuleId, Symbol};
 use roc_mono::ir::ProcLayout;
 use roc_mono::layout::{union_sorted_tags_help, Builtin, Layout, UnionLayout, UnionVariant};

compiler/can/src/builtins.rs

@@ -3,9 +3,9 @@ use crate::expr::{ClosureData, Expr::*};
 use crate::expr::{Expr, Field, Recursive, WhenBranch};
 use crate::pattern::Pattern;
 use roc_collections::all::SendMap;
+use roc_module::called_via::CalledVia;
 use roc_module::ident::{Lowercase, TagName};
 use roc_module::low_level::LowLevel;
-use roc_module::operator::CalledVia;
 use roc_module::symbol::Symbol;
 use roc_region::all::{Located, Region};
 use roc_types::subs::{VarStore, Variable};

compiler/can/src/expr.rs

@@ -10,9 +10,9 @@ use crate::pattern::{canonicalize_pattern, Pattern};
 use crate::procedure::References;
 use crate::scope::Scope;
 use roc_collections::all::{ImSet, MutMap, MutSet, SendMap};
+use roc_module::called_via::CalledVia;
 use roc_module::ident::{ForeignSymbol, Lowercase, TagName};
 use roc_module::low_level::LowLevel;
-use roc_module::operator::CalledVia;
 use roc_module::symbol::Symbol;
 use roc_parse::ast::{self, EscapedChar, StrLiteral};
 use roc_parse::pattern::PatternType::*;
@@ -1711,7 +1711,7 @@ fn desugar_str_segments(var_store: &mut VarStore, segments: Vec<StrSegment>) ->
                 (var_store.fresh(), loc_new_expr),
                 (var_store.fresh(), loc_expr),
             ],
-            CalledVia::Space,
+            CalledVia::StringInterpolation,
         );
 
         loc_expr = Located::new(0, 0, 0, 0, expr);

compiler/can/src/operator.rs

@@ -2,9 +2,9 @@
 
 use bumpalo::collections::Vec;
 use bumpalo::Bump;
+use roc_module::called_via::BinOp::Pizza;
+use roc_module::called_via::{BinOp, CalledVia};
 use roc_module::ident::ModuleName;
-use roc_module::operator::BinOp::Pizza;
-use roc_module::operator::{BinOp, CalledVia};
 use roc_parse::ast::Expr::{self, *};
 use roc_parse::ast::{AssignedField, Def, WhenBranch};
 use roc_region::all::{Located, Region};
@@ -277,7 +277,7 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Located<Expr<'a>>) -> &'a
             })
         }
         UnaryOp(loc_arg, loc_op) => {
-            use roc_module::operator::UnaryOp::*;
+            use roc_module::called_via::UnaryOp::*;
 
             let region = loc_op.region;
             let op = loc_op.value;
@@ -475,7 +475,7 @@ fn binop_step<'a>(
     op_stack: &mut Vec<Located<BinOp>>,
     next_op: Located<BinOp>,
 ) -> Step<'a> {
-    use roc_module::operator::Associativity::*;
+    use roc_module::called_via::Associativity::*;
     use std::cmp::Ordering;
 
     match op_stack.pop() {

compiler/constrain/src/expr.rs

@@ -254,7 +254,7 @@ pub fn constrain_expr(
                 exists(vec![*elem_var], And(constraints))
             }
         }
-        Call(boxed, loc_args, _application_style) => {
+        Call(boxed, loc_args, called_via) => {
             let (fn_var, loc_fn, closure_var, ret_var) = &**boxed;
             // The expression that evaluates to the function being called, e.g. `foo` in
             // (foo) bar baz
@@ -317,7 +317,7 @@ pub fn constrain_expr(
                 region,
             );
 
-            let category = Category::CallResult(opt_symbol);
+            let category = Category::CallResult(opt_symbol, *called_via);
 
             exists(
                 vars,

compiler/fmt/src/expr.rs

@@ -3,7 +3,7 @@ use crate::def::fmt_def;
 use crate::pattern::fmt_pattern;
 use crate::spaces::{add_spaces, fmt_comments_only, fmt_spaces, newline, NewlineAt, INDENT};
 use bumpalo::collections::String;
-use roc_module::operator::{self, BinOp};
+use roc_module::called_via::{self, BinOp};
 use roc_parse::ast::StrSegment;
 use roc_parse::ast::{
     AssignedField, Base, Collection, CommentOrNewline, Expr, Pattern, WhenBranch,
@@ -296,10 +296,10 @@ impl<'a> Formattable<'a> for Expr<'a> {
             BinOps(lefts, right) => fmt_bin_ops(buf, lefts, right, false, parens, indent),
             UnaryOp(sub_expr, unary_op) => {
                 match &unary_op.value {
-                    operator::UnaryOp::Negate => {
+                    called_via::UnaryOp::Negate => {
                         buf.push('-');
                     }
-                    operator::UnaryOp::Not => {
+                    called_via::UnaryOp::Not => {
                         buf.push('!');
                     }
                 }
@@ -354,26 +354,26 @@ fn format_str_segment<'a>(seg: &StrSegment<'a>, buf: &mut String<'a>, indent: u1
 
 fn push_op(buf: &mut String, op: BinOp) {
     match op {
-        operator::BinOp::Caret => buf.push('^'),
-        operator::BinOp::Star => buf.push('*'),
-        operator::BinOp::Slash => buf.push('/'),
-        operator::BinOp::DoubleSlash => buf.push_str("//"),
-        operator::BinOp::Percent => buf.push('%'),
-        operator::BinOp::DoublePercent => buf.push_str("%%"),
-        operator::BinOp::Plus => buf.push('+'),
-        operator::BinOp::Minus => buf.push('-'),
-        operator::BinOp::Equals => buf.push_str("=="),
-        operator::BinOp::NotEquals => buf.push_str("!="),
-        operator::BinOp::LessThan => buf.push('<'),
-        operator::BinOp::GreaterThan => buf.push('>'),
-        operator::BinOp::LessThanOrEq => buf.push_str("<="),
-        operator::BinOp::GreaterThanOrEq => buf.push_str(">="),
-        operator::BinOp::And => buf.push_str("&&"),
-        operator::BinOp::Or => buf.push_str("||"),
-        operator::BinOp::Pizza => buf.push_str("|>"),
-        operator::BinOp::Assignment => unreachable!(),
-        operator::BinOp::HasType => unreachable!(),
-        operator::BinOp::Backpassing => unreachable!(),
+        called_via::BinOp::Caret => buf.push('^'),
+        called_via::BinOp::Star => buf.push('*'),
+        called_via::BinOp::Slash => buf.push('/'),
+        called_via::BinOp::DoubleSlash => buf.push_str("//"),
+        called_via::BinOp::Percent => buf.push('%'),
+        called_via::BinOp::DoublePercent => buf.push_str("%%"),
+        called_via::BinOp::Plus => buf.push('+'),
+        called_via::BinOp::Minus => buf.push('-'),
+        called_via::BinOp::Equals => buf.push_str("=="),
+        called_via::BinOp::NotEquals => buf.push_str("!="),
+        called_via::BinOp::LessThan => buf.push('<'),
+        called_via::BinOp::GreaterThan => buf.push('>'),
+        called_via::BinOp::LessThanOrEq => buf.push_str("<="),
+        called_via::BinOp::GreaterThanOrEq => buf.push_str(">="),
+        called_via::BinOp::And => buf.push_str("&&"),
+        called_via::BinOp::Or => buf.push_str("||"),
+        called_via::BinOp::Pizza => buf.push_str("|>"),
+        called_via::BinOp::Assignment => unreachable!(),
+        called_via::BinOp::HasType => unreachable!(),
+        called_via::BinOp::Backpassing => unreachable!(),
     }
 }
 

compiler/load/src/effect_module.rs

@@ -5,8 +5,8 @@ use roc_can::expr::{ClosureData, Expr, Recursive};
 use roc_can::pattern::Pattern;
 use roc_can::scope::Scope;
 use roc_collections::all::{MutSet, SendMap};
+use roc_module::called_via::CalledVia;
 use roc_module::ident::TagName;
-use roc_module::operator::CalledVia;
 use roc_module::symbol::Symbol;
 use roc_region::all::{Located, Region};
 use roc_types::subs::{VarStore, Variable};

compiler/module/src/called_via.rs

@@ -12,6 +12,10 @@ pub enum CalledVia {
 
     /// Calling with a unary operator, e.g. (!foo bar baz) or (-foo bar baz)
     UnaryOp(UnaryOp),
+
+    /// This call is the result of desugaring string interpolation,
+    /// e.g. "\(first) \(last)" is transformed into Str.concat (Str.concat first " ") last.
+    StringInterpolation,
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]

compiler/module/src/lib.rs

@@ -2,10 +2,10 @@
 // See github.com/rtfeldman/roc/issues/800 for discussion of the large_enum_variant check.
 #![allow(clippy::large_enum_variant, clippy::upper_case_acronyms)]
 
+pub mod called_via;
 pub mod ident;
 pub mod low_level;
 pub mod module_err;
-pub mod operator;
 pub mod symbol;
 
 #[macro_use]

compiler/parse/src/ast.rs

@@ -4,7 +4,7 @@ use crate::header::{AppHeader, ImportsEntry, InterfaceHeader, PlatformHeader, Ty
 use crate::ident::Ident;
 use bumpalo::collections::{String, Vec};
 use bumpalo::Bump;
-use roc_module::operator::{BinOp, CalledVia, UnaryOp};
+use roc_module::called_via::{BinOp, CalledVia, UnaryOp};
 use roc_region::all::{Loc, Position, Region};
 
 #[derive(Clone, Debug, PartialEq)]

compiler/parse/src/expr.rs

@@ -13,7 +13,7 @@ use crate::pattern::loc_closure_param;
 use crate::type_annotation;
 use bumpalo::collections::Vec;
 use bumpalo::Bump;
-use roc_module::operator::{BinOp, CalledVia, UnaryOp};
+use roc_module::called_via::{BinOp, CalledVia, UnaryOp};
 use roc_region::all::{Located, Position, Region};
 
 use crate::parser::Progress::{self, *};

compiler/problem/src/can.rs

@@ -1,6 +1,6 @@
 use roc_collections::all::MutSet;
+use roc_module::called_via::BinOp;
 use roc_module::ident::{Ident, Lowercase, ModuleName, TagName};
-use roc_module::operator::BinOp;
 use roc_module::symbol::{ModuleId, Symbol};
 use roc_parse::ast::Base;
 use roc_parse::pattern::PatternType;

compiler/types/src/types.rs

@@ -3,6 +3,7 @@ use crate::subs::{
     GetSubsSlice, RecordFields, Subs, UnionTags, VarStore, Variable, VariableSubsSlice,
 };
 use roc_collections::all::{ImMap, ImSet, Index, MutSet, SendMap};
+use roc_module::called_via::CalledVia;
 use roc_module::ident::{ForeignSymbol, Ident, Lowercase, TagName};
 use roc_module::low_level::LowLevel;
 use roc_module::symbol::{Interns, ModuleId, Symbol};
@@ -1134,7 +1135,7 @@ pub enum Reason {
 #[derive(PartialEq, Debug, Clone)]
 pub enum Category {
     Lookup(Symbol),
-    CallResult(Option<Symbol>),
+    CallResult(Option<Symbol>, CalledVia),
     LowLevelOpResult(LowLevel),
     ForeignCall,
     TagApply {

reporting/src/error/type.rs

@@ -1,5 +1,6 @@
 use roc_can::expected::{Expected, PExpected};
 use roc_collections::all::{Index, MutSet, SendMap};
+use roc_module::called_via::{BinOp, CalledVia};
 use roc_module::ident::{Ident, IdentStr, Lowercase, TagName};
 use roc_module::symbol::Symbol;
 use roc_region::all::{Located, Region};
@@ -1043,13 +1044,26 @@ fn add_category<'b>(
             alloc.record_field(field.to_owned()),
             alloc.text(" is a:"),
         ]),
-
-        CallResult(Some(symbol)) => alloc.concat(vec![
+        CallResult(
+            Some(_),
+            CalledVia::BinOp(
+                BinOp::Equals
+                | BinOp::NotEquals
+                | BinOp::LessThan
+                | BinOp::GreaterThan
+                | BinOp::LessThanOrEq
+                | BinOp::GreaterThanOrEq,
+            ),
+        ) => alloc.concat(vec![alloc.text("This comparison produces:")]),
+        CallResult(Some(_), CalledVia::StringInterpolation) => {
+            alloc.concat(vec![this_is, alloc.text(" a string of type:")])
+        }
+        CallResult(Some(symbol), _) => alloc.concat(vec![
             alloc.text("This "),
             alloc.symbol_foreign_qualified(*symbol),
             alloc.text(" call produces:"),
         ]),
-        CallResult(None) => alloc.concat(vec![this_is, alloc.text(":")]),
+        CallResult(None, _) => alloc.concat(vec![this_is, alloc.text(":")]),
         LowLevelOpResult(op) => {
             panic!(
                 "Compiler bug: invalid return type from low-level op {:?}",

reporting/src/report.rs

@@ -353,7 +353,7 @@ impl<'a> RocDocAllocator<'a> {
 
     pub fn binop(
         &'a self,
-        content: roc_module::operator::BinOp,
+        content: roc_module::called_via::BinOp,
     ) -> DocBuilder<'a, Self, Annotation> {
         self.text(content.to_string()).annotate(Annotation::BinOp)
     }

reporting/tests/test_reporting.rs

@@ -5586,6 +5586,82 @@ mod test_reporting {
         )
     }
 
+    #[test]
+    // https://github.com/rtfeldman/roc/issues/1714
+    fn interpolate_concat_is_transparent_1714() {
+        report_problem_as(
+            indoc!(
+                r#"
+                greeting = "Privet"
+
+                if True then 1 else "\(greeting), World!"
+                "#,
+            ),
+            indoc!(
+                r#"
+                ── TYPE MISMATCH ───────────────────────────────────────────────────────────────
+
+                This `if` has an `else` branch with a different type from its `then` branch:
+
+                3│  if True then 1 else "\(greeting), World!"
+                                        ^^^^^^^^^^^^^^^^^^^^^
+
+                The `else` branch is a string of type:
+
+                    Str
+
+                but the `then` branch has the type:
+
+                    Num a
+
+                I need all branches in an `if` to have the same type!
+                "#
+            ),
+        )
+    }
+
+    macro_rules! comparison_binop_transparency_tests {
+        ($($op:expr, $name:ident),* $(,)?) => {
+            $(
+            #[test]
+            fn $name() {
+                report_problem_as(
+                    &format!(r#"if True then "abc" else 1 {} 2"#, $op),
+                    &format!(
+r#"── TYPE MISMATCH ───────────────────────────────────────────────────────────────
+
+This `if` has an `else` branch with a different type from its `then` branch:
+
+1│  if True then "abc" else 1 {} 2
+                            ^^{}^^
+
+This comparison produces:
+
+    Bool
+
+but the `then` branch has the type:
+
+    Str
+
+I need all branches in an `if` to have the same type!
+"#,
+                        $op, "^".repeat($op.len())
+                    ),
+                )
+            }
+            )*
+        }
+    }
+
+    comparison_binop_transparency_tests! {
+        "<", lt_binop_is_transparent,
+        ">", gt_binop_is_transparent,
+        "==", eq_binop_is_transparent,
+        "!=", neq_binop_is_transparent,
+        "<=", leq_binop_is_transparent,
+        ">=", geq_binop_is_transparent,
+    }
+
     #[test]
     fn keyword_record_field_access() {
         report_problem_as(