record update hints

This commit is contained in:
Folkert 2020-04-11 14:55:12 +02:00
parent a67fe6540c
commit ab19529077
6 changed files with 443 additions and 55 deletions

View file

@ -174,7 +174,13 @@ pub fn constrain_expr(
let con = Lookup( let con = Lookup(
*symbol, *symbol,
ForReason( ForReason(
Reason::RecordUpdateKeys(*symbol, fields), Reason::RecordUpdateKeys(
*symbol,
updates
.iter()
.map(|(key, field)| (key.clone(), field.region))
.collect(),
),
record_type, record_type,
region, region,
), ),

View file

@ -1195,7 +1195,13 @@ pub fn constrain_expr(
let con = Lookup( let con = Lookup(
*symbol, *symbol,
Expected::ForReason( Expected::ForReason(
Reason::RecordUpdateKeys(*symbol, fields), Reason::RecordUpdateKeys(
*symbol,
updates
.iter()
.map(|(key, field)| (key.clone(), field.region))
.collect(),
),
record_type, record_type,
region, region,
), ),

View file

@ -452,6 +452,7 @@ impl<'a> RocDocAllocator<'a> {
} }
} }
/// vertical concatenation. Adds a newline between elements
pub fn vcat<A, I>(&'a self, docs: I) -> DocBuilder<'a, Self, A> pub fn vcat<A, I>(&'a self, docs: I) -> DocBuilder<'a, Self, A>
where where
A: 'a + Clone, A: 'a + Clone,
@ -461,6 +462,8 @@ impl<'a> RocDocAllocator<'a> {
self.intersperse(docs, self.line()) self.intersperse(docs, self.line())
} }
/// live vcat, but adds a double line break between elements. Visually this means an empty line
/// between elements.
pub fn stack<A, I>(&'a self, docs: I) -> DocBuilder<'a, Self, A> pub fn stack<A, I>(&'a self, docs: I) -> DocBuilder<'a, Self, A>
where where
A: 'a + Clone, A: 'a + Clone,
@ -470,6 +473,7 @@ impl<'a> RocDocAllocator<'a> {
self.intersperse(docs, self.line().append(self.line())) self.intersperse(docs, self.line().append(self.line()))
} }
/// text from a String. Note that this does not reflow!
pub fn string(&'a self, string: String) -> DocBuilder<'a, Self, Annotation> { pub fn string(&'a self, string: String) -> DocBuilder<'a, Self, Annotation> {
let x: std::borrow::Cow<'a, str> = string.into(); let x: std::borrow::Cow<'a, str> = string.into();

View file

@ -1,5 +1,5 @@
use roc_can::expected::{Expected, PExpected}; use roc_can::expected::{Expected, PExpected};
use roc_collections::all::SendMap; use roc_collections::all::{MutSet, SendMap};
use roc_module::ident::{Lowercase, TagName}; use roc_module::ident::{Lowercase, TagName};
use roc_module::symbol::Symbol; use roc_module::symbol::Symbol;
use roc_solve::solve; use roc_solve::solve;
@ -171,12 +171,14 @@ fn to_expr_report<'b>(
// TODO special-case 2-branch if // TODO special-case 2-branch if
let thing = match annotation_source { let thing = match annotation_source {
TypedIfBranch(index) => alloc.concat(vec![ TypedIfBranch(index) => alloc.concat(vec![
alloc.string(format!("{} branch of this ", int_to_ordinal(index))), alloc.string(format!("{}", int_to_ordinal(index))),
alloc.reflow(" branch of this "),
alloc.keyword("if"), alloc.keyword("if"),
alloc.text(" expression:"), alloc.text(" expression:"),
]), ]),
TypedWhenBranch(index) => alloc.concat(vec![ TypedWhenBranch(index) => alloc.concat(vec![
alloc.string(format!("{} branch of this ", int_to_ordinal(index))), alloc.string(format!("{}", int_to_ordinal(index))),
alloc.reflow(" branch of this "),
alloc.keyword("when"), alloc.keyword("when"),
alloc.text(" expression:"), alloc.text(" expression:"),
]), ]),
@ -210,7 +212,7 @@ fn to_expr_report<'b>(
title: "TYPE MISMATCH".to_string(), title: "TYPE MISMATCH".to_string(),
filename, filename,
doc: alloc.stack(vec![ doc: alloc.stack(vec![
alloc.text("Something is off with the ").append( thing), alloc.text("Something is off with the ").append(thing),
alloc.region(expr_region), alloc.region(expr_region),
comparison, comparison,
]), ]),
@ -237,15 +239,15 @@ fn to_expr_report<'b>(
problem, problem,
alloc.text("Right now its"), alloc.text("Right now its"),
alloc.concat(vec![ alloc.concat(vec![
alloc.text("But I need every "), alloc.reflow("But I need every "),
alloc.keyword("if"), alloc.keyword("if"),
alloc.text(" condition to evaluate to a "), alloc.reflow(" condition to evaluate to a "),
alloc.type_str("Bool"), alloc.type_str("Bool"),
alloc.text("—either "), alloc.reflow("—either "),
alloc.global_tag_name("True".into()), alloc.global_tag_name("True".into()),
alloc.text(" or "), alloc.reflow(" or "),
alloc.global_tag_name("False".into()), alloc.global_tag_name("False".into()),
alloc.text("."), alloc.reflow("."),
]), ]),
// Note: Elm has a hint here about truthiness. I think that // Note: Elm has a hint here about truthiness. I think that
// makes sense for Elm, since most Elm users will come from // makes sense for Elm, since most Elm users will come from
@ -260,7 +262,8 @@ fn to_expr_report<'b>(
alloc.text("This "), alloc.text("This "),
alloc.keyword("if"), alloc.keyword("if"),
alloc.text(" guard condition needs to be a "), alloc.text(" guard condition needs to be a "),
alloc.type_str("Bool"), alloc.text(":"), alloc.type_str("Bool"),
alloc.text(":"),
]); ]);
report_bad_type( report_bad_type(
alloc, alloc,
@ -273,14 +276,15 @@ alloc.type_str("Bool"), alloc.text(":"),
problem, problem,
alloc.text("Right now its"), alloc.text("Right now its"),
alloc.concat(vec![ alloc.concat(vec![
alloc.text("But I need every "), alloc.reflow("But I need every "),
alloc.keyword("if"), alloc.keyword("if"),
alloc.text(" guard condition to evaluate to a "), alloc.reflow(" guard condition to evaluate to a "),
alloc.type_str("Bool"), alloc.text("—either "), alloc.type_str("Bool"),
alloc.reflow("—either "),
alloc.global_tag_name("True".into()), alloc.global_tag_name("True".into()),
alloc.text(" or "), alloc.reflow(" or "),
alloc.global_tag_name("False".into()), alloc.global_tag_name("False".into()),
alloc.text("."), alloc.reflow("."),
]), ]),
) )
} }
@ -315,8 +319,7 @@ alloc.type_str("Bool"), alloc.text(":"),
alloc.keyword("then"), alloc.keyword("then"),
alloc.text(" branch has the type:"), alloc.text(" branch has the type:"),
]), ]),
Some( Some(alloc.concat(vec![
alloc.concat(vec![
alloc.text("I need all branches in an "), alloc.text("I need all branches in an "),
alloc.keyword("if"), alloc.keyword("if"),
alloc.text(" to have the same type!"), alloc.text(" to have the same type!"),
@ -333,16 +336,19 @@ alloc.type_str("Bool"), alloc.text(":"),
expected_type, expected_type,
region, region,
Some(expr_region), Some(expr_region),
alloc.string(format!( alloc.concat(vec![
"The {} branch of this `if` does not match all the previous branches:", alloc.reflow("The "),
ith alloc.string(format!("{}", ith)),
)), alloc.reflow(" branch of this "),
alloc.string(format!("The {} branch is", ith)),
alloc.text("But all the previous branches have type:"),
Some(alloc.concat(vec![
alloc.text("I need all branches in an "),
alloc.keyword("if"), alloc.keyword("if"),
alloc.text(" to have the same type!"), alloc.reflow(" does not match all the previous branches:"),
]),
alloc.string(format!("The {} branch is", ith)),
alloc.reflow("But all the previous branches have type:"),
Some(alloc.concat(vec![
alloc.reflow("I need all branches in an "),
alloc.keyword("if"),
alloc.reflow(" to have the same type!"),
])), ])),
) )
} }
@ -360,16 +366,22 @@ alloc.type_str("Bool"), alloc.text(":"),
region, region,
Some(expr_region), Some(expr_region),
alloc.concat(vec![ alloc.concat(vec![
alloc.string(format!("The {} branch of this ", ith)), alloc.reflow("The "),
alloc.string(format!("{}", ith)),
alloc.reflow(" branch of this "),
alloc.keyword("when"), alloc.keyword("when"),
alloc.text(" does not match all the previous branches:"), alloc.reflow(" does not match all the previous branches:"),
]), ]),
alloc.string(format!("The {} branch is", ith)), alloc.concat(vec![
alloc.text("But all the previous branches have type:"), alloc.reflow("The "),
alloc.string(format!("{}", ith)),
alloc.reflow(" branch is"),
]),
alloc.reflow("But all the previous branches have type:"),
Some(alloc.concat(vec![ Some(alloc.concat(vec![
alloc.text("I need all branches of a "), alloc.reflow("I need all branches of a "),
alloc.keyword("when"), alloc.keyword("when"),
alloc.text(" to have the same type!"), alloc.reflow(" to have the same type!"),
])), ])),
) )
} }
@ -391,8 +403,8 @@ alloc.type_str("Bool"), alloc.text(":"),
ith ith
)), )),
alloc.string(format!("The {} element is", ith)), alloc.string(format!("The {} element is", ith)),
alloc.text("But all the previous elements in the list have type:"), alloc.reflow("But all the previous elements in the list have type:"),
Some(alloc.text("I need all elements of a list to have the same type!")), Some(alloc.reflow("I need all elements of a list to have the same type!")),
) )
} }
Reason::RecordUpdateValue(field) => report_mismatch( Reason::RecordUpdateValue(field) => report_mismatch(
@ -414,10 +426,116 @@ alloc.type_str("Bool"), alloc.text(":"),
alloc.text(" to be"), alloc.text(" to be"),
]), ]),
alloc.text("But it should be:"), alloc.text("But it should be:"),
Some(alloc.text( Some(alloc.reflow(
r#"Record update syntax does not allow you to change the type of fields. You can achieve that with record literal syntax."#, "Record update syntax does not allow you \
to change the type of fields. \
You can achieve that with record literal syntax.",
)), )),
), ),
Reason::RecordUpdateKeys(symbol, expected_fields) => match found.clone().unwrap_alias()
{
ErrorType::Record(actual_fields, ext) => {
let expected_set: MutSet<_> = expected_fields.keys().cloned().collect();
let actual_set: MutSet<_> = actual_fields.keys().cloned().collect();
let diff = expected_set.difference(&actual_set);
match diff
.into_iter()
.next()
.and_then(|k| Some((k, expected_fields.get(k)?)))
{
None => report_mismatch(
alloc,
filename,
&category,
found,
expected_type,
region,
Some(expr_region),
alloc.reflow("Something is off with this record update:"),
alloc.concat(vec![
alloc.reflow("The"),
alloc.symbol_unqualified(symbol),
alloc.reflow(" record is"),
]),
alloc.reflow("But this update needs it to be compatible with:"),
None,
),
Some((field, field_region)) => {
let r_doc = alloc.symbol_unqualified(symbol);
let f_doc = alloc.record_field(field.clone().clone());
let header = alloc.concat(vec![
alloc.reflow("The "),
r_doc.clone(),
alloc.reflow(" record does not have a "),
f_doc.clone(),
alloc.reflow(" field:"),
]);
let mut suggestions = suggest::sort(
field.as_str(),
actual_fields.into_iter().collect::<Vec<_>>(),
);
let doc = alloc.stack(vec![
header,
alloc.region(*field_region),
if suggestions.is_empty() {
alloc.concat(vec![
alloc.reflow("In fact, "),
r_doc,
alloc.reflow(" is a record with NO fields!"),
])
} else {
let f = suggestions.remove(0);
let fs = suggestions;
alloc.stack(vec![
alloc.concat(vec![
alloc.reflow("This is usually a typo. Here are the "),
r_doc,
alloc.reflow(" fields that are most similar:"),
]),
report_text::to_suggestion_record(
alloc,
f.clone(),
fs,
ext,
),
alloc.concat(vec![
alloc.reflow("So maybe "),
f_doc,
alloc.reflow(" should be "),
alloc.record_field(f.0),
alloc.reflow("?"),
]),
])
},
]);
Report {
filename,
title: "TYPE MISMATCH".to_string(),
doc,
}
}
}
}
_ => report_bad_type(
alloc,
filename,
&category,
found,
expected_type,
region,
Some(expr_region),
alloc.reflow("This is not a record, so it has no fields to update!"),
alloc.reflow("It is"),
alloc.reflow("But I need a record!"),
),
},
Reason::FnCall { name, arity } => match count_arguments(&found) { Reason::FnCall { name, arity } => match count_arguments(&found) {
0 => { 0 => {
let this_value = match name { let this_value = match name {
@ -442,7 +560,7 @@ alloc.type_str("Bool"), alloc.text(":"),
)), )),
]), ]),
alloc.region(expr_region), alloc.region(expr_region),
alloc.text("Are there any missing commas? Or missing parentheses?"), alloc.reflow("Are there any missing commas? Or missing parentheses?"),
]; ];
Report { Report {
@ -476,7 +594,7 @@ alloc.type_str("Bool"), alloc.text(":"),
)), )),
]), ]),
alloc.region(expr_region), alloc.region(expr_region),
alloc.text("Are there any missing commas? Or missing parentheses?"), alloc.reflow("Are there any missing commas? Or missing parentheses?"),
]; ];
Report { Report {
@ -499,8 +617,9 @@ alloc.type_str("Bool"), alloc.text(":"),
)), )),
]), ]),
alloc.region(expr_region), alloc.region(expr_region),
alloc.text( alloc.reflow(
r#"Roc does not allow functions to be partially applied. Use a closure to make partial application explicit."#, "Roc does not allow functions to be partially applied. \
Use a closure to make partial application explicit.",
), ),
]; ];
@ -552,8 +671,6 @@ alloc.type_str("Bool"), alloc.text(":"),
// IntLiteral, // IntLiteral,
// NumLiteral, // NumLiteral,
// InterpolatedStringVar, // InterpolatedStringVar,
// RecordUpdateValue(Lowercase),
// RecordUpdateKeys(Symbol, SendMap<Lowercase, Type>),
todo!("I don't have a message yet for reason {:?}", other) todo!("I don't have a message yet for reason {:?}", other)
} }
}, },
@ -912,6 +1029,12 @@ pub mod suggest {
} }
} }
impl ToStr for &Lowercase {
fn to_str(&self) -> &str {
self.as_str()
}
}
impl ToStr for InlinableString { impl ToStr for InlinableString {
fn to_str(&self) -> &str { fn to_str(&self) -> &str {
self.as_ref() self.as_ref()
@ -924,6 +1047,15 @@ pub mod suggest {
} }
} }
impl<A, B> ToStr for (A, B)
where
A: ToStr,
{
fn to_str(&self) -> &str {
self.0.to_str()
}
}
pub fn sort<'a, T>(typo: &'a str, mut options: Vec<T>) -> Vec<T> pub fn sort<'a, T>(typo: &'a str, mut options: Vec<T>) -> Vec<T>
where where
T: ToStr, T: ToStr,
@ -999,9 +1131,7 @@ fn ext_to_doc<'b>(alloc: &'b RocDocAllocator<'b>, ext: TypeExt) -> Option<RocDoc
match ext { match ext {
Closed => None, Closed => None,
FlexOpen(lowercase) | RigidOpen(lowercase) => { FlexOpen(lowercase) | RigidOpen(lowercase) => Some(alloc.type_variable(lowercase)),
Some(alloc.string(lowercase.as_str().to_string()))
}
} }
} }
@ -1024,8 +1154,8 @@ pub fn to_doc<'b>(
Infinite => alloc.text(""), Infinite => alloc.text(""),
Error => alloc.text("?"), Error => alloc.text("?"),
FlexVar(lowercase) => alloc.string(lowercase.as_str().to_string()), FlexVar(lowercase) => alloc.type_variable(lowercase),
RigidVar(lowercase) => alloc.string(lowercase.as_str().to_string()), RigidVar(lowercase) => alloc.type_variable(lowercase),
Type(symbol, args) => report_text::apply( Type(symbol, args) => report_text::apply(
alloc, alloc,
@ -1600,8 +1730,10 @@ fn ext_to_status(ext1: &TypeExt, ext2: &TypeExt) -> Status {
} }
mod report_text { mod report_text {
use crate::report::{RocDocAllocator, RocDocBuilder}; use crate::report::{Annotation, RocDocAllocator, RocDocBuilder};
use roc_module::ident::Lowercase;
use roc_types::pretty_print::Parens; use roc_types::pretty_print::Parens;
use roc_types::types::{ErrorType, TypeExt};
use ven_pretty::DocAllocator; use ven_pretty::DocAllocator;
fn with_parens<'b>( fn with_parens<'b>(
@ -1684,6 +1816,106 @@ mod report_text {
} }
} }
pub fn to_suggestion_record<'b>(
alloc: &'b RocDocAllocator<'b>,
f: (Lowercase, ErrorType),
fs: Vec<(Lowercase, ErrorType)>,
ext: TypeExt,
) -> RocDocBuilder<'b> {
use crate::type_error::{ext_to_doc, to_doc};
let entry_to_doc = |(name, tipe): (Lowercase, ErrorType)| {
(
alloc.string(name.as_str().to_string()),
to_doc(alloc, Parens::Unnecessary, tipe),
)
};
if fs.len() <= 3 {
let mut selection = vec![f];
selection.extend(fs);
let fields = selection.into_iter().map(entry_to_doc).collect();
vertical_record(alloc, fields, ext_to_doc(alloc, ext))
.annotate(Annotation::TypeBlock)
.indent(4)
} else {
let fields = fs.into_iter().take(3).map(entry_to_doc).collect();
vertical_record_snippet(alloc, entry_to_doc(f), fields)
.annotate(Annotation::TypeBlock)
.indent(4)
}
}
fn vertical_record<'b>(
alloc: &'b RocDocAllocator<'b>,
entries: Vec<(RocDocBuilder<'b>, RocDocBuilder<'b>)>,
opt_ext: Option<RocDocBuilder<'b>>,
) -> RocDocBuilder<'b> {
let entry_to_doc = |(field_name, field_type): (RocDocBuilder<'b>, RocDocBuilder<'b>)| {
field_name
.append(alloc.text(" : "))
.hang(4)
.append(field_type)
};
match opt_ext {
None => {
if entries.is_empty() {
alloc.text("{}")
} else {
let start = std::iter::once(alloc.reflow("{ "))
.chain(std::iter::repeat(alloc.reflow(", ")));
let entry_docs = start
.zip(entries.into_iter().map(entry_to_doc))
.map(|(a, b)| a.append(b));
alloc.vcat(entry_docs.chain(std::iter::once(alloc.text("}"))))
}
}
Some(ext) => {
let start = std::iter::once(alloc.reflow("{ "))
.chain(std::iter::repeat(alloc.reflow(", ")));
let entry_docs = start
.zip(entries.into_iter().map(entry_to_doc))
.map(|(a, b)| a.append(b));
alloc
.vcat(entry_docs.chain(std::iter::once(alloc.text("}"))))
.append(ext)
}
}
}
fn vertical_record_snippet<'b>(
alloc: &'b RocDocAllocator<'b>,
entry: (RocDocBuilder<'b>, RocDocBuilder<'b>),
entries: Vec<(RocDocBuilder<'b>, RocDocBuilder<'b>)>,
) -> RocDocBuilder<'b> {
let entry_to_doc = |(field_name, field_type): (RocDocBuilder<'b>, RocDocBuilder<'b>)| {
field_name
.append(alloc.text(" : "))
.hang(4)
.append(field_type)
};
let field = alloc.reflow("{ ").append(entry_to_doc(entry));
let fields = std::iter::repeat(alloc.reflow(", "))
.zip(
entries
.into_iter()
.map(entry_to_doc)
.chain(std::iter::once(alloc.text("..."))),
)
.map(|(a, b)| a.append(b));
alloc.vcat(
std::iter::once(field)
.chain(fields)
.chain(std::iter::once(alloc.text("}"))),
)
}
pub fn tag_union<'b>( pub fn tag_union<'b>(
alloc: &'b RocDocAllocator<'b>, alloc: &'b RocDocAllocator<'b>,
entries: Vec<(RocDocBuilder<'b>, Vec<RocDocBuilder<'b>>)>, entries: Vec<(RocDocBuilder<'b>, Vec<RocDocBuilder<'b>>)>,

View file

@ -608,7 +608,8 @@ mod test_reporting {
Str Str
But I need every `if` condition to evaluate to a Booleither `True` or `False`. But I need every `if` condition to evaluate to a Booleither `True` or
`False`.
"# "#
), ),
) )
@ -637,7 +638,8 @@ mod test_reporting {
Num a Num a
But I need every `if` guard condition to evaluate to a Booleither `True` or `False`. But I need every `if` guard condition to evaluate to a Booleither
`True` or `False`.
"# "#
), ),
) )
@ -796,7 +798,8 @@ mod test_reporting {
{} {}
Record update syntax does not allow you to change the type of fields. You can achieve that with record literal syntax. Record update syntax does not allow you to change the type of fields.
You can achieve that with record literal syntax.
"# "#
), ),
) )
@ -1207,7 +1210,8 @@ mod test_reporting {
4 f 1 4 f 1
^ ^
Roc does not allow functions to be partially applied. Use a closure to make partial application explicit. Roc does not allow functions to be partially applied. Use a closure to
make partial application explicit.
"# "#
), ),
) )
@ -1743,4 +1747,140 @@ mod test_reporting {
), ),
) )
} }
#[test]
fn update_empty_record() {
report_problem_as(
indoc!(
r#"
x = {}
{ x & foo: 3 }
"#
),
indoc!(
r#"
-- TYPE MISMATCH ---------------------------------------------------------------
The `x` record does not have a `.foo` field:
3 { x & foo: 3 }
^^^^^^
In fact, `x` is a record with NO fields!
"#
),
)
}
#[test]
fn update_record() {
report_problem_as(
indoc!(
r#"
x = { fo: 3, bar: 4 }
{ x & foo: 3 }
"#
),
// TODO also suggest fields with the correct type
indoc!(
r#"
-- TYPE MISMATCH ---------------------------------------------------------------
The `x` record does not have a `.foo` field:
3 { x & foo: 3 }
^^^^^^
This is usually a typo. Here are the `x` fields that are most similar:
{ fo : Num b
, bar : Num a
}
So maybe `.foo` should be `.fo`?
"#
),
)
}
#[test]
fn update_record_ext() {
report_problem_as(
indoc!(
r#"
f : { fo: Int }ext -> Int
f = \r ->
r2 = { r & foo: r.fo }
r2.fo
f
"#
),
// TODO also suggest fields with the correct type
indoc!(
r#"
-- TYPE MISMATCH ---------------------------------------------------------------
Something is off with the body of the `f` definition:
2 > f = \r ->
3 > r2 = { r & foo: r.fo }
4 >
5 > r2.fo
The body is an anonymous function of type:
{ fo : Int, foo : Int }a -> Int
But the type annotation on `f` says it should be:
{ fo : Int }ext -> Int
Hint: Seems like a record field typo. Maybe `foo` should be `fo`?
Hint: Can more type annotations be added? Type annotations always help
me give more specific messages, and I think they could help a lot in
this case
"#
),
)
}
#[test]
fn update_record_snippet() {
report_problem_as(
indoc!(
r#"
x = { fo: 3, bar: 4, baz: 3, spam: 42, foobar: 3 }
{ x & foo: 3 }
"#
),
// TODO also suggest fields with the correct type
indoc!(
r#"
-- TYPE MISMATCH ---------------------------------------------------------------
The `x` record does not have a `.foo` field:
3 { x & foo: 3 }
^^^^^^
This is usually a typo. Here are the `x` fields that are most similar:
{ fo : Num c
, foobar : Num a
, bar : Num e
, baz : Num b
, ...
}
So maybe `.foo` should be `.fo`?
"#
),
)
}
} }

View file

@ -626,7 +626,7 @@ pub enum Reason {
IfBranch { index: usize, total_branches: usize }, IfBranch { index: usize, total_branches: usize },
ElemInList { index: usize }, ElemInList { index: usize },
RecordUpdateValue(Lowercase), RecordUpdateValue(Lowercase),
RecordUpdateKeys(Symbol, SendMap<Lowercase, Type>), RecordUpdateKeys(Symbol, SendMap<Lowercase, Region>),
} }
#[derive(PartialEq, Debug, Clone)] #[derive(PartialEq, Debug, Clone)]