Skip to content

Commit 88ed993

Browse files
warn on assignment to const (#4960)
* warn on assignment to const * fix formatting and switch to error * check most local scopes first * fix logic and add more tests * more formatting * Fix broken test * use find_owner instead Co-authored-by: tanhauhau <[email protected]>
1 parent ab1285a commit 88ed993

File tree

12 files changed

+207
-3
lines changed

12 files changed

+207
-3
lines changed

src/compiler/compile/Component.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,42 @@ export default class Component {
792792
scope = map.get(node);
793793
}
794794

795+
let deep = false;
796+
let names: string[] | undefined;
797+
798+
if (node.type === 'AssignmentExpression') {
799+
deep = node.left.type === 'MemberExpression';
800+
names = deep
801+
? [get_object(node.left).name]
802+
: extract_names(node.left);
803+
} else if (node.type === 'UpdateExpression') {
804+
deep = node.argument.type === 'MemberExpression';
805+
const { name } = get_object(node.argument);
806+
names = [name];
807+
}
808+
809+
if (names) {
810+
names.forEach(name => {
811+
let current_scope = scope;
812+
let declaration;
813+
814+
while (current_scope) {
815+
if (current_scope.declarations.has(name)) {
816+
declaration = current_scope.declarations.get(name);
817+
break;
818+
}
819+
current_scope = current_scope.parent;
820+
}
821+
822+
if (declaration && declaration.kind === 'const' && !deep) {
823+
component.error(node as any, {
824+
code: 'assignment-to-const',
825+
message: 'You are assigning to a const'
826+
});
827+
}
828+
});
829+
}
830+
795831
if (node.type === 'ImportDeclaration') {
796832
component.extract_imports(node);
797833
// TODO: to use actual remove

src/compiler/compile/nodes/shared/Expression.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ export default class Expression {
128128
deep = node.left.type === 'MemberExpression';
129129
names = extract_names(deep ? get_object(node.left) : node.left);
130130
} else if (node.type === 'UpdateExpression') {
131+
deep = node.argument.type === 'MemberExpression';
131132
names = extract_names(get_object(node.argument));
132133
}
133134
}
@@ -149,7 +150,26 @@ export default class Expression {
149150
component.add_reference(node, name);
150151

151152
const variable = component.var_lookup.get(name);
152-
if (variable) variable[deep ? 'mutated' : 'reassigned'] = true;
153+
154+
if (variable) {
155+
variable[deep ? 'mutated' : 'reassigned'] = true;
156+
}
157+
158+
const declaration: any = scope.find_owner(name)?.declarations.get(name);
159+
160+
if (declaration) {
161+
if (declaration.kind === 'const' && !deep) {
162+
component.error(node, {
163+
code: 'assignment-to-const',
164+
message: 'You are assigning to a const'
165+
});
166+
}
167+
} else if (variable && variable.writable === false && !deep) {
168+
component.error(node, {
169+
code: 'assignment-to-const',
170+
message: 'You are assigning to a const'
171+
});
172+
}
153173
}
154174
});
155175
}

test/js/samples/unchanged-expression/expected.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ let world1 = 'world';
7373
let world2 = 'world';
7474

7575
function instance($$self, $$props, $$invalidate) {
76-
const world3 = 'world';
76+
let world3 = 'world';
7777

7878
function foo() {
7979
$$invalidate(0, world3 = 'svelte');

test/js/samples/unchanged-expression/input.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<script>
22
let world1 = 'world';
33
let world2 = 'world';
4-
const world3 = 'world';
4+
let world3 = 'world';
55
function foo() {
66
world3 = 'svelte';
77
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
[
2+
{
3+
"code": "assignment-to-const",
4+
"message": "You are assigning to a const",
5+
"start": {
6+
"line": 13,
7+
"column": 24,
8+
"character": 282
9+
},
10+
"end": {
11+
"line": 13,
12+
"column": 35,
13+
"character": 293
14+
},
15+
"pos": 282
16+
}
17+
]
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<script>
2+
const immutable = 0;
3+
4+
const obj1 = { prop: true };
5+
const obj2 = { prop: 0 }
6+
</script>
7+
8+
<!-- should not error -->
9+
<button on:click={() => obj1.prop = false}>click</button>
10+
<button on:click={() => obj2.prop++}>click</button>
11+
12+
<!-- should error -->
13+
<button on:click={() => immutable++}>click</button>
14+
15+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
[
2+
{
3+
"code": "assignment-to-const",
4+
"message": "You are assigning to a const",
5+
"start": {
6+
"line": 14,
7+
"column": 3,
8+
"character": 172
9+
},
10+
"end": {
11+
"line": 14,
12+
"column": 10,
13+
"character": 179
14+
},
15+
"pos": 172
16+
}
17+
]
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<script>
2+
const foo = 'hello';
3+
4+
function shouldNotError() {
5+
let foo = 0;
6+
7+
function inner() {
8+
foo = 1;
9+
}
10+
}
11+
12+
function shouldError() {
13+
function inner() {
14+
foo = 1;
15+
}
16+
}
17+
</script>
18+
19+
<button on:click={shouldNotError}>click</button>
20+
<button on:click={shouldError}>click</button>
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
[
2+
{
3+
"code": "assignment-to-const",
4+
"message": "You are assigning to a const",
5+
"start": {
6+
"line": 17,
7+
"column": 2,
8+
"character": 189
9+
},
10+
"end": {
11+
"line": 17,
12+
"column": 9,
13+
"character": 196
14+
},
15+
"pos": 189
16+
}
17+
]
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<script>
2+
const foo = 'hello';
3+
</script>
4+
5+
<button on:click={() => {
6+
let foo = 0;
7+
8+
function inner() {
9+
foo = 1;
10+
}
11+
}}>
12+
click
13+
</button>
14+
15+
<button on:click={() => {
16+
function inner() {
17+
foo = 1;
18+
}
19+
}}>
20+
click
21+
</button>

0 commit comments

Comments
 (0)