From 7f3805afc3f80e74f3940cfb766064251cfa7c9f Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 31 Jan 2018 13:51:28 -0800 Subject: [PATCH 1/4] Fixing an edge case in the no-octal-literal rule where numbers preceeded by an escaped backslash would fail the rule. --- src/noOctalLiteralRule.ts | 15 ++++--- src/tests/NoOctalLiteralTests.ts | 42 ++++++++++++++++--- .../NoOctalLiteralTestInput-passing.ts | 2 + 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/src/noOctalLiteralRule.ts b/src/noOctalLiteralRule.ts index 12039f222..ae8d4df7c 100644 --- a/src/noOctalLiteralRule.ts +++ b/src/noOctalLiteralRule.ts @@ -40,14 +40,19 @@ class NoOctalLiteral extends ErrorTolerantWalker { } private failOnOctalString(node: ts.LiteralExpression) { - const match = /("|')(.*(\\-?[0-7]{1,3}(?![0-9])).*("|'))/g.exec(node.getText()); + const match = /("|'|`)[^\\]*(\\+-?[0-7]{1,3}(?![0-9])).*(?:\1)/g.exec(node.getText()); if (match) { - const octalValue : string = match[3]; // match[3] is the matched octal value. - const startOfMatch = node.getStart() + node.getText().indexOf(octalValue); - const width = octalValue.length; + let octalValue: string = match[2]; // match[2] is the matched octal value. + const backslashCount: number = octalValue.lastIndexOf('\\') + 1; + if (backslashCount % 2 === 1) { // Make sure the string starts with an odd number of backslashes + octalValue = octalValue.substr(backslashCount - 1); - this.addFailureAt(startOfMatch, width, Rule.FAILURE_STRING + octalValue); + const startOfMatch = node.getStart() + node.getText().indexOf(octalValue); + const width = octalValue.length; + + this.addFailureAt(startOfMatch, width, Rule.FAILURE_STRING + octalValue); + } } } } diff --git a/src/tests/NoOctalLiteralTests.ts b/src/tests/NoOctalLiteralTests.ts index b904ad46a..6168aad71 100644 --- a/src/tests/NoOctalLiteralTests.ts +++ b/src/tests/NoOctalLiteralTests.ts @@ -65,7 +65,9 @@ function demoScriptFail1() { return "Sample text \\7"; return "Sample text \\025"; return "Sample text \\0"; + return "Sample text \\\\\\0"; return "Sample text \\-0"; + return "Sample text \\\\\\-0"; return "Sample text \\-035"; return "Sample text \\-235"; }`; @@ -106,25 +108,39 @@ function demoScriptFail1() { "startPosition": { "character": 25, "line": 11 } }, { + "failure": "Octal literals should not be used: \\0", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 27, "line": 12 } + }, + { "failure": "Octal literals should not be used: \\-0", "name": "file.ts", "ruleName": "no-octal-literal", "ruleSeverity": "ERROR", - "startPosition": { "character": 25, "line": 12 } + "startPosition": { "character": 25, "line": 13 } }, { + "failure": "Octal literals should not be used: \\-0", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 27, "line": 14 } + }, + { "failure": "Octal literals should not be used: \\-035", "name": "file.ts", "ruleName": "no-octal-literal", "ruleSeverity": "ERROR", - "startPosition": { "character": 25, "line": 13 } + "startPosition": { "character": 25, "line": 15 } }, { "failure": "Octal literals should not be used: \\-235", "name": "file.ts", "ruleName": "no-octal-literal", "ruleSeverity": "ERROR", - "startPosition": { "character": 25, "line": 14 } + "startPosition": { "character": 25, "line": 16 } } ]); }); @@ -142,6 +158,8 @@ function demoScriptFail2() { return 'Sample text \\125'; return 'Sample text \\0'; return 'Sample text \\-0'; + return 'Sample text \\\\\\0'; + return 'Sample text \\\\\\-0'; return 'Sample text \\-035'; return 'Sample text \\-235'; }`; @@ -195,19 +213,33 @@ function demoScriptFail2() { "ruleSeverity": "ERROR", "startPosition": { "character": 25, "line": 12 } }, + { + "failure": "Octal literals should not be used: \\0", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 27, "line": 13 } + }, + { + "failure": "Octal literals should not be used: \\-0", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 27, "line": 14 } + }, { "failure": "Octal literals should not be used: \\-035", "name": "file.ts", "ruleName": "no-octal-literal", "ruleSeverity": "ERROR", - "startPosition": { "character": 25, "line": 13 } + "startPosition": { "character": 25, "line": 15 } }, { "failure": "Octal literals should not be used: \\-235", "name": "file.ts", "ruleName": "no-octal-literal", "ruleSeverity": "ERROR", - "startPosition": { "character": 25, "line": 14 } + "startPosition": { "character": 25, "line": 16 } } ]); }); diff --git a/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts b/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts index 5ee28e66d..d144d96c2 100644 --- a/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts +++ b/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts @@ -1,4 +1,6 @@ function demoScriptPass1() { var x = "Sample text \xB2"; var y = "Sample text \0111"; // longer than octal + var z = "Sample text \\1"; + var z = "Sample text \\\\1"; }; From cecfe52a4e1fd05aa2cbec402db6ea747370c20f Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 31 Jan 2018 14:29:26 -0800 Subject: [PATCH 2/4] Fixing an issue where the no-octal-rule wouldn't handle template strings or newlines inside strings. --- src/noOctalLiteralRule.ts | 4 +- src/tests/NoOctalLiteralTests.ts | 155 +++++++++++++++++- .../NoOctalLiteralTestInput-passing.ts | 18 +- 3 files changed, 169 insertions(+), 8 deletions(-) diff --git a/src/noOctalLiteralRule.ts b/src/noOctalLiteralRule.ts index ae8d4df7c..deda4d82b 100644 --- a/src/noOctalLiteralRule.ts +++ b/src/noOctalLiteralRule.ts @@ -33,14 +33,14 @@ export class Rule extends Lint.Rules.AbstractRule { class NoOctalLiteral extends ErrorTolerantWalker { public visitNode(node: ts.Node) { - if (node.kind === ts.SyntaxKind.StringLiteral) { + if (node.kind === ts.SyntaxKind.StringLiteral || node.kind === ts.SyntaxKind.FirstTemplateToken) { this.failOnOctalString(node); } super.visitNode(node); } private failOnOctalString(node: ts.LiteralExpression) { - const match = /("|'|`)[^\\]*(\\+-?[0-7]{1,3}(?![0-9])).*(?:\1)/g.exec(node.getText()); + const match = /("|'|`)[^\\]*(\\+-?[0-7]{1,3}(?![0-9]))(?:.|\n|\t|\u2028|\u2029)*(?:\1)/g.exec(node.getText()); if (match) { let octalValue: string = match[2]; // match[2] is the matched octal value. diff --git a/src/tests/NoOctalLiteralTests.ts b/src/tests/NoOctalLiteralTests.ts index 6168aad71..4ce60a9c6 100644 --- a/src/tests/NoOctalLiteralTests.ts +++ b/src/tests/NoOctalLiteralTests.ts @@ -16,7 +16,7 @@ describe('noOctalLiteralRule', () : void => { it('should fail on 3 digit octal literals', () : void => { const script : string = ` /** - * The following code should have no errors: + * The following code should have errors: */ function demoScriptFail() { var a = "Sample text \\251"; @@ -150,7 +150,7 @@ function demoScriptFail1() { /** * The following code should have errors: */ -function demoScriptFail2() { +function demoScriptFail3() { return 'Sample text \\351'; return 'Sample text \\354 more text'; return 'Sample text \\33'; @@ -243,6 +243,157 @@ function demoScriptFail2() { } ]); }); + + it('should produce violations - batch3', () : void => { + const inputFile : string = ` +/** + * The following code should have errors: + */ +function demoScriptFail4() { + return \`Sample text \\351\`; + return \`Sample text \\354 more text\`; + return \`Sample text \\33\`; + return \`Sample text \\6\`; + return \`Sample text \\125\`; + return \`Sample text \\0\`; + return \`Sample text \\-0\`; + return \`Sample text \\\\\\0\`; + return \`Sample text \\\\\\-0\`; + return \`Sample text \\-035\`; + return \`Sample text \\-235\`; +}`; + TestHelper.assertViolations(ruleName, inputFile, [ + { + "failure": "Octal literals should not be used: \\351", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 6 } + }, + { + "failure": "Octal literals should not be used: \\354", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 7 } + }, + { + "failure": "Octal literals should not be used: \\33", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 8 } + }, + { + "failure": "Octal literals should not be used: \\6", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 9 } + }, + { + "failure": "Octal literals should not be used: \\125", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 10 } + }, + { + "failure": "Octal literals should not be used: \\0", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 11 } + }, + { + "failure": "Octal literals should not be used: \\-0", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 12 } + }, + { + "failure": "Octal literals should not be used: \\0", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 27, "line": 13 } + }, + { + "failure": "Octal literals should not be used: \\-0", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 27, "line": 14 } + }, + { + "failure": "Octal literals should not be used: \\-035", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 15 } + }, + { + "failure": "Octal literals should not be used: \\-235", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 16 } + } + ]); + }); + + it('should produce violations - batch4', () : void => { + const inputFile : string = ` +/** + * The following code should have errors: + */ +function demoScriptFail2() { + return 'Sample text \\354 \\n more text'; + return 'Sample text \\354 \\t more text'; + return 'Sample text \\354 \\u2028 more text'; + return 'Sample text \\354 \\u2029 more text'; + return \`Sample text \\354 +more text\`; +}`; + TestHelper.assertViolations(ruleName, inputFile, [ + { + "failure": "Octal literals should not be used: \\354", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 6 } + }, + { + "failure": "Octal literals should not be used: \\354", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 7 } + }, + { + "failure": "Octal literals should not be used: \\354", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 8 } + }, + { + "failure": "Octal literals should not be used: \\354", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 9 } + }, + { + "failure": "Octal literals should not be used: \\354", + "name": "file.ts", + "ruleName": "no-octal-literal", + "ruleSeverity": "ERROR", + "startPosition": { "character": 25, "line": 10 } + } + ]); + }); }); /* tslint:enable:no-octal-literal */ diff --git a/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts b/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts index d144d96c2..d2d4c926a 100644 --- a/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts +++ b/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts @@ -1,6 +1,16 @@ function demoScriptPass1() { - var x = "Sample text \xB2"; - var y = "Sample text \0111"; // longer than octal - var z = "Sample text \\1"; - var z = "Sample text \\\\1"; + var x1 = "Sample text \xB2"; + var y1 = "Sample text \0111"; // longer than octal + var z1 = "Sample text \\1"; + var z1 = "Sample text \\\\1"; + + var x2 = 'Sample text \xB2'; + var y2 = 'Sample text \0111'; // longer than octal + var z2 = 'Sample text \\1'; + var z2 = 'Sample text \\\\1'; + + var x3 = `Sample text \xB2`; + var y3 = `Sample text \0111`; // longer than octal + var z3 = `Sample text \\1`; + var z3 = `Sample text \\\\1`; }; From 2ff99e5615ddc8faf34fea091cdf714f892b9164 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 31 Jan 2018 15:15:17 -0800 Subject: [PATCH 3/4] Fixing test numbering. --- src/tests/NoOctalLiteralTests.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests/NoOctalLiteralTests.ts b/src/tests/NoOctalLiteralTests.ts index 4ce60a9c6..97de348db 100644 --- a/src/tests/NoOctalLiteralTests.ts +++ b/src/tests/NoOctalLiteralTests.ts @@ -150,7 +150,7 @@ function demoScriptFail1() { /** * The following code should have errors: */ -function demoScriptFail3() { +function demoScriptFail2() { return 'Sample text \\351'; return 'Sample text \\354 more text'; return 'Sample text \\33'; @@ -249,7 +249,7 @@ function demoScriptFail3() { /** * The following code should have errors: */ -function demoScriptFail4() { +function demoScriptFail3() { return \`Sample text \\351\`; return \`Sample text \\354 more text\`; return \`Sample text \\33\`; @@ -348,7 +348,7 @@ function demoScriptFail4() { /** * The following code should have errors: */ -function demoScriptFail2() { +function demoScriptFail4() { return 'Sample text \\354 \\n more text'; return 'Sample text \\354 \\t more text'; return 'Sample text \\354 \\u2028 more text'; From 01ea495877947b0afcdddade434640f8cb60613a Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 31 Jan 2018 15:17:33 -0800 Subject: [PATCH 4/4] Fixing duplicate variable declaration. --- .../NoOctalLiteralTestInput-passing.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts b/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts index d2d4c926a..95d9926fc 100644 --- a/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts +++ b/test-data/NoOctalLiteral/NoOctalLiteralTestInput-passing.ts @@ -1,16 +1,16 @@ function demoScriptPass1() { - var x1 = "Sample text \xB2"; - var y1 = "Sample text \0111"; // longer than octal - var z1 = "Sample text \\1"; + var w1 = "Sample text \xB2"; + var x1 = "Sample text \0111"; // longer than octal + var y1 = "Sample text \\1"; var z1 = "Sample text \\\\1"; - var x2 = 'Sample text \xB2'; - var y2 = 'Sample text \0111'; // longer than octal - var z2 = 'Sample text \\1'; + var w2 = 'Sample text \xB2'; + var x2 = 'Sample text \0111'; // longer than octal + var y2 = 'Sample text \\1'; var z2 = 'Sample text \\\\1'; - var x3 = `Sample text \xB2`; - var y3 = `Sample text \0111`; // longer than octal - var z3 = `Sample text \\1`; + var w3 = `Sample text \xB2`; + var x3 = `Sample text \0111`; // longer than octal + var y3 = `Sample text \\1`; var z3 = `Sample text \\\\1`; };