diff --git a/src/rules/terNewlineAfterVarRule.ts b/src/rules/terNewlineAfterVarRule.ts index d1a445d..e5fdf0d 100644 --- a/src/rules/terNewlineAfterVarRule.ts +++ b/src/rules/terNewlineAfterVarRule.ts @@ -2,13 +2,41 @@ import * as ts from 'typescript'; import * as Lint from 'tslint'; const RULE_NAME: string = 'ter-newline-after-var'; +const EXPECTED_BLANK_LINE_MESSAGE: string = 'Expected blank line after variable declarations.'; +const UNEXPECTED_BLANK_LINE_MESSAGE: string = 'Unexpected blank line after variable declarations.'; +const enum SymbolCodes { + TAB = 9, + NEWLINE = 10, + CARRIAGE_RETURN = 13, + WHITESPACE = 32 +} +const enum Symbols { + NEWLINE = '\n' +} interface ITerNewlineAfterVarOptions { always: boolean; } -const EXPECTED_BLANK_LINE_MESSAGE: string = 'Expected blank line after variable declarations.'; -const UNEXPECTED_BLANK_LINE_MESSAGE: string = 'Unexpected blank line after variable declarations.'; +function formatOptions ([alwaysOrNever]: string[]): ITerNewlineAfterVarOptions { + return { + always: alwaysOrNever !== 'never' + }; +} + +function getNextSiblingNode (node: ts.Node): ts.Node|void { + let breakOnSibling: boolean = false; + + return ts.forEachChild(node.parent, (sibling: ts.Node): ts.Node|void => { + if (breakOnSibling) { + + // stop looking for a next node + return sibling; + } + + breakOnSibling = sibling === node; + }); +} export class Rule extends Lint.Rules.AbstractRule { public static metadata: Lint.IRuleMetadata = { @@ -48,101 +76,108 @@ export class Rule extends Lint.Rules.AbstractRule { type: 'style' }; - private formatOptions ([alwaysOrNever]: string[]): ITerNewlineAfterVarOptions { - return { - always: alwaysOrNever !== 'never' - }; - } - public apply (sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const opt = this.formatOptions(this.ruleArguments); + const opt = formatOptions(this.ruleArguments); const walker = new RuleWalker(sourceFile, this.ruleName, opt); + return this.applyWithWalker(walker); } } class RuleWalker extends Lint.AbstractWalker { - // last variable statement node - private lastVariableStatementNode: ts.Node|undefined; - private sourceFileText: string; - public walk (sourceFile: ts.SourceFile) { - this.sourceFileText = sourceFile.getFullText(); - - const onNode = (node: ts.Node): void => { - const { lastVariableStatementNode, sourceFileText } = this; - - // save the variable statement + const isNewLineAlwaysRequired: boolean = this.options.always; + const sourceFileText: string = sourceFile.getFullText(); + const onNode = (node: ts.Node): ts.Node|void => { if (node.kind === ts.SyntaxKind.VariableStatement) { - this.lastVariableStatementNode = node; - return; - } - - if (node.kind === ts.SyntaxKind.EndOfFileToken) { - this.lastVariableStatementNode = undefined; - return; - } - - if (lastVariableStatementNode) { - const unexpectedLineFixes: Lint.Replacement[] = []; - const expectedLineFixes: Lint.Replacement[] = []; - const isNewLineRequired: boolean = this.options.always; - let expectedLinePos: number = lastVariableStatementNode.end; - let newLinesCount: number = 0; + const nextSibling: ts.Node|void = getNextSiblingNode(node); + const nextSiblingKind: number|void = nextSibling && nextSibling.kind; + + if ( + !(isNewLineAlwaysRequired && nextSiblingKind === ts.SyntaxKind.VariableStatement) && + + // prevent a conflict with "eofline" rule + !(nextSibling && nextSiblingKind === ts.SyntaxKind.EndOfFileToken) + ) { + const isNewLineRequired: boolean = isNewLineAlwaysRequired && nextSiblingKind !== ts.SyntaxKind.VariableStatement; + const unexpectedLineFixes: Lint.Replacement[] = []; + const expectedLineFixes: Lint.Replacement[] = []; + const parentNodeEnd: number = node.parent.end; + const nodeEnd: number = node.end; + const trailingComments: ts.CommentRange[]|undefined = ts.getTrailingCommentRanges( + sourceFileText.slice(nodeEnd, parentNodeEnd), + 0 + ); + const lastTrailingComment: ts.CommentRange|undefined = trailingComments && trailingComments.pop(); + const newLinesSearchStart: number = lastTrailingComment + ? nodeEnd + lastTrailingComment.end + : nextSibling + ? nextSibling.pos + : node.end; + const newLinesSearchEnd: number = nextSibling ? nextSibling.getStart() : parentNodeEnd; + let expectedLinePos: number = newLinesSearchStart; + let newLinesCount: number = 0; + + if (isNewLineRequired && newLinesSearchStart === newLinesSearchEnd) { + expectedLineFixes.push(Lint.Replacement.appendText(expectedLinePos, Symbols.NEWLINE)); + } - for (let i = lastVariableStatementNode.end; i < node.end; i++) { - const code: number = sourceFileText.charCodeAt(i); + for (let i = newLinesSearchStart; i <= newLinesSearchEnd; i++) { + const code: number = sourceFileText.charCodeAt(i); - if (code === 10) { - newLinesCount++; + if (code === SymbolCodes.NEWLINE) { + newLinesCount++; - if (!isNewLineRequired && newLinesCount > 1) { - unexpectedLineFixes.push(Lint.Replacement.deleteText(i, 1)); - } - } else if (code !== 9 && code !== 13 && code !== 32) { - const leadingComments: ts.CommentRange[]|undefined = ts.getLeadingCommentRanges( - `\n${ sourceFileText.slice(i) }`, - 0 - ); - const lastLeadingComment: ts.CommentRange|undefined = leadingComments && leadingComments.pop(); - - if (lastLeadingComment && (!isNewLineRequired || (isNewLineRequired && newLinesCount < 2))) { - newLinesCount = 0; - expectedLinePos = i - 1 + lastLeadingComment.end; - i = expectedLinePos - 1; - } else { - if (isNewLineRequired && newLinesCount < 2) { - expectedLineFixes.push(Lint.Replacement.appendText(expectedLinePos, '\n')); + if (!isNewLineRequired && newLinesCount > 1) { + unexpectedLineFixes.push(Lint.Replacement.deleteText(i, 1)); + } + } else if ( + code !== SymbolCodes.TAB && + code !== SymbolCodes.CARRIAGE_RETURN && + code !== SymbolCodes.WHITESPACE + ) { + const leadingComments: ts.CommentRange[]|undefined = ts.getLeadingCommentRanges( + `\n${ sourceFileText.slice(i, newLinesSearchEnd) }`, + 0 + ); + const lastLeadingComment: ts.CommentRange|undefined = leadingComments && leadingComments.pop(); + + if (lastLeadingComment && (!isNewLineRequired || (isNewLineRequired && newLinesCount < 2))) { + newLinesCount = 0; + expectedLinePos = i - 1 + lastLeadingComment.end; + i = expectedLinePos - 1; + } else { + if (isNewLineRequired && newLinesCount < 2) { + expectedLineFixes.push(Lint.Replacement.appendText(expectedLinePos, Symbols.NEWLINE)); + } + + break; } - - break; } } - } - if (isNewLineRequired && expectedLineFixes[0]) { - // add the an empty line after previous node - this.addFailureAt( - lastVariableStatementNode.getStart(), - 1, - EXPECTED_BLANK_LINE_MESSAGE, - expectedLineFixes - ); - } else if (unexpectedLineFixes[0]) { - this.addFailureAt( - lastVariableStatementNode.getStart(), - 1, - UNEXPECTED_BLANK_LINE_MESSAGE, - unexpectedLineFixes - ); + if (isNewLineRequired && expectedLineFixes[0]) { + // add the an empty line after previous node + this.addFailureAt( + node.getStart(), + 1, + EXPECTED_BLANK_LINE_MESSAGE, + expectedLineFixes + ); + } else if (unexpectedLineFixes[0]) { + this.addFailureAt( + node.getStart(), + 1, + UNEXPECTED_BLANK_LINE_MESSAGE, + unexpectedLineFixes + ); + } } - - this.lastVariableStatementNode = undefined; } - return ts.forEachChild(node, onNode); + ts.forEachChild(node, onNode); }; - return ts.forEachChild(sourceFile, onNode); + ts.forEachChild(sourceFile, onNode); } } diff --git a/src/test/rules/terNewlineAfterVarRuleTests.ts b/src/test/rules/terNewlineAfterVarRuleTests.ts index 147fa7a..6eb8c11 100644 --- a/src/test/rules/terNewlineAfterVarRuleTests.ts +++ b/src/test/rules/terNewlineAfterVarRuleTests.ts @@ -20,6 +20,29 @@ function expecting (errors: ['expectedBlankLine' | 'unexpectedBlankLine', number } ruleTester.addTestGroup('always', 'should always require an empty line after variable declarations ', [ + { + code: dedent` + const foo1 = 1; + + const bar1 = 2; // Inline comment + + const foo2 = 3; /* Multiline + comment */ + + const bar2 = 4; + `, + output: dedent` + const foo1 = 1; + + const bar1 = 2; // Inline comment + + const foo2 = 3; /* Multiline + comment */ + + const bar2 = 4; + `, + options: [] + }, { code: dedent` var greet = "hello,", @@ -102,7 +125,6 @@ ruleTester.addTestGroup('always', 'should always require an empty line after var console.log(greet, name); var greet = "hello,"; - var name = "world"; `, options: ['always'] @@ -182,10 +204,77 @@ ruleTester.addTestGroup('always', 'should always require an empty line after var errors: expecting([ ['expectedBlankLine', 1] ]) + }, + { + code: dedent` + const myFunc = () => { + const x = 2; + const y = 3; + return x + y; + }`, + output: dedent` + const myFunc = () => { + const x = 2; + const y = 3; + + return x + y; + }`, + options: ['always'], + errors: expecting([ + ['expectedBlankLine', 3] + ]) + }, + { + code: dedent` + let pendingStatus: string[] = []; // contains all pending checks from travis as multiple are sent + + /** + * @param {string} s The commit hash string + * @return {string} Returns formatted commit hash string + */ + function fmt_url(s: string): string { + return \`\x0302\x1F\${s}\x0F\`; + }`, + output: dedent` + let pendingStatus: string[] = []; // contains all pending checks from travis as multiple are sent + + /** + * @param {string} s The commit hash string + * @return {string} Returns formatted commit hash string + */ + function fmt_url(s: string): string { + return \`\x0302\x1F\${s}\x0F\`; + }`, + options: ['always'] } ]); ruleTester.addTestGroup('never', 'should disallow empty lines after variable declarations ', [ + { + code: dedent` + const foo1 = 1; + + const bar1 = 2; // Inline comment + + const foo2 = 3; /* Multiline + comment */ + + const bar2 = 4; + `, + output: dedent` + const foo1 = 1; + const bar1 = 2; // Inline comment + const foo2 = 3; /* Multiline + comment */ + const bar2 = 4; + `, + options: ['never'], + errors: expecting([ + ['unexpectedBlankLine', 1], + ['unexpectedBlankLine', 3], + ['unexpectedBlankLine', 5] + ]) + }, { code: dedent` var greet = "hello,", diff --git a/src/test/rules/terNoSelfCompareRuleTests.ts b/src/test/rules/terNoSelfCompareRuleTests.ts index 78fe18e..65983d8 100644 --- a/src/test/rules/terNoSelfCompareRuleTests.ts +++ b/src/test/rules/terNoSelfCompareRuleTests.ts @@ -4,7 +4,7 @@ const ruleTester = new RuleTester('ter-no-self-compare'); // Change this function to better test the rule. In some cases the message never changes so we // can avoid passing it in. See other rule tests for examples. -function expecting(errors: [number, number, number][]): Failure[] { +function expecting(errors: [number, number, number, number?][]): Failure[] { return errors.map((err) => { return { failure: 'Comparing to itself is potentially pointless.',