diff --git a/docs/rules/no-empty-collection.md b/docs/rules/no-empty-collection.md index 1a8c8118..af541917 100644 --- a/docs/rules/no-empty-collection.md +++ b/docs/rules/no-empty-collection.md @@ -11,5 +11,5 @@ if (strings.includes("foo")) {} // Noncompliant for (str of strings) {} // Noncompliant -strings.forEach(str => doSomething(str)); // Noncompliant +strings.forEach(str => doSomething(str)); // Noncompliant ``` diff --git a/docs/rules/prefer-single-boolean-return.md b/docs/rules/prefer-single-boolean-return.md index 4bce8472..8848f27c 100644 --- a/docs/rules/prefer-single-boolean-return.md +++ b/docs/rules/prefer-single-boolean-return.md @@ -1,6 +1,6 @@ # prefer-single-boolean-return -Return of boolean literal statements wrapped into `if-then-else` ones should be simplified. +Return of boolean literal statements wrapped into `if-then-else` flow should be simplified. ## Noncompliant Code Example @@ -12,6 +12,15 @@ if (expression) { } ``` +or + +```javascript +if (expression) { + return true; +} +return false; +``` + ## Compliant Solution ```javascript diff --git a/ruling/snapshots/prefer-single-boolean-return b/ruling/snapshots/prefer-single-boolean-return index 7dae9a0e..4d3b7bdd 100644 --- a/ruling/snapshots/prefer-single-boolean-return +++ b/ruling/snapshots/prefer-single-boolean-return @@ -1,5 +1,58 @@ src/Ghost/core/server/adapters/storage/utils.js: 39 src/Ghost/core/server/api/notifications.js: 73 -src/brackets/src/extensions/default/DebugCommands/main.js: 345 +src/Ghost/core/server/config/utils.js: 13 +src/Ghost/core/server/update-check.js: 237 +src/Ghost/core/server/web/utils.js: 45 +src/angular.js/src/ngMock/angular-mocks.js: 1980 +src/angular.js/src/ngParseExt/ucd.js: 561,1215 +src/brackets/src/JSUtils/ScopeManager.js: 234,262 +src/brackets/src/LiveDevelopment/Documents/HTMLDocument.js: 255 +src/brackets/src/command/KeyBindingManager.js: 1069 +src/brackets/src/extensibility/node/package-validator.js: 74 +src/brackets/src/extensions/default/DebugCommands/main.js: 345,433 +src/brackets/src/extensions/default/InlineTimingFunctionEditor/TimingFunctionUtils.js: 157,239 +src/brackets/src/extensions/default/JavaScriptCodeHints/main.js: 784 +src/brackets/src/extensions/default/JavaScriptRefactoring/RefactoringUtils.js: 189 +src/brackets/src/extensions/default/PrefsCodeHints/main.js: 201,369 +src/brackets/src/extensions/default/UrlCodeHints/main.js: 308 +src/brackets/src/language/CSSUtils.js: 1130 +src/brackets/src/preferences/PreferencesBase.js: 1143 src/brackets/src/search/FindUtils.js: 431 +src/brackets/src/utils/ValidationUtils.js: 46 +src/create-react-app/packages/react-dev-utils/eslintFormatter.js: 15 +src/jest/packages/expect/src/utils.js: 163 +src/jest/packages/jest-cli/src/cli/index.js: 252 +src/jest/packages/jest-cli/src/run_jest.js: 161 +src/jest/packages/jest-message-util/src/index.js: 188 +src/jest/packages/jest-runtime/src/should_instrument.js: 78 +src/jquery/external/qunit/qunit.js: 1703 +src/jquery/external/sinon/sinon.js: 220 +src/react-native/Libraries/Components/ScrollResponder.js: 179 +src/react-native/Libraries/Renderer/ReactFabric-dev.js: 10866 +src/react-native/Libraries/Renderer/ReactNativeRenderer-dev.js: 11236 +src/react/packages/react-dom/src/shared/DOMProperty.js: 101 +src/react/packages/react-reconciler/src/ReactFiberHydrationContext.js: 272 +src/react/packages/shared/isTextInputElement.js: 38 +src/reveal.js/js/reveal.js: 5203 +src/spectrum/admin/src/helpers/utils.js: 41 +src/spectrum/api/models/invoice.js: 83 +src/spectrum/api/queries/channel/isArchived.js: 5 +src/spectrum/api/queries/directMessageThread/utils.js: 16 +src/spectrum/api/queries/meta/isAdmin.js: 6 +src/spectrum/src/components/chatInput/index.js: 101 +src/spectrum/src/components/composer/utils.js: 25 src/spectrum/src/components/message/index.js: 17 +src/spectrum/src/components/threadFeed/index.js: 156 +src/spectrum/src/helpers/utils.js: 85 +src/spectrum/src/views/community/components/memberGrid.js: 54 +src/spectrum/src/views/community/components/moderatorList.js: 36 +src/spectrum/src/views/dashboard/components/communityList.js: 68 +src/spectrum/src/views/dashboard/components/threadFeed.js: 81 +src/spectrum/src/views/dashboard/components/upsellExploreCommunities.js: 48 +src/spectrum/src/views/directMessages/index.js: 63 +src/spectrum/src/views/navbar/components/messagesTab.js: 64 +src/spectrum/src/views/navbar/components/notificationsTab.js: 109 +src/spectrum/src/views/navbar/index.js: 70 +src/spectrum/src/views/notifications/index.js: 120 +src/spectrum/src/views/pages/pricing/components/communityList.js: 43 +src/three.js/src/math/Ray.js: 352 diff --git a/src/rules/prefer-single-boolean-return.ts b/src/rules/prefer-single-boolean-return.ts index 9f41abe5..9c8128e9 100644 --- a/src/rules/prefer-single-boolean-return.ts +++ b/src/rules/prefer-single-boolean-return.ts @@ -30,8 +30,7 @@ import docsUrl from '../utils/docs-url'; const rule: TSESLint.RuleModule = { meta: { messages: { - replaceIfThenElseByReturn: - 'Replace this if-then-else statement by a single return statement.', + replaceIfThenElseByReturn: 'Replace this if-then-else flow by a single return statement.', }, schema: [], type: 'suggestion', @@ -44,23 +43,35 @@ const rule: TSESLint.RuleModule = { }, create(context) { return { - IfStatement(node: TSESTree.Node) { - const ifStmt = node as TSESTree.IfStatement; + IfStatement(node: TSESTree.IfStatement) { if ( // ignore `else if` - !isIfStatement(ifStmt.parent) && - // `ifStmt.alternate` can be `null`, replace it with `undefined` in this case - returnsBoolean(ifStmt.alternate || undefined) && - returnsBoolean(ifStmt.consequent) + !isIfStatement(node.parent) && + returnsBoolean(node.consequent) && + alternateReturnsBoolean(node) ) { context.report({ messageId: 'replaceIfThenElseByReturn', - node: ifStmt, + node, }); } }, }; + function alternateReturnsBoolean(node: TSESTree.IfStatement) { + if (node.alternate) { + return returnsBoolean(node.alternate); + } + + const { parent } = node; + if (parent?.type === 'BlockStatement') { + const ifStmtIndex = parent.body.findIndex(stmt => stmt === node); + return isSimpleReturnBooleanLiteral(parent.body[ifStmtIndex + 1]); + } + + return false; + } + function returnsBoolean(statement: TSESTree.Statement | undefined) { return ( statement !== undefined && @@ -76,7 +87,7 @@ const rule: TSESLint.RuleModule = { ); } - function isSimpleReturnBooleanLiteral(statement: TSESTree.Statement) { + function isSimpleReturnBooleanLiteral(statement: TSESTree.Node) { // `statement.argument` can be `null`, replace it with `undefined` in this case return isReturnStatement(statement) && isBooleanLiteral(statement.argument || undefined); } diff --git a/tests/rules/prefer-single-boolean-return.test.ts b/tests/rules/prefer-single-boolean-return.test.ts index 32fff52f..d661016d 100644 --- a/tests/rules/prefer-single-boolean-return.test.ts +++ b/tests/rules/prefer-single-boolean-return.test.ts @@ -186,5 +186,46 @@ ruleTester.run('prefer-single-boolean-return', rule, { }, ], }, + { + code: ` + function fn() { + if (foo) { + if (something) { + return true + } + return false + } + + if (bar) { + if (something) { + return false + } + return true + } + + if (baz) { + if (something) { + return false + } + } + } + `, + errors: [ + { + messageId: 'replaceIfThenElseByReturn', + line: 4, + column: 13, + endLine: 6, + endColumn: 14, + }, + { + messageId: 'replaceIfThenElseByReturn', + line: 11, + column: 13, + endLine: 13, + endColumn: 14, + }, + ], + }, ], });