From 5665f711fd699d8b7d2e07524b19cab194951d11 Mon Sep 17 00:00:00 2001
From: Yuichiro Yamashita <xydybaseball@gmail.com>
Date: Thu, 27 Jan 2022 00:16:20 +0900
Subject: [PATCH] [fix] raise compile error if variable name is same as
 imported variable name (#7145)

---
 src/compiler/compile/Component.ts             | 36 +++++++++++--------
 src/compiler/compile/compiler_errors.ts       |  4 +++
 src/compiler/compile/nodes/Action.ts          |  2 +-
 src/compiler/compile/nodes/Animation.ts       |  2 +-
 src/compiler/compile/nodes/InlineComponent.ts |  6 ++--
 src/compiler/compile/nodes/Transition.ts      |  2 +-
 src/compiler/compile/nodes/shared/Context.ts  |  2 +-
 .../compile/nodes/shared/Expression.ts        |  8 ++---
 src/compiler/compile/render_dom/Renderer.ts   |  2 +-
 .../illegal-variable-declaration/errors.json  | 17 +++++++++
 .../illegal-variable-declaration/input.svelte |  9 +++++
 11 files changed, 64 insertions(+), 26 deletions(-)
 create mode 100644 test/validator/samples/illegal-variable-declaration/errors.json
 create mode 100644 test/validator/samples/illegal-variable-declaration/input.svelte

diff --git a/src/compiler/compile/Component.ts b/src/compiler/compile/Component.ts
index 0347a28743..5f7d4b9758 100644
--- a/src/compiler/compile/Component.ts
+++ b/src/compiler/compile/Component.ts
@@ -191,27 +191,33 @@ export default class Component {
 		this.stylesheet.warn_on_unused_selectors(this);
 	}
 
-	add_var(variable: Var, add_to_lookup = true) {
+	add_var(node: Node, variable: Var, add_to_lookup = true) {
 		this.vars.push(variable);
 
 		if (add_to_lookup) {
+			if (this.var_lookup.has(variable.name)) {
+				const exists_var = this.var_lookup.get(variable.name);
+				if (exists_var.module && exists_var.imported) {
+					this.error(node as any, compiler_errors.illegal_variable_declaration);
+				}
+			}
 			this.var_lookup.set(variable.name, variable);
 		}
 	}
 
-	add_reference(name: string) {
+	add_reference(node: Node, name: string) {
 		const variable = this.var_lookup.get(name);
 
 		if (variable) {
 			variable.referenced = true;
 		} else if (is_reserved_keyword(name)) {
-			this.add_var({
+			this.add_var(node, {
 				name,
 				injected: true,
 				referenced: true
 			});
 		} else if (name[0] === '$') {
-			this.add_var({
+			this.add_var(node, {
 				name,
 				injected: true,
 				referenced: true,
@@ -228,7 +234,7 @@ export default class Component {
 			}
 		} else {
 			if (this.compile_options.varsReport === 'full') {
-				this.add_var({ name, referenced: true }, false);
+				this.add_var(node, { name, referenced: true }, false);
 			}
 
 			this.used_names.add(name);
@@ -599,12 +605,14 @@ export default class Component {
 			}
 
 			const writable = node.type === 'VariableDeclaration' && (node.kind === 'var' || node.kind === 'let');
+			const imported = node.type.startsWith('Import');
 
-			this.add_var({
+			this.add_var(node, {
 				name,
 				module: true,
 				hoistable: true,
-				writable
+				writable,
+				imported
 			});
 		});
 
@@ -612,7 +620,7 @@ export default class Component {
 			if (name[0] === '$') {
 				return this.error(node as any, compiler_errors.illegal_subscription);
 			} else {
-				this.add_var({
+				this.add_var(node, {
 					name,
 					global: true,
 					hoistable: true
@@ -674,7 +682,7 @@ export default class Component {
 			const writable = node.type === 'VariableDeclaration' && (node.kind === 'var' || node.kind === 'let');
 			const imported = node.type.startsWith('Import');
 
-			this.add_var({
+			this.add_var(node, {
 				name,
 				initialised: instance_scope.initialised_declarations.has(name),
 				writable,
@@ -697,7 +705,7 @@ export default class Component {
 			const node = globals.get(name);
 
 			if (this.injected_reactive_declaration_vars.has(name)) {
-				this.add_var({
+				this.add_var(node, {
 					name,
 					injected: true,
 					writable: true,
@@ -705,7 +713,7 @@ export default class Component {
 					initialised: true
 				});
 			} else if (is_reserved_keyword(name)) {
-				this.add_var({
+				this.add_var(node, {
 					name,
 					injected: true
 				});
@@ -714,14 +722,14 @@ export default class Component {
 					return this.error(node as any, compiler_errors.illegal_global(name));
 				}
 
-				this.add_var({
+				this.add_var(node, {
 					name,
 					injected: true,
 					mutated: true,
 					writable: true
 				});
 
-				this.add_reference(name.slice(1));
+				this.add_reference(node, name.slice(1));
 
 				const variable = this.var_lookup.get(name.slice(1));
 				if (variable) {
@@ -729,7 +737,7 @@ export default class Component {
 					variable.referenced_from_script = true;
 				}
 			} else {
-				this.add_var({
+				this.add_var(node, {
 					name,
 					global: true,
 					hoistable: true
diff --git a/src/compiler/compile/compiler_errors.ts b/src/compiler/compile/compiler_errors.ts
index f6e9d17ea3..c9282189f1 100644
--- a/src/compiler/compile/compiler_errors.ts
+++ b/src/compiler/compile/compiler_errors.ts
@@ -190,6 +190,10 @@ export default {
 		code: 'illegal-global',
 		message: `${name} is an illegal variable name`
 	}),
+	illegal_variable_declaration: {
+		code: 'illegal-variable-declaration',
+		message: 'Cannot declare same variable name which is imported inside <script context="module">'
+	},
 	cyclical_reactive_declaration: (cycle: string[]) => ({
 		code: 'cyclical-reactive-declaration',
 		message: `Cyclical dependency detected: ${cycle.join(' → ')}`
diff --git a/src/compiler/compile/nodes/Action.ts b/src/compiler/compile/nodes/Action.ts
index 347ea2fb30..f0d92f8537 100644
--- a/src/compiler/compile/nodes/Action.ts
+++ b/src/compiler/compile/nodes/Action.ts
@@ -18,7 +18,7 @@ export default class Action extends Node {
 		component.warn_if_undefined(object, info, scope);
 
 		this.name = info.name;
-		component.add_reference(object);
+		component.add_reference(this as any, object);
 
 		this.expression = info.expression
 			? new Expression(component, this, scope, info.expression)
diff --git a/src/compiler/compile/nodes/Animation.ts b/src/compiler/compile/nodes/Animation.ts
index ac9dddd275..507aa1df04 100644
--- a/src/compiler/compile/nodes/Animation.ts
+++ b/src/compiler/compile/nodes/Animation.ts
@@ -18,7 +18,7 @@ export default class Animation extends Node {
 		component.warn_if_undefined(info.name, info, scope);
 
 		this.name = info.name;
-		component.add_reference(info.name.split('.')[0]);
+		component.add_reference(this as any, info.name.split('.')[0]);
 
 		if (parent.animation) {
 			component.error(this, compiler_errors.duplicate_animation);
diff --git a/src/compiler/compile/nodes/InlineComponent.ts b/src/compiler/compile/nodes/InlineComponent.ts
index 9d88b136cd..e65bf445da 100644
--- a/src/compiler/compile/nodes/InlineComponent.ts
+++ b/src/compiler/compile/nodes/InlineComponent.ts
@@ -29,7 +29,7 @@ export default class InlineComponent extends Node {
 		if (info.name !== 'svelte:component' && info.name !== 'svelte:self') {
 			const name = info.name.split('.')[0]; // accommodate namespaces
 			component.warn_if_undefined(name, info, scope);
-			component.add_reference(name);
+			component.add_reference(this as any, name);
 		}
 
 		this.name = info.name;
@@ -141,10 +141,10 @@ export default class InlineComponent extends Node {
 		}
 
 		if (info.children.some(node => not_whitespace_text(node))) {
-			children.push({ 
+			children.push({
 				start: info.start,
 				end: info.end,
-				type: 'SlotTemplate', 
+				type: 'SlotTemplate',
 				name: 'svelte:fragment',
 				attributes: [],
 				children: info.children
diff --git a/src/compiler/compile/nodes/Transition.ts b/src/compiler/compile/nodes/Transition.ts
index 78799ac7a9..3a4c5a04a7 100644
--- a/src/compiler/compile/nodes/Transition.ts
+++ b/src/compiler/compile/nodes/Transition.ts
@@ -19,7 +19,7 @@ export default class Transition extends Node {
 		component.warn_if_undefined(info.name, info, scope);
 
 		this.name = info.name;
-		component.add_reference(info.name.split('.')[0]);
+		component.add_reference(this as any, info.name.split('.')[0]);
 
 		this.directive = info.intro && info.outro ? 'transition' : info.intro ? 'in' : 'out';
 		this.is_local = info.modifiers.includes('local');
diff --git a/src/compiler/compile/nodes/shared/Context.ts b/src/compiler/compile/nodes/shared/Context.ts
index 7507d1e37e..670f0531e6 100644
--- a/src/compiler/compile/nodes/shared/Context.ts
+++ b/src/compiler/compile/nodes/shared/Context.ts
@@ -187,7 +187,7 @@ function mark_referenced(
 			if (is_reference(node, parent)) {
 				const { name } = flatten_reference(node);
 				if (!scope.is_let(name) && !scope.names.has(name)) {
-					component.add_reference(name);
+					component.add_reference(node, name);
 				}
 			}
 		}
diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts
index dfefafd8d8..98fb2f1e3a 100644
--- a/src/compiler/compile/nodes/shared/Expression.ts
+++ b/src/compiler/compile/nodes/shared/Expression.ts
@@ -110,7 +110,7 @@ export default class Expression {
 							dependencies.add(name);
 						}
 
-						component.add_reference(name);
+						component.add_reference(node, name);
 						component.warn_if_undefined(name, nodes[0], template_scope);
 					}
 
@@ -144,7 +144,7 @@ export default class Expression {
 							const each_block = template_scope.get_owner(name);
 							(each_block as EachBlock).has_binding = true;
 						} else {
-							component.add_reference(name);
+							component.add_reference(node, name);
 
 							const variable = component.var_lookup.get(name);
 							if (variable) variable[deep ? 'mutated' : 'reassigned'] = true;
@@ -220,7 +220,7 @@ export default class Expression {
 							});
 						} else {
 							dependencies.add(name);
-							component.add_reference(name); // TODO is this redundant/misplaced?
+							component.add_reference(node, name); // TODO is this redundant/misplaced?
 						}
 					} else if (is_contextual(component, template_scope, name)) {
 						const reference = block.renderer.reference(node, ctx);
@@ -267,7 +267,7 @@ export default class Expression {
 
 						this.replace(id as any);
 
-						component.add_var({
+						component.add_var(node, {
 							name: id.name,
 							internal: true,
 							hoistable: true,
diff --git a/src/compiler/compile/render_dom/Renderer.ts b/src/compiler/compile/render_dom/Renderer.ts
index c585cb3f72..179fc0c956 100644
--- a/src/compiler/compile/render_dom/Renderer.ts
+++ b/src/compiler/compile/render_dom/Renderer.ts
@@ -264,7 +264,7 @@ export default class Renderer {
 
 		// TODO is this correct?
 		if (this.component.var_lookup.get(name)) {
-			this.component.add_reference(name);
+			this.component.add_reference(node, name);
 		}
 
 		if (member !== undefined) {
diff --git a/test/validator/samples/illegal-variable-declaration/errors.json b/test/validator/samples/illegal-variable-declaration/errors.json
new file mode 100644
index 0000000000..e3d273138e
--- /dev/null
+++ b/test/validator/samples/illegal-variable-declaration/errors.json
@@ -0,0 +1,17 @@
+[
+	{
+		"code": "illegal-variable-declaration",
+		"message": "Cannot declare same variable name which is imported inside <script context=\"module\">",
+		"start": {
+			"line": 6,
+			"column": 1,
+			"character": 86
+		},
+		"end": {
+			"line": 6,
+			"column": 9,
+			"character": 94
+		},
+		"pos": 86
+	}
+]
diff --git a/test/validator/samples/illegal-variable-declaration/input.svelte b/test/validator/samples/illegal-variable-declaration/input.svelte
new file mode 100644
index 0000000000..4542380f49
--- /dev/null
+++ b/test/validator/samples/illegal-variable-declaration/input.svelte
@@ -0,0 +1,9 @@
+<script context="module">
+	import { FOO } from './dummy.svelte';
+</script>
+
+<script>
+	let FOO;
+</script>
+
+{FOO}