Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: check if module have type with the same name #3696

Merged
merged 7 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@
O(1) operation instead of O(N), significantly improving performance.
([Richard Viney](https://github.com/richard-viney))

- Better error message for existed type constructor being used as value constructor.
([Jiangda Wang](https://github.com/Frank-III))

### Build tool

- Improved the error message you get when trying to add a package that doesn't
Expand Down
1 change: 1 addition & 0 deletions compiler-core/src/analyse/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ impl<'context, 'problems> Importer<'context, 'problems> {
module_name: module.name.clone(),
value_constructors: module.public_value_names(),
type_with_same_name: module.get_public_type(import_name).is_some(),
context: crate::type_::error::ModuleValueUsageContext::UnqualifiedImport,
});
return;
}
Expand Down
15 changes: 11 additions & 4 deletions compiler-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
use crate::build::{Outcome, Runtime, Target};
use crate::diagnostic::{Diagnostic, ExtraLabel, Label, Location};
use crate::type_::error::{
MissingAnnotation, Named, UnknownField, UnknownTypeHint, UnsafeRecordUpdateReason,
MissingAnnotation, ModuleValueUsageContext, Named, UnknownField, UnknownTypeHint,
UnsafeRecordUpdateReason,
};
use crate::type_::printer::{Names, Printer};
use crate::type_::{error::PatternMatchKind, FieldAccessUsage};
Expand Down Expand Up @@ -2190,11 +2191,17 @@ Private types can only be used within the module that defines them.",
module_name,
value_constructors,
type_with_same_name: imported_value_as_type,
context,
} => {
let text = if *imported_value_as_type {
format!("`{name}` is only a type, it cannot be imported as a value.")
match context {
ModuleValueUsageContext::UnqualifiedImport =>
wrap_format!("`{name}` is only a type, it cannot be imported as a value."),
ModuleValueUsageContext::ModuleAccess =>
wrap_format!("{module_name}.{name} is a type constructor, it cannot be used as a value"),
}
} else {
format!("The module `{module_name}` does not have a `{name}` value.")
wrap_format!("The module `{module_name}` does not have a `{name}` value.")
};
Diagnostic {
title: "Unknown module value".into(),
Expand All @@ -2203,7 +2210,7 @@ Private types can only be used within the module that defines them.",
level: Level::Error,
location: Some(Location {
label: Label {
text: if *imported_value_as_type {
text: if *imported_value_as_type && matches!(context, ModuleValueUsageContext::UnqualifiedImport) {
Some(format!("Did you mean `type {name}`?"))
} else {
did_you_mean(name, value_constructors)
Expand Down
8 changes: 8 additions & 0 deletions compiler-core/src/type_/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ pub enum Error {
module_name: EcoString,
value_constructors: Vec<EcoString>,
type_with_same_name: bool,
context: ModuleValueUsageContext,
},

ModuleAliasUsedAsName {
Expand Down Expand Up @@ -583,6 +584,12 @@ pub enum Error {
},
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ModuleValueUsageContext {
UnqualifiedImport,
ModuleAccess,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum MissingAnnotation {
Parameter,
Expand Down Expand Up @@ -1135,6 +1142,7 @@ pub fn convert_get_value_constructor_error(
module_name,
value_constructors,
type_with_same_name: imported_value_as_type,
context: ModuleValueUsageContext::ModuleAccess,
},
}
}
Expand Down
6 changes: 4 additions & 2 deletions compiler-core/src/type_/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2224,7 +2224,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
},
module_name: module.name.clone(),
value_constructors: module.public_value_names(),
type_with_same_name: false,
type_with_same_name: module.get_public_type(&label).is_some(),
context: ModuleValueUsageContext::ModuleAccess,
})?;

// Emit a warning if the value being used is deprecated.
Expand Down Expand Up @@ -2785,7 +2786,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
module_name: module_name.clone(),
name: name.clone(),
value_constructors: module.public_value_names(),
type_with_same_name: false,
type_with_same_name: module.get_public_type(name).is_some(),
context: ModuleValueUsageContext::ModuleAccess,
})?
}
};
Expand Down
8 changes: 4 additions & 4 deletions compiler-core/src/type_/tests/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub const a = two.Thing(1)
}

#[test]
fn using_private_constructo() {
fn using_private_constructor() {
assert_with_module_error!(
("one", "type Two { Two }"),
"import one
Expand All @@ -51,7 +51,7 @@ pub fn main(x) {
}

#[test]
fn using_opaque_constructo() {
fn using_opaque_constructor() {
assert_with_module_error!(
("one", "pub opaque type Two { Two }"),
"import one
Expand Down Expand Up @@ -147,7 +147,7 @@ pub fn main() {
}

#[test]
fn unqualified_using_private_constructo() {
fn unqualified_using_private_constructor() {
assert_with_module_error!(
("one", "type Two { Two }"),
"import one.{Two}
Expand All @@ -171,7 +171,7 @@ pub fn main(x) {
}

#[test]
fn unqualified_using_opaque_constructo() {
fn unqualified_using_opaque_constructor() {
assert_with_module_error!(
("one", "pub opaque type Two { Two }"),
"import one.{Two}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ error: Unknown module value
4 │ one.Two
│ ^^^^

The module `one` does not have a `Two` value.
one.Two is a type constructor, it cannot be used as a value
Loading