From 1779f84830dfc0b8c9c12834c0795359524a7ab0 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Sun, 28 May 2017 19:16:24 -0700 Subject: [PATCH 1/3] curly: Add "never" option --- src/rules/curlyRule.ts | 28 +++++++++++++++++++++++++--- test/rules/curly/never/test.ts.lint | 24 ++++++++++++++++++++++++ test/rules/curly/never/tslint.json | 5 +++++ 3 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 test/rules/curly/never/test.ts.lint create mode 100644 test/rules/curly/never/tslint.json diff --git a/src/rules/curlyRule.ts b/src/rules/curlyRule.ts index 399d7a9c076..aabe3ea58a7 100644 --- a/src/rules/curlyRule.ts +++ b/src/rules/curlyRule.ts @@ -15,11 +15,12 @@ * limitations under the License. */ -import { isIfStatement, isIterationStatement, isSameLine } from "tsutils"; +import { isBlock, isIfStatement, isIterationStatement, isSameLine } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; +const OPTION_NEVER = "never"; const OPTION_IGNORE_SAME_LINE = "ignore-same-line"; interface Options { @@ -42,8 +43,9 @@ export class Rule extends Lint.Rules.AbstractRule { to be executed only if \`foo === bar\`. However, he forgot braces and \`bar++\` will be executed no matter what. This rule could prevent such a mistake.`, optionsDescription: Lint.Utils.dedent` - The rule may be set to \`true\`, or to the following: + One of the following options may be provided: + * \`"${OPTION_NEVER}"\` forbids any unnecessary curly braces. * \`"${OPTION_IGNORE_SAME_LINE}"\` skips checking braces for control-flow statements that are on one line and start on the same line as their control-flow keyword `, @@ -52,27 +54,47 @@ export class Rule extends Lint.Rules.AbstractRule { items: { type: "string", enum: [ + OPTION_NEVER, OPTION_IGNORE_SAME_LINE, ], }, }, - optionExamples: [true, [true, "ignore-same-line"]], + optionExamples: [ + true, + [true, "ignore-same-line"], + [true, "never"], + ], type: "functionality", typescriptOnly: false, }; /* tslint:enable:object-literal-sort-keys */ + public static FAILURE_STRING_NEVER = "Block contains only one statement; remove the curly braces."; public static FAILURE_STRING_FACTORY(kind: string) { return `${kind} statements must be braced`; } public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + if (this.ruleArguments.indexOf(OPTION_NEVER) !== -1) { + return this.applyWithFunction(sourceFile, walkNever); + } + return this.applyWithWalker(new CurlyWalker(sourceFile, this.ruleName, { ignoreSameLine: this.ruleArguments.indexOf(OPTION_IGNORE_SAME_LINE) !== -1, })); } } +function walkNever(ctx: Lint.WalkContext): void { + ts.forEachChild(ctx.sourceFile, function cb(node) { + if (isBlock(node) && node.statements.length === 1 + && (isIterationStatement(node.parent!) || isIfStatement(node.parent!))) { + ctx.addFailureAtNode(Lint.childOfKind(node, ts.SyntaxKind.OpenBraceToken)!, Rule.FAILURE_STRING_NEVER); + } + ts.forEachChild(node, cb); + }); +} + class CurlyWalker extends Lint.AbstractWalker { public walk(sourceFile: ts.SourceFile) { const cb = (node: ts.Node): void => { diff --git a/test/rules/curly/never/test.ts.lint b/test/rules/curly/never/test.ts.lint new file mode 100644 index 00000000000..61aefe53769 --- /dev/null +++ b/test/rules/curly/never/test.ts.lint @@ -0,0 +1,24 @@ +if (so) { + ~ [0] + foo(); +} else { + ~ [0] + foo(); +} + +while (true) { + ~ [0] + foo(); +} + +// Some blocks are necessary. + +function f() { + foo(); +} + +() => { foo(); }; + +try { foo(); } catch (e) { foo(); } finally { foo(); } + +[0]: Block contains only one statement; remove the curly braces. diff --git a/test/rules/curly/never/tslint.json b/test/rules/curly/never/tslint.json new file mode 100644 index 00000000000..9f0eded4c57 --- /dev/null +++ b/test/rules/curly/never/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "curly": [true, "never"] + } +} From 77d80c206866b3ea3a2b9eb6ccb8f73afae94325 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 29 May 2017 07:25:07 -0700 Subject: [PATCH 2/3] Handle special case --- src/rules/curlyRule.ts | 22 ++++++++++++++++++++-- test/rules/curly/never/test.ts.lint | 29 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/rules/curlyRule.ts b/src/rules/curlyRule.ts index aabe3ea58a7..2d71840d4ca 100644 --- a/src/rules/curlyRule.ts +++ b/src/rules/curlyRule.ts @@ -87,14 +87,32 @@ export class Rule extends Lint.Rules.AbstractRule { function walkNever(ctx: Lint.WalkContext): void { ts.forEachChild(ctx.sourceFile, function cb(node) { - if (isBlock(node) && node.statements.length === 1 - && (isIterationStatement(node.parent!) || isIfStatement(node.parent!))) { + if (isBlock(node) && isBlockUnnecessary(node)) { ctx.addFailureAtNode(Lint.childOfKind(node, ts.SyntaxKind.OpenBraceToken)!, Rule.FAILURE_STRING_NEVER); } ts.forEachChild(node, cb); }); } +function isBlockUnnecessary(node: ts.Block): boolean { + const parent = node.parent!; + if (node.statements.length !== 1) { return false; } + const statement = node.statements[0]; + if (isIterationStatement(parent)) { return true; } + /* + Watch out for this case: + if (so) { + if (also) + foo(); + } else + bar(); + */ + return isIfStatement(parent) && !(isIfStatement(statement) + && statement.elseStatement === undefined + && parent.thenStatement === node + && parent.elseStatement !== undefined); +} + class CurlyWalker extends Lint.AbstractWalker { public walk(sourceFile: ts.SourceFile) { const cb = (node: ts.Node): void => { diff --git a/test/rules/curly/never/test.ts.lint b/test/rules/curly/never/test.ts.lint index 61aefe53769..448d9151fa8 100644 --- a/test/rules/curly/never/test.ts.lint +++ b/test/rules/curly/never/test.ts.lint @@ -11,8 +11,37 @@ while (true) { foo(); } +if (so) { + ~ [0] + if (also) + foo(); +} + +if (so) { + ~ [0] + if (also) + foo(); + else + foo(); +} else + foo(); + +if (so) + bar(); +else { + ~ [0] + if (also) + foo(); +} + // Some blocks are necessary. +if (so) { + if (also) + foo(); +} else + bar(); + function f() { foo(); } From eaa78dd6b763b121f1fb5a9ab49126156aaa1432 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 30 May 2017 18:11:17 -0700 Subject: [PATCH 3/3] Rename to "as-needed" --- src/rules/curlyRule.ts | 20 +++++++++---------- .../curly/{never => as-needed}/test.ts.lint | 0 test/rules/curly/as-needed/tslint.json | 5 +++++ test/rules/curly/never/tslint.json | 5 ----- 4 files changed, 15 insertions(+), 15 deletions(-) rename test/rules/curly/{never => as-needed}/test.ts.lint (100%) create mode 100644 test/rules/curly/as-needed/tslint.json delete mode 100644 test/rules/curly/never/tslint.json diff --git a/src/rules/curlyRule.ts b/src/rules/curlyRule.ts index 2d71840d4ca..d5f1cb16720 100644 --- a/src/rules/curlyRule.ts +++ b/src/rules/curlyRule.ts @@ -20,7 +20,7 @@ import * as ts from "typescript"; import * as Lint from "../index"; -const OPTION_NEVER = "never"; +const OPTION_AS_NEEDED = "as-needed"; const OPTION_IGNORE_SAME_LINE = "ignore-same-line"; interface Options { @@ -45,7 +45,7 @@ export class Rule extends Lint.Rules.AbstractRule { optionsDescription: Lint.Utils.dedent` One of the following options may be provided: - * \`"${OPTION_NEVER}"\` forbids any unnecessary curly braces. + * \`"${OPTION_AS_NEEDED}"\` forbids any unnecessary curly braces. * \`"${OPTION_IGNORE_SAME_LINE}"\` skips checking braces for control-flow statements that are on one line and start on the same line as their control-flow keyword `, @@ -54,29 +54,29 @@ export class Rule extends Lint.Rules.AbstractRule { items: { type: "string", enum: [ - OPTION_NEVER, + OPTION_AS_NEEDED, OPTION_IGNORE_SAME_LINE, ], }, }, optionExamples: [ true, - [true, "ignore-same-line"], - [true, "never"], + [true, OPTION_IGNORE_SAME_LINE], + [true, OPTION_AS_NEEDED], ], type: "functionality", typescriptOnly: false, }; /* tslint:enable:object-literal-sort-keys */ - public static FAILURE_STRING_NEVER = "Block contains only one statement; remove the curly braces."; + public static FAILURE_STRING_AS_NEEDED = "Block contains only one statement; remove the curly braces."; public static FAILURE_STRING_FACTORY(kind: string) { return `${kind} statements must be braced`; } public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - if (this.ruleArguments.indexOf(OPTION_NEVER) !== -1) { - return this.applyWithFunction(sourceFile, walkNever); + if (this.ruleArguments.indexOf(OPTION_AS_NEEDED) !== -1) { + return this.applyWithFunction(sourceFile, walkAsNeeded); } return this.applyWithWalker(new CurlyWalker(sourceFile, this.ruleName, { @@ -85,10 +85,10 @@ export class Rule extends Lint.Rules.AbstractRule { } } -function walkNever(ctx: Lint.WalkContext): void { +function walkAsNeeded(ctx: Lint.WalkContext): void { ts.forEachChild(ctx.sourceFile, function cb(node) { if (isBlock(node) && isBlockUnnecessary(node)) { - ctx.addFailureAtNode(Lint.childOfKind(node, ts.SyntaxKind.OpenBraceToken)!, Rule.FAILURE_STRING_NEVER); + ctx.addFailureAtNode(Lint.childOfKind(node, ts.SyntaxKind.OpenBraceToken)!, Rule.FAILURE_STRING_AS_NEEDED); } ts.forEachChild(node, cb); }); diff --git a/test/rules/curly/never/test.ts.lint b/test/rules/curly/as-needed/test.ts.lint similarity index 100% rename from test/rules/curly/never/test.ts.lint rename to test/rules/curly/as-needed/test.ts.lint diff --git a/test/rules/curly/as-needed/tslint.json b/test/rules/curly/as-needed/tslint.json new file mode 100644 index 00000000000..81fdd50f632 --- /dev/null +++ b/test/rules/curly/as-needed/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "curly": [true, "as-needed"] + } +} diff --git a/test/rules/curly/never/tslint.json b/test/rules/curly/never/tslint.json deleted file mode 100644 index 9f0eded4c57..00000000000 --- a/test/rules/curly/never/tslint.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "rules": { - "curly": [true, "never"] - } -}