Skip to content

Commit 113990c

Browse files
AlbertMarashiAlbertMarashi
committed
Add if block type narrowing
1 parent 4284cbb commit 113990c

File tree

8 files changed

+110
-91
lines changed

8 files changed

+110
-91
lines changed

Diff for: ‎packages/eslint-plugin-svelte/src/meta.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// IMPORTANT!
22
// This file has been automatically generated,
33
// in order to update its content execute "pnpm run update"
4-
export const name = 'eslint-plugin-svelte';
5-
export const version = '2.43.0';
4+
export const name = 'eslint-plugin-svelte-albert';
5+
export const version = '2.50.0';

Diff for: ‎packages/eslint-plugin-svelte/src/rules/restrict-mustache-expressions.ts

+86-83
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ import { createRule } from '../utils';
33
import { TypeFlags } from 'typescript';
44
import type { TSESTree } from '@typescript-eslint/types';
55
import type { TS, TSTools } from '../utils/ts-utils';
6-
import { getConstrainedTypeAtLocation, getTypeName, getTypeScriptTools } from '../utils/ts-utils';
6+
import { getTypeName, getTypeScriptTools } from '../utils/ts-utils';
77
import { findVariable } from '../utils/ast-utils';
88
import type { RuleContext } from '../types';
9+
import type { Scope, Variable } from '@typescript-eslint/scope-manager';
910

1011
const props = {
1112
allowBoolean: {
@@ -38,6 +39,11 @@ type Props = {
3839
allowUndefined: boolean;
3940
};
4041

42+
enum NodeType {
43+
Unknown,
44+
Allowed
45+
}
46+
4147
type Config = {
4248
stringTemplateExpressions?: Props;
4349
textExpressions?: Props;
@@ -86,6 +92,7 @@ export default createRule('restrict-mustache-expressions', {
8692
if (node.parent.type === 'SvelteAttribute') {
8793
if (!node.parent.value.find((n) => n.type === 'SvelteLiteral')) {
8894
// we are rendering a non-literal attribute (eg: class:disabled={disabled}, so we allow any type
95+
// (todo): maybe we could maybe check the expected type of the attribute here, but I think the language server already does that?
8996
return;
9097
}
9198
// we are rendering an template string attribute (eg: href="/page/{page.id}"), so we only allow stringifiable types
@@ -107,14 +114,13 @@ export default createRule('restrict-mustache-expressions', {
107114
if (allowNull) allowed_types.add('null');
108115
if (allowUndefined) allowed_types.add('undefined');
109116

110-
const disallowed = disallowed_expression(node.expression, allowed_types, context, tools!);
111-
if (!disallowed) return;
112-
117+
const type = disallowed_expression(node.expression, allowed_types, context, tools!);
118+
if (NodeType.Allowed === type) return;
113119
context.report({
114120
node,
115121
messageId: 'expectedStringifyableType',
116122
data: {
117-
disallowed: getTypeName(disallowed, tools!),
123+
disallowed: type === NodeType.Unknown ? 'unknown' : getTypeName(type, tools!),
118124
types: [...allowed_types].map((t) => `\`${t}\``).join(', ')
119125
}
120126
});
@@ -126,49 +132,87 @@ export default createRule('restrict-mustache-expressions', {
126132
}
127133
});
128134

129-
function getNodeType(
130-
node: TSESTree.Expression | TSESTree.PrivateIdentifier | TSESTree.SpreadElement,
131-
tools: TSTools
132-
): TS.Type | null {
133-
const tsNode = tools.service.esTreeNodeToTSNodeMap.get(node);
134-
return (
135-
(tsNode && getConstrainedTypeAtLocation(tools.service.program.getTypeChecker(), tsNode)) || null
136-
);
135+
function getNodeType(node: TSESTree.Node, tools: TSTools): TS.Type | NodeType.Unknown {
136+
const checker = tools.service.program.getTypeChecker();
137+
const ts_node = tools.service.esTreeNodeToTSNodeMap.get(node);
138+
if (!ts_node) return NodeType.Unknown;
139+
const nodeType = checker.getTypeAtLocation(ts_node);
140+
const constrained = checker.getBaseConstraintOfType(nodeType);
141+
return constrained ?? nodeType;
137142
}
138143

139144
function disallowed_identifier(
140145
expression: TSESTree.Identifier,
141146
allowed_types: Set<string>,
142147
context: RuleContext,
143148
tools: TSTools
144-
): TS.Type | null {
145-
const type = getNodeType(expression, tools);
149+
): TS.Type | NodeType {
150+
const type = get_variable_type(expression, context, tools);
146151

147-
if (!type) return null;
152+
if (type === NodeType.Unknown) return NodeType.Unknown;
148153

149154
return disallowed_type(type, allowed_types, context, tools);
150155
}
151156

157+
function get_variable_type(
158+
identifier: TSESTree.Identifier,
159+
context: RuleContext,
160+
tools: TSTools
161+
): TS.Type | NodeType.Unknown {
162+
const variable = findVariable(context, identifier);
163+
164+
const identifiers = variable?.identifiers[0];
165+
166+
if (!identifiers) return getNodeType(identifier, tools);
167+
168+
const type = getNodeType(variable.identifiers[0], tools);
169+
170+
if (NodeType.Unknown === type) return NodeType.Unknown;
171+
172+
return narrow_variable_type(identifier, type, tools);
173+
}
174+
175+
function narrow_variable_type(
176+
identifier: TSESTree.Identifier,
177+
type: TS.Type,
178+
tools: TSTools
179+
): TS.Type {
180+
const checker = tools.service.program.getTypeChecker();
181+
let currentNode: TSESTree.Node | AST.SvelteNode | undefined = identifier as TSESTree.Node;
182+
183+
while (currentNode) {
184+
if (currentNode.type === 'SvelteIfBlock') {
185+
const condition = currentNode.expression;
186+
if (condition.type === 'Identifier' && condition.name === identifier.name) {
187+
return checker.getNonNullableType(type);
188+
}
189+
}
190+
currentNode = currentNode.parent as TSESTree.Node | AST.SvelteNode;
191+
}
192+
193+
return type;
194+
}
195+
152196
function disallowed_type(
153197
type: TS.Type,
154198
allowed_types: Set<string>,
155199
context: RuleContext,
156200
tools: TSTools
157-
): TS.Type | null {
201+
): TS.Type | NodeType {
158202
if (type.flags & TypeFlags.StringLike) {
159-
return null;
203+
return NodeType.Allowed;
160204
}
161205
if (type.flags & TypeFlags.BooleanLike) {
162-
return allowed_types.has('boolean') ? null : type;
206+
return allowed_types.has('boolean') ? NodeType.Allowed : type;
163207
}
164208
if (type.flags & TypeFlags.NumberLike) {
165-
return allowed_types.has('number') ? null : type;
209+
return allowed_types.has('number') ? NodeType.Allowed : type;
166210
}
167211
if (type.flags & TypeFlags.Null) {
168-
return allowed_types.has('null') ? null : type;
212+
return allowed_types.has('null') ? NodeType.Allowed : type;
169213
}
170214
if (type.flags & TypeFlags.Undefined) {
171-
return allowed_types.has('undefined') ? null : type;
215+
return allowed_types.has('undefined') ? NodeType.Allowed : type;
172216
}
173217
if (type.isUnion()) {
174218
for (const sub_type of type.types) {
@@ -177,7 +221,7 @@ function disallowed_type(
177221
return disallowed;
178222
}
179223
}
180-
return null;
224+
return NodeType.Allowed;
181225
}
182226

183227
return type;
@@ -188,10 +232,10 @@ function disallowed_literal(
188232
allowed_types: Set<string>,
189233
context: RuleContext,
190234
tools: TSTools
191-
): TS.Type | null {
235+
): TS.Type | NodeType {
192236
const type = getNodeType(expression, tools);
193237

194-
if (!type) return null;
238+
if (NodeType.Unknown === type) return NodeType.Unknown;
195239

196240
return disallowed_type(type, allowed_types, context, tools);
197241
}
@@ -201,7 +245,7 @@ function disallowed_expression(
201245
allowed_types: Set<string>,
202246
context: RuleContext,
203247
tools: TSTools
204-
): TS.Type | null {
248+
): TS.Type | NodeType {
205249
switch (expression.type) {
206250
case 'Literal':
207251
return disallowed_literal(expression, allowed_types, context, tools);
@@ -213,8 +257,11 @@ function disallowed_expression(
213257
return disallowed_member_expression(expression, allowed_types, context, tools);
214258
case 'LogicalExpression':
215259
return disallowed_logical_expression(expression, allowed_types, context, tools);
216-
default:
217-
return getNodeType(expression, tools);
260+
default: {
261+
const type = getNodeType(expression, tools);
262+
if (NodeType.Unknown === type) return NodeType.Unknown;
263+
return disallowed_type(type, allowed_types, context, tools);
264+
}
218265
}
219266
}
220267

@@ -223,58 +270,24 @@ function disallowed_logical_expression(
223270
allowed_types: Set<string>,
224271
context: RuleContext,
225272
tools: TSTools
226-
): TS.Type | null {
273+
): TS.Type | NodeType {
227274
const type = getNodeType(expression, tools);
228275

229-
if (!type) return null;
276+
if (NodeType.Unknown === type) return NodeType.Unknown;
230277

231278
return disallowed_type(type, allowed_types, context, tools);
232279
}
233280

234-
// function disallowed_member_expression(
235-
// expression: TSESTree.MemberExpression,
236-
// allowed_types: Set<string>,
237-
// context: RuleContext,
238-
// tools: TSTools
239-
// ): TS.Type | null {
240-
// const checker = tools.service.program.getTypeChecker();
241-
// const type = getNodeType(expression, tools);
242-
// if (!type) return null;
243-
244-
// const object = expression.object;
245-
// if (object.type === 'Identifier') {
246-
// const variable = findVariable(context, object);
247-
// if (!variable) return null;
248-
// const node_def = variable.defs[0].node;
249-
// if (node_def.type !== 'VariableDeclarator') return null;
250-
// if (!node_def.init) return null;
251-
// // let type = getNodeType(node_def.init, tools);
252-
// if (node_def.init.type !== 'ObjectExpression') return null;
253-
// if (expression.property.type !== 'Identifier') return null;
254-
255-
// const type = getNodeType(node_def.init, tools);
256-
// if (!type) return null;
257-
// const symbol = checker.getPropertyOfType(type, expression.property.name);
258-
// if (!symbol) return null;
259-
260-
// const prop_type = checker.getTypeOfSymbol(symbol);
261-
262-
// return disallowed_type(prop_type, allowed_types, context, tools);
263-
// }
264-
265-
// return disallowed_type(type, allowed_types, context, tools);
266-
// }
267-
268281
function disallowed_member_expression(
269282
expression: TSESTree.MemberExpression,
270283
allowed_types: Set<string>,
271284
context: RuleContext,
272285
tools: TSTools
273-
): TS.Type | null {
286+
): TS.Type | NodeType {
274287
const checker = tools.service.program.getTypeChecker();
275-
let objectType = getNodeType(expression.object, tools);
288+
let objectType: TS.Type | NodeType = getNodeType(expression.object, tools);
276289

277-
if (!objectType) return null;
290+
if (NodeType.Unknown === objectType) return NodeType.Unknown;
278291

279292
// Handle nested member expressions
280293
if (expression.object.type === 'MemberExpression') {
@@ -287,6 +300,8 @@ function disallowed_member_expression(
287300
if (nestedType) objectType = nestedType;
288301
}
289302

303+
if (NodeType.Allowed === objectType) return NodeType.Allowed;
304+
290305
// Handle identifiers (variables)
291306
if (expression.object.type === 'Identifier') {
292307
const variable = findVariable(context, expression.object);
@@ -303,26 +318,14 @@ function disallowed_member_expression(
303318
const propertyName = getPropertyName(expression.property);
304319
if (!propertyName) return objectType;
305320

306-
let propertyType: TS.Type | undefined;
307-
308321
// Try to get property type using getPropertyOfType
309322
const symbol = checker.getPropertyOfType(objectType, propertyName);
310323
if (symbol) {
311-
propertyType = checker.getTypeOfSymbol(symbol);
312-
}
313-
314-
// If property type is still not found, try using getTypeOfPropertyOfType
315-
if (!propertyType) {
316-
const property_symbol = checker.getPropertyOfType(objectType, propertyName);
317-
if (property_symbol) {
318-
propertyType = checker.getTypeOfSymbol(property_symbol);
319-
}
324+
const property_type = checker.getTypeOfSymbol(symbol);
325+
return disallowed_type(property_type, allowed_types, context, tools);
320326
}
321327

322-
// If we found a property type, use it; otherwise, fall back to the object type
323-
return propertyType
324-
? disallowed_type(propertyType, allowed_types, context, tools)
325-
: disallowed_type(objectType, allowed_types, context, tools);
328+
return NodeType.Unknown;
326329
}
327330

328331
function getPropertyName(
@@ -333,7 +336,7 @@ function getPropertyName(
333336
} else if (property.type === 'Literal' && typeof property.value === 'string') {
334337
return property.value;
335338
} else if (property.type === 'TemplateLiteral' && property.quasis.length === 1) {
336-
return property.quasis[0].value.cooked;
339+
// return property.quasis[0].value.cooked;
337340
}
338341
return undefined;
339342
}

Diff for: ‎packages/eslint-plugin-svelte/tests/fixtures/rules/restrict-mustache-expressions/invalid/conditional/conditional-errors.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
- message: 'Expected `null` to be one of the following: `string`, `boolean`,
2+
`number`. You must cast or convert the expression to one of the allowed
3+
types.'
4+
line: 6
5+
column: 5
6+
suggestions: null

Diff for: ‎packages/eslint-plugin-svelte/tests/fixtures/rules/restrict-mustache-expressions/invalid/conditional/conditional-input.svelte

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<script lang="ts">
2+
3+
let foo: null | string = "test";
4+
</script>
5+
{#if foo === null}
6+
{foo }
7+
{/if}

Diff for: ‎packages/eslint-plugin-svelte/tests/fixtures/rules/restrict-mustache-expressions/invalid/textExpression/object/object-input.svelte

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script lang="ts">
2-
let invalid_object = { foo: 'bar' };
2+
let invalid_object = { foo: 'test' };
33
</script>
44
{ { foo: 'bar' } }
55
{ invalid_object }

Diff for: ‎packages/eslint-plugin-svelte/tests/fixtures/rules/restrict-mustache-expressions/valid/conditional/conditional-input.svelte

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<script lang="ts">
2+
let foo: string | null = null as unknown as string | null
3+
</script>
4+
{#if foo}
5+
{ foo }
6+
{/if}

Diff for: ‎packages/eslint-plugin-svelte/tests/fixtures/rules/restrict-mustache-expressions/valid/object-access/_config.json

-3
This file was deleted.

Diff for: ‎packages/eslint-plugin-svelte/tests/src/rules/restrict-mustache-expressions.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import { loadTestCases } from '../../utils/utils';
55
const tester = new RuleTester({
66
languageOptions: {
77
ecmaVersion: 2020,
8-
sourceType: 'module'
9-
}
8+
sourceType: 'module',
9+
},
1010
});
1111

1212
tester.run('restrict-mustache-expressions', rule as any, loadTestCases('restrict-mustache-expressions'));

0 commit comments

Comments
 (0)