1 | /**
|
2 | * @fileoverview Rule to flag `else` after a `return` in `if`
|
3 | * @author Ian Christian Myers
|
4 | */
|
5 |
|
6 | ;
|
7 |
|
8 | //------------------------------------------------------------------------------
|
9 | // Requirements
|
10 | //------------------------------------------------------------------------------
|
11 |
|
12 | const astUtils = require("./utils/ast-utils");
|
13 | const FixTracker = require("./utils/fix-tracker");
|
14 |
|
15 | //------------------------------------------------------------------------------
|
16 | // Rule Definition
|
17 | //------------------------------------------------------------------------------
|
18 |
|
19 | module.exports = {
|
20 | meta: {
|
21 | type: "suggestion",
|
22 |
|
23 | docs: {
|
24 | description: "disallow `else` blocks after `return` statements in `if` statements",
|
25 | category: "Best Practices",
|
26 | recommended: false,
|
27 | url: "https://eslint.org/docs/rules/no-else-return"
|
28 | },
|
29 |
|
30 | schema: [{
|
31 | type: "object",
|
32 | properties: {
|
33 | allowElseIf: {
|
34 | type: "boolean",
|
35 | default: true
|
36 | }
|
37 | },
|
38 | additionalProperties: false
|
39 | }],
|
40 |
|
41 | fixable: "code",
|
42 |
|
43 | messages: {
|
44 | unexpected: "Unnecessary 'else' after 'return'."
|
45 | }
|
46 | },
|
47 |
|
48 | create(context) {
|
49 |
|
50 | //--------------------------------------------------------------------------
|
51 | // Helpers
|
52 | //--------------------------------------------------------------------------
|
53 |
|
54 | /**
|
55 | * Checks whether the given names can be safely used to declare block-scoped variables
|
56 | * in the given scope. Name collisions can produce redeclaration syntax errors,
|
57 | * or silently change references and modify behavior of the original code.
|
58 | *
|
59 | * This is not a generic function. In particular, it is assumed that the scope is a function scope or
|
60 | * a function's inner scope, and that the names can be valid identifiers in the given scope.
|
61 | * @param {string[]} names Array of variable names.
|
62 | * @param {eslint-scope.Scope} scope Function scope or a function's inner scope.
|
63 | * @returns {boolean} True if all names can be safely declared, false otherwise.
|
64 | */
|
65 | function isSafeToDeclare(names, scope) {
|
66 |
|
67 | if (names.length === 0) {
|
68 | return true;
|
69 | }
|
70 |
|
71 | const functionScope = scope.variableScope;
|
72 |
|
73 | /*
|
74 | * If this is a function scope, scope.variables will contain parameters, implicit variables such as "arguments",
|
75 | * all function-scoped variables ('var'), and block-scoped variables defined in the scope.
|
76 | * If this is an inner scope, scope.variables will contain block-scoped variables defined in the scope.
|
77 | *
|
78 | * Redeclaring any of these would cause a syntax error, except for the implicit variables.
|
79 | */
|
80 | const declaredVariables = scope.variables.filter(({ defs }) => defs.length > 0);
|
81 |
|
82 | if (declaredVariables.some(({ name }) => names.includes(name))) {
|
83 | return false;
|
84 | }
|
85 |
|
86 | // Redeclaring a catch variable would also cause a syntax error.
|
87 | if (scope !== functionScope && scope.upper.type === "catch") {
|
88 | if (scope.upper.variables.some(({ name }) => names.includes(name))) {
|
89 | return false;
|
90 | }
|
91 | }
|
92 |
|
93 | /*
|
94 | * Redeclaring an implicit variable, such as "arguments", would not cause a syntax error.
|
95 | * However, if the variable was used, declaring a new one with the same name would change references
|
96 | * and modify behavior.
|
97 | */
|
98 | const usedImplicitVariables = scope.variables.filter(({ defs, references }) =>
|
99 | defs.length === 0 && references.length > 0);
|
100 |
|
101 | if (usedImplicitVariables.some(({ name }) => names.includes(name))) {
|
102 | return false;
|
103 | }
|
104 |
|
105 | /*
|
106 | * Declaring a variable with a name that was already used to reference a variable from an upper scope
|
107 | * would change references and modify behavior.
|
108 | */
|
109 | if (scope.through.some(t => names.includes(t.identifier.name))) {
|
110 | return false;
|
111 | }
|
112 |
|
113 | /*
|
114 | * If the scope is an inner scope (not the function scope), an uninitialized `var` variable declared inside
|
115 | * the scope node (directly or in one of its descendants) is neither declared nor 'through' in the scope.
|
116 | *
|
117 | * For example, this would be a syntax error "Identifier 'a' has already been declared":
|
118 | * function foo() { if (bar) { let a; if (baz) { var a; } } }
|
119 | */
|
120 | if (scope !== functionScope) {
|
121 | const scopeNodeRange = scope.block.range;
|
122 | const variablesToCheck = functionScope.variables.filter(({ name }) => names.includes(name));
|
123 |
|
124 | if (variablesToCheck.some(v => v.defs.some(({ node: { range } }) =>
|
125 | scopeNodeRange[0] <= range[0] && range[1] <= scopeNodeRange[1]))) {
|
126 | return false;
|
127 | }
|
128 | }
|
129 |
|
130 | return true;
|
131 | }
|
132 |
|
133 |
|
134 | /**
|
135 | * Checks whether the removal of `else` and its braces is safe from variable name collisions.
|
136 | * @param {Node} node The 'else' node.
|
137 | * @param {eslint-scope.Scope} scope The scope in which the node and the whole 'if' statement is.
|
138 | * @returns {boolean} True if it is safe, false otherwise.
|
139 | */
|
140 | function isSafeFromNameCollisions(node, scope) {
|
141 |
|
142 | if (node.type === "FunctionDeclaration") {
|
143 |
|
144 | // Conditional function declaration. Scope and hoisting are unpredictable, different engines work differently.
|
145 | return false;
|
146 | }
|
147 |
|
148 | if (node.type !== "BlockStatement") {
|
149 | return true;
|
150 | }
|
151 |
|
152 | const elseBlockScope = scope.childScopes.find(({ block }) => block === node);
|
153 |
|
154 | if (!elseBlockScope) {
|
155 |
|
156 | // ecmaVersion < 6, `else` block statement cannot have its own scope, no possible collisions.
|
157 | return true;
|
158 | }
|
159 |
|
160 | /*
|
161 | * elseBlockScope is supposed to merge into its upper scope. elseBlockScope.variables array contains
|
162 | * only block-scoped variables (such as let and const variables or class and function declarations)
|
163 | * defined directly in the elseBlockScope. These are exactly the only names that could cause collisions.
|
164 | */
|
165 | const namesToCheck = elseBlockScope.variables.map(({ name }) => name);
|
166 |
|
167 | return isSafeToDeclare(namesToCheck, scope);
|
168 | }
|
169 |
|
170 | /**
|
171 | * Display the context report if rule is violated
|
172 | * @param {Node} node The 'else' node
|
173 | * @returns {void}
|
174 | */
|
175 | function displayReport(node) {
|
176 | const currentScope = context.getScope();
|
177 |
|
178 | context.report({
|
179 | node,
|
180 | messageId: "unexpected",
|
181 | fix: fixer => {
|
182 |
|
183 | if (!isSafeFromNameCollisions(node, currentScope)) {
|
184 | return null;
|
185 | }
|
186 |
|
187 | const sourceCode = context.getSourceCode();
|
188 | const startToken = sourceCode.getFirstToken(node);
|
189 | const elseToken = sourceCode.getTokenBefore(startToken);
|
190 | const source = sourceCode.getText(node);
|
191 | const lastIfToken = sourceCode.getTokenBefore(elseToken);
|
192 | let fixedSource, firstTokenOfElseBlock;
|
193 |
|
194 | if (startToken.type === "Punctuator" && startToken.value === "{") {
|
195 | firstTokenOfElseBlock = sourceCode.getTokenAfter(startToken);
|
196 | } else {
|
197 | firstTokenOfElseBlock = startToken;
|
198 | }
|
199 |
|
200 | /*
|
201 | * If the if block does not have curly braces and does not end in a semicolon
|
202 | * and the else block starts with (, [, /, +, ` or -, then it is not
|
203 | * safe to remove the else keyword, because ASI will not add a semicolon
|
204 | * after the if block
|
205 | */
|
206 | const ifBlockMaybeUnsafe = node.parent.consequent.type !== "BlockStatement" && lastIfToken.value !== ";";
|
207 | const elseBlockUnsafe = /^[([/+`-]/u.test(firstTokenOfElseBlock.value);
|
208 |
|
209 | if (ifBlockMaybeUnsafe && elseBlockUnsafe) {
|
210 | return null;
|
211 | }
|
212 |
|
213 | const endToken = sourceCode.getLastToken(node);
|
214 | const lastTokenOfElseBlock = sourceCode.getTokenBefore(endToken);
|
215 |
|
216 | if (lastTokenOfElseBlock.value !== ";") {
|
217 | const nextToken = sourceCode.getTokenAfter(endToken);
|
218 |
|
219 | const nextTokenUnsafe = nextToken && /^[([/+`-]/u.test(nextToken.value);
|
220 | const nextTokenOnSameLine = nextToken && nextToken.loc.start.line === lastTokenOfElseBlock.loc.start.line;
|
221 |
|
222 | /*
|
223 | * If the else block contents does not end in a semicolon,
|
224 | * and the else block starts with (, [, /, +, ` or -, then it is not
|
225 | * safe to remove the else block, because ASI will not add a semicolon
|
226 | * after the remaining else block contents
|
227 | */
|
228 | if (nextTokenUnsafe || (nextTokenOnSameLine && nextToken.value !== "}")) {
|
229 | return null;
|
230 | }
|
231 | }
|
232 |
|
233 | if (startToken.type === "Punctuator" && startToken.value === "{") {
|
234 | fixedSource = source.slice(1, -1);
|
235 | } else {
|
236 | fixedSource = source;
|
237 | }
|
238 |
|
239 | /*
|
240 | * Extend the replacement range to include the entire
|
241 | * function to avoid conflicting with no-useless-return.
|
242 | * https://github.com/eslint/eslint/issues/8026
|
243 | *
|
244 | * Also, to avoid name collisions between two else blocks.
|
245 | */
|
246 | return new FixTracker(fixer, sourceCode)
|
247 | .retainEnclosingFunction(node)
|
248 | .replaceTextRange([elseToken.range[0], node.range[1]], fixedSource);
|
249 | }
|
250 | });
|
251 | }
|
252 |
|
253 | /**
|
254 | * Check to see if the node is a ReturnStatement
|
255 | * @param {Node} node The node being evaluated
|
256 | * @returns {boolean} True if node is a return
|
257 | */
|
258 | function checkForReturn(node) {
|
259 | return node.type === "ReturnStatement";
|
260 | }
|
261 |
|
262 | /**
|
263 | * Naive return checking, does not iterate through the whole
|
264 | * BlockStatement because we make the assumption that the ReturnStatement
|
265 | * will be the last node in the body of the BlockStatement.
|
266 | * @param {Node} node The consequent/alternate node
|
267 | * @returns {boolean} True if it has a return
|
268 | */
|
269 | function naiveHasReturn(node) {
|
270 | if (node.type === "BlockStatement") {
|
271 | const body = node.body,
|
272 | lastChildNode = body[body.length - 1];
|
273 |
|
274 | return lastChildNode && checkForReturn(lastChildNode);
|
275 | }
|
276 | return checkForReturn(node);
|
277 | }
|
278 |
|
279 | /**
|
280 | * Check to see if the node is valid for evaluation,
|
281 | * meaning it has an else.
|
282 | * @param {Node} node The node being evaluated
|
283 | * @returns {boolean} True if the node is valid
|
284 | */
|
285 | function hasElse(node) {
|
286 | return node.alternate && node.consequent;
|
287 | }
|
288 |
|
289 | /**
|
290 | * If the consequent is an IfStatement, check to see if it has an else
|
291 | * and both its consequent and alternate path return, meaning this is
|
292 | * a nested case of rule violation. If-Else not considered currently.
|
293 | * @param {Node} node The consequent node
|
294 | * @returns {boolean} True if this is a nested rule violation
|
295 | */
|
296 | function checkForIf(node) {
|
297 | return node.type === "IfStatement" && hasElse(node) &&
|
298 | naiveHasReturn(node.alternate) && naiveHasReturn(node.consequent);
|
299 | }
|
300 |
|
301 | /**
|
302 | * Check the consequent/body node to make sure it is not
|
303 | * a ReturnStatement or an IfStatement that returns on both
|
304 | * code paths.
|
305 | * @param {Node} node The consequent or body node
|
306 | * @returns {boolean} `true` if it is a Return/If node that always returns.
|
307 | */
|
308 | function checkForReturnOrIf(node) {
|
309 | return checkForReturn(node) || checkForIf(node);
|
310 | }
|
311 |
|
312 |
|
313 | /**
|
314 | * Check whether a node returns in every codepath.
|
315 | * @param {Node} node The node to be checked
|
316 | * @returns {boolean} `true` if it returns on every codepath.
|
317 | */
|
318 | function alwaysReturns(node) {
|
319 | if (node.type === "BlockStatement") {
|
320 |
|
321 | // If we have a BlockStatement, check each consequent body node.
|
322 | return node.body.some(checkForReturnOrIf);
|
323 | }
|
324 |
|
325 | /*
|
326 | * If not a block statement, make sure the consequent isn't a
|
327 | * ReturnStatement or an IfStatement with returns on both paths.
|
328 | */
|
329 | return checkForReturnOrIf(node);
|
330 | }
|
331 |
|
332 |
|
333 | /**
|
334 | * Check the if statement, but don't catch else-if blocks.
|
335 | * @returns {void}
|
336 | * @param {Node} node The node for the if statement to check
|
337 | * @private
|
338 | */
|
339 | function checkIfWithoutElse(node) {
|
340 | const parent = node.parent;
|
341 |
|
342 | /*
|
343 | * Fixing this would require splitting one statement into two, so no error should
|
344 | * be reported if this node is in a position where only one statement is allowed.
|
345 | */
|
346 | if (!astUtils.STATEMENT_LIST_PARENTS.has(parent.type)) {
|
347 | return;
|
348 | }
|
349 |
|
350 | const consequents = [];
|
351 | let alternate;
|
352 |
|
353 | for (let currentNode = node; currentNode.type === "IfStatement"; currentNode = currentNode.alternate) {
|
354 | if (!currentNode.alternate) {
|
355 | return;
|
356 | }
|
357 | consequents.push(currentNode.consequent);
|
358 | alternate = currentNode.alternate;
|
359 | }
|
360 |
|
361 | if (consequents.every(alwaysReturns)) {
|
362 | displayReport(alternate);
|
363 | }
|
364 | }
|
365 |
|
366 | /**
|
367 | * Check the if statement
|
368 | * @returns {void}
|
369 | * @param {Node} node The node for the if statement to check
|
370 | * @private
|
371 | */
|
372 | function checkIfWithElse(node) {
|
373 | const parent = node.parent;
|
374 |
|
375 |
|
376 | /*
|
377 | * Fixing this would require splitting one statement into two, so no error should
|
378 | * be reported if this node is in a position where only one statement is allowed.
|
379 | */
|
380 | if (!astUtils.STATEMENT_LIST_PARENTS.has(parent.type)) {
|
381 | return;
|
382 | }
|
383 |
|
384 | const alternate = node.alternate;
|
385 |
|
386 | if (alternate && alwaysReturns(node.consequent)) {
|
387 | displayReport(alternate);
|
388 | }
|
389 | }
|
390 |
|
391 | const allowElseIf = !(context.options[0] && context.options[0].allowElseIf === false);
|
392 |
|
393 | //--------------------------------------------------------------------------
|
394 | // Public API
|
395 | //--------------------------------------------------------------------------
|
396 |
|
397 | return {
|
398 |
|
399 | "IfStatement:exit": allowElseIf ? checkIfWithoutElse : checkIfWithElse
|
400 |
|
401 | };
|
402 |
|
403 | }
|
404 | };
|