Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Commit

Permalink
[quotemark] Exclude some cases from backtick rule (#4693)
Browse files Browse the repository at this point in the history
* [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.
  • Loading branch information
ericbf authored and Josh Goldberg committed May 7, 2019
1 parent d79cd18 commit c98a859
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 19 deletions.
83 changes: 64 additions & 19 deletions src/rules/quotemarkRule.ts
Expand Up @@ -17,8 +17,12 @@

import { lt } from "semver";
import {
isEnumMember,
isExportDeclaration,
isExpressionStatement,
isImportDeclaration,
isIndexedAccessTypeNode,
isLiteralTypeNode,
isNoSubstitutionTemplateLiteral,
isSameLine,
isStringLiteral,
Expand All @@ -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;
Expand Down Expand Up @@ -131,21 +136,7 @@ function walk(ctx: Lint.WalkContext<Options>) {
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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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");
}
48 changes: 48 additions & 0 deletions 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'`;
Expand All @@ -23,6 +51,26 @@ function test<T extends ("generic" & number)>() {

}

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 = <U extends "generic">() => `hi` as number | string

var hello: `world`;
Expand Down
57 changes: 57 additions & 0 deletions 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";
Expand Down Expand Up @@ -28,6 +62,29 @@ function test<T extends ("generic" & number)>() {

}

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 = <U extends "generic">() => "hi" as number | string
~~~~ [double]

Expand Down

0 comments on commit c98a859

Please sign in to comment.