Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Added unnecessary-else Rule #4502

Merged
merged 20 commits into from Mar 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5bd58d2
Added no-unnecessary-else Rule
debsmita1 Feb 4, 2019
021b97c
* Updated the branch with upstream/master
debsmita1 Feb 5, 2019
90e9d7a
Added 'unnecessary-else rule' under latest.ts instead of recommended.ts
debsmita1 Feb 8, 2019
ee45076
Changed year to 2019 under License
debsmita1 Feb 9, 2019
8d4f035
Changed name of the rule to 'unnecessary-else'
debsmita1 Feb 9, 2019
c2db576
Removed comments like valid from test.ts.lint file
debsmita1 Feb 9, 2019
a1ec00c
Modified the description, rationale and the FAILURE_STRING
debsmita1 Feb 9, 2019
b46d538
Added codeExamples
debsmita1 Feb 9, 2019
89c814e
Changed the Logic and Added new testcases
debsmita1 Feb 12, 2019
0f7d798
Merge branch 'master' of github.com:palantir/tslint into no-unnecessa…
debsmita1 Feb 16, 2019
2e0825f
Incorporated review comments and Added more testcases
debsmita1 Feb 24, 2019
7578c43
Revert "Incorporated review comments and Added more testcases"
debsmita1 Feb 24, 2019
b281653
Merge branch 'master' of github.com:palantir/tslint into no-unnecessa…
debsmita1 Feb 24, 2019
be2f8b4
Incorporated review comments and Added more Testcases
debsmita1 Feb 24, 2019
f36d420
Modified the logic and error message
debsmita1 Feb 24, 2019
a2842a9
Modified Code Examples
debsmita1 Feb 24, 2019
d7425c2
Fixed linting errors
debsmita1 Feb 24, 2019
717baef
Merge branch 'master' of github.com:palantir/tslint into no-unnecessa…
debsmita1 Feb 24, 2019
4c29b43
Optimized the rule logic
debsmita1 Feb 24, 2019
418c234
Merge branch 'master' of github.com:palantir/tslint into no-unnecessa…
debsmita1 Feb 28, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/configs/all.ts
Expand Up @@ -153,6 +153,7 @@ export const rules = {
"unnecessary-constructor": true,
"use-default-type-parameter": true,
"use-isnan": true,
"unnecessary-else": true,

// Maintainability

Expand Down
1 change: 1 addition & 0 deletions src/configs/latest.ts
Expand Up @@ -67,6 +67,7 @@ export const rules = {
// added in v5.12
"function-constructor": true,
"unnecessary-bind": true,
"unnecessary-else": true,
};
// tslint:enable object-literal-sort-keys

Expand Down
76 changes: 36 additions & 40 deletions src/configuration.ts
Expand Up @@ -155,42 +155,40 @@ export function findConfigurationPath(
throw new FatalError(
`Could not find config file at: ${path.resolve(suppliedConfigFilePath)}`,
);
} else {
return path.resolve(suppliedConfigFilePath);
}
} 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
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()) {
useDirName = true;
}
if (useDirName) {
inputFilePath = path.dirname(inputFilePath!);
}
} catch (e) {
// throws if file doesn't exist
useDirName = true;
}
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;
}

/**
Expand Down Expand Up @@ -246,16 +244,15 @@ 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. */
Expand All @@ -266,9 +263,8 @@ 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
Expand Down
8 changes: 4 additions & 4 deletions src/formatterLoader.ts
Expand Up @@ -29,7 +29,8 @@ export function findFormatter(
): FormatterConstructor | undefined {
if (typeof name === "function") {
return name;
} else if (typeof name === "string") {
}
if (typeof name === "string") {
name = name.trim();
const camelizedName = camelize(`${name}Formatter`);

Expand All @@ -49,10 +50,9 @@ 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(
Expand Down
9 changes: 3 additions & 6 deletions src/language/utils.ts
Expand Up @@ -79,9 +79,8 @@ 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` */
Expand All @@ -99,9 +98,8 @@ 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;
}
Expand Down Expand Up @@ -147,9 +145,8 @@ export function isAssignment(node: ts.Node) {
binaryExpression.operatorToken.kind >= ts.SyntaxKind.FirstAssignment &&
binaryExpression.operatorToken.kind <= ts.SyntaxKind.LastAssignment
);
} else {
return false;
}
return false;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/language/walker/ruleWalker.ts
Expand Up @@ -66,9 +66,8 @@ 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. */
Expand Down
6 changes: 2 additions & 4 deletions src/linter.ts
Expand Up @@ -283,9 +283,8 @@ 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);
Expand Down Expand Up @@ -323,9 +322,8 @@ export class Linter {
throw new FatalError(INVALID_SOURCE_ERROR);
}
return sourceFile;
} else {
return utils.getSourceFile(fileName, source);
}
return utils.getSourceFile(fileName, source);
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/rules/callableTypesRule.ts
Expand Up @@ -115,9 +115,8 @@ 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;
}
Expand Down
76 changes: 76 additions & 0 deletions src/rules/code-examples/unnecessaryElse.examples.ts
@@ -0,0 +1,76 @@
/**
* @license
* Copyright 2019 Palantir Technologies, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as Lint from "../../index";

// tslint:disable: object-literal-sort-keys
export const codeExamples = [
{
description:
'Disallows "else" following "if" blocks ending with "return", "break", "continue" or "throw" statement. ',
config: Lint.Utils.dedent`
"rules": { "unnecessary-else": true }
`,
pass: Lint.Utils.dedent`
if (someCondition()) {
return;
}
// some code here

if (someCondition()) {
continue;
}
// some code here

if (someCondition()) {
throw;
}
// some code here

if (someCondition()) {
break;
}
// some code here

`,
fail: Lint.Utils.dedent`
if (someCondition()) {
return;
} else {
// some code here
}

if (someCondition()) {
break;
} else {
// some code here
}

if (someCondition()) {
throw;
} else {
// some code here
}

if (someCondition()) {
continue;
} else {
// some code here
}
`,
},
];
11 changes: 5 additions & 6 deletions src/rules/curlyRule.ts
Expand Up @@ -172,12 +172,11 @@ 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}}`),
];
}
}
6 changes: 3 additions & 3 deletions src/rules/noBooleanLiteralCompareRule.ts
Expand Up @@ -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;
} else if (needsParenthesesForNegate(expression)) {
}
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 {
Expand Down
7 changes: 3 additions & 4 deletions src/rules/noDefaultImportRule.ts
Expand Up @@ -104,11 +104,10 @@ export class Rule extends Lint.Rules.AbstractRule {
fromModuleConfigOption[fromModulesConfigOptionName],
),
};
} else {
return {
[fromModulesConfigOptionName]: new RegExp("^\\./|^\\.\\./"),
};
}
return {
[fromModulesConfigOptionName]: new RegExp("^\\./|^\\.\\./"),
};
}
}

Expand Down
17 changes: 8 additions & 9 deletions src/rules/noInvalidThisRule.ts
Expand Up @@ -102,16 +102,15 @@ 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)) {
Expand Down
8 changes: 1 addition & 7 deletions src/rules/noRestrictedGlobalsRule.ts
Expand Up @@ -73,14 +73,8 @@ 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());
}
}

Expand Down