Skip to content

Commit

Permalink
Merge pull request palantir#362 from gscshoyru/template-string-handling
Browse files Browse the repository at this point in the history
Fix things that walk tokens to properly handle templates, etc.
  • Loading branch information
ashwinr committed Apr 14, 2015
2 parents 55391e8 + 0c62f06 commit 56cf19f
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 73 deletions.
14 changes: 13 additions & 1 deletion lib/tslint.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,19 @@ declare module Lint {
function findFormatter(name: string, formattersDirectory?: string): any;
}
declare module Lint {
class EnableDisableRulesWalker extends Lint.RuleWalker {
class SkippableTokenAwareRuleWalker extends RuleWalker {
protected tokensToSkipStartEndMap: {
[start: number]: number;
};
constructor(sourceFile: ts.SourceFile, options: Lint.IOptions);
protected visitRegularExpressionLiteral(node: ts.Node): void;
protected visitIdentifier(node: ts.Identifier): void;
protected visitTemplateExpression(node: ts.TemplateExpression): void;
private addTokenToSkipFromNode(node);
}
}
declare module Lint {
class EnableDisableRulesWalker extends Lint.SkippableTokenAwareRuleWalker {
enableDisableRuleMap: {
[rulename: string]: Lint.IEnableDisablePosition[];
};
Expand Down
14 changes: 11 additions & 3 deletions src/enableDisableRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,29 @@
* limitations under the License.
*/

/// <reference path='language/walker/ruleWalker.ts'/>
/// <reference path='language/walker/skippableTokenAwareRuleWalker.ts'/>

module Lint {
export class EnableDisableRulesWalker extends Lint.RuleWalker {
export class EnableDisableRulesWalker extends Lint.SkippableTokenAwareRuleWalker {
public enableDisableRuleMap: {[rulename: string]: Lint.IEnableDisablePosition[]} = {};

public visitSourceFile(node: ts.SourceFile): void {
super.visitSourceFile(node);
Lint.scanAllTokens(ts.createScanner(ts.ScriptTarget.ES5, false, node.text), (scanner: ts.Scanner) => {
var startPos = scanner.getStartPos();
if (this.tokensToSkipStartEndMap[startPos] != null) {
// tokens to skip are places where the scanner gets confused about what the token is, without the proper context
// (specifically, regex, identifiers, and templates). So skip those tokens.
scanner.setTextPos(this.tokensToSkipStartEndMap[startPos]);
return;
}

if (scanner.getToken() === ts.SyntaxKind.MultiLineCommentTrivia) {
var commentText = scanner.getTokenText();
var startPosition = scanner.getTokenPos();
this.handlePossibleTslintSwitch(commentText, startPosition);
}
});
// no need to call super to visit the rest of the nodes, so don't call super here
}

private handlePossibleTslintSwitch(commentText: string, startingPosition: number) {
Expand Down
50 changes: 50 additions & 0 deletions src/language/walker/skippableTokenAwareRuleWalker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2015 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.
*/

/// <reference path='ruleWalker.ts'/>

module Lint {
export class SkippableTokenAwareRuleWalker extends RuleWalker {
protected tokensToSkipStartEndMap: {[start: number]: number};

constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);
this.tokensToSkipStartEndMap = {};
}

protected visitRegularExpressionLiteral(node: ts.Node) {
this.addTokenToSkipFromNode(node);
super.visitRegularExpressionLiteral(node);
}

protected visitIdentifier(node: ts.Identifier) {
this.addTokenToSkipFromNode(node);
super.visitIdentifier(node);
}

protected visitTemplateExpression(node: ts.TemplateExpression) {
this.addTokenToSkipFromNode(node);
super.visitTemplateExpression(node);
}

private addTokenToSkipFromNode(node: ts.Node) {
if (node.getStart() < node.getEnd()) {
// only add to the map nodes whose end comes after their start, to prevent infinite loops
this.tokensToSkipStartEndMap[node.getStart()] = node.getEnd();
}
}
}
}
33 changes: 2 additions & 31 deletions src/rules/commentFormatRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* 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.
*/
*/

var OPTION_SPACE = "check-space";
var OPTION_LOWERCASE = "check-lowercase";
Expand All @@ -28,14 +28,7 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

class CommentWalker extends Lint.RuleWalker {
private tokensToSkipStartEndMap: {[start: number]: number};

constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);
this.tokensToSkipStartEndMap = {};
}

class CommentWalker extends Lint.SkippableTokenAwareRuleWalker {
public visitSourceFile(node: ts.SourceFile): void {
super.visitSourceFile(node);
Lint.scanAllTokens(ts.createScanner(ts.ScriptTarget.ES5, false, node.text), (scanner: ts.Scanner) => {
Expand Down Expand Up @@ -73,28 +66,6 @@ class CommentWalker extends Lint.RuleWalker {
});
}

public visitRegularExpressionLiteral(node: ts.Node) {
this.addTokenToSkipFromNode(node);
super.visitRegularExpressionLiteral(node);
}

public visitIdentifier(node: ts.Identifier) {
this.addTokenToSkipFromNode(node);
super.visitIdentifier(node);
}

public visitTemplateExpression(node: ts.TemplateExpression) {
this.addTokenToSkipFromNode(node);
super.visitTemplateExpression(node);
}

public addTokenToSkipFromNode(node: ts.Node) {
if (node.getStart() < node.getEnd()) {
// only add to the map nodes whose end comes after their start, to prevent infinite loops
this.tokensToSkipStartEndMap[node.getStart()] = node.getEnd();
}
}

private startsWithSpace(commentText: string): boolean {
if (commentText.length <= 2) {
return true; // comment is "//"? Technically not a violation.
Expand Down
12 changes: 10 additions & 2 deletions src/rules/jsdocFormatRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,24 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

class JsdocWalker extends Lint.RuleWalker {
class JsdocWalker extends Lint.SkippableTokenAwareRuleWalker {
public visitSourceFile(node: ts.SourceFile): void {
super.visitSourceFile(node);
Lint.scanAllTokens(ts.createScanner(ts.ScriptTarget.ES5, false, node.text), (scanner: ts.Scanner) => {
var startPos = scanner.getStartPos();
if (this.tokensToSkipStartEndMap[startPos] != null) {
// tokens to skip are places where the scanner gets confused about what the token is, without the proper context
// (specifically, regex, identifiers, and templates). So skip those tokens.
scanner.setTextPos(this.tokensToSkipStartEndMap[startPos]);
return;
}

if (scanner.getToken() === ts.SyntaxKind.MultiLineCommentTrivia) {
var commentText = scanner.getTokenText();
var startPosition = scanner.getTokenPos();
this.findFailuresForJsdocComment(commentText, startPosition, node);
}
});
// no need to call super to visit the rest of the nodes, so don't call super here
}

private findFailuresForJsdocComment(commentText: string, startingPosition: number, sourceFile: ts.SourceFile) {
Expand Down
13 changes: 11 additions & 2 deletions src/rules/noConsecutiveBlankLinesRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,21 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

class BlankLinesWalker extends Lint.RuleWalker {
class BlankLinesWalker extends Lint.SkippableTokenAwareRuleWalker {
public visitSourceFile(node: ts.SourceFile): void {
super.visitSourceFile(node);
// starting with 1 to cover the case where the file starts with two blank lines
var newLinesInARowSeenSoFar = 1;
Lint.scanAllTokens(ts.createScanner(ts.ScriptTarget.ES5, false, node.text), (scanner: ts.Scanner) => {
var startPos = scanner.getStartPos();
if (this.tokensToSkipStartEndMap[startPos] != null) {
// tokens to skip are places where the scanner gets confused about what the token is, without the proper context
// (specifically, regex, identifiers, and templates). So skip those tokens.
scanner.setTextPos(this.tokensToSkipStartEndMap[startPos]);
newLinesInARowSeenSoFar = 0;
return;
}

if (scanner.getToken() === ts.SyntaxKind.NewLineTrivia) {
newLinesInARowSeenSoFar += 1;
if (newLinesInARowSeenSoFar >= 3) {
Expand All @@ -37,6 +47,5 @@ class BlankLinesWalker extends Lint.RuleWalker {
newLinesInARowSeenSoFar = 0;
}
});
// no need to call super to visit the rest of the nodes, so don't call super here
}
}
13 changes: 11 additions & 2 deletions src/rules/noTrailingWhitespaceRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,21 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

class NoTrailingWhitespaceWalker extends Lint.RuleWalker {
class NoTrailingWhitespaceWalker extends Lint.SkippableTokenAwareRuleWalker {
public visitSourceFile(node: ts.SourceFile): void {
super.visitSourceFile(node);
var lastSeenWasWhitespace = false;
var lastSeenWhitespacePosition = 0;
Lint.scanAllTokens(ts.createScanner(ts.ScriptTarget.ES5, false, node.text), (scanner: ts.Scanner) => {
var startPos = scanner.getStartPos();
if (this.tokensToSkipStartEndMap[startPos] != null) {
// tokens to skip are places where the scanner gets confused about what the token is, without the proper context
// (specifically, regex, identifiers, and templates). So skip those tokens.
scanner.setTextPos(this.tokensToSkipStartEndMap[startPos]);
lastSeenWasWhitespace = false;
return;
}

if (scanner.getToken() === ts.SyntaxKind.NewLineTrivia) {
if (lastSeenWasWhitespace) {
var width = scanner.getStartPos() - lastSeenWhitespacePosition;
Expand All @@ -41,6 +51,5 @@ class NoTrailingWhitespaceWalker extends Lint.RuleWalker {
lastSeenWasWhitespace = false;
}
});
// no need to call super to visit the rest of the nodes, so don't call super here
}
}
26 changes: 1 addition & 25 deletions src/rules/whitespaceRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,12 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

class WhitespaceWalker extends Lint.RuleWalker {
class WhitespaceWalker extends Lint.SkippableTokenAwareRuleWalker {
private scanner: ts.Scanner;
private tokensToSkipStartEndMap: {[start: number]: number};

constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);
this.scanner = ts.createScanner(ts.ScriptTarget.ES5, false, sourceFile.text);
this.tokensToSkipStartEndMap = {};
}

public visitSourceFile(node: ts.SourceFile): void {
Expand Down Expand Up @@ -95,28 +93,6 @@ class WhitespaceWalker extends Lint.RuleWalker {
});
}

public visitRegularExpressionLiteral(node: ts.Node) {
this.addTokenToSkipFromNode(node);
super.visitRegularExpressionLiteral(node);
}

public visitIdentifier(node: ts.Identifier) {
this.addTokenToSkipFromNode(node);
super.visitIdentifier(node);
}

public visitTemplateExpression(node: ts.TemplateExpression) {
this.addTokenToSkipFromNode(node);
super.visitTemplateExpression(node);
}

public addTokenToSkipFromNode(node: ts.Node) {
if (node.getStart() < node.getEnd()) {
// only add to the map nodes whose end comes after their start, to prevent infinite loops
this.tokensToSkipStartEndMap[node.getStart()] = node.getEnd();
}
}

// check for spaces between the operator symbol (except in the case of comma statements)
public visitBinaryExpression(node: ts.BinaryExpression): void {
var operatorKind = node.operatorToken.kind;
Expand Down
1 change: 1 addition & 0 deletions src/tslint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
/// <reference path='language/rule/abstractRule.ts'/>
/// <reference path='language/walker/ruleWalker.ts'/>
/// <reference path='language/walker/scopeAwareRuleWalker.ts'/>
/// <reference path='language/walker/skippableTokenAwareRuleWalker.ts'/>

module Lint {
var path = require("path");
Expand Down
7 changes: 7 additions & 0 deletions test/files/rules/enabledisable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,10 @@ var AAAaA = 'test'
var AAAaA = 'test'
/* tslint:enable:zasdadsa */
var AAAaA = 'test'

/* tslint:enable */
var re;
re = /`/;

/* tslint:disable:quotemark */
var s = 'xxx';
10 changes: 10 additions & 0 deletions test/files/rules/jsdoc.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
function makeHeader(pkg: Package) {
return [
`\t/**`,
`\t * ${pkg.name} v${pkg.version}`,
`\t */`
].join("\r\n");
}

class Clazz { //this is not a block comment
/* block comment
*Not a jsdoc and not subject to the rules lalala
Expand Down Expand Up @@ -50,3 +58,5 @@ one *
/** a good one */

}


6 changes: 6 additions & 0 deletions test/files/rules/whitespace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,9 @@ a.then(() => {
}).if(() => {
return 1;
});

var name = "something";
var test = `
<sl-property-group label=${name} Axes">
<div class="repeat"
`;
14 changes: 7 additions & 7 deletions test/rules/jsdocFormatRuleTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ describe("<jsdoc-format>", () => {
var fileName = "rules/jsdoc.test.ts";
var createFormatFailure = Lint.Test.createFailuresOnFile(fileName, JsdocFormatRule.FORMAT_FAILURE_STRING);
var createAlignmentFailure = Lint.Test.createFailuresOnFile(fileName, JsdocFormatRule.ALIGNMENT_FAILURE_STRING);
var expectedFailure1 = createFormatFailure([20, 1], [20, 40]);
var expectedFailure2 = createAlignmentFailure([26, 1], [26, 6]);
var expectedFailure3 = createFormatFailure([30, 1], [30, 8]);
var expectedFailure4 = createFormatFailure([34, 1], [34, 7]);
var expectedFailure5 = createAlignmentFailure([39, 1], [39, 19]);
var expectedFailure6 = createFormatFailure([42, 5], [42, 26]);
var expectedFailure7 = createFormatFailure([44, 5], [44, 32]);
var expectedFailure1 = createFormatFailure([28, 1], [28, 40]);
var expectedFailure2 = createAlignmentFailure([34, 1], [34, 6]);
var expectedFailure3 = createFormatFailure([38, 1], [38, 8]);
var expectedFailure4 = createFormatFailure([42, 1], [42, 7]);
var expectedFailure5 = createAlignmentFailure([47, 1], [47, 19]);
var expectedFailure6 = createFormatFailure([50, 5], [50, 26]);
var expectedFailure7 = createFormatFailure([52, 5], [52, 32]);

var actualFailures = Lint.Test.applyRuleOnFile(fileName, JsdocFormatRule);

Expand Down
1 change: 1 addition & 0 deletions test/rules/noTrailingWhitespaceRuleTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,6 @@ describe("<no-trailing-whitespace>", () => {
Lint.Test.assertContainsFailure(actualFailures, expectedFailure3);
Lint.Test.assertContainsFailure(actualFailures, expectedFailure4);
Lint.Test.assertContainsFailure(actualFailures, expectedFailure5);
assert.lengthOf(actualFailures, 5);
});
});

0 comments on commit 56cf19f

Please sign in to comment.