From 95899d6a665f1729300069038906ac643a83403e Mon Sep 17 00:00:00 2001 From: "Eric Ferreira (ericbf)" <2483476+ericbf@users.noreply.github.com> Date: Fri, 5 Apr 2019 14:30:56 -0400 Subject: [PATCH 1/4] [quotemark] Excuse more backtick edge cases This edge cases were previously flagged when they should be ignored, as changing them breaks typescript. This commit makes it so they are ignored. It also organizes a little better, using functions instead of multi-layered conditionals (it was getting confusing). --- src/rules/quotemarkRule.ts | 90 ++++++++++++++++++++-- test/rules/quotemark/backtick/test.ts.fix | 28 +++++++ test/rules/quotemark/backtick/test.ts.lint | 52 ++++++++++--- 3 files changed, 155 insertions(+), 15 deletions(-) diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index e85447a2315..88991e793e5 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -134,13 +134,14 @@ function walk(ctx: Lint.WalkContext) { (isExportDeclaration(node.parent) || // This captures `import blah from "package"` isImportDeclaration(node.parent) || - // This captures kebab-case property names in object literals (only when the node is not at the end of the parent node) - (node.parent.kind === ts.SyntaxKind.PropertyAssignment && - node.end !== node.parent.end) || - // This captures the kebab-case property names in type definitions - node.parent.kind === ts.SyntaxKind.PropertySignature || + // 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 - node.parent.parent.kind === ts.SyntaxKind.TypeReference) + isTypeConstraint(node) || + // Whether this is the type in a typeof check with older tsc + isTypeCheckWithOldTsc(node)) ) { return; } @@ -245,3 +246,80 @@ function getJSXQuotemarkPreference( // If the regular pref is backtick, use double quotes instead. return regularQuotemarkPreference !== "`" ? regularQuotemarkPreference : '"'; } + +/** + * 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) { + let parent = node.parent.parent; + + // If this node doesn't have a grandparent, it's not a type constraint + if (parent == undefined) { + return false; + } + + // Iterate through all levels of union, intersection, or parethesized types + while ( + parent.kind === ts.SyntaxKind.UnionType || + parent.kind === ts.SyntaxKind.IntersectionType || + parent.kind === ts.SyntaxKind.ParenthesizedType + ) { + parent = parent.parent; + } + + return ( + // If the next level is a type reference, the node is a type constraint + parent.kind === ts.SyntaxKind.TypeReference || + // If the next level is a type parameter, the node is a type constraint + parent.kind === ts.SyntaxKind.TypeParameter + ); +} + +/** + * Whether this node is the signature of a property or method in a type. + * @param node The node to check + * @return Whether this node is a property/method signature. + */ +function isSignature(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { + return ( + // This captures the kebab-case property names in type definitions + node.parent.kind === ts.SyntaxKind.PropertySignature || + // This captures the kebab-case method names in type definitions + node.parent.kind === ts.SyntaxKind.MethodSignature + ); +} + +/** + * Whether this node is the method or property name in an assignment/declaration. + * @param node The node to check + * @return Whether this node is the name in an assignment/decleration. + */ +function isNameInAssignment(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { + // If the node is in a property assignment or method declaration and is not at + // the end of its parent node, it is then the actual prop/method name. + return ( + // If the node is in a property assignment + (node.parent.kind === ts.SyntaxKind.PropertyAssignment || + // If the node is in a method declaration + node.parent.kind === ts.SyntaxKind.MethodDeclaration) && + // If this node is not at the end of the parent + node.end !== node.parent.end + ); +} + +function isTypeCheckWithOldTsc(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { + if (ts.version >= "2.7.1") { + // This one only affects older typescript versions (below 2.7.1) + return false; + } + + if (node.parent.kind !== ts.SyntaxKind.BinaryExpression) { + // If this isn't in a binary expression + return false; + } + + // If this node has a sibling that is a TypeOf + return node.parent.getChildren().some(n => n.kind === ts.SyntaxKind.TypeOfExpression); +} diff --git a/test/rules/quotemark/backtick/test.ts.fix b/test/rules/quotemark/backtick/test.ts.fix index 856d96dd3c1..8bf970dcb9d 100644 --- a/test/rules/quotemark/backtick/test.ts.fix +++ b/test/rules/quotemark/backtick/test.ts.fix @@ -6,7 +6,31 @@ var single = `single`; var singleWithinDouble = `'singleWithinDouble'`; var doubleWithinSingle = `"doubleWithinSingle"`; var tabNewlineWithinSingle = `tab\tNewline\nWithinSingle`; + +var array: Array<"literal string"> = []; +var arrayTwo: Array<"literal string" | number> = []; +var arrayThree: Array<"literal string" | "hello world"> = []; +var arrayFour: Array<"literal string" | "hello world" | "foo bar"> = []; var array: Array<"literal string"> = []; +var arrayTwo: Array<"literal string" & number> = []; +var arrayFour: Array<"literal string" | "hello world" & "foo bar"> = []; + +function test() { + +} + +function test() { + +} + +const callback = () => `hi` as number | string + +const v = callback() + +if (typeof v === `string`) {} + +if (typeof `string` === `number`) {} + var hello: `world`; `escaped'quotemark`; @@ -17,10 +41,14 @@ const object: { "hello-kebab" : number "kebab-case": number + "optional-prop"?: `hello-optional` + "optional-function"?(): void "another-kebab": `hello-value` } = { "hello-kebab" : 4 "kebab-case": 3, + "optional-kebab": undefined, + "optional-function"() {}, "another-kebab": `hello-value` }; diff --git a/test/rules/quotemark/backtick/test.ts.lint b/test/rules/quotemark/backtick/test.ts.lint index 19cc40ffe1a..d48050b2b25 100644 --- a/test/rules/quotemark/backtick/test.ts.lint +++ b/test/rules/quotemark/backtick/test.ts.lint @@ -2,20 +2,48 @@ import { Something } from "some-package" export { SomethingElse } from "another-package" var single = 'single'; - ~~~~~~~~ [' should be `] + ~~~~~~~~ [single] var double = "married"; - ~~~~~~~~~ [" should be `] + ~~~~~~~~~ [double] var singleWithinDouble = "'singleWithinDouble'"; - ~~~~~~~~~~~~~~~~~~~~~~ [" should be `] + ~~~~~~~~~~~~~~~~~~~~~~ [double] var doubleWithinSingle = '"doubleWithinSingle"'; - ~~~~~~~~~~~~~~~~~~~~~~ [' should be `] + ~~~~~~~~~~~~~~~~~~~~~~ [single] var tabNewlineWithinSingle = 'tab\tNewline\nWithinSingle'; - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [' should be `] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [single] + +var array: Array<"literal string"> = []; +var arrayTwo: Array<"literal string" | number> = []; +var arrayThree: Array<"literal string" | "hello world"> = []; +var arrayFour: Array<"literal string" | "hello world" | "foo bar"> = []; var array: Array<"literal string"> = []; +var arrayTwo: Array<"literal string" & number> = []; +var arrayFour: Array<"literal string" | "hello world" & "foo bar"> = []; + +function test() { + +} + +function test() { + +} + +const callback = () => "hi" as number | string + ~~~~ [double] + +const v = callback() + +if (typeof v === "string") {} + ~~~~~~~~ [double] + +if (typeof "string" === 'number') {} + ~~~~~~~~ [double] + ~~~~~~~~ [single] + var hello: "world"; - ~~~~~~~ [" should be `] + ~~~~~~~ [double] 'escaped\'quotemark'; -~~~~~~~~~~~~~~~~~~~~ [' should be `] +~~~~~~~~~~~~~~~~~~~~ [single] // "avoid-template" option is not set. `foo`; @@ -24,12 +52,18 @@ const object: { "hello-kebab" : number "kebab-case": number + "optional-prop"?: `hello-optional` + "optional-function"?(): void "another-kebab": "hello-value" - ~~~~~~~~~~~~~ [" should be `] + ~~~~~~~~~~~~~ [double] } = { "hello-kebab" : 4 "kebab-case": 3, + "optional-kebab": undefined, + "optional-function"() {}, "another-kebab": "hello-value" - ~~~~~~~~~~~~~ [" should be `] + ~~~~~~~~~~~~~ [double] }; +[single]: ' should be ` +[double]: " should be ` From 765a634022d17f62ea4e9075f617b7a88d106c23 Mon Sep 17 00:00:00 2001 From: "Eric Ferreira (ericbf)" <2483476+ericbf@users.noreply.github.com> Date: Fri, 5 Apr 2019 16:07:24 -0400 Subject: [PATCH 2/4] [quotemark] Add tests for different versions This commit adds different tests for different version ranges, as those versions behave differently. --- src/rules/quotemarkRule.ts | 27 +++++++++++++------ test/rules/quotemark/backtick/test.ts.fix | 22 --------------- test/rules/quotemark/backtick/test.ts.lint | 27 ------------------- .../quotemark/backtick/test<2.7.1.ts.fix | 11 ++++++++ .../quotemark/backtick/test<2.7.1.ts.lint | 15 +++++++++++ .../quotemark/backtick/test>=2.7.1.ts.fix | 13 +++++++++ .../quotemark/backtick/test>=2.7.1.ts.lint | 21 +++++++++++++++ 7 files changed, 79 insertions(+), 57 deletions(-) create mode 100644 test/rules/quotemark/backtick/test<2.7.1.ts.fix create mode 100644 test/rules/quotemark/backtick/test<2.7.1.ts.lint create mode 100644 test/rules/quotemark/backtick/test>=2.7.1.ts.fix create mode 100644 test/rules/quotemark/backtick/test>=2.7.1.ts.lint diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index 88991e793e5..6a6d45a43a4 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -283,11 +283,18 @@ function isTypeConstraint(node: ts.StringLiteral | ts.NoSubstitutionTemplateLite * @return Whether this node is a property/method signature. */ function isSignature(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { + let parent = node.parent; + + if (ts.version < "2.7.1" && node.parent.kind === ts.SyntaxKind.LastTypeNode) { + // In older versions, there's a "LastTypeNode" here + parent = parent.parent; + } + return ( // This captures the kebab-case property names in type definitions - node.parent.kind === ts.SyntaxKind.PropertySignature || + parent.kind === ts.SyntaxKind.PropertySignature || // This captures the kebab-case method names in type definitions - node.parent.kind === ts.SyntaxKind.MethodSignature + parent.kind === ts.SyntaxKind.MethodSignature ); } @@ -297,13 +304,17 @@ function isSignature(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) * @return Whether this node is the name in an assignment/decleration. */ function isNameInAssignment(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { - // If the node is in a property assignment or method declaration and is not at - // the end of its parent node, it is then the actual prop/method name. + if ( + node.parent.kind !== ts.SyntaxKind.PropertyAssignment && + node.parent.kind !== ts.SyntaxKind.MethodDeclaration + ) { + // If the node is neither a property assignment or method declaration, it's not a name in an assignment + return false; + } + return ( - // If the node is in a property assignment - (node.parent.kind === ts.SyntaxKind.PropertyAssignment || - // If the node is in a method declaration - node.parent.kind === ts.SyntaxKind.MethodDeclaration) && + // In typescript below 2.7.1, don't change values either + ts.version < "2.7.1" || // If this node is not at the end of the parent node.end !== node.parent.end ); diff --git a/test/rules/quotemark/backtick/test.ts.fix b/test/rules/quotemark/backtick/test.ts.fix index 8bf970dcb9d..c2c13346619 100644 --- a/test/rules/quotemark/backtick/test.ts.fix +++ b/test/rules/quotemark/backtick/test.ts.fix @@ -25,30 +25,8 @@ function test() { const callback = () => `hi` as number | string -const v = callback() - -if (typeof v === `string`) {} - -if (typeof `string` === `number`) {} - var hello: `world`; `escaped'quotemark`; // "avoid-template" option is not set. `foo`; - -const object: { - "hello-kebab" - : number - "kebab-case": number - "optional-prop"?: `hello-optional` - "optional-function"?(): void - "another-kebab": `hello-value` -} = { - "hello-kebab" - : 4 - "kebab-case": 3, - "optional-kebab": undefined, - "optional-function"() {}, - "another-kebab": `hello-value` -}; diff --git a/test/rules/quotemark/backtick/test.ts.lint b/test/rules/quotemark/backtick/test.ts.lint index d48050b2b25..e556b68b42d 100644 --- a/test/rules/quotemark/backtick/test.ts.lint +++ b/test/rules/quotemark/backtick/test.ts.lint @@ -31,15 +31,6 @@ function test() { const callback = () => "hi" as number | string ~~~~ [double] -const v = callback() - -if (typeof v === "string") {} - ~~~~~~~~ [double] - -if (typeof "string" === 'number') {} - ~~~~~~~~ [double] - ~~~~~~~~ [single] - var hello: "world"; ~~~~~~~ [double] 'escaped\'quotemark'; @@ -47,23 +38,5 @@ var hello: "world"; // "avoid-template" option is not set. `foo`; - -const object: { - "hello-kebab" - : number - "kebab-case": number - "optional-prop"?: `hello-optional` - "optional-function"?(): void - "another-kebab": "hello-value" - ~~~~~~~~~~~~~ [double] -} = { - "hello-kebab" - : 4 - "kebab-case": 3, - "optional-kebab": undefined, - "optional-function"() {}, - "another-kebab": "hello-value" - ~~~~~~~~~~~~~ [double] -}; [single]: ' should be ` [double]: " should be ` diff --git a/test/rules/quotemark/backtick/test<2.7.1.ts.fix b/test/rules/quotemark/backtick/test<2.7.1.ts.fix new file mode 100644 index 00000000000..30b5e84aaa9 --- /dev/null +++ b/test/rules/quotemark/backtick/test<2.7.1.ts.fix @@ -0,0 +1,11 @@ +if (typeof v === "string") {} + +if (typeof `string` === 'number') {} + +const object: { + "optional-prop"?: "hello-optional" + "another-kebab": "hello-value" +} = { + "optional-prop": undefined, + "another-kebab": "hello-value" +}; diff --git a/test/rules/quotemark/backtick/test<2.7.1.ts.lint b/test/rules/quotemark/backtick/test<2.7.1.ts.lint new file mode 100644 index 00000000000..c4ee7762f0c --- /dev/null +++ b/test/rules/quotemark/backtick/test<2.7.1.ts.lint @@ -0,0 +1,15 @@ +[typescript]: <2.7.1 +if (typeof v === "string") {} + +if (typeof "string" === 'number') {} + ~~~~~~~~ [double] + +const object: { + "optional-prop"?: "hello-optional" + "another-kebab": "hello-value" +} = { + "optional-prop": undefined, + "another-kebab": "hello-value" +}; +[single]: ' should be ` +[double]: " should be ` diff --git a/test/rules/quotemark/backtick/test>=2.7.1.ts.fix b/test/rules/quotemark/backtick/test>=2.7.1.ts.fix new file mode 100644 index 00000000000..f3e005b4d9f --- /dev/null +++ b/test/rules/quotemark/backtick/test>=2.7.1.ts.fix @@ -0,0 +1,13 @@ +if (typeof v === `string`) {} + +if (typeof `string` === `number`) {} + +const object: { + "optional-prop"?: `hello-optional` + "optional-function"?(): void + "another-kebab": `hello-value` +} = { + "optional-prop": undefined, + "optional-function"() {}, + "another-kebab": `hello-value` +}; diff --git a/test/rules/quotemark/backtick/test>=2.7.1.ts.lint b/test/rules/quotemark/backtick/test>=2.7.1.ts.lint new file mode 100644 index 00000000000..1f33caa5d7b --- /dev/null +++ b/test/rules/quotemark/backtick/test>=2.7.1.ts.lint @@ -0,0 +1,21 @@ +[typescript]: >=2.7.1 +if (typeof v === "string") {} + ~~~~~~~~ [double] + +if (typeof "string" === 'number') {} + ~~~~~~~~ [double] + ~~~~~~~~ [single] + +const object: { + "optional-prop"?: `hello-optional` + "optional-function"?(): void + "another-kebab": "hello-value" + ~~~~~~~~~~~~~ [double] +} = { + "optional-prop": undefined, + "optional-function"() {}, + "another-kebab": "hello-value" + ~~~~~~~~~~~~~ [double] +}; +[single]: ' should be ` +[double]: " should be ` From 4aa87c16aa8b0f197f33357472332a879ab918da Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Tue, 16 Apr 2019 17:46:00 -0400 Subject: [PATCH 3/4] Update quotemarkRule.ts --- src/rules/quotemarkRule.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index 6a6d45a43a4..8a51fbfc8e6 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -14,6 +14,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +import { lt } from "semver"; import { isExportDeclaration, isImportDeclaration, @@ -24,6 +26,7 @@ import { import * as ts from "typescript"; import * as Lint from "../index"; +import { getNormalizedTypescriptVersion } from "../verify/parse"; const OPTION_SINGLE = "single"; const OPTION_DOUBLE = "double"; @@ -285,7 +288,7 @@ function isTypeConstraint(node: ts.StringLiteral | ts.NoSubstitutionTemplateLite function isSignature(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { let parent = node.parent; - if (ts.version < "2.7.1" && node.parent.kind === ts.SyntaxKind.LastTypeNode) { + if (hasOldTscBacktickBehavior() && node.parent.kind === ts.SyntaxKind.LastTypeNode) { // In older versions, there's a "LastTypeNode" here parent = parent.parent; } @@ -313,16 +316,16 @@ function isNameInAssignment(node: ts.StringLiteral | ts.NoSubstitutionTemplateLi } return ( - // In typescript below 2.7.1, don't change values either - ts.version < "2.7.1" || + // In old typescript versions, don't change values either + hasOldTscBacktickBehavior() || // If this node is not at the end of the parent node.end !== node.parent.end ); } function isTypeCheckWithOldTsc(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { - if (ts.version >= "2.7.1") { - // This one only affects older typescript versions (below 2.7.1) + if (hasOldTscBacktickBehavior()) { + // This one only affects older typescript versions return false; } @@ -334,3 +337,7 @@ function isTypeCheckWithOldTsc(node: ts.StringLiteral | ts.NoSubstitutionTemplat // If this node has a sibling that is a TypeOf return node.parent.getChildren().some(n => n.kind === ts.SyntaxKind.TypeOfExpression); } + +function hasOldTscBacktickBehavior() { + return lt(getNormalizedTypescriptVersion(), "2.7.1"); +} From 886a8b1aa18b845105d76571a5267bea5b387f82 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Tue, 16 Apr 2019 17:48:21 -0400 Subject: [PATCH 4/4] Update quotemarkRule.ts --- 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 8a51fbfc8e6..39c3124405b 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -324,7 +324,7 @@ function isNameInAssignment(node: ts.StringLiteral | ts.NoSubstitutionTemplateLi } function isTypeCheckWithOldTsc(node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral) { - if (hasOldTscBacktickBehavior()) { + if (!hasOldTscBacktickBehavior()) { // This one only affects older typescript versions return false; }