Improve error message when shadowing builtin type

Closes #3109
This commit is contained in:
Ayaz Hafiz 2022-09-21 12:54:26 -05:00
parent ae122a0aea
commit 5f117be306
No known key found for this signature in database
GPG key ID: 0E2A37416A25EF58
5 changed files with 90 additions and 62 deletions

View file

@ -2302,11 +2302,6 @@ fn to_pending_alias_or_opaque<'a>(
opt_derived: Option<&'a Loc<ast::HasAbilities<'a>>>, opt_derived: Option<&'a Loc<ast::HasAbilities<'a>>>,
kind: AliasKind, kind: AliasKind,
) -> PendingTypeDef<'a> { ) -> PendingTypeDef<'a> {
let shadow_kind = match kind {
AliasKind::Structural => ShadowKind::Alias,
AliasKind::Opaque => ShadowKind::Opaque,
};
let region = Region::span_across(&name.region, &ann.region); let region = Region::span_across(&name.region, &ann.region);
match scope.introduce_without_shadow_symbol(&Ident::from(name.value), region) { match scope.introduce_without_shadow_symbol(&Ident::from(name.value), region) {
@ -2361,7 +2356,12 @@ fn to_pending_alias_or_opaque<'a>(
} }
} }
Err((original_region, loc_shadowed_symbol)) => { Err((original_sym, original_region, loc_shadowed_symbol)) => {
let shadow_kind = match kind {
AliasKind::Structural => ShadowKind::Alias(original_sym),
AliasKind::Opaque => ShadowKind::Opaque(original_sym),
};
env.problem(Problem::Shadowing { env.problem(Problem::Shadowing {
original_region, original_region,
shadow: loc_shadowed_symbol, shadow: loc_shadowed_symbol,
@ -2422,11 +2422,11 @@ fn to_pending_type_def<'a>(
.introduce_without_shadow_symbol(&Ident::from(name.value), name.region) .introduce_without_shadow_symbol(&Ident::from(name.value), name.region)
{ {
Ok(symbol) => Loc::at(name.region, symbol), Ok(symbol) => Loc::at(name.region, symbol),
Err((original_region, shadowed_symbol)) => { Err((original_symbol, original_region, shadowed_symbol)) => {
env.problem(Problem::Shadowing { env.problem(Problem::Shadowing {
original_region, original_region,
shadow: shadowed_symbol, shadow: shadowed_symbol,
kind: ShadowKind::Ability, kind: ShadowKind::Ability(original_symbol),
}); });
return PendingTypeDef::AbilityShadows; return PendingTypeDef::AbilityShadows;
} }

View file

@ -284,14 +284,14 @@ impl Scope {
&mut self, &mut self,
ident: &Ident, ident: &Ident,
region: Region, region: Region,
) -> Result<Symbol, (Region, Loc<Ident>)> { ) -> Result<Symbol, (Symbol, Region, Loc<Ident>)> {
match self.introduce_help(ident.as_str(), region) { match self.introduce_help(ident.as_str(), region) {
Err((_, original_region)) => { Err((symbol, original_region)) => {
let shadow = Loc { let shadow = Loc {
value: ident.clone(), value: ident.clone(),
region, region,
}; };
Err((original_region, shadow)) Err((symbol, original_region, shadow))
} }
Ok(symbol) => Ok(symbol), Ok(symbol) => Ok(symbol),
} }

View file

@ -22,9 +22,9 @@ pub enum BadPattern {
#[derive(Clone, Copy, Debug, PartialEq)] #[derive(Clone, Copy, Debug, PartialEq)]
pub enum ShadowKind { pub enum ShadowKind {
Variable, Variable,
Alias, Alias(Symbol),
Opaque, Opaque(Symbol),
Ability, Ability(Symbol),
} }
/// Problems that can occur in the course of canonicalization. /// Problems that can occur in the course of canonicalization.

View file

@ -257,9 +257,11 @@ pub fn can_problem<'b>(
shadow, shadow,
kind, kind,
} => { } => {
doc = report_shadowing(alloc, lines, original_region, shadow, kind); let (res_title, res_doc) =
report_shadowing(alloc, lines, original_region, shadow, kind);
title = DUPLICATE_NAME.to_string(); doc = res_doc;
title = res_title.to_string();
severity = Severity::RuntimeError; severity = Severity::RuntimeError;
} }
Problem::CyclicAlias(symbol, region, others, alias_kind) => { Problem::CyclicAlias(symbol, region, others, alias_kind) => {
@ -1371,28 +1373,48 @@ fn report_shadowing<'b>(
original_region: Region, original_region: Region,
shadow: Loc<Ident>, shadow: Loc<Ident>,
kind: ShadowKind, kind: ShadowKind,
) -> RocDocBuilder<'b> { ) -> (&'static str, RocDocBuilder<'b>) {
let what = match kind { let (what, what_plural, is_builtin) = match kind {
ShadowKind::Variable => "variables", ShadowKind::Variable => ("variable", "variables", false),
ShadowKind::Alias => "aliases", ShadowKind::Alias(sym) => ("alias", "aliases", sym.is_builtin()),
ShadowKind::Opaque => "opaques", ShadowKind::Opaque(sym) => ("opaque type", "opaque types", sym.is_builtin()),
ShadowKind::Ability => "abilities", ShadowKind::Ability(sym) => ("ability", "abilities", sym.is_builtin()),
}; };
alloc.stack([ let doc = if is_builtin {
alloc alloc.stack([
.text("The ") alloc.concat([
.append(alloc.ident(shadow.value)) alloc.reflow("This "),
.append(alloc.reflow(" name is first defined here:")), alloc.reflow(what),
alloc.region(lines.convert_region(original_region)), alloc.reflow(" has the same name as a builtin:"),
alloc.reflow("But then it's defined a second time here:"), ]),
alloc.region(lines.convert_region(shadow.region)), alloc.region(lines.convert_region(shadow.region)),
alloc.concat([ alloc.concat([
alloc.reflow("Since these "), alloc.reflow("All builtin "),
alloc.reflow(what), alloc.reflow(what_plural),
alloc.reflow(" have the same name, it's easy to use the wrong one on accident. Give one of them a new name."), alloc.reflow(" are in scope by default, so I need this "),
]), alloc.reflow(what),
]) alloc.reflow(" to have a different name!"),
]),
])
} else {
alloc.stack([
alloc
.text("The ")
.append(alloc.ident(shadow.value))
.append(alloc.reflow(" name is first defined here:")),
alloc.region(lines.convert_region(original_region)),
alloc.reflow("But then it's defined a second time here:"),
alloc.region(lines.convert_region(shadow.region)),
alloc.concat([
alloc.reflow("Since these "),
alloc.reflow(what_plural),
alloc.reflow(" have the same name, it's easy to use the wrong one on accident. Give one of them a new name."),
]),
])
};
(DUPLICATE_NAME, doc)
} }
fn pretty_runtime_error<'b>( fn pretty_runtime_error<'b>(
@ -1420,8 +1442,7 @@ fn pretty_runtime_error<'b>(
shadow, shadow,
kind, kind,
} => { } => {
doc = report_shadowing(alloc, lines, original_region, shadow, kind); (title, doc) = report_shadowing(alloc, lines, original_region, shadow, kind);
title = DUPLICATE_NAME;
} }
RuntimeError::LookupNotInScope(loc_name, options) => { RuntimeError::LookupNotInScope(loc_name, options) => {

View file

@ -6198,18 +6198,13 @@ All branches in an `if` must have the same type!
@r###" @r###"
DUPLICATE NAME /code/proj/Main.roc DUPLICATE NAME /code/proj/Main.roc
The `Result` name is first defined here: This alias has the same name as a builtin:
1 app "test" provides [main] to "./platform"
But then it's defined a second time here:
4 Result a b : [Ok a, Err b] 4 Result a b : [Ok a, Err b]
^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^
Since these aliases have the same name, it's easy to use the wrong one All builtin aliases are in scope by default, so I need this alias to
on accident. Give one of them a new name. have a different name!
TOO FEW TYPE ARGUMENTS /code/proj/Main.roc TOO FEW TYPE ARGUMENTS /code/proj/Main.roc
@ -6241,18 +6236,13 @@ All branches in an `if` must have the same type!
@r###" @r###"
DUPLICATE NAME /code/proj/Main.roc DUPLICATE NAME /code/proj/Main.roc
The `Result` name is first defined here: This alias has the same name as a builtin:
1 app "test" provides [main] to "./platform"
But then it's defined a second time here:
4 Result a b : [Ok a, Err b] 4 Result a b : [Ok a, Err b]
^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^
Since these aliases have the same name, it's easy to use the wrong one All builtin aliases are in scope by default, so I need this alias to
on accident. Give one of them a new name. have a different name!
TOO MANY TYPE ARGUMENTS /code/proj/Main.roc TOO MANY TYPE ARGUMENTS /code/proj/Main.roc
@ -7435,18 +7425,13 @@ All branches in an `if` must have the same type!
@r###" @r###"
DUPLICATE NAME /code/proj/Main.roc DUPLICATE NAME /code/proj/Main.roc
The `Result` name is first defined here: This alias has the same name as a builtin:
1 app "test" provides [main] to "./platform"
But then it's defined a second time here:
4 Result a b : [Ok a, Err b] 4 Result a b : [Ok a, Err b]
^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^
Since these aliases have the same name, it's easy to use the wrong one All builtin aliases are in scope by default, so I need this alias to
on accident. Give one of them a new name. have a different name!
"### "###
); );
@ -10710,4 +10695,26 @@ All branches in an `if` must have the same type!
be safely removed! be safely removed!
"### "###
); );
test_report!(
custom_type_conflicts_with_builtin,
indoc!(
r#"
Nat := [ S Nat, Z ]
""
"#
),
@r###"
DUPLICATE NAME /code/proj/Main.roc
This opaque type has the same name as a builtin:
4 Nat := [ S Nat, Z ]
^^^^^^^^^^^^^^^^^^^
All builtin opaque types are in scope by default, so I need this
opaque type to have a different name!
"###
);
} }