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

Commit

Permalink
Excuse edge cases in quotemark backtick (#4588) (#4618)
Browse files Browse the repository at this point in the history
This commit fixes the cases where export statements, string literal type as generic type parameter, and string literals in object literal property assignments were incorrectly flagged. This also fixes a small lint error (specifying a type parameter same as default).
  • Loading branch information
ericbf authored and Josh Goldberg committed Mar 31, 2019
1 parent 307fba4 commit e544769
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/rules/noNullUndefinedUnionRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class Rule extends Lint.Rules.TypedRule {
}
}

function walk(ctx: Lint.WalkContext<void>, tc: ts.TypeChecker): void {
function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker): void {
return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
const type = getType(node, tc);
if (type !== undefined && isNullUndefinedUnion(type)) {
Expand Down
30 changes: 24 additions & 6 deletions src/rules/quotemarkRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
* limitations under the License.
*/
import {
isExportDeclaration,
isImportDeclaration,
isNoSubstitutionTemplateLiteral,
isPropertyAssignment,
isSameLine,
isStringLiteral,
} from "tsutils";
Expand Down Expand Up @@ -130,20 +130,37 @@ function walk(ctx: Lint.WalkContext<Options>) {
// Don't use backticks instead of single/double quotes when it breaks TypeScript syntax.
if (
expectedQuotemark === "`" &&
(isImportDeclaration(node.parent) || isPropertyAssignment(node.parent))
// This captures `export blah from "package"`
(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 literal types in generic type constraints
node.parent.parent.kind === ts.SyntaxKind.TypeReference)
) {
return;
}

// We already have the expected quotemark. Done.
if (actualQuotemark === expectedQuotemark) {
return;
}

/** The quotemark we intend to use to fix this node. */
let fixQuotemark = expectedQuotemark;
const needsQuoteEscapes = node.text.includes(expectedQuotemark);

/**
* Whether this node needs to be escaped (because
* it contains the expected quotemark).
*/
const needsToBeEscaped = node.text.includes(expectedQuotemark);

// This string requires escapes to use the expected quote mark, but `avoid-escape` was passed
if (needsQuoteEscapes && options.avoidEscape) {
if (needsToBeEscaped && options.avoidEscape) {
if (node.kind === ts.SyntaxKind.StringLiteral) {
return;
}
Expand All @@ -153,7 +170,7 @@ function walk(ctx: Lint.WalkContext<Options>) {
const alternativeFixQuotemark = expectedQuotemark === '"' ? "'" : '"';

if (node.text.includes(alternativeFixQuotemark)) {
// It also includes the alternative fix quote mark. Let's try to use single quotes instead,
// It also includes the alternative fix quotemark. Let's try to use single quotes instead,
// unless we originally expected single quotes, in which case we will try to use backticks.
// This means that we may use backtick even with avoid-template in trying to avoid escaping.
fixQuotemark = expectedQuotemark === "'" ? "`" : "'";
Expand All @@ -173,7 +190,7 @@ function walk(ctx: Lint.WalkContext<Options>) {
const start = node.getStart(sourceFile);
let text = sourceFile.text.substring(start + 1, node.end - 1);

if (needsQuoteEscapes) {
if (needsToBeEscaped) {
text = text.replace(new RegExp(fixQuotemark, "g"), `\\${fixQuotemark}`);
}

Expand All @@ -186,6 +203,7 @@ function walk(ctx: Lint.WalkContext<Options>) {
new Lint.Replacement(start, node.end - start, fixQuotemark + text + fixQuotemark),
);
}

ts.forEachChild(node, cb);
});
}
Expand Down
15 changes: 14 additions & 1 deletion test/rules/quotemark/backtick/test.ts.fix
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
import { Something } from "some-package"
export { SomethingElse } from "another-package"

var single = `single`;
var double = `married`;
var singleWithinDouble = `'singleWithinDouble'`;
var doubleWithinSingle = `"doubleWithinSingle"`;
var tabNewlineWithinSingle = `tab\tNewline\nWithinSingle`;
var array: Array<"literal string"> = [];
var hello: `world`;
`escaped'quotemark`;

// "avoid-template" option is not set.
`foo`;

const object = { "kebab-case": 3 }
const object: {
"hello-kebab"
: number
"kebab-case": number
"another-kebab": `hello-value`
} = {
"hello-kebab"
: 4
"kebab-case": 3,
"another-kebab": `hello-value`
};
18 changes: 17 additions & 1 deletion test/rules/quotemark/backtick/test.ts.lint
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Something } from "some-package"
export { SomethingElse } from "another-package"

var single = 'single';
~~~~~~~~ [' should be `]
Expand All @@ -10,10 +11,25 @@ var doubleWithinSingle = '"doubleWithinSingle"';
~~~~~~~~~~~~~~~~~~~~~~ [' should be `]
var tabNewlineWithinSingle = 'tab\tNewline\nWithinSingle';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [' should be `]
var array: Array<"literal string"> = [];
var hello: "world";
~~~~~~~ [" should be `]
'escaped\'quotemark';
~~~~~~~~~~~~~~~~~~~~ [' should be `]

// "avoid-template" option is not set.
`foo`;

const object = { "kebab-case": 3 }
const object: {
"hello-kebab"
: number
"kebab-case": number
"another-kebab": "hello-value"
~~~~~~~~~~~~~ [" should be `]
} = {
"hello-kebab"
: 4
"kebab-case": 3,
"another-kebab": "hello-value"
~~~~~~~~~~~~~ [" should be `]
};

0 comments on commit e544769

Please sign in to comment.