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("../ast-utils");
|
13 | const FixTracker = require("../util/fix-tracker");
|
14 |
|
15 | //------------------------------------------------------------------------------
|
16 | // Rule Definition
|
17 | //------------------------------------------------------------------------------
|
18 |
|
19 | module.exports = {
|
20 | meta: {
|
21 | docs: {
|
22 | description: "disallow `else` blocks after `return` statements in `if` statements",
|
23 | category: "Best Practices",
|
24 | recommended: false,
|
25 | url: "https://eslint.org/docs/rules/no-else-return"
|
26 | },
|
27 |
|
28 | schema: [{
|
29 | type: "object",
|
30 | properties: {
|
31 | allowElseIf: {
|
32 | type: "boolean"
|
33 | }
|
34 | },
|
35 | additionalProperties: false
|
36 | }],
|
37 |
|
38 | fixable: "code",
|
39 |
|
40 | messages: {
|
41 | unexpected: "Unnecessary 'else' after 'return'."
|
42 | }
|
43 | },
|
44 |
|
45 | create(context) {
|
46 |
|
47 | //--------------------------------------------------------------------------
|
48 | // Helpers
|
49 | //--------------------------------------------------------------------------
|
50 |
|
51 | /**
|
52 | * Display the context report if rule is violated
|
53 | *
|
54 | * @param {Node} node The 'else' node
|
55 | * @returns {void}
|
56 | */
|
57 | function displayReport(node) {
|
58 | context.report({
|
59 | node,
|
60 | messageId: "unexpected",
|
61 | fix: fixer => {
|
62 | const sourceCode = context.getSourceCode();
|
63 | const startToken = sourceCode.getFirstToken(node);
|
64 | const elseToken = sourceCode.getTokenBefore(startToken);
|
65 | const source = sourceCode.getText(node);
|
66 | const lastIfToken = sourceCode.getTokenBefore(elseToken);
|
67 | let fixedSource, firstTokenOfElseBlock;
|
68 |
|
69 | if (startToken.type === "Punctuator" && startToken.value === "{") {
|
70 | firstTokenOfElseBlock = sourceCode.getTokenAfter(startToken);
|
71 | } else {
|
72 | firstTokenOfElseBlock = startToken;
|
73 | }
|
74 |
|
75 | /*
|
76 | * If the if block does not have curly braces and does not end in a semicolon
|
77 | * and the else block starts with (, [, /, +, ` or -, then it is not
|
78 | * safe to remove the else keyword, because ASI will not add a semicolon
|
79 | * after the if block
|
80 | */
|
81 | const ifBlockMaybeUnsafe = node.parent.consequent.type !== "BlockStatement" && lastIfToken.value !== ";";
|
82 | const elseBlockUnsafe = /^[([/+`-]/.test(firstTokenOfElseBlock.value);
|
83 |
|
84 | if (ifBlockMaybeUnsafe && elseBlockUnsafe) {
|
85 | return null;
|
86 | }
|
87 |
|
88 | const endToken = sourceCode.getLastToken(node);
|
89 | const lastTokenOfElseBlock = sourceCode.getTokenBefore(endToken);
|
90 |
|
91 | if (lastTokenOfElseBlock.value !== ";") {
|
92 | const nextToken = sourceCode.getTokenAfter(endToken);
|
93 |
|
94 | const nextTokenUnsafe = nextToken && /^[([/+`-]/.test(nextToken.value);
|
95 | const nextTokenOnSameLine = nextToken && nextToken.loc.start.line === lastTokenOfElseBlock.loc.start.line;
|
96 |
|
97 | /*
|
98 | * If the else block contents does not end in a semicolon,
|
99 | * and the else block starts with (, [, /, +, ` or -, then it is not
|
100 | * safe to remove the else block, because ASI will not add a semicolon
|
101 | * after the remaining else block contents
|
102 | */
|
103 | if (nextTokenUnsafe || (nextTokenOnSameLine && nextToken.value !== "}")) {
|
104 | return null;
|
105 | }
|
106 | }
|
107 |
|
108 | if (startToken.type === "Punctuator" && startToken.value === "{") {
|
109 | fixedSource = source.slice(1, -1);
|
110 | } else {
|
111 | fixedSource = source;
|
112 | }
|
113 |
|
114 | /*
|
115 | * Extend the replacement range to include the entire
|
116 | * function to avoid conflicting with no-useless-return.
|
117 | * https://github.com/eslint/eslint/issues/8026
|
118 | */
|
119 | return new FixTracker(fixer, sourceCode)
|
120 | .retainEnclosingFunction(node)
|
121 | .replaceTextRange([elseToken.range[0], node.range[1]], fixedSource);
|
122 | }
|
123 | });
|
124 | }
|
125 |
|
126 | /**
|
127 | * Check to see if the node is a ReturnStatement
|
128 | *
|
129 | * @param {Node} node The node being evaluated
|
130 | * @returns {boolean} True if node is a return
|
131 | */
|
132 | function checkForReturn(node) {
|
133 | return node.type === "ReturnStatement";
|
134 | }
|
135 |
|
136 | /**
|
137 | * Naive return checking, does not iterate through the whole
|
138 | * BlockStatement because we make the assumption that the ReturnStatement
|
139 | * will be the last node in the body of the BlockStatement.
|
140 | *
|
141 | * @param {Node} node The consequent/alternate node
|
142 | * @returns {boolean} True if it has a return
|
143 | */
|
144 | function naiveHasReturn(node) {
|
145 | if (node.type === "BlockStatement") {
|
146 | const body = node.body,
|
147 | lastChildNode = body[body.length - 1];
|
148 |
|
149 | return lastChildNode && checkForReturn(lastChildNode);
|
150 | }
|
151 | return checkForReturn(node);
|
152 | }
|
153 |
|
154 | /**
|
155 | * Check to see if the node is valid for evaluation,
|
156 | * meaning it has an else.
|
157 | *
|
158 | * @param {Node} node The node being evaluated
|
159 | * @returns {boolean} True if the node is valid
|
160 | */
|
161 | function hasElse(node) {
|
162 | return node.alternate && node.consequent;
|
163 | }
|
164 |
|
165 | /**
|
166 | * If the consequent is an IfStatement, check to see if it has an else
|
167 | * and both its consequent and alternate path return, meaning this is
|
168 | * a nested case of rule violation. If-Else not considered currently.
|
169 | *
|
170 | * @param {Node} node The consequent node
|
171 | * @returns {boolean} True if this is a nested rule violation
|
172 | */
|
173 | function checkForIf(node) {
|
174 | return node.type === "IfStatement" && hasElse(node) &&
|
175 | naiveHasReturn(node.alternate) && naiveHasReturn(node.consequent);
|
176 | }
|
177 |
|
178 | /**
|
179 | * Check the consequent/body node to make sure it is not
|
180 | * a ReturnStatement or an IfStatement that returns on both
|
181 | * code paths.
|
182 | *
|
183 | * @param {Node} node The consequent or body node
|
184 | * @param {Node} alternate The alternate node
|
185 | * @returns {boolean} `true` if it is a Return/If node that always returns.
|
186 | */
|
187 | function checkForReturnOrIf(node) {
|
188 | return checkForReturn(node) || checkForIf(node);
|
189 | }
|
190 |
|
191 |
|
192 | /**
|
193 | * Check whether a node returns in every codepath.
|
194 | * @param {Node} node The node to be checked
|
195 | * @returns {boolean} `true` if it returns on every codepath.
|
196 | */
|
197 | function alwaysReturns(node) {
|
198 | if (node.type === "BlockStatement") {
|
199 |
|
200 | // If we have a BlockStatement, check each consequent body node.
|
201 | return node.body.some(checkForReturnOrIf);
|
202 | }
|
203 |
|
204 | /*
|
205 | * If not a block statement, make sure the consequent isn't a
|
206 | * ReturnStatement or an IfStatement with returns on both paths.
|
207 | */
|
208 | return checkForReturnOrIf(node);
|
209 | }
|
210 |
|
211 |
|
212 | /**
|
213 | * Check the if statement, but don't catch else-if blocks.
|
214 | * @returns {void}
|
215 | * @param {Node} node The node for the if statement to check
|
216 | * @private
|
217 | */
|
218 | function checkIfWithoutElse(node) {
|
219 | const parent = node.parent;
|
220 |
|
221 | /*
|
222 | * Fixing this would require splitting one statement into two, so no error should
|
223 | * be reported if this node is in a position where only one statement is allowed.
|
224 | */
|
225 | if (!astUtils.STATEMENT_LIST_PARENTS.has(parent.type)) {
|
226 | return;
|
227 | }
|
228 |
|
229 | const consequents = [];
|
230 | let alternate;
|
231 |
|
232 | for (let currentNode = node; currentNode.type === "IfStatement"; currentNode = currentNode.alternate) {
|
233 | if (!currentNode.alternate) {
|
234 | return;
|
235 | }
|
236 | consequents.push(currentNode.consequent);
|
237 | alternate = currentNode.alternate;
|
238 | }
|
239 |
|
240 | if (consequents.every(alwaysReturns)) {
|
241 | displayReport(alternate);
|
242 | }
|
243 | }
|
244 |
|
245 | /**
|
246 | * Check the if statement
|
247 | * @returns {void}
|
248 | * @param {Node} node The node for the if statement to check
|
249 | * @private
|
250 | */
|
251 | function checkIfWithElse(node) {
|
252 | const parent = node.parent;
|
253 |
|
254 |
|
255 | /*
|
256 | * Fixing this would require splitting one statement into two, so no error should
|
257 | * be reported if this node is in a position where only one statement is allowed.
|
258 | */
|
259 | if (!astUtils.STATEMENT_LIST_PARENTS.has(parent.type)) {
|
260 | return;
|
261 | }
|
262 |
|
263 | const alternate = node.alternate;
|
264 |
|
265 | if (alternate && alwaysReturns(node.consequent)) {
|
266 | displayReport(alternate);
|
267 | }
|
268 | }
|
269 |
|
270 | const allowElseIf = !(context.options[0] && context.options[0].allowElseIf === false);
|
271 |
|
272 | //--------------------------------------------------------------------------
|
273 | // Public API
|
274 | //--------------------------------------------------------------------------
|
275 |
|
276 | return {
|
277 |
|
278 | "IfStatement:exit": allowElseIf ? checkIfWithoutElse : checkIfWithElse
|
279 |
|
280 | };
|
281 |
|
282 | }
|
283 | };
|