From 8eda20a110678ce45d32a1910f9104240529fd53 Mon Sep 17 00:00:00 2001 From: "Eric Ferreira (ericbf)" <2483476+ericbf@users.noreply.github.com> Date: Wed, 1 May 2019 11:43:10 -0400 Subject: [PATCH 1/3] [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. --- src/rules/noInferredEmptyObjectTypeRule.ts | 2 +- src/rules/quotemarkRule.ts | 68 ++++++++++++++++------ test/rules/quotemark/backtick/test.ts.fix | 46 +++++++++++++++ test/rules/quotemark/backtick/test.ts.lint | 54 +++++++++++++++++ 4 files changed, 150 insertions(+), 20 deletions(-) diff --git a/src/rules/noInferredEmptyObjectTypeRule.ts b/src/rules/noInferredEmptyObjectTypeRule.ts index 3c6dbb4166d..c22be7ea6db 100644 --- a/src/rules/noInferredEmptyObjectTypeRule.ts +++ b/src/rules/noInferredEmptyObjectTypeRule.ts @@ -30,7 +30,7 @@ export class Rule extends Lint.Rules.TypedRule { options: null, optionExamples: [true], rationale: Lint.Utils.dedent` - Prior to TypeScript 3.4, generic type parameters for functions and constructors are inferred as + Prior to TypeScript 3.5, generic type parameters for functions and constructors are inferred as \`{}\` (the empty object type) by default when no type parameter is explicitly supplied or when the compiler cannot infer a more specific type. This is often undesirable as the call is meant to be of a more specific type. diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index 39c3124405b..3535fb4a91e 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) || + // Typescript converts old octal escape sequences to just the numbers therein + 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,20 @@ 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 + return /\\[1-7][0-7]?/.test(node.getText(sourceFile)); +} + +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..18563d40acc 100644 --- a/test/rules/quotemark/backtick/test.ts.fix +++ b/test/rules/quotemark/backtick/test.ts.fix @@ -1,6 +1,32 @@ +"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 notOctalEscapeSequence = `\877` +const notOctalEscapeSequence2 = `\017` +const notOctalEscapeSequence3 = `\t` +const notOctalEscapeSequence4 = `\0` + +function strictFunction() { + "use strict" + + const str = `use strict` +} + var single = `single`; var double = `married`; var singleWithinDouble = `'singleWithinDouble'`; @@ -23,6 +49,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..2d25bf7a692 100644 --- a/test/rules/quotemark/backtick/test.ts.lint +++ b/test/rules/quotemark/backtick/test.ts.lint @@ -1,6 +1,37 @@ +"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 notOctalEscapeSequence = '\877' + ~~~~~~ [single] +const notOctalEscapeSequence2 = '\017' + ~~~~~~ [single] +const notOctalEscapeSequence3 = '\t' + ~~~~ [single] +const notOctalEscapeSequence4 = '\0' + ~~~~ [single] + +function strictFunction() { + "use strict" + + const str = "use strict" + ~~~~~~~~~~~~ [double] +} + var single = 'single'; ~~~~~~~~ [single] var double = "married"; @@ -28,6 +59,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] From ee3c5698826c939a200cf6756edd442274424b9c Mon Sep 17 00:00:00 2001 From: "Eric Ferreira (ericbf)" <2483476+ericbf@users.noreply.github.com> Date: Wed, 1 May 2019 15:04:16 -0400 Subject: [PATCH 2/3] [quotemark] Fix comment on use strict check call I had copy pasted and forgotten to change it. This changes that comment. --- src/rules/quotemarkRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index 3535fb4a91e..0618685d704 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -259,7 +259,7 @@ function isNotValidToUseBackticksInNode(node: StringLiteralLike, sourceFile: ts. isEnumMember(node.parent) || // Typescript converts old octal escape sequences to just the numbers therein containsOctalEscapeSequence(node, sourceFile) || - // Typescript converts old octal escape sequences to just the numbers therein + // Use strict declarations have to be single or double quoted isUseStrictDeclaration(node) || // Lookup type parameters must be single/double quoted isLookupTypeParameter(node) From cb1fd9255cd861e7c8734796c2b8b58817f268cf Mon Sep 17 00:00:00 2001 From: Eric Ferreira <2483476+ericbf@users.noreply.github.com> Date: Sun, 5 May 2019 09:26:02 -0400 Subject: [PATCH 3/3] 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/noInferredEmptyObjectTypeRule.ts | 2 +- src/rules/quotemarkRule.ts | 17 ++++++++++++++++- test/rules/quotemark/backtick/test.ts.fix | 2 ++ test/rules/quotemark/backtick/test.ts.lint | 3 +++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/rules/noInferredEmptyObjectTypeRule.ts b/src/rules/noInferredEmptyObjectTypeRule.ts index c22be7ea6db..3c6dbb4166d 100644 --- a/src/rules/noInferredEmptyObjectTypeRule.ts +++ b/src/rules/noInferredEmptyObjectTypeRule.ts @@ -30,7 +30,7 @@ export class Rule extends Lint.Rules.TypedRule { options: null, optionExamples: [true], rationale: Lint.Utils.dedent` - Prior to TypeScript 3.5, generic type parameters for functions and constructors are inferred as + Prior to TypeScript 3.4, generic type parameters for functions and constructors are inferred as \`{}\` (the empty object type) by default when no type parameter is explicitly supplied or when the compiler cannot infer a more specific type. This is often undesirable as the call is meant to be of a more specific type. diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index 0618685d704..d98b4b586c4 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -356,7 +356,22 @@ function isTypeCheckWithOldTsc(node: StringLiteralLike) { 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 - return /\\[1-7][0-7]?/.test(node.getText(sourceFile)); + // 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) { diff --git a/test/rules/quotemark/backtick/test.ts.fix b/test/rules/quotemark/backtick/test.ts.fix index 18563d40acc..55cf8ac4dc2 100644 --- a/test/rules/quotemark/backtick/test.ts.fix +++ b/test/rules/quotemark/backtick/test.ts.fix @@ -15,11 +15,13 @@ 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" diff --git a/test/rules/quotemark/backtick/test.ts.lint b/test/rules/quotemark/backtick/test.ts.lint index 2d25bf7a692..99329be41f4 100644 --- a/test/rules/quotemark/backtick/test.ts.lint +++ b/test/rules/quotemark/backtick/test.ts.lint @@ -15,6 +15,7 @@ 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] @@ -24,6 +25,8 @@ const notOctalEscapeSequence3 = '\t' ~~~~ [single] const notOctalEscapeSequence4 = '\0' ~~~~ [single] +const notOctalEscapeSequence4 = '\\77 \\37 \\\\47 \\\\\\77' + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ [single] function strictFunction() { "use strict"