From ce4dbfee542c286dbf6b09b65927c58ad0830480 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Sat, 20 May 2017 14:02:03 -0700 Subject: [PATCH] type-literal-delimiter: Prefer ';' instead of ',', and lint for trailing delimiter (#2787) --- src/configuration.ts | 6 +-- src/enableDisableRules.ts | 2 +- src/rules/adjacentOverloadSignaturesRule.ts | 2 +- src/rules/memberOrderingRule.ts | 6 +-- src/rules/preferForOfRule.ts | 2 +- src/rules/typeLiteralDelimiterRule.ts | 38 ++++++++++++------- src/rules/unifiedSignaturesRule.ts | 6 +-- src/test.ts | 2 +- src/test/parse.ts | 2 +- .../rules/type-literal-delimiter/test.ts.lint | 38 +++++++++---------- 10 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index 6d66d4602b8..df2f5a91608 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -44,7 +44,7 @@ export interface IConfigurationFile { * Other linter options, currently for testing. Not publicly supported. */ linterOptions?: { - typeCheck?: boolean, + typeCheck?: boolean; }; /** @@ -447,8 +447,8 @@ export interface RawRulesConfig { [key: string]: RawRuleConfig; } export type RawRuleConfig = null | undefined | boolean | any[] | { - severity?: RuleSeverity | "warn" | "none" | "default", - options?: any, + severity?: RuleSeverity | "warn" | "none" | "default"; + options?: any; }; /** diff --git a/src/enableDisableRules.ts b/src/enableDisableRules.ts index d72f6816d50..fd653840fa8 100644 --- a/src/enableDisableRules.ts +++ b/src/enableDisableRules.ts @@ -124,7 +124,7 @@ function getSwitchRange(modifier: Modifier, range: ts.TextRange, sourceFile: ts. } type Modifier = "line" | "next-line" | undefined; -function parseComment(commentText: string): { rulesList: string[] | "all", isEnabled: boolean, modifier: Modifier } | undefined { +function parseComment(commentText: string): { rulesList: string[] | "all"; isEnabled: boolean; modifier: Modifier } | undefined { // regex is: start of string followed by any amount of whitespace // followed by tslint and colon // followed by either "enable" or "disable" diff --git a/src/rules/adjacentOverloadSignaturesRule.ts b/src/rules/adjacentOverloadSignaturesRule.ts index 4b573f80212..83c04d9fc9f 100644 --- a/src/rules/adjacentOverloadSignaturesRule.ts +++ b/src/rules/adjacentOverloadSignaturesRule.ts @@ -117,7 +117,7 @@ export function getOverloadKey(node: ts.SignatureDeclaration): string | undefine return (computed ? "0" : "1") + (isStatic ? "0" : "1") + name; } -function getOverloadInfo(node: ts.SignatureDeclaration): string | { name: string, computed?: boolean } | undefined { +function getOverloadInfo(node: ts.SignatureDeclaration): string | { name: string; computed?: boolean } | undefined { switch (node.kind) { case ts.SyntaxKind.ConstructSignature: case ts.SyntaxKind.Constructor: diff --git a/src/rules/memberOrderingRule.ts b/src/rules/memberOrderingRule.ts index c30cc726ed2..e64b3ac2c20 100644 --- a/src/rules/memberOrderingRule.ts +++ b/src/rules/memberOrderingRule.ts @@ -353,7 +353,7 @@ function getMemberKind(member: Member): MemberKind | undefined { } } -type MemberCategoryJson = { name: string, kinds: string[] } | string; +type MemberCategoryJson = { name: string; kinds: string[] } | string; class MemberCategory { constructor(readonly name: string, private readonly kinds: Set) {} public has(kind: MemberKind) { return this.kinds.has(kind); } @@ -376,12 +376,12 @@ function parseOptions(options: any[]): Options { : new MemberCategory(cat.name, new Set(flatMap(cat.kinds, memberKindFromName)))); return { order, alphabetize }; } -function getOptionsJson(allOptions: any[]): { order: MemberCategoryJson[], alphabetize: boolean } { +function getOptionsJson(allOptions: any[]): { order: MemberCategoryJson[]; alphabetize: boolean } { if (allOptions == null || allOptions.length === 0 || allOptions[0] == null) { throw new Error("Got empty options"); } - const firstOption = allOptions[0] as { order: MemberCategoryJson[] | string, alphabetize?: boolean } | string; + const firstOption = allOptions[0] as { order: MemberCategoryJson[] | string; alphabetize?: boolean } | string; if (typeof firstOption !== "object") { // Undocumented direct string option. Deprecate eventually. return { order: convertFromOldStyleOptions(allOptions), alphabetize: false }; // presume allOptions to be string[] diff --git a/src/rules/preferForOfRule.ts b/src/rules/preferForOfRule.ts index 028c9b41383..50f5ed75160 100644 --- a/src/rules/preferForOfRule.ts +++ b/src/rules/preferForOfRule.ts @@ -120,7 +120,7 @@ function nodeEquals(a: ts.Node, b: ts.Node, sourceFile: ts.SourceFile): boolean } // returns the iterator and array of a `for` loop if the `for` loop is basic. -function getForLoopHeaderInfo(forLoop: ts.ForStatement): { indexVariable: ts.Identifier, arrayExpr: ts.Expression } | undefined { +function getForLoopHeaderInfo(forLoop: ts.ForStatement): { indexVariable: ts.Identifier; arrayExpr: ts.Expression } | undefined { const { initializer, condition, incrementor } = forLoop; if (initializer === undefined || condition === undefined || incrementor === undefined) { return undefined; diff --git a/src/rules/typeLiteralDelimiterRule.ts b/src/rules/typeLiteralDelimiterRule.ts index 996e539f0fe..245f6c78ccb 100644 --- a/src/rules/typeLiteralDelimiterRule.ts +++ b/src/rules/typeLiteralDelimiterRule.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { isTypeLiteralNode } from "tsutils"; +import { isSameLine, isTypeLiteralNode } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; @@ -25,8 +25,8 @@ export class Rule extends Lint.Rules.AbstractRule { public static metadata: Lint.IRuleMetadata = { ruleName: "type-literal-delimiter", description: Lint.Utils.dedent` - Checks that type literal members are separated by commas. - Does not check the last member, as that is done by 'trailing-comma'.`, + Checks that type literal members are separated by semicolons. + Enforces a trailing semicolon for multiline type literals.`, optionsDescription: "Not configurable.", options: null, optionExamples: [true], @@ -36,9 +36,11 @@ export class Rule extends Lint.Rules.AbstractRule { /* tslint:enable:object-literal-sort-keys */ public static FAILURE_STRING_MISSING = - "Expected type literal to use ',' to separate members."; - public static FAILURE_STRING_SEMICOLON = - "Expected type literal to use ',' instead of ';'."; + "Expected type literal to use ';' to separate members."; + public static FAILURE_STRING_COMMA = + "Expected type literal to use ';' instead of ','."; + public static FAILURE_STRING_TRAILING = + "Did not expect single-line type literal to have a trailing ';'."; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { return this.applyWithFunction(sourceFile, walk); @@ -54,23 +56,31 @@ function walk(ctx: Lint.WalkContext): void { ts.forEachChild(node, cb); }); - function check({ members }: ts.TypeLiteralNode): void { - members.forEach((member, idx) => { + function check(node: ts.TypeLiteralNode): void { + node.members.forEach((member, idx) => { const end = member.end - 1; + // Trailing delimiter should be ommitted for a single-line type literal. + const shouldOmit = idx === node.members.length - 1 && isSameLine(sourceFile, node.getStart(sourceFile), node.getEnd()); const delimiter = sourceFile.text[end]; switch (delimiter) { - case ",": - break; case ";": - ctx.addFailureAt(end, 1, Rule.FAILURE_STRING_SEMICOLON); + if (shouldOmit) { + fail(Rule.FAILURE_STRING_TRAILING); + } + break; + case ",": + fail(Rule.FAILURE_STRING_COMMA); break; default: - // Allow missing trailing ',' - if (idx !== members.length - 1) { - ctx.addFailureAt(end, 1, Rule.FAILURE_STRING_MISSING); + if (!shouldOmit) { + fail(Rule.FAILURE_STRING_MISSING); } break; } + + function fail(failure: string): void { + ctx.addFailureAt(end, 1, failure); + } }); } } diff --git a/src/rules/unifiedSignaturesRule.ts b/src/rules/unifiedSignaturesRule.ts index c4fed4fea0f..be46d3625be 100644 --- a/src/rules/unifiedSignaturesRule.ts +++ b/src/rules/unifiedSignaturesRule.ts @@ -147,8 +147,8 @@ interface Failure { only2: boolean; } type Unify = - | { kind: "single-parameter-difference", p0: ts.ParameterDeclaration, p1: ts.ParameterDeclaration } - | { kind: "extra-parameter", extraParameter: ts.ParameterDeclaration, otherSignature: ts.NodeArray }; + | { kind: "single-parameter-difference"; p0: ts.ParameterDeclaration; p1: ts.ParameterDeclaration } + | { kind: "extra-parameter"; extraParameter: ts.ParameterDeclaration; otherSignature: ts.NodeArray }; function checkOverloads( signatures: T[], @@ -247,7 +247,7 @@ function signaturesDifferByOptionalOrRestParameter(sig1: ts.NodeArray = (node: T) => { signature: ts.SignatureDeclaration, key: string } | undefined; +type GetOverload = (node: T) => { signature: ts.SignatureDeclaration; key: string } | undefined; /** * Returns true if typeName is the name of an *outer* type parameter. diff --git a/src/test.ts b/src/test.ts index 2eedbcebd5c..cae803933cf 100644 --- a/src/test.ts +++ b/src/test.ts @@ -50,7 +50,7 @@ export interface SkippedTest { export interface TestResult { directory: string; results: { - [fileName: string]: TestOutput | SkippedTest, + [fileName: string]: TestOutput | SkippedTest; }; } diff --git a/src/test/parse.ts b/src/test/parse.ts index cb40fa91125..b302fb08477 100644 --- a/src/test/parse.ts +++ b/src/test/parse.ts @@ -74,7 +74,7 @@ export function parseErrorsFromMarkup(text: string): LintError[] { const errorLinesForCodeLines = createCodeLineNoToErrorsMap(lines); const lintErrors: LintError[] = []; - function addError(errorLine: EndErrorLine, errorStartPos: { line: number, col: number }, lineNo: number) { + function addError(errorLine: EndErrorLine, errorStartPos: { line: number; col: number }, lineNo: number) { lintErrors.push({ startPos: errorStartPos, endPos: { line: lineNo, col: errorLine.endCol }, diff --git a/test/rules/type-literal-delimiter/test.ts.lint b/test/rules/type-literal-delimiter/test.ts.lint index 43ae2b8c829..d5b99e77592 100644 --- a/test/rules/type-literal-delimiter/test.ts.lint +++ b/test/rules/type-literal-delimiter/test.ts.lint @@ -1,32 +1,30 @@ -type T = { - x: number - ~ [MISSING] - y: string -} - type T = { x: number, - // Doesn't care about missing final comma; trailing-comma will handle. + ~ [COMMA] y: string -} + ~ [MISSING] +}; -// Does care about semicolon. type T = { - x: number, - y: string; - ~ [SEMI] -} + x: number + ~ [MISSING] + y: string, + ~ [COMMA] +}; type T = { x: number; - ~ [SEMI] - y: string -} + y: string; +}; + +type T = { x: number; y: string }; // Works even when there's extra whitespace -type T = { x: number , y: number }; type T = { x: number ; y: number }; - ~ [SEMI] +type T = { x: number , y: number ; }; + ~ [COMMA] + ~ [EXTRA] -[MISSING]: Expected type literal to use ',' to separate members. -[SEMI]: Expected type literal to use ',' instead of ';'. +[MISSING]: Expected type literal to use ';' to separate members. +[COMMA]: Expected type literal to use ';' instead of ','. +[EXTRA]: Did not expect single-line type literal to have a trailing ';'.