diff --git a/CHANGELOG.md b/CHANGELOG.md index a7b8d54e3fd..9de76511011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ on the Erlang target. ([Giacomo Cavalieri](https://github.com/giacomocavalieri)) +- Type mismatch errors now emphasize the exact location of mismatches in complex + types by highlighting the mismatched type parameters. This makes it easier to + spot errors in types with multiple parameters or deep nesting. + ([Adi Salimgereyev](https://github.com/abs0luty)) + ### Build tool - The help text displayed by `gleam dev --help`, `gleam test --help`, and diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs index 2aef77b3b50..ac9c273c732 100644 --- a/compiler-core/src/error.rs +++ b/compiler-core/src/error.rs @@ -12,7 +12,7 @@ use crate::type_::error::{ IncorrectArityContext, InvalidImportKind, MissingAnnotation, ModuleValueUsageContext, Named, UnknownField, UnknownTypeHint, UnsafeRecordUpdateReason, }; -use crate::type_::printer::{Names, Printer}; +use crate::type_::printer::{Names, Printer, TypeAnnotation, TypePathStep}; use crate::type_::{FieldAccessUsage, error::PatternMatchKind}; use crate::{ast::BinOp, parse::error::ParseErrorType, type_::Type}; use crate::{bit_array, diagnostic::Level, type_::UnifyErrorSituation}; @@ -2143,6 +2143,9 @@ But function expects: situation, } => { let mut printer = Printer::new(names); + let expected = collapse_links(expected.clone()); + let given = collapse_links(given.clone()); + let mut text = if let Some(description) = situation.as_ref().and_then(|s| s.description()) { @@ -2153,10 +2156,42 @@ But function expects: } else { "".into() }; - text.push_str("Expected type:\n\n "); - text.push_str(&printer.print_type(expected)); - text.push_str("\n\nFound type:\n\n "); - text.push_str(&printer.print_type(given)); + + let expected_display = printer.print_type(&expected); + let (given_display, annotations) = + printer.print_type_with_annotations(&given); + let differences = collect_type_differences(expected.clone(), given.clone()); + + // Only show smart diff suggestions if the type is complex enough + let highlight_lines = if should_show_smart_diff(&differences) { + build_highlight_lines(&differences, &annotations, &mut printer) + } else { + Vec::new() + }; + + // Only show "Expected type:" if there are no smart diff suggestions + if highlight_lines.is_empty() { + text.push_str("Expected type:\n\n "); + text.push_str(&expected_display); + text.push_str("\n\n"); + } + + text.push_str("Found type:\n\n "); + text.push_str(&given_display); + + for highlight in highlight_lines { + text.push('\n'); + text.push_str(" "); + for _ in 0..highlight.start { + text.push(' '); + } + let span_len = highlight.end.saturating_sub(highlight.start).max(1); + for _ in 0..span_len { + text.push('^'); + } + text.push_str(" expected: "); + text.push_str(&highlight.expected); + } let (main_message_location, main_message_text, extra_labels) = match situation { @@ -2170,9 +2205,11 @@ But function expects: }) => (clause_location, None, vec![]), // In all other cases we just highlight the offending expression, optionally // adding the wrapping hint if it makes sense. - Some(_) | None => { - (location, hint_wrap_value_in_result(expected, given), vec![]) - } + Some(_) | None => ( + location, + hint_wrap_value_in_result(&expected, &given), + vec![], + ), }; Diagnostic { @@ -4578,6 +4615,205 @@ fn hint_alternative_operator(op: &BinOp, given: &Type) -> Option { } } +#[derive(Debug, Clone)] +struct TypeDifference { + path: Vec, + expected: Arc, +} + +fn collect_type_differences(expected: Arc, given: Arc) -> Vec { + let mut differences = Vec::new(); + let mut path = Vec::new(); + collect_type_differences_inner(expected, given, &mut path, &mut differences); + differences +} + +fn collect_type_differences_inner( + expected: Arc, + given: Arc, + path: &mut Vec, + differences: &mut Vec, +) { + let expected = collapse_links(expected); + let given = collapse_links(given); + + if Arc::ptr_eq(&expected, &given) { + return; + } + + match (&*expected, &*given) { + ( + Type::Named { + module: module_expected, + name: name_expected, + arguments: arguments_expected, + .. + }, + Type::Named { + module: module_given, + name: name_given, + arguments: arguments_given, + .. + }, + ) if module_expected == module_given + && name_expected == name_given + && arguments_expected.len() == arguments_given.len() => + { + for (index, (expected_argument, given_argument)) in arguments_expected + .iter() + .zip(arguments_given.iter()) + .enumerate() + { + path.push(TypePathStep::NamedArgument(index)); + collect_type_differences_inner( + expected_argument.clone(), + given_argument.clone(), + path, + differences, + ); + let _ = path.pop(); + } + } + ( + Type::Fn { + arguments: arguments_expected, + return_: return_expected, + }, + Type::Fn { + arguments: arguments_given, + return_: return_given, + }, + ) if arguments_expected.len() == arguments_given.len() => { + for (index, (expected_argument, given_argument)) in arguments_expected + .iter() + .zip(arguments_given.iter()) + .enumerate() + { + path.push(TypePathStep::FnArgument(index)); + collect_type_differences_inner( + expected_argument.clone(), + given_argument.clone(), + path, + differences, + ); + let _ = path.pop(); + } + path.push(TypePathStep::FnReturn); + collect_type_differences_inner( + return_expected.clone(), + return_given.clone(), + path, + differences, + ); + let _ = path.pop(); + } + ( + Type::Tuple { + elements: elements_expected, + .. + }, + Type::Tuple { + elements: elements_given, + .. + }, + ) if elements_expected.len() == elements_given.len() => { + for (index, (expected_element, given_element)) in elements_expected + .iter() + .zip(elements_given.iter()) + .enumerate() + { + path.push(TypePathStep::TupleElement(index)); + collect_type_differences_inner( + expected_element.clone(), + given_element.clone(), + path, + differences, + ); + let _ = path.pop(); + } + } + _ => differences.push(TypeDifference { + path: path.clone(), + expected, + }), + } +} + +#[derive(Debug)] +struct HighlightLine { + start: usize, + end: usize, + expected: EcoString, +} + +fn build_highlight_lines( + differences: &[TypeDifference], + annotations: &[TypeAnnotation], + printer: &mut Printer<'_>, +) -> Vec { + let mut highlights = Vec::new(); + + for difference in differences { + if difference.path.is_empty() { + continue; + } + + if let Some(annotation) = annotations + .iter() + .find(|annotation| annotation.path == difference.path) + { + if annotation.range.start == annotation.range.end { + continue; + } + let expected = printer.print_type(&difference.expected); + highlights.push(HighlightLine { + start: annotation.range.start, + end: annotation.range.end, + expected, + }); + } + } + + highlights.sort_by_key(|highlight| highlight.start); + highlights +} + +/// Determines if the type mismatch is "complex enough" to warrant showing smart diff suggestions. +/// +/// Smart diffs are helpful when: +/// 1. There are multiple type parameter mismatches (2+) +/// 2. The mismatch occurs at depth >= 2 (nested within nested types) +/// 3. The parent type has multiple parameters (2+) with at least one mismatch +fn should_show_smart_diff(differences: &[TypeDifference]) -> bool { + // Filter out empty path differences (top-level mismatches) + let nested_diffs: Vec<_> = differences.iter().filter(|d| !d.path.is_empty()).collect(); + + if nested_diffs.is_empty() { + return false; + } + + // Count how many distinct mismatches we have + let num_mismatches = nested_diffs.len(); + + // Check the maximum depth of mismatches + let max_depth = nested_diffs.iter().map(|d| d.path.len()).max().unwrap_or(0); + + // Criterion 1: Multiple mismatches (2+) at any depth + if num_mismatches >= 2 { + return true; + } + + // Criterion 2: Deep nesting (depth >= 2) + // For example: Wrapper(List(Int)) has depth 2 + if max_depth >= 2 { + return true; + } + + // If we get here, we have exactly 1 mismatch at depth 1 + // This is like List(Int) vs List(String) - simple enough to not need smart diff + false +} + fn hint_wrap_value_in_result(expected: &Arc, given: &Arc) -> Option { let expected = collapse_links(expected.clone()); let (expected_ok_type, expected_error_type) = expected.result_types()?; diff --git a/compiler-core/src/type_/printer.rs b/compiler-core/src/type_/printer.rs index 34dc3829e5c..18453b1ceb3 100644 --- a/compiler-core/src/type_/printer.rs +++ b/compiler-core/src/type_/printer.rs @@ -1,7 +1,7 @@ use bimap::BiMap; use ecow::{EcoString, eco_format}; use im::HashMap; -use std::{collections::HashSet, sync::Arc}; +use std::{collections::HashSet, ops::Range, sync::Arc}; use crate::{ ast::SrcSpan, @@ -379,6 +379,20 @@ pub enum PrintMode { ExpandAliases, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum TypePathStep { + NamedArgument(usize), + FnArgument(usize), + FnReturn, + TupleElement(usize), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TypeAnnotation { + pub path: Vec, + pub range: Range, +} + /// A type printer that does not wrap and indent, but does take into account the /// names that types and modules have been aliased with in the current module. #[derive(Debug)] @@ -460,6 +474,25 @@ impl<'a> Printer<'a> { buffer } + pub fn print_type_with_annotations( + &mut self, + type_: &Type, + ) -> (EcoString, Vec) { + self.print_type_with_annotations_mode(type_, PrintMode::Normal) + } + + pub fn print_type_with_annotations_mode( + &mut self, + type_: &Type, + print_mode: PrintMode, + ) -> (EcoString, Vec) { + let mut buffer = EcoString::new(); + let mut annotations = Vec::new(); + let mut path = Vec::new(); + self.print_with_annotations(type_, &mut buffer, print_mode, &mut path, &mut annotations); + (buffer, annotations) + } + pub fn print_module(&self, module: &str) -> EcoString { match self.names.imported_modules.get(module) { Some((module, _)) => module.clone(), @@ -526,6 +559,104 @@ impl<'a> Printer<'a> { } } + fn print_with_annotations( + &mut self, + type_: &Type, + buffer: &mut EcoString, + print_mode: PrintMode, + path: &mut Vec, + annotations: &mut Vec, + ) { + let start = buffer.len(); + + match type_ { + Type::Named { + name, + arguments, + module, + .. + } => { + let (module, name) = match self.names.named_type(module, name, print_mode) { + NameContextInformation::Qualified(module, name) => (Some(module), name), + NameContextInformation::Unqualified(name) => (None, name), + NameContextInformation::Unimported(module, name) => { + (module.split('/').next_back(), name) + } + }; + + if let Some(module) = module { + buffer.push_str(module); + buffer.push('.'); + } + buffer.push_str(name); + + if !arguments.is_empty() { + buffer.push('('); + for (index, argument) in arguments.iter().enumerate() { + if index > 0 { + buffer.push_str(", "); + } + path.push(TypePathStep::NamedArgument(index)); + self.print_with_annotations( + argument, + buffer, + print_mode, + path, + annotations, + ); + let _ = path.pop(); + } + buffer.push(')'); + } + } + + Type::Fn { arguments, return_ } => { + buffer.push_str("fn("); + for (index, argument) in arguments.iter().enumerate() { + if index > 0 { + buffer.push_str(", "); + } + path.push(TypePathStep::FnArgument(index)); + self.print_with_annotations(argument, buffer, print_mode, path, annotations); + let _ = path.pop(); + } + buffer.push_str(") -> "); + path.push(TypePathStep::FnReturn); + self.print_with_annotations(return_, buffer, print_mode, path, annotations); + let _ = path.pop(); + } + + Type::Var { type_, .. } => match *type_.borrow() { + TypeVar::Link { ref type_, .. } => { + self.print_with_annotations(type_, buffer, print_mode, path, annotations); + return; + } + TypeVar::Unbound { id, .. } | TypeVar::Generic { id, .. } => { + buffer.push_str(&self.type_variable(id)) + } + }, + + Type::Tuple { elements, .. } => { + buffer.push_str("#("); + for (index, element) in elements.iter().enumerate() { + if index > 0 { + buffer.push_str(", "); + } + path.push(TypePathStep::TupleElement(index)); + self.print_with_annotations(element, buffer, print_mode, path, annotations); + let _ = path.pop(); + } + buffer.push(')'); + } + } + + let end = buffer.len(); + annotations.push(TypeAnnotation { + path: path.clone(), + range: start..end, + }); + } + pub fn print_constructor(&mut self, module: &EcoString, name: &EcoString) -> EcoString { let (module, name) = match self.names.named_constructor(module, name) { NameContextInformation::Qualified(module, name) => (Some(module), name), diff --git a/compiler-core/src/type_/tests/errors.rs b/compiler-core/src/type_/tests/errors.rs index 37084a6e083..df52cd2f3fc 100644 --- a/compiler-core/src/type_/tests/errors.rs +++ b/compiler-core/src/type_/tests/errors.rs @@ -360,6 +360,32 @@ fn pipe_value_type_mismatch_error() { ); } +#[test] +fn emphasize_multiple_type_mismatches() { + assert_module_error!( + "type Container(a, b) { + Container(first: a, second: b) + } + + pub fn main() { + let container: Container(String, #(Int, Float)) = Container(123, #(2, \"hello\")) + }" + ); +} + +#[test] +fn emphasize_nested_type_mismatches() { + assert_module_error!( + "type Wrapper(a) { + Wrapper(value: a) + } + + pub fn main() { + let nested: Wrapper(List(String)) = Wrapper([1, 2, 3]) + }" + ); +} + #[test] fn case_tuple_guard() { assert_error!("case #(1, 2, 3) { x if x == #(1, 1.0) -> 1 }"); diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__const_annotation_wrong_4.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__const_annotation_wrong_4.snap index eda44ff3187..ba61d76aec0 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__const_annotation_wrong_4.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__const_annotation_wrong_4.snap @@ -12,10 +12,8 @@ error: Type mismatch 1 │ pub const pair: #(Int, Float) = #(4.1, 1) │ ^^^^^^^^^ -Expected type: - - #(Int, Float) - Found type: #(Float, Int) + ^^^^^ expected: Int + ^^^ expected: Float diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__emphasize_multiple_type_mismatches.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__emphasize_multiple_type_mismatches.snap new file mode 100644 index 00000000000..c47f3a87d54 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__emphasize_multiple_type_mismatches.snap @@ -0,0 +1,37 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "type Container(a, b) {\n Container(first: a, second: b)\n }\n\n pub fn main() {\n let container: Container(String, #(Int, Float)) = Container(123, #(2, \"hello\"))\n }" +--- +----- SOURCE CODE +type Container(a, b) { + Container(first: a, second: b) + } + + pub fn main() { + let container: Container(String, #(Int, Float)) = Container(123, #(2, "hello")) + } + +----- ERROR +error: Private type used in public interface + ┌─ /src/one/two.gleam:5:5 + │ +5 │ pub fn main() { + │ ^^^^^^^^^^^^^ + +The following type is private, but is being used by this public export. + + Container(Int, #(Int, String)) + +Private types can only be used within the module that defines them. + +error: Type mismatch + ┌─ /src/one/two.gleam:6:59 + │ +6 │ let container: Container(String, #(Int, Float)) = Container(123, #(2, "hello")) + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Found type: + + Container(Int, #(Int, String)) + ^^^ expected: String + ^^^^^^ expected: Float diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__emphasize_nested_type_mismatches.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__emphasize_nested_type_mismatches.snap new file mode 100644 index 00000000000..39d4dc804e2 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__emphasize_nested_type_mismatches.snap @@ -0,0 +1,36 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "type Wrapper(a) {\n Wrapper(value: a)\n }\n\n pub fn main() {\n let nested: Wrapper(List(String)) = Wrapper([1, 2, 3])\n }" +--- +----- SOURCE CODE +type Wrapper(a) { + Wrapper(value: a) + } + + pub fn main() { + let nested: Wrapper(List(String)) = Wrapper([1, 2, 3]) + } + +----- ERROR +error: Private type used in public interface + ┌─ /src/one/two.gleam:5:5 + │ +5 │ pub fn main() { + │ ^^^^^^^^^^^^^ + +The following type is private, but is being used by this public export. + + Wrapper(List(Int)) + +Private types can only be used within the module that defines them. + +error: Type mismatch + ┌─ /src/one/two.gleam:6:45 + │ +6 │ let nested: Wrapper(List(String)) = Wrapper([1, 2, 3]) + │ ^^^^^^^^^^^^^^^^^^ + +Found type: + + Wrapper(List(Int)) + ^^^ expected: String diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__module_could_not_unify10.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__module_could_not_unify10.snap index f6d8da43a2e..a07d17f9091 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__module_could_not_unify10.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__module_could_not_unify10.snap @@ -15,10 +15,8 @@ error: Type mismatch 2 │ let #(y, [..x]): #(x, List(x)) = #("one", [1,2,3]) │ ^^^^^^^^^^^^^^^^^ -Expected type: - - #(x, List(x)) - Found type: #(String, List(Int)) + ^^^^^^ expected: x + ^^^ expected: x diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__module_could_not_unify8.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__module_could_not_unify8.snap index 5023a02fe09..1822b390ed8 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__module_could_not_unify8.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__module_could_not_unify8.snap @@ -12,10 +12,8 @@ error: Type mismatch 1 │ fn main() { let x: #(x, x) = #(5, 5.0) x } │ ^^^^^^^^^ -Expected type: - - #(x, x) - Found type: #(Int, Float) + ^^^ expected: x + ^^^^^ expected: x