Disable unnecessary-else rule, revert parts of #4502 (#4575)

palantir · Mar 13, 2019 · 0796224 · 0796224
1 parent e080fc4
commit 0796224
Show file tree

Hide file tree

Showing 35 changed files with 235 additions and 221 deletions.
diff --git a/src/configs/all.ts b/src/configs/all.ts
@@ -154,7 +154,6 @@ export const rules = {
     "unnecessary-constructor": true,
     "use-default-type-parameter": true,
     "use-isnan": true,
-    "unnecessary-else": true,
 
     // Maintainability
 
@@ -266,6 +265,7 @@ export const rules = {
     "switch-final-break": true,
     "type-literal-delimiter": true,
     "unnecessary-bind": true,
+    "unnecessary-else": true,
     "variable-name": { options: ["ban-keywords", "check-format", "require-const-for-all-caps"] },
     whitespace: {
         options: [

diff --git a/src/configs/latest.ts b/src/configs/latest.ts
@@ -67,7 +67,6 @@ export const rules = {
     // added in v5.12
     "function-constructor": true,
     "unnecessary-bind": true,
-    "unnecessary-else": true,
 };
 // tslint:enable object-literal-sort-keys
 

diff --git a/src/configuration.ts b/src/configuration.ts
@@ -155,40 +155,42 @@ export function findConfigurationPath(
             throw new FatalError(
                 `Could not find config file at: ${path.resolve(suppliedConfigFilePath)}`,
             );
+        } else {
+            return path.resolve(suppliedConfigFilePath);
         }
-        return path.resolve(suppliedConfigFilePath);
-    }
-    // convert to dir if it's a file or doesn't exist
-    let useDirName = false;
-    try {
-        const stats = fs.statSync(inputFilePath!);
-        if (stats.isFile()) {
+    } else {
+        // convert to dir if it's a file or doesn't exist
+        let useDirName = false;
+        try {
+            const stats = fs.statSync(inputFilePath!);
+            if (stats.isFile()) {
+                useDirName = true;
+            }
+        } catch (e) {
+            // throws if file doesn't exist
             useDirName = true;
         }
-    } catch (e) {
-        // throws if file doesn't exist
-        useDirName = true;
-    }
-    if (useDirName) {
-        inputFilePath = path.dirname(inputFilePath!);
-    }
+        if (useDirName) {
+            inputFilePath = path.dirname(inputFilePath!);
+        }
 
-    // search for tslint.json from input file location
-    let configFilePath = findup(CONFIG_FILENAMES, path.resolve(inputFilePath!));
-    if (configFilePath !== undefined) {
-        return configFilePath;
-    }
+        // search for tslint.json from input file location
+        let configFilePath = findup(CONFIG_FILENAMES, path.resolve(inputFilePath!));
+        if (configFilePath !== undefined) {
+            return configFilePath;
+        }
 
-    // search for tslint.json in home directory
-    const homeDir = os.homedir();
-    for (const configFilename of CONFIG_FILENAMES) {
-        configFilePath = path.join(homeDir, configFilename);
-        if (fs.existsSync(configFilePath)) {
-            return path.resolve(configFilePath);
+        // search for tslint.json in home directory
+        const homeDir = os.homedir();
+        for (const configFilename of CONFIG_FILENAMES) {
+            configFilePath = path.join(homeDir, configFilename);
+            if (fs.existsSync(configFilePath)) {
+                return path.resolve(configFilePath);
+            }
         }
+        // no path could be found
+        return undefined;
     }
-    // no path could be found
-    return undefined;
 }
 
 /**
@@ -244,15 +246,16 @@ function findup(filenames: string[], directory: string): string | undefined {
 export function loadConfigurationFromPath(configFilePath?: string, _originalFilePath?: string) {
     if (configFilePath == undefined) {
         return DEFAULT_CONFIG;
+    } else {
+        const resolvedConfigFilePath = resolveConfigurationPath(configFilePath);
+        const rawConfigFile = readConfigurationFile(resolvedConfigFilePath);
+
+        return parseConfigFile(
+            rawConfigFile,
+            path.dirname(resolvedConfigFilePath),
+            readConfigurationFile,
+        );
     }
-    const resolvedConfigFilePath = resolveConfigurationPath(configFilePath);
-    const rawConfigFile = readConfigurationFile(resolvedConfigFilePath);
-
-    return parseConfigFile(
-        rawConfigFile,
-        path.dirname(resolvedConfigFilePath),
-        readConfigurationFile,
-    );
 }
 
 /** Reads the configuration file from disk and parses it as raw JSON, YAML or JS depending on the extension. */
@@ -263,8 +266,9 @@ export function readConfigurationFile(filepath: string): RawConfigFile {
         try {
             if (resolvedConfigFileExt === ".json") {
                 return JSON.parse(stripComments(fileContent)) as RawConfigFile;
+            } else {
+                return yaml.safeLoad(fileContent) as RawConfigFile;
             }
-            return yaml.safeLoad(fileContent) as RawConfigFile;
         } catch (e) {
             const error = e as Error;
             // include the configuration file being parsed in the error since it may differ from the directly referenced config

diff --git a/src/formatterLoader.ts b/src/formatterLoader.ts
@@ -29,8 +29,7 @@ export function findFormatter(
 ): FormatterConstructor | undefined {
     if (typeof name === "function") {
         return name;
-    }
-    if (typeof name === "string") {
+    } else if (typeof name === "string") {
         name = name.trim();
         const camelizedName = camelize(`${name}Formatter`);
 
@@ -50,9 +49,10 @@ export function findFormatter(
 
         // else try to resolve as module
         return loadFormatterModule(name);
+    } else {
+        // If an something else is passed as a name (e.g. object)
+        throw new Error(`Name of type ${typeof name} is not supported.`);
     }
-    // If an something else is passed as a name (e.g. object)
-    throw new Error(`Name of type ${typeof name} is not supported.`);
 }
 
 function loadFormatter(

diff --git a/src/language/utils.ts b/src/language/utils.ts
@@ -79,8 +79,9 @@ export function isBlockScopedVariable(
             parent.kind === ts.SyntaxKind.CatchClause ||
             isBlockScopedVariableDeclarationList(parent)
         );
+    } else {
+        return isBlockScopedVariableDeclarationList(node.declarationList);
     }
-    return isBlockScopedVariableDeclarationList(node.declarationList);
 }
 
 /** @deprecated use `isBlockScopedVariableDeclarationList` and `getDeclarationOfBindingElement` from `tsutils` */
@@ -98,8 +99,9 @@ export function getBindingElementVariableDeclaration(
     while (currentParent.kind !== ts.SyntaxKind.VariableDeclaration) {
         if (currentParent.parent === undefined) {
             return null; // function parameter, no variable declaration
+        } else {
+            currentParent = currentParent.parent;
         }
-        currentParent = currentParent.parent;
     }
     return currentParent as ts.VariableDeclaration;
 }
@@ -145,8 +147,9 @@ export function isAssignment(node: ts.Node) {
             binaryExpression.operatorToken.kind >= ts.SyntaxKind.FirstAssignment &&
             binaryExpression.operatorToken.kind <= ts.SyntaxKind.LastAssignment
         );
+    } else {
+        return false;
     }
-    return false;
 }
 
 /**

diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts
@@ -66,8 +66,9 @@ export class RuleWalker extends SyntaxWalker implements IWalker {
     public hasOption(option: string): boolean {
         if (this.options !== undefined) {
             return this.options.indexOf(option) !== -1;
+        } else {
+            return false;
         }
-        return false;
     }
 
     /** @deprecated Prefer `addFailureAt` and its variants. */

diff --git a/src/linter.ts b/src/linter.ts
@@ -283,8 +283,9 @@ export class Linter {
         try {
             if (this.program !== undefined && isTypedRule(rule)) {
                 return rule.applyWithProgram(sourceFile, this.program);
+            } else {
+                return rule.apply(sourceFile);
             }
-            return rule.apply(sourceFile);
         } catch (error) {
             if (isError(error) && error.stack !== undefined) {
                 showRuleCrashWarning(error.stack, rule.getOptions().ruleName, sourceFile.fileName);
@@ -322,8 +323,9 @@ export class Linter {
                 throw new FatalError(INVALID_SOURCE_ERROR);
             }
             return sourceFile;
+        } else {
+            return utils.getSourceFile(fileName, source);
         }
-        return utils.getSourceFile(fileName, source);
     }
 }
 

diff --git a/src/rules/callableTypesRule.ts b/src/rules/callableTypesRule.ts
@@ -115,8 +115,9 @@ function renderSuggestion(
                 parent.name.pos,
                 parent.typeParameters.end + 1,
             )} = ${suggestion}`;
+        } else {
+            return `type ${parent.name.text} = ${suggestion}`;
         }
-        return `type ${parent.name.text} = ${suggestion}`;
     }
     return suggestion.endsWith(";") ? suggestion.slice(0, -1) : suggestion;
 }

diff --git a/src/rules/curlyRule.ts b/src/rules/curlyRule.ts
@@ -172,11 +172,12 @@ class CurlyWalker extends Lint.AbstractWalker<Options> {
                 Lint.Replacement.appendText(statement.pos, " {"),
                 Lint.Replacement.appendText(statement.end, " }"),
             ];
+        } else {
+            const newLine = newLineWithIndentation(node, this.sourceFile);
+            return [
+                Lint.Replacement.appendText(statement.pos, " {"),
+                Lint.Replacement.appendText(statement.end, `${newLine}}`),
+            ];
         }
-        const newLine = newLineWithIndentation(node, this.sourceFile);
-        return [
-            Lint.Replacement.appendText(statement.pos, " {"),
-            Lint.Replacement.appendText(statement.end, `${newLine}}`),
-        ];
     }
 }
diff --git a/src/rules/noBooleanLiteralCompareRule.ts b/src/rules/noBooleanLiteralCompareRule.ts
@@ -89,15 +89,15 @@ function fix(node: ts.BinaryExpression, { negate, expression }: Compare): Lint.F
             : Lint.Replacement.deleteFromTo(node.getStart(), node.right.getStart());
     if (!negate) {
         return deleted;
-    }
-    if (needsParenthesesForNegate(expression)) {
+    } else if (needsParenthesesForNegate(expression)) {
         return [
             deleted,
             Lint.Replacement.appendText(node.getStart(), "!("),
             Lint.Replacement.appendText(node.getEnd(), ")"),
         ];
+    } else {
+        return [deleted, Lint.Replacement.appendText(node.getStart(), "!")];
     }
-    return [deleted, Lint.Replacement.appendText(node.getStart(), "!")];
 }
 
 function needsParenthesesForNegate(node: ts.Expression): boolean {

diff --git a/src/rules/noDefaultImportRule.ts b/src/rules/noDefaultImportRule.ts
@@ -104,10 +104,11 @@ export class Rule extends Lint.Rules.AbstractRule {
                     fromModuleConfigOption[fromModulesConfigOptionName],
                 ),
             };
+        } else {
+            return {
+                [fromModulesConfigOptionName]: new RegExp("^\\./|^\\.\\./"),
+            };
         }
-        return {
-            [fromModulesConfigOptionName]: new RegExp("^\\./|^\\.\\./"),
-        };
     }
 }
 

diff --git a/src/rules/noInvalidThisRule.ts b/src/rules/noInvalidThisRule.ts
@@ -102,15 +102,16 @@ function walk(ctx: Lint.WalkContext<boolean>): void {
                     ts.forEachChild(node, cb);
                     currentParent = originalParent;
                     return;
+                } else {
+                    currentParent = (node as ts.FunctionLikeDeclaration).parameters.some(
+                        isThisParameter,
+                    )
+                        ? ParentType.BoundRegularFunction
+                        : ParentType.UnboundRegularFunction;
+                    ts.forEachChild(node, cb);
+                    currentParent = originalParent;
+                    return;
                 }
-                currentParent = (node as ts.FunctionLikeDeclaration).parameters.some(
-                    isThisParameter,
-                )
-                    ? ParentType.BoundRegularFunction
-                    : ParentType.UnboundRegularFunction;
-                ts.forEachChild(node, cb);
-                currentParent = originalParent;
-                return;
 
             case ts.SyntaxKind.ThisKeyword:
                 if (!thisAllowedParents.has(currentParent)) {

diff --git a/src/rules/noRestrictedGlobalsRule.ts b/src/rules/noRestrictedGlobalsRule.ts
@@ -73,8 +73,14 @@ export class Rule extends Lint.Rules.TypedRule {
         const bannedGlobals = new Set(bannedList);
         if (sourceFile.isDeclarationFile) {
             return [];
+        } else {
+            return this.applyWithFunction(
+                sourceFile,
+                walk,
+                bannedGlobals,
+                program.getTypeChecker(),
+            );
         }
-        return this.applyWithFunction(sourceFile, walk, bannedGlobals, program.getTypeChecker());
     }
 }
 

diff --git a/src/rules/noSparseArraysRule.ts b/src/rules/noSparseArraysRule.ts
@@ -54,8 +54,9 @@ function walk(ctx: Lint.WalkContext<void>): void {
                 // Ignore LHS of assignments.
                 traverseExpressionsInLHS(node.left, cb);
                 return cb(node.right);
+            } else {
+                return ts.forEachChild(node, cb);
             }
-            return ts.forEachChild(node, cb);
         }
 
         for (const element of node.elements) {

diff --git a/src/rules/noUnnecessaryQualifierRule.ts b/src/rules/noUnnecessaryQualifierRule.ts
@@ -122,8 +122,7 @@ function walk(ctx: Lint.WalkContext<void>, checker: ts.TypeChecker): void {
         const symbolDeclarations = symbol.getDeclarations();
         if (symbolDeclarations === undefined) {
             return false;
-        }
-        if (symbolDeclarations.some(decl => namespacesInScope.some(ns => ns === decl))) {
+        } else if (symbolDeclarations.some(decl => namespacesInScope.some(ns => ns === decl))) {
             return true;
         }
 

diff --git a/src/rules/noUnusedExpressionRule.ts b/src/rules/noUnusedExpressionRule.ts
@@ -91,12 +91,10 @@ function walk(ctx: Lint.WalkContext<Options>) {
         if (checking) {
             if (isParenthesizedExpression(node) || isVoidExpression(node)) {
                 return cb(node.expression);
-            }
-            if (isConditionalExpression(node)) {
+            } else if (isConditionalExpression(node)) {
                 noCheck(node.condition, cb);
                 return both(node.whenTrue, node.whenFalse);
-            }
-            if (isBinaryExpression(node)) {
+            } else if (isBinaryExpression(node)) {
                 switch (node.operatorToken.kind) {
                     case ts.SyntaxKind.CommaToken:
                         if (isIndirectEval(node)) {
@@ -121,8 +119,7 @@ function walk(ctx: Lint.WalkContext<Options>) {
             }
             allowFastNullChecks = true;
             return false;
-        }
-        if (isVoidExpression(node)) {
+        } else if (isVoidExpression(node)) {
             // allow `void 0` and `void(0)`
             if (
                 !isLiteralZero(
@@ -134,8 +131,7 @@ function walk(ctx: Lint.WalkContext<Options>) {
                 check(node.expression);
             }
             return false;
-        }
-        if (
+        } else if (
             isBinaryExpression(node) &&
             node.operatorToken.kind === ts.SyntaxKind.CommaToken &&
             !isIndirectEval(node)
@@ -171,8 +167,9 @@ function walk(ctx: Lint.WalkContext<Options>) {
         if (cb(one)) {
             if (cb(two)) {
                 return true;
+            } else {
+                ctx.addFailureAtNode(one, Rule.FAILURE_STRING);
             }
-            ctx.addFailureAtNode(one, Rule.FAILURE_STRING);
         } else if (cb(two)) {
             ctx.addFailureAtNode(two, Rule.FAILURE_STRING);
         }