From 46776861e10813071e662f4a37a17c67a75c4097 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Sat, 17 Dec 2016 20:28:13 -0800 Subject: [PATCH] Replace `node.getSourceFile()`, which climbs ancestors, with the `sourceFile` field on a RuleWalker. (#1896) --- src/language/walker/ruleWalker.ts | 4 ++++ src/rules/alignRule.ts | 14 ++------------ src/rules/jsdocFormatRule.ts | 6 +++--- src/rules/noEmptyRule.ts | 2 +- src/rules/noMergeableNamespaceRule.ts | 4 ++-- src/rules/noSwitchCaseFallThroughRule.ts | 8 ++++---- src/rules/objectLiteralSortKeysRule.ts | 4 ++-- src/rules/oneLineRule.ts | 19 ++++++++----------- src/rules/orderedImportsRule.ts | 12 ++++++------ 9 files changed, 32 insertions(+), 41 deletions(-) diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index eab79631cf2..775a3ce3dfd 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -44,6 +44,10 @@ export class RuleWalker extends SyntaxWalker { return this.sourceFile; } + public getLineAndCharacterOfPosition(position: number): ts.LineAndCharacter { + return this.sourceFile.getLineAndCharacterOfPosition(position); + } + public getFailures(): RuleFailure[] { return this.failures; } diff --git a/src/rules/alignRule.ts b/src/rules/alignRule.ts index d2c52e049a5..f4568e5d538 100644 --- a/src/rules/alignRule.ts +++ b/src/rules/alignRule.ts @@ -58,11 +58,6 @@ export class Rule extends Lint.Rules.AbstractRule { } } -type SourcePosition = { - line: number; - character: number; -}; - class AlignWalker extends Lint.RuleWalker { public visitConstructorDeclaration(node: ts.ConstructorDeclaration) { this.checkAlignment(Rule.PARAMETERS_OPTION, node.parameters); @@ -104,12 +99,12 @@ class AlignWalker extends Lint.RuleWalker { return; } - let prevPos = getPosition(nodes[0]); + let prevPos = this.getLineAndCharacterOfPosition(nodes[0].getStart()); const alignToColumn = prevPos.character; // skip first node in list for (let node of nodes.slice(1)) { - const curPos = getPosition(node); + const curPos = this.getLineAndCharacterOfPosition(node.getStart()); if (curPos.line !== prevPos.line && curPos.character !== alignToColumn) { this.addFailureAtNode(node, kind + Rule.FAILURE_STRING_SUFFIX); break; @@ -117,9 +112,4 @@ class AlignWalker extends Lint.RuleWalker { prevPos = curPos; } } - -} - -function getPosition(node: ts.Node): SourcePosition { - return node.getSourceFile().getLineAndCharacterOfPosition(node.getStart()); } diff --git a/src/rules/jsdocFormatRule.ts b/src/rules/jsdocFormatRule.ts index 238c1320d89..16bd38440d7 100644 --- a/src/rules/jsdocFormatRule.ts +++ b/src/rules/jsdocFormatRule.ts @@ -63,12 +63,12 @@ class JsdocWalker extends Lint.SkippableTokenAwareRuleWalker { if (scanner.getToken() === ts.SyntaxKind.MultiLineCommentTrivia) { const commentText = scanner.getTokenText(); const startPosition = scanner.getTokenPos(); - this.findFailuresForJsdocComment(commentText, startPosition, node); + this.findFailuresForJsdocComment(commentText, startPosition); } }); } - private findFailuresForJsdocComment(commentText: string, startingPosition: number, sourceFile: ts.SourceFile) { + private findFailuresForJsdocComment(commentText: string, startingPosition: number) { const currentPosition = startingPosition; // the file may be different depending on the OS it was originally authored on // can't rely on require('os').EOL or process.platform as that is the execution env @@ -88,7 +88,7 @@ class JsdocWalker extends Lint.SkippableTokenAwareRuleWalker { return; } - const indexToMatch = firstLine.indexOf("**") + sourceFile.getLineAndCharacterOfPosition(currentPosition).character; + const indexToMatch = firstLine.indexOf("**") + this.getLineAndCharacterOfPosition(currentPosition).character; // all lines but the first and last const otherLines = lines.splice(1, lines.length - 2); jsdocPosition += firstLine.length + 1; // + 1 for the splitted-out newline diff --git a/src/rules/noEmptyRule.ts b/src/rules/noEmptyRule.ts index f824cfe4e16..f5421584d16 100644 --- a/src/rules/noEmptyRule.ts +++ b/src/rules/noEmptyRule.ts @@ -47,7 +47,7 @@ class BlockWalker extends Lint.RuleWalker { public visitBlock(node: ts.Block) { const openBrace = node.getChildAt(0); const closeBrace = node.getChildAt(node.getChildCount() - 1); - const sourceFileText = node.getSourceFile().text; + const sourceFileText = this.getSourceFile().text; const hasCommentAfter = ts.getTrailingCommentRanges(sourceFileText, openBrace.getEnd()) != null; const hasCommentBefore = ts.getLeadingCommentRanges(sourceFileText, closeBrace.getFullStart()) != null; const isSkipped = this.ignoredBlocks.indexOf(node) !== -1; diff --git a/src/rules/noMergeableNamespaceRule.ts b/src/rules/noMergeableNamespaceRule.ts index a9ee9a9fb7f..c09307c4237 100644 --- a/src/rules/noMergeableNamespaceRule.ts +++ b/src/rules/noMergeableNamespaceRule.ts @@ -66,10 +66,10 @@ class NoMergeableNamespaceWalker extends Lint.RuleWalker { } private findLocationToMerge(currentPosition: number, highlightSpans: ts.HighlightSpan[]): ts.LineAndCharacter { - const { line } = ts.getLineAndCharacterOfPosition(this.getSourceFile(), currentPosition); + const { line } = this.getLineAndCharacterOfPosition(currentPosition); for (const span of highlightSpans) { - const lineAndCharacter = ts.getLineAndCharacterOfPosition(this.getSourceFile(), span.textSpan.start); + const lineAndCharacter = this.getLineAndCharacterOfPosition(span.textSpan.start); if (lineAndCharacter.line !== line) { return lineAndCharacter; } diff --git a/src/rules/noSwitchCaseFallThroughRule.ts b/src/rules/noSwitchCaseFallThroughRule.ts index 4b191ca5968..a22aafedb6a 100644 --- a/src/rules/noSwitchCaseFallThroughRule.ts +++ b/src/rules/noSwitchCaseFallThroughRule.ts @@ -79,13 +79,13 @@ export class NoSwitchCaseFallThroughWalker extends Lint.RuleWalker { // no break statements and no statements means the fallthrough is expected. // last item doesn't need a break if (isFallingThrough && switchClause.statements.length > 0 && ((switchClauses.length - 1) > i)) { - if (!isFallThroughAllowed(switchClauses[i + 1])) { + if (!isFallThroughAllowed(this.getSourceFile(), switchClauses[i + 1])) { this.addFailureAt(switchClauses[i + 1].getStart(), "case".length, `${Rule.FAILURE_STRING_PART}'case'`); } } } else { // case statement falling through a default - if (isFallingThrough && !isFallThroughAllowed(child)) { + if (isFallingThrough && !isFallThroughAllowed(this.getSourceFile(), child)) { this.addFailureAt(switchClauses[i].getStart(), "default".length, Rule.FAILURE_STRING_PART + "'default'"); } } @@ -103,8 +103,8 @@ function fallsThrough(statements: ts.NodeArray) { }); } -function isFallThroughAllowed(nextCaseOrDefaultStatement: ts.Node) { - const sourceFileText = nextCaseOrDefaultStatement.getSourceFile().text; +function isFallThroughAllowed(sourceFile: ts.SourceFile, nextCaseOrDefaultStatement: ts.Node) { + const sourceFileText = sourceFile.text; const firstChild = nextCaseOrDefaultStatement.getChildAt(0); const commentRanges = ts.getLeadingCommentRanges(sourceFileText, firstChild.getFullStart()); if (commentRanges != null) { diff --git a/src/rules/objectLiteralSortKeysRule.ts b/src/rules/objectLiteralSortKeysRule.ts index ad2a2f66815..b6f6aae4d13 100644 --- a/src/rules/objectLiteralSortKeysRule.ts +++ b/src/rules/objectLiteralSortKeysRule.ts @@ -85,8 +85,8 @@ class ObjectLiteralSortKeysWalker extends Lint.RuleWalker { } private isMultilineListNode(node: ts.ObjectLiteralExpression) { - const startLineOfNode = this.getSourceFile().getLineAndCharacterOfPosition(node.getStart()).line; - const endLineOfNode = this.getSourceFile().getLineAndCharacterOfPosition(node.getEnd()).line; + const startLineOfNode = this.getLineAndCharacterOfPosition(node.getStart()).line; + const endLineOfNode = this.getLineAndCharacterOfPosition(node.getEnd()).line; return endLineOfNode !== startLineOfNode; } } diff --git a/src/rules/oneLineRule.ts b/src/rules/oneLineRule.ts index 6c02772842d..246d667dcc4 100644 --- a/src/rules/oneLineRule.ts +++ b/src/rules/oneLineRule.ts @@ -67,7 +67,6 @@ export class Rule extends Lint.Rules.AbstractRule { class OneLineWalker extends Lint.RuleWalker { public visitIfStatement(node: ts.IfStatement) { - const sourceFile = node.getSourceFile(); const thenStatement = node.thenStatement; const thenIsBlock = thenStatement.kind === ts.SyntaxKind.Block; if (thenIsBlock) { @@ -85,8 +84,8 @@ class OneLineWalker extends Lint.RuleWalker { this.handleOpeningBrace(elseKeyword, elseOpeningBrace); } if (thenIsBlock && this.hasOption(OPTION_ELSE)) { - const thenStatementEndLine = sourceFile.getLineAndCharacterOfPosition(thenStatement.getEnd()).line; - const elseKeywordLine = sourceFile.getLineAndCharacterOfPosition(elseKeyword.getStart()).line; + const thenStatementEndLine = this.getLineAndCharacterOfPosition(thenStatement.getEnd()).line; + const elseKeywordLine = this.getLineAndCharacterOfPosition(elseKeyword.getStart()).line; if (thenStatementEndLine !== elseKeywordLine) { this.addFailureAtNode(elseKeyword, Rule.ELSE_FAILURE_STRING); } @@ -104,7 +103,6 @@ class OneLineWalker extends Lint.RuleWalker { } public visitTryStatement(node: ts.TryStatement) { - const sourceFile = node.getSourceFile(); const catchClause = node.catchClause; const finallyBlock = node.finallyBlock; const finallyKeyword = node.getChildren().filter((n) => n.kind === ts.SyntaxKind.FinallyKeyword)[0]; @@ -118,8 +116,8 @@ class OneLineWalker extends Lint.RuleWalker { if (this.hasOption(OPTION_CATCH) && catchClause != null) { const tryClosingBrace = node.tryBlock.getChildAt(node.tryBlock.getChildCount() - 1); const catchKeyword = catchClause.getChildAt(0); - const tryClosingBraceLine = sourceFile.getLineAndCharacterOfPosition(tryClosingBrace.getEnd()).line; - const catchKeywordLine = sourceFile.getLineAndCharacterOfPosition(catchKeyword.getStart()).line; + const tryClosingBraceLine = this.getLineAndCharacterOfPosition(tryClosingBrace.getEnd()).line; + const catchKeywordLine = this.getLineAndCharacterOfPosition(catchKeyword.getStart()).line; if (tryClosingBraceLine !== catchKeywordLine) { this.addFailureAtNode(catchKeyword, Rule.CATCH_FAILURE_STRING); } @@ -132,8 +130,8 @@ class OneLineWalker extends Lint.RuleWalker { if (this.hasOption(OPTION_FINALLY)) { const previousBlock = catchClause != null ? catchClause.block : node.tryBlock; const closingBrace = previousBlock.getChildAt(previousBlock.getChildCount() - 1); - const closingBraceLine = sourceFile.getLineAndCharacterOfPosition(closingBrace.getEnd()).line; - const finallyKeywordLine = sourceFile.getLineAndCharacterOfPosition(finallyKeyword.getStart()).line; + const closingBraceLine = this.getLineAndCharacterOfPosition(closingBrace.getEnd()).line; + const finallyKeywordLine = this.getLineAndCharacterOfPosition(finallyKeyword.getStart()).line; if (closingBraceLine !== finallyKeywordLine) { this.addFailureAtNode(finallyKeyword, Rule.FINALLY_FAILURE_STRING); } @@ -291,9 +289,8 @@ class OneLineWalker extends Lint.RuleWalker { return; } - const sourceFile = previousNode.getSourceFile(); - const previousNodeLine = sourceFile.getLineAndCharacterOfPosition(previousNode.getEnd()).line; - const openBraceLine = sourceFile.getLineAndCharacterOfPosition(openBraceToken.getStart()).line; + const previousNodeLine = this.getLineAndCharacterOfPosition(previousNode.getEnd()).line; + const openBraceLine = this.getLineAndCharacterOfPosition(openBraceToken.getStart()).line; let failure: string; if (this.hasOption(OPTION_BRACE) && previousNodeLine !== openBraceLine) { diff --git a/src/rules/orderedImportsRule.ts b/src/rules/orderedImportsRule.ts index ce7bb8fac5b..b25e1ca1f7a 100644 --- a/src/rules/orderedImportsRule.ts +++ b/src/rules/orderedImportsRule.ts @@ -173,7 +173,7 @@ class OrderedImportsWalker extends Lint.RuleWalker { source = removeQuotes(source); source = this.importSourcesOrderTransform(source); const previousSource = this.currentImportsBlock.getLastImportSource(); - this.currentImportsBlock.addImportDeclaration(node, source); + this.currentImportsBlock.addImportDeclaration(this.getSourceFile(), node, source); if (previousSource && compare(source, previousSource) === -1) { this.lastFix = new Lint.Fix(Rule.metadata.ruleName, []); @@ -244,10 +244,10 @@ interface ImportDeclaration { class ImportsBlock { private importDeclarations: ImportDeclaration[] = []; - public addImportDeclaration(node: ts.ImportDeclaration, sourcePath: string) { + public addImportDeclaration(sourceFile: ts.SourceFile, node: ts.ImportDeclaration, sourcePath: string) { const start = this.getStartOffset(node); - const end = this.getEndOffset(node); - const text = node.getSourceFile().text.substring(start, end); + const end = this.getEndOffset(sourceFile, node); + const text = sourceFile.text.substring(start, end); if (start > node.getStart() || end === 0) { // skip block if any statements don't end with a newline to simplify implementation @@ -309,8 +309,8 @@ class ImportsBlock { } // gets the offset of the end of the import's line, including newline, to include comment to the right - private getEndOffset(node: ts.ImportDeclaration) { - let endLineOffset = node.getSourceFile().text.indexOf("\n", node.end) + 1; + private getEndOffset(sourceFile: ts.SourceFile, node: ts.ImportDeclaration) { + let endLineOffset = sourceFile.text.indexOf("\n", node.end) + 1; return endLineOffset; }