From c98a8596c4b3ed90efb521d5861c62bd0c1c9c39 Mon Sep 17 00:00:00 2001 From: Eric Ferreira <2483476+ericbf@users.noreply.github.com> Date: Tue, 7 May 2019 08:33:28 -0400 Subject: [PATCH] [quotemark] Exclude some cases from backtick rule (#4693) * [quotemark] Exclude some cases from backtick rule This commit makes quotemark backtick ignore use strict declarations, enum members, lookup types, and strings containing octal escape sequences. * [quotemark] Fix comment on use strict check call I had copy pasted and forgotten to change it. This changes that comment. * Revert unrelated change, fix octal escape sequence check This commit makes it so that if a string has a literal backslash instead of an actual octal escape sequence, it is not flagged. --- src/rules/quotemarkRule.ts | 83 +++++++++++++++++----- test/rules/quotemark/backtick/test.ts.fix | 48 +++++++++++++ test/rules/quotemark/backtick/test.ts.lint | 57 +++++++++++++++ 3 files changed, 169 insertions(+), 19 deletions(-) diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index 39c3124405b..d98b4b586c4 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -17,8 +17,12 @@ import { lt } from "semver"; import { + isEnumMember, isExportDeclaration, + isExpressionStatement, isImportDeclaration, + isIndexedAccessTypeNode, + isLiteralTypeNode, isNoSubstitutionTemplateLiteral, isSameLine, isStringLiteral, @@ -38,6 +42,7 @@ const OPTION_AVOID_ESCAPE = "avoid-escape"; type QUOTEMARK = "'" | '"' | "`"; type JSX_QUOTEMARK = "'" | '"'; +type StringLiteralLike = ts.StringLiteral | ts.NoSubstitutionTemplateLiteral; interface Options { quotemark: QUOTEMARK; @@ -131,21 +136,7 @@ function walk(ctx: Lint.WalkContext) { const actualQuotemark = sourceFile.text[node.end - 1]; // Don't use backticks instead of single/double quotes when it breaks TypeScript syntax. - if ( - expectedQuotemark === "`" && - // This captures `export blah from "package"` - (isExportDeclaration(node.parent) || - // This captures `import blah from "package"` - isImportDeclaration(node.parent) || - // This captures quoted names in object literal keys - isNameInAssignment(node) || - // This captures quoted signatures (property or method) - isSignature(node) || - // This captures literal types in generic type constraints - isTypeConstraint(node) || - // Whether this is the type in a typeof check with older tsc - isTypeCheckWithOldTsc(node)) - ) { + if (expectedQuotemark === "`" && isNotValidToUseBackticksInNode(node, sourceFile)) { return; } @@ -250,12 +241,37 @@ function getJSXQuotemarkPreference( return regularQuotemarkPreference !== "`" ? regularQuotemarkPreference : '"'; } +function isNotValidToUseBackticksInNode(node: StringLiteralLike, sourceFile: ts.SourceFile) { + return ( + // This captures `export blah from "package"` + isExportDeclaration(node.parent) || + // This captures `import blah from "package"` + isImportDeclaration(node.parent) || + // This captures quoted names in object literal keys + isNameInAssignment(node) || + // This captures quoted signatures (property or method) + isSignature(node) || + // This captures literal types in generic type constraints + isTypeConstraint(node) || + // Older TS doesn't narrow a type when backticks are used to compare typeof + isTypeCheckWithOldTsc(node) || + // Enum members can't use backticks + isEnumMember(node.parent) || + // Typescript converts old octal escape sequences to just the numbers therein + containsOctalEscapeSequence(node, sourceFile) || + // Use strict declarations have to be single or double quoted + isUseStrictDeclaration(node) || + // Lookup type parameters must be single/double quoted + isLookupTypeParameter(node) + ); +} + /** * Whether this node is a type constraint in a generic type. * @param node The node to check * @return Whether this node is a type constraint */ -function isTypeConstraint(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { +function isTypeConstraint(node: StringLiteralLike) { let parent = node.parent.parent; // If this node doesn't have a grandparent, it's not a type constraint @@ -285,7 +301,7 @@ function isTypeConstraint(node: ts.StringLiteral | ts.NoSubstitutionTemplateLite * @param node The node to check * @return Whether this node is a property/method signature. */ -function isSignature(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { +function isSignature(node: StringLiteralLike) { let parent = node.parent; if (hasOldTscBacktickBehavior() && node.parent.kind === ts.SyntaxKind.LastTypeNode) { @@ -306,7 +322,7 @@ function isSignature(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) * @param node The node to check * @return Whether this node is the name in an assignment/decleration. */ -function isNameInAssignment(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { +function isNameInAssignment(node: StringLiteralLike) { if ( node.parent.kind !== ts.SyntaxKind.PropertyAssignment && node.parent.kind !== ts.SyntaxKind.MethodDeclaration @@ -323,7 +339,7 @@ function isNameInAssignment(node: ts.StringLiteral | ts.NoSubstitutionTemplateLi ); } -function isTypeCheckWithOldTsc(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { +function isTypeCheckWithOldTsc(node: StringLiteralLike) { if (!hasOldTscBacktickBehavior()) { // This one only affects older typescript versions return false; @@ -338,6 +354,35 @@ function isTypeCheckWithOldTsc(node: ts.StringLiteral | ts.NoSubstitutionTemplat return node.parent.getChildren().some(n => n.kind === ts.SyntaxKind.TypeOfExpression); } +function containsOctalEscapeSequence(node: StringLiteralLike, sourceFile: ts.SourceFile) { + // Octal sequences can go from 1-377 (255 in octal), but let's match the prefix, which will at least be \1-\77 + // Using node.getText here strips the backslashes from the string. We also need to make sure there isn't an even + // number of backslashes (then it would not be an escape sequence, but a literal backslash followed by numbers). + const matches = node.getText(sourceFile).match(/(\\)+[1-7][0-7]?/g); + + if (matches != undefined) { + for (const match of matches) { + const numBackslashes = match.match(/\\/g)!.length; + + if (numBackslashes % 2 === 1) { + // There was an odd number of backslashes preceeding this node – it's an octal escape sequence + return true; + } + } + } + + return false; +} + +function isUseStrictDeclaration(node: StringLiteralLike) { + return node.text === "use strict" && isExpressionStatement(node.parent); +} + +function isLookupTypeParameter(node: StringLiteralLike) { + return isLiteralTypeNode(node.parent) && isIndexedAccessTypeNode(node.parent.parent); +} + +/** Versions of typescript below 2.7.1 treat backticks differently */ function hasOldTscBacktickBehavior() { return lt(getNormalizedTypescriptVersion(), "2.7.1"); } diff --git a/test/rules/quotemark/backtick/test.ts.fix b/test/rules/quotemark/backtick/test.ts.fix index c2c13346619..55cf8ac4dc2 100644 --- a/test/rules/quotemark/backtick/test.ts.fix +++ b/test/rules/quotemark/backtick/test.ts.fix @@ -1,6 +1,34 @@ +"use strict" + import { Something } from "some-package" export { SomethingElse } from "another-package" +enum Sides { + '<- Left', + 'Right ->' +} + +const octalEscapeSequence = '\1' +const octalEscapeSequence2 = '\7' +const octalEscapeSequence3 = '\77' +const octalEscapeSequence4 = '\377' +const octalEscapeSequence5 = '\123' +// Prefix (\47) is an octal escape sequence +const octalEscapeSequence6 = '\477' +const octalEscapeSequence7 = '\\77 \77 \\37 \\77' + +const notOctalEscapeSequence = `\877` +const notOctalEscapeSequence2 = `\017` +const notOctalEscapeSequence3 = `\t` +const notOctalEscapeSequence4 = `\0` +const notOctalEscapeSequence4 = `\\77 \\37 \\\\47 \\\\\\77` + +function strictFunction() { + "use strict" + + const str = `use strict` +} + var single = `single`; var double = `married`; var singleWithinDouble = `'singleWithinDouble'`; @@ -23,6 +51,26 @@ function test() { } +declare var obj: { + helloWorldString: string +} + +interface obj2 { + helloWorldString: string +} + +type objHello = typeof obj["helloWorldString"] +type objHello2 = obj2["helloWorldString"] +let helloValue = obj[`helloWorldString`] + +helloValue = obj[`helloWorldString`] + +const obj3: { + value: typeof obj["helloWorldString"] +} = { + value: obj[`helloWorldString`] +} + const callback = () => `hi` as number | string var hello: `world`; diff --git a/test/rules/quotemark/backtick/test.ts.lint b/test/rules/quotemark/backtick/test.ts.lint index e556b68b42d..99329be41f4 100644 --- a/test/rules/quotemark/backtick/test.ts.lint +++ b/test/rules/quotemark/backtick/test.ts.lint @@ -1,6 +1,40 @@ +"use strict" + import { Something } from "some-package" export { SomethingElse } from "another-package" +enum Sides { + '<- Left', + 'Right ->' +} + +const octalEscapeSequence = '\1' +const octalEscapeSequence2 = '\7' +const octalEscapeSequence3 = '\77' +const octalEscapeSequence4 = '\377' +const octalEscapeSequence5 = '\123' +// Prefix (\47) is an octal escape sequence +const octalEscapeSequence6 = '\477' +const octalEscapeSequence7 = '\\77 \77 \\37 \\77' + +const notOctalEscapeSequence = '\877' + ~~~~~~ [single] +const notOctalEscapeSequence2 = '\017' + ~~~~~~ [single] +const notOctalEscapeSequence3 = '\t' + ~~~~ [single] +const notOctalEscapeSequence4 = '\0' + ~~~~ [single] +const notOctalEscapeSequence4 = '\\77 \\37 \\\\47 \\\\\\77' + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ [single] + +function strictFunction() { + "use strict" + + const str = "use strict" + ~~~~~~~~~~~~ [double] +} + var single = 'single'; ~~~~~~~~ [single] var double = "married"; @@ -28,6 +62,29 @@ function test() { } +declare var obj: { + helloWorldString: string +} + +interface obj2 { + helloWorldString: string +} + +type objHello = typeof obj["helloWorldString"] +type objHello2 = obj2["helloWorldString"] +let helloValue = obj["helloWorldString"] + ~~~~~~~~~~~~~~~~~~ [double] + +helloValue = obj["helloWorldString"] + ~~~~~~~~~~~~~~~~~~ [double] + +const obj3: { + value: typeof obj["helloWorldString"] +} = { + value: obj["helloWorldString"] + ~~~~~~~~~~~~~~~~~~ [double] +} + const callback = () => "hi" as number | string ~~~~ [double]