Skip to content

Commit be99997

Browse files
XvezdaXvezda
authored
Merge pull request #1 from Xvezda/feature/fix-edgecase
Fix union type matching
2 parents eb5c7a4 + 8bf73e6 commit be99997

File tree

5 files changed

+247
-10
lines changed

5 files changed

+247
-10
lines changed

‎src/rules/no-implicit-propagation.js‎

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// @ts-check
22
const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/utils');
3+
const utils = require('@typescript-eslint/type-utils');
4+
const ts = require('typescript');
35
const { hasThrowsTag } = require('../utils');
46
const { findParent } = require('../utils');
57

@@ -19,6 +21,8 @@ module.exports = createRule({
1921
messages: {
2022
implicitPropagation:
2123
'Implicit propagation of exceptions is not allowed. Use try/catch to handle exceptions.',
24+
throwTypeMismatch:
25+
'The type of the exception thrown does not match the type specified in the @throws (or @exception) tag.',
2226
},
2327
defaultOptions: [
2428
{ tabLength: 4 },
@@ -41,10 +45,13 @@ module.exports = createRule({
4145

4246
const sourceCode = context.sourceCode;
4347
const services = ESLintUtils.getParserServices(context);
48+
const checker = services.program.getTypeChecker();
4449

4550
return {
4651
/** @param {import('@typescript-eslint/utils').TSESTree.ExpressionStatement} node */
4752
'FunctionDeclaration :not(TryStatement > BlockStatement) ExpressionStatement:has(> CallExpression)'(node) {
53+
if (node.expression.type !== AST_NODE_TYPES.CallExpression) return;
54+
4855
const declaration =
4956
/** @type {import('@typescript-eslint/utils').TSESTree.FunctionDeclaration} */
5057
(findParent(node, (n) => n.type === AST_NODE_TYPES.FunctionDeclaration));
@@ -56,12 +63,83 @@ module.exports = createRule({
5663
.map(({ value }) => value)
5764
.some(hasThrowsTag);
5865

59-
if (isCommented) return;
66+
// TODO: Branching type checking or not
67+
if (isCommented) {
68+
const calleeDeclaration = services.getTypeAtLocation(node.expression.callee).symbol.valueDeclaration;
69+
if (!calleeDeclaration) return;
6070

61-
if (node.expression.type !== AST_NODE_TYPES.CallExpression) return;
71+
const calleeTags =
72+
/** @type {import('typescript').JSDocThrowsTag[]} */
73+
(ts.getAllJSDocTagsOfKind(calleeDeclaration, ts.SyntaxKind.JSDocThrowsTag));
74+
75+
const calleeThrowTypeNodes =
76+
calleeTags
77+
.map((tag) => tag.typeExpression?.type)
78+
.filter((tag) => !!tag);
79+
80+
const tsDeclaration = services.getTypeAtLocation(declaration).symbol.valueDeclaration;
81+
if (!tsDeclaration) return;
82+
83+
const declarationTags =
84+
/** @type {import('typescript').JSDocThrowsTag[]} */
85+
(ts.getAllJSDocTagsOfKind(tsDeclaration, ts.SyntaxKind.JSDocThrowsTag));
86+
87+
const declarationThrowTypeNodes =
88+
declarationTags
89+
.map((tag) => tag.typeExpression?.type)
90+
.filter((tag) => !!tag);
91+
92+
const calleeThrowTypes = calleeThrowTypeNodes
93+
.map((node) => checker.getTypeFromTypeNode(node))
94+
.flatMap(t => t.isUnion() ? t.types : t);
95+
96+
const declarationThrowTypes = declarationThrowTypeNodes
97+
.map((node) => checker.getTypeFromTypeNode(node));
98+
99+
const isAllCalleeThrowsAssignable = calleeThrowTypes
100+
.every((t) => declarationThrowTypes
101+
.some((n) => checker.isTypeAssignableTo(t, n)));
102+
103+
if (isAllCalleeThrowsAssignable) return;
104+
105+
context.report({
106+
node,
107+
messageId: 'throwTypeMismatch',
108+
fix(fixer) {
109+
const lastTagtypeNode =
110+
declarationThrowTypeNodes[declarationThrowTypeNodes.length - 1];
111+
112+
if (declarationTags.length > 1) {
113+
const lastTag = declarationTags[declarationTags.length - 1];
114+
const notAssignableThrows = calleeThrowTypes
115+
.filter((t) => !declarationThrowTypes
116+
.some((n) => checker.isTypeAssignableTo(t, n)));
117+
118+
return fixer.replaceTextRange(
119+
[lastTag.parent.getStart(), lastTag.parent.getEnd()],
120+
notAssignableThrows
121+
.reduce((acc, t) =>
122+
acc.replace(
123+
/([^*\n]+)(\*+[/])/,
124+
`$1* @throws {${utils.getTypeName(checker, t)}}\n$1$2`
125+
),
126+
lastTag.parent.getFullText()
127+
)
128+
);
129+
}
130+
131+
return fixer.replaceTextRange(
132+
[lastTagtypeNode.pos, lastTagtypeNode.end],
133+
calleeThrowTypes.map(t => utils.getTypeName(checker, t)).join(' | '),
134+
);
135+
},
136+
});
137+
return;
138+
}
62139

63140
const calleeType = services.getTypeAtLocation(node.expression.callee);
64141
if (!calleeType.symbol) return;
142+
65143
const calleeTags = calleeType.symbol.getJsDocTags();
66144

67145
const isCalleeThrowable = calleeTags
@@ -71,9 +149,9 @@ module.exports = createRule({
71149

72150
const lines = sourceCode.getLines();
73151
const currentLine = lines[node.loc.start.line - 1];
152+
const prevLine = lines[node.loc.start.line - 2];
74153
const indent = currentLine.match(/^\s*/)?.[0] ?? '';
75154
const newIndent = indent + ' '.repeat(tabLength);
76-
const prevLine = lines[node.loc.start.line - 2];
77155

78156
// TODO: Better way to handle this?
79157
if (/^\s*try\s*\{/.test(prevLine)) return;

‎src/rules/no-implicit-propagation.test.js‎

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,69 @@ ruleTester.run(
5050
}
5151
`,
5252
},
53+
{
54+
code: `
55+
/**
56+
* foo bar baz
57+
*
58+
* @throws {string | number}
59+
*/
60+
function foo() {
61+
if (Math.random() > 0.5) {
62+
throw "lol";
63+
} else {
64+
throw 42;
65+
}
66+
}
67+
/** @throws {string | number} */
68+
function bar() {
69+
foo();
70+
}
71+
`,
72+
},
73+
{
74+
code: `
75+
/**
76+
* foo bar baz
77+
*
78+
* @throws {string | number}
79+
*/
80+
function foo() {
81+
if (Math.random() > 0.5) {
82+
throw "lol";
83+
} else {
84+
throw 42;
85+
}
86+
}
87+
/** @throws {number | string} */
88+
function bar() {
89+
foo();
90+
}
91+
`,
92+
},
93+
{
94+
code: `
95+
/**
96+
* foo bar baz
97+
*
98+
* @throws {string | number}
99+
*/
100+
function foo() {
101+
if (Math.random() > 0.5) {
102+
throw "lol";
103+
} else {
104+
throw 42;
105+
}
106+
}
107+
/**
108+
* @throws {string}
109+
* @throws {number}
110+
*/
111+
function bar() {
112+
foo();
113+
}
114+
`,
115+
},
53116
],
54117
invalid: [
55118
{
@@ -181,7 +244,102 @@ ruleTester.run(
181244
tabLength: 2,
182245
},
183246
],
184-
}
247+
},
248+
{
249+
code: `
250+
/**
251+
* foo bar baz
252+
*
253+
* @throws {string | number}
254+
*/
255+
function foo() {
256+
if (Math.random() > 0.5) {
257+
throw "lol";
258+
} else {
259+
throw 42;
260+
}
261+
}
262+
/** @throws {string} */
263+
function bar() {
264+
foo();
265+
}
266+
`,
267+
output: `
268+
/**
269+
* foo bar baz
270+
*
271+
* @throws {string | number}
272+
*/
273+
function foo() {
274+
if (Math.random() > 0.5) {
275+
throw "lol";
276+
} else {
277+
throw 42;
278+
}
279+
}
280+
/** @throws {string | number} */
281+
function bar() {
282+
foo();
283+
}
284+
`,
285+
errors: [{
286+
messageId: 'throwTypeMismatch',
287+
}],
288+
},
289+
{
290+
code: `
291+
/**
292+
* foo bar baz
293+
*
294+
* @throws {string | number | Error}
295+
*/
296+
function foo() {
297+
const rand = Math.random();
298+
if (rand > 0.5) {
299+
throw "lol";
300+
} else if (rand < 0.2) {
301+
throw 42;
302+
} else {
303+
throw new Error();
304+
}
305+
}
306+
/**
307+
* @throws {string}
308+
* @throws {number}
309+
*/
310+
function bar() {
311+
foo();
312+
}
313+
`,
314+
output: `
315+
/**
316+
* foo bar baz
317+
*
318+
* @throws {string | number | Error}
319+
*/
320+
function foo() {
321+
const rand = Math.random();
322+
if (rand > 0.5) {
323+
throw "lol";
324+
} else if (rand < 0.2) {
325+
throw 42;
326+
} else {
327+
throw new Error();
328+
}
329+
}
330+
/**
331+
* @throws {string}
332+
* @throws {number}
333+
* @throws {Error}
334+
*/
335+
function bar() {
336+
foo();
337+
}
338+
`,
339+
errors: [{
340+
messageId: 'throwTypeMismatch',
341+
}],
342+
},
185343
],
186344
},
187345
);

‎src/rules/no-undocumented-throws.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ module.exports = createRule({
5454

5555
/** @param {import('typescript').Type[]} types */
5656
const typesToUnionString = (types) =>
57-
types.map(t => utils.getTypeName(checker, t)).join('|');
57+
types.map(t => utils.getTypeName(checker, t)).join(' | ');
5858

5959
return {
6060
/** @param {import('@typescript-eslint/utils').TSESTree.ThrowStatement} node */
@@ -95,7 +95,8 @@ module.exports = createRule({
9595
return options.useBaseTypeOfLiteral && ts.isLiteralTypeLiteral(tsNode)
9696
? checker.getBaseTypeOfLiteralType(type)
9797
: type;
98-
});
98+
})
99+
.flatMap(t => t.isUnion() ? t.types : t);
99100

100101
if (isCommented) {
101102
const tags =

‎src/rules/no-undocumented-throws.test.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ ruleTester.run(
8080
/**
8181
* foo bar baz
8282
*
83-
* @throws {string|number}
83+
* @throws {string | number}
8484
*/
8585
function foo() {
8686
if (Math.random() > 0.5) {
@@ -96,7 +96,7 @@ ruleTester.run(
9696
/**
9797
* foo bar baz
9898
*
99-
* @throws {number|string}
99+
* @throws {number | string}
100100
*/
101101
function foo() {
102102
if (Math.random() > 0.5) {
@@ -200,7 +200,7 @@ ruleTester.run(
200200
/**
201201
* foo bar baz
202202
*
203-
* @throws {string|number}
203+
* @throws {string | number}
204204
*/
205205
function foo() {
206206
if (Math.random() > 0.5) {

‎tsconfig.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
// "disableReferencedProjectLoad": true, /* Reduce the number of projects loaded automatically by TypeScript. */
1212

1313
/* Language and Environment */
14-
"target": "es2016", /* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */
14+
"target": "es2018", /* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */
1515
// "lib": [], /* Specify a set of bundled library declaration files that describe the target runtime environment. */
1616
// "jsx": "preserve", /* Specify what JSX code is generated. */
1717
// "libReplacement": true, /* Enable lib replacement. */

0 commit comments

Comments
 (0)