Handle explicit builtin imports with empty exposing lists

Also includes related style suggestions by Ayaz on #6658

crates/compiler/can/src/def.rs

@@ -2975,76 +2975,65 @@ fn to_pending_value_def<'a>(
                 return PendingValue::ImportNameConflict;
             }
 
-            let mut exposed_symbols;
+            let exposed_names = module_import
+                .exposed
+                .map(|kw| kw.item.items)
+                .unwrap_or_default();
 
-            let is_automatically_imported =
+            if exposed_names.is_empty() && !env.home.is_builtin() && module_id.is_automatically_imported() {
-                !env.home.is_builtin() && module_id.is_automatically_imported();
+                env.problems.push(Problem::ExplicitBuiltinImport(module_id, region));
+            }
 
-            match module_import.exposed {
+            let exposed_ids = env
-                None => {
+                .dep_idents
-                    exposed_symbols = Vec::new();
+                .get(&module_id)
+                .expect("Module id should have been added in load");
 
-                    if is_automatically_imported {
+            let mut exposed_symbols = Vec::with_capacity(exposed_names.len());
-                        env.problems
-                            .push(Problem::ExplicitBuiltinImport(module_id, region));
-                    }
-                }
-                Some(exposed_kw) => {
-                    let exposed_ids = env
-                        .dep_idents
-                        .get(&module_id)
-                        .expect("Module id should have been added in load");
 
-                    exposed_symbols = Vec::with_capacity(exposed_kw.item.len());
+            for loc_name in exposed_names {
+                let exposed_name = loc_name.value.item();
+                let name = exposed_name.as_str();
+                let ident = Ident::from(name);
 
-                    for loc_name in exposed_kw.item.items {
+                match exposed_ids.get_id(name) {
-                        let exposed_name = loc_name.value.item();
+                    Some(ident_id) => {
-                        let name = exposed_name.as_str();
+                        let symbol = Symbol::new(module_id, ident_id);
-                        let ident = Ident::from(name);
+                        exposed_symbols.push((symbol, loc_name.region));
 
-                        match exposed_ids.get_id(name) {
+                        if let Err((_shadowed_symbol, existing_symbol_region)) = scope.import_symbol(ident, symbol, loc_name.region) {
-                            Some(ident_id) => {
+                            if symbol.is_automatically_imported() {
-                                let symbol = Symbol::new(module_id, ident_id);
+                                env.problem(Problem::ExplicitBuiltinTypeImport(
-                                exposed_symbols.push((symbol, loc_name.region));
+                                    symbol,
-
+                                    loc_name.region,
-                                if let Err((_shadowed_symbol, existing_symbol_region)) = scope.import_symbol(ident, symbol, loc_name.region) {
+                                ));
-                                    if is_automatically_imported
+                            } else {
-                                        && Symbol::builtin_types_in_scope(module_id)
+                                env.problem(Problem::ImportShadowsSymbol {
-                                            .iter()
-                                            .any(|(_, (s, _))| *s == symbol)
-                                    {
-                                        env.problem(Problem::ExplicitBuiltinTypeImport(
-                                            symbol,
-                                            loc_name.region,
-                                        ));
-                                    } else {
-                                        env.problem(Problem::ImportShadowsSymbol {
-                                            region: loc_name.region,
-                                            new_symbol: symbol,
-                                            existing_symbol_region,
-                                        })
-                                    }
-                                }
-                            }
-                            None => {
-                                let exposed_values = exposed_ids
-                                    .ident_strs()
-                                    .filter(|(_, ident)| {
-                                        ident.starts_with(|c: char| c.is_lowercase())
-                                    })
-                                    .map(|(_, ident)| Lowercase::from(ident))
-                                    .collect();
-
-                                env.problem(Problem::RuntimeError(RuntimeError::ValueNotExposed {
-                                    module_name: module_name.clone(),
-                                    ident,
                                     region: loc_name.region,
-                                    exposed_values,
+                                    new_symbol: symbol,
-                                }))
+                                    existing_symbol_region,
+                                })
                             }
                         }
                     }
+                    None => {
+                        let exposed_values = exposed_ids
+                            .ident_strs()
+                            .filter(|(_, ident)| {
+                                ident.starts_with(|c: char| c.is_lowercase())
+                            })
+                            .map(|(_, ident)| Lowercase::from(ident))
+                            .collect();
+
+                        env.problem(Problem::RuntimeError(RuntimeError::ValueNotExposed {
+                            module_name: module_name.clone(),
+                            ident,
+                            region: loc_name.region,
+                            exposed_values,
+                        }))
+                    }
                 }
+
             }
 
             PendingValue::ModuleImport(IntroducedImport {

crates/compiler/load_internal/tests/test_load.rs

@@ -1146,6 +1146,41 @@ fn explicit_builtin_import() {
     );
 }
 
+#[test]
+fn explicit_builtin_import_empty_exposing() {
+    let modules = vec![(
+        "Main.roc",
+        indoc!(
+            r#"
+                interface Main exposes [main] imports []
+
+                import Bool exposing []
+
+                main = Bool.true
+                "#
+        ),
+    )];
+    let err = multiple_modules("empty_exposing_builtin_import", modules).unwrap_err();
+    assert_eq!(
+        err,
+        indoc!(
+            r"
+            ── EXPLICIT BUILTIN IMPORT in tmp/empty_exposing_builtin_import/Main.roc ───────
+            
+            The builtin Bool was imported here:
+            
+            3│  import Bool exposing []
+                ^^^^^^^^^^^^^^^^^^^^^^^
+            
+            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![(

crates/compiler/module/src/symbol.rs

@@ -115,6 +115,15 @@ impl Symbol {
         )
     }
 
+    pub fn is_automatically_imported(self) -> bool {
+        let module_id = self.module_id();
+
+        module_id.is_automatically_imported()
+            && Self::builtin_types_in_scope(module_id)
+                .iter()
+                .any(|(_, (s, _))| *s == self)
+    }
+
     pub fn module_string<'a>(&self, interns: &'a Interns) -> &'a ModuleName {
         interns
             .module_ids