Explicit builtin import warning

We will now show a warning if a builtin is imported explicitly,
since this is unncessary.

We will not show the warning if they expose functions from the builtin:

	import Dict exposing [isEmpty]

However, we will show a special warning if they expose types from it:

	import Dict exposing [Dict, isEmpty]
This commit is contained in:
Agus Zubiaga 2024-04-14 10:46:37 -03:00
parent 3217e5a3f0
commit 979aff8bf7
No known key found for this signature in database
6 changed files with 144 additions and 8 deletions

View File

@ -3007,9 +3007,17 @@ fn to_pending_value_def<'a>(
let mut exposed_symbols;
let is_automatically_imported =
!env.home.is_builtin() && module_id.is_automatically_imported();
match module_import.exposed {
None => {
exposed_symbols = Vec::new();
if is_automatically_imported {
env.problems
.push(Problem::ExplicitBuiltinImport(module_id, region));
}
}
Some(exposed_kw) => {
let exposed_ids = env
@ -3032,9 +3040,18 @@ fn to_pending_value_def<'a>(
match scope.import_symbol(ident, symbol, loc_name.region) {
Ok(()) => {}
Err((_shadowed_symbol, _region)) => {
internal_error!(
"TODO gracefully handle shadowing in imports."
)
if is_automatically_imported
&& Symbol::builtin_types_in_scope(module_id)
.iter()
.any(|(_, (s, _))| *s == symbol)
{
env.problem(Problem::ExplicitBuiltinTypeImport(
symbol,
loc_name.region,
));
} else {
todo!("[modules-revamp] Handle shadowing of imported symbols");
}
}
}
}

View File

@ -4921,8 +4921,6 @@ mod test_reporting {
r#"
app "dict" imports [ Dict ] provides [main] to "./platform"
import Dict
myDict : Dict.Dict Num.I64 Str
myDict = Dict.insert (Dict.empty {}) "foo" 42
@ -4934,8 +4932,8 @@ mod test_reporting {
Something is off with the body of the `myDict` definition:
5 myDict : Dict.Dict Num.I64 Str
6 myDict = Dict.insert (Dict.empty {}) "foo" 42
3 myDict : Dict.Dict Num.I64 Str
4 myDict = Dict.insert (Dict.empty {}) "foo" 42
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This `insert` call produces:
@ -4952,7 +4950,7 @@ mod test_reporting {
alias_type_diff,
indoc!(
r#"
app "test" imports [Set.{ Set }] provides [main] to "./platform"
app "test" imports [] provides [main] to "./platform"
HSet a : Set a

View File

@ -1111,6 +1111,81 @@ fn used_exposed_and_qualified() {
assert!(result.is_ok())
}
#[test]
fn explicit_builtin_import() {
let modules = vec![(
"Main.roc",
indoc!(
r#"
interface Main exposes [main] imports []
import Bool
main = Bool.true
"#
),
)];
let err = multiple_modules("explicit_builtin_import", modules).unwrap_err();
assert_eq!(
err,
indoc!(
r"
EXPLICIT BUILTIN IMPORT in tmp/explicit_builtin_import/Main.roc
The builtin Bool was imported here:
3 import Bool
^^^^^^^^^^^
Builtins are imported automatically, so you can remove this import.
Tip: Learn more about builtins in the tutorial:
<https://www.roc-lang.org/tutorial#builtin-modules>
"
)
);
}
#[test]
fn explicit_builtin_type_import() {
let modules = vec![(
"Main.roc",
indoc!(
r#"
interface Main exposes [main] imports []
import Dict exposing [Dict, isEmpty]
myDict : Dict * *
myDict =
Dict.empty {}
main = isEmpty myDict
"#
),
)];
let err = multiple_modules("explicit_builtin_type_import", modules).unwrap_err();
assert_eq!(
err,
indoc!(
r"
EXPLICIT BUILTIN IMPORT in tmp/explicit_builtin_type_import/Main.roc
`Dict.Dict` was imported here:
3 import Dict exposing [Dict, isEmpty]
^^^^
All types from builtins are automatically exposed, so you can remove
`Dict` from the exposing list.
Tip: Learn more about builtins in the tutorial:
<https://www.roc-lang.org/tutorial#builtin-modules>
"
)
);
}
#[test]
fn import_with_alias() {
let modules = vec![

View File

@ -384,6 +384,11 @@ impl ModuleId {
.get_name(self)
.unwrap_or_else(|| internal_error!("Could not find ModuleIds for {:?}", self))
}
pub fn is_automatically_imported(self) -> bool {
// The deprecated TotallyNotJson module is not automatically imported.
self.is_builtin() && self != ModuleId::JSON
}
}
impl fmt::Debug for ModuleId {

View File

@ -47,6 +47,8 @@ pub enum Problem {
new_import_region: Region,
existing_import: ScopeModuleSource,
},
ExplicitBuiltinImport(ModuleId, Region),
ExplicitBuiltinTypeImport(Symbol, Region),
/// First symbol is the name of the closure with that argument
/// Bool is whether the closure is anonymous
/// Second symbol is the name of the argument that is unused
@ -229,6 +231,8 @@ impl Problem {
Problem::UnusedImport(_, _) => Warning,
Problem::UnusedModuleImport(_, _) => Warning,
Problem::ImportNameConflict { .. } => RuntimeError,
Problem::ExplicitBuiltinImport(_, _) => Warning,
Problem::ExplicitBuiltinTypeImport(_, _) => Warning,
Problem::ExposedButNotDefined(_) => RuntimeError,
Problem::UnknownGeneratesWith(_) => RuntimeError,
Problem::UnusedArgument(_, _, _, _) => Warning,
@ -307,6 +311,8 @@ impl Problem {
new_import_region: region,
..
}
| Problem::ExplicitBuiltinImport(_, region)
| Problem::ExplicitBuiltinTypeImport(_, region)
| Problem::UnknownGeneratesWith(Loc { region, .. })
| Problem::UnusedArgument(_, _, _, region)
| Problem::UnusedBranchDef(_, region)

View File

@ -21,6 +21,7 @@ const UNRECOGNIZED_NAME: &str = "UNRECOGNIZED NAME";
const UNUSED_DEF: &str = "UNUSED DEFINITION";
const UNUSED_IMPORT: &str = "UNUSED IMPORT";
const IMPORT_NAME_CONFLICT: &str = "IMPORT NAME CONFLICT";
const EXPLICIT_BUILTIN_IMPORT: &str = "EXPLICIT BUILTIN IMPORT";
const UNUSED_ALIAS_PARAM: &str = "UNUSED TYPE ALIAS PARAMETER";
const UNBOUND_TYPE_VARIABLE: &str = "UNBOUND TYPE VARIABLE";
const UNUSED_ARG: &str = "UNUSED ARGUMENT";
@ -181,6 +182,40 @@ pub fn can_problem<'b>(
]);
title = IMPORT_NAME_CONFLICT.to_string();
}
Problem::ExplicitBuiltinImport(module_id, region) => {
doc = alloc.stack([
alloc.concat([
alloc.reflow("The builtin "),
alloc.module(module_id),
alloc.reflow(" was imported here:"),
]),
alloc.region(lines.convert_region(region)),
alloc.reflow("Builtins are imported automatically, so you can remove this import."),
alloc.reflow("Tip: Learn more about builtins in the tutorial:\n\n<https://www.roc-lang.org/tutorial#builtin-modules>"),
]);
title = EXPLICIT_BUILTIN_IMPORT.to_string();
}
Problem::ExplicitBuiltinTypeImport(symbol, region) => {
doc = alloc.stack([
alloc.concat([
alloc.symbol_qualified(symbol),
alloc.reflow(" was imported here:"),
]),
alloc.region(lines.convert_region(region)),
alloc.concat([
alloc.reflow("All types from builtins are automatically exposed, so you can remove "),
alloc.symbol_unqualified(symbol),
alloc.reflow(" from the exposing list.")
]),
alloc.reflow("Tip: Learn more about builtins in the tutorial:\n\n<https://www.roc-lang.org/tutorial#builtin-modules>"),
]);
title = EXPLICIT_BUILTIN_IMPORT.to_string();
}
Problem::DefsOnlyUsedInRecursion(1, region) => {
doc = alloc.stack([
alloc.reflow("This definition is only used in recursion with itself:"),