Add "duplicate import name" and "missing default export" diagnostics (#…

…192) * add error for duplicate import names * add error for importing non existing default export * add specification test for `NoDefaultExport` and `DuplicateImportName`
kaleidawave · Aug 18, 2024 · 2d7c08f · 2d7c08f
1 parent 2ef0ec5
commit 2d7c08f
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 7 deletions.
diff --git a/checker/specification/specification.md b/checker/specification/specification.md
@@ -4121,16 +4121,35 @@ export const a = 2;
 #### Import where export does not exist
 
 ```ts
-import { a } from "./export";
+import b, { a } from "./export";
 
 console.log(a.prop);
 
 // in export.ts
-export const b = 2;
+export const c = 2;
 ```
 
+- Cannot find default export from module './export'
 - a not exported from ./export
 
+#### Import conflicts with existing name
+
+```ts
+import { x } from "./export1";
+import x, { z } from "./export2";
+
+// in export1.ts
+export const x = 1;
+
+// in export2.ts
+const y = 2;
+
+export default y;
+export const z = 2;
+```
+
+- Cannot import using conflicting name
+
 #### Import from invalid file
 
 ```ts

diff --git a/checker/src/diagnostics.rs b/checker/src/diagnostics.rs
@@ -387,8 +387,16 @@ pub(crate) enum TypeCheckError<'a> {
 	},
 	#[allow(dead_code)]
 	NotTopLevelImport(SpanWithSource),
+	DuplicateImportName {
+		import_position: SpanWithSource,
+		existing_position: SpanWithSource,
+	},
 	#[allow(dead_code)]
 	DoubleDefaultExport(SpanWithSource),
+	NoDefaultExport {
+		position: SpanWithSource,
+		partial_import_path: &'a str,
+	},
 	CannotOpenFile {
 		file: CouldNotOpenFile,
 		import_position: Option<SpanWithSource>,
@@ -674,7 +682,18 @@ impl From<TypeCheckError<'_>> for Diagnostic {
 				position,
 				kind,
 			},
+			TypeCheckError::DuplicateImportName { import_position: position, existing_position, ..} => Diagnostic::PositionWithAdditionalLabels {
+				reason: "Cannot import using conflicting name".to_string(),
+				position,
+				kind,
+				labels: vec![("Existing import with same name".to_string(), existing_position)],
+			},
 			TypeCheckError::DoubleDefaultExport(_) => todo!(),
+			TypeCheckError::NoDefaultExport { partial_import_path, position, ..} => Diagnostic::Position {
+				reason: format!("Cannot find default export from module '{partial_import_path}'"),
+				position,
+				kind
+			},
 			TypeCheckError::CannotOpenFile { file, import_position, possibles, partial_import_path } => if let Some(import_position) = import_position {
 				Diagnostic::PositionWithAdditionalLabels {
 					reason: format!("Cannot find {partial_import_path}"),

diff --git a/checker/src/features/modules.rs b/checker/src/features/modules.rs
@@ -156,11 +156,27 @@ pub fn import_items<
 				};
 				environment.info.variable_current_value.insert(id, *item);
 				let existing = environment.variables.insert(default_name.to_owned(), v);
-				if let Some(_existing) = existing {
-					todo!("diagnostic")
+				if let Some(existing) = existing {
+					checking_data.diagnostics_container.add_error(
+						crate::diagnostics::TypeCheckError::DuplicateImportName {
+							import_position: position.with_source(current_source),
+							existing_position: match existing {
+								VariableOrImport::Variable { declared_at, .. } => declared_at,
+								VariableOrImport::MutableImport { import_specified_at, .. }
+								| VariableOrImport::ConstantImport {
+									import_specified_at, ..
+								} => import_specified_at,
+							},
+						},
+					);
 				}
 			} else {
-				todo!("emit 'no default export' diagnostic")
+				checking_data.diagnostics_container.add_error(
+					crate::diagnostics::TypeCheckError::NoDefaultExport {
+						position: position.with_source(current_source),
+						partial_import_path,
+					},
+				);
 			}
 		} else {
 			environment.register_variable_handle_error(
@@ -243,8 +259,27 @@ pub fn import_items<
 						};
 						crate::utilities::notify!("{:?}", part.r#as.to_owned());
 						let existing = environment.variables.insert(part.r#as.to_owned(), v);
-						if let Some(_existing) = existing {
-							todo!("diagnostic")
+						if let Some(existing) = existing {
+							checking_data.diagnostics_container.add_error(
+								crate::diagnostics::TypeCheckError::DuplicateImportName {
+									import_position: part
+										.position
+										.with_source(environment.get_source()),
+									existing_position: match existing {
+										VariableOrImport::Variable { declared_at, .. } => {
+											declared_at
+										}
+										VariableOrImport::MutableImport {
+											import_specified_at,
+											..
+										}
+										| VariableOrImport::ConstantImport {
+											import_specified_at,
+											..
+										} => import_specified_at,
+									},
+								},
+							);
 						}
 						if also_export {
 							if let Scope::Module { ref mut exported, .. } =