From 9cd8af3d67bebe06b80ca1fd00d86b88dc24e7da Mon Sep 17 00:00:00 2001 From: John Wiseheart Date: Thu, 10 Jan 2019 21:53:42 +0000 Subject: [PATCH] Fix quotemark avoid-template issues (#4408) --- src/rules/quotemarkRule.ts | 117 ++++++++---------- .../quotemark/avoid-template/tslint.json | 5 - .../test.ts.fix | 2 + .../test.ts.lint | 3 + .../double-avoid-template/tslint.json | 5 + .../quotemark/single-avoid-escape/tslint.json | 2 +- .../single-avoid-template/test.ts.fix | 20 +++ .../single-avoid-template/test.ts.lint | 26 ++++ .../single-avoid-template/tslint.json | 5 + 9 files changed, 116 insertions(+), 69 deletions(-) delete mode 100644 test/rules/quotemark/avoid-template/tslint.json rename test/rules/quotemark/{avoid-template => double-avoid-template}/test.ts.fix (75%) rename test/rules/quotemark/{avoid-template => double-avoid-template}/test.ts.lint (69%) create mode 100644 test/rules/quotemark/double-avoid-template/tslint.json create mode 100644 test/rules/quotemark/single-avoid-template/test.ts.fix create mode 100644 test/rules/quotemark/single-avoid-template/test.ts.lint create mode 100644 test/rules/quotemark/single-avoid-template/tslint.json diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index ddc0312f189..aef05c8c963 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -27,12 +27,12 @@ const OPTION_JSX_DOUBLE = "jsx-double"; const OPTION_AVOID_TEMPLATE = "avoid-template"; const OPTION_AVOID_ESCAPE = "avoid-escape"; -type QUOTE_MARK = "'" | '"' | "`"; -type JSX_QUOTE_MARK = "'" | '"'; +type QUOTEMARK = "'" | '"' | "`"; +type JSX_QUOTEMARK = "'" | '"'; interface Options { - quoteMark: QUOTE_MARK; - jsxQuoteMark: JSX_QUOTE_MARK; + quotemark: QUOTEMARK; + jsxQuotemark: JSX_QUOTEMARK; avoidEscape: boolean; avoidTemplate: boolean; } @@ -52,9 +52,11 @@ export class Rule extends Lint.Rules.AbstractRule { * \`"${OPTION_JSX_SINGLE}"\` enforces single quotes for JSX attributes. * \`"${OPTION_JSX_DOUBLE}"\` enforces double quotes for JSX attributes. * \`"${OPTION_AVOID_TEMPLATE}"\` forbids single-line untagged template strings that do not contain string interpolations. + Note that backticks may still be used if \`"${OPTION_AVOID_ESCAPE}"\` is enabled and both single and double quotes are + present in the string (the latter option takes precedence). * \`"${OPTION_AVOID_ESCAPE}"\` allows you to use the "other" quotemark in cases where escaping would normally be required. - For example, \`[true, "${OPTION_DOUBLE}", "${OPTION_AVOID_ESCAPE}"]\` would not report a failure on the string literal - \`'Hello "World"'\`.`, + For example, \`[true, "${OPTION_DOUBLE}", "${OPTION_AVOID_ESCAPE}"]\` would not report a failure on the string literal + \`'Hello "World"'\`.`, options: { type: "array", items: { @@ -87,15 +89,14 @@ export class Rule extends Lint.Rules.AbstractRule { public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { const args = this.ruleArguments; - - const quoteMark = getQuotemarkPreference(args); - const jsxQuoteMark = getJSXQuotemarkPreference(args); + const quotemark = getQuotemarkPreference(args); + const jsxQuotemark = getJSXQuotemarkPreference(args, quotemark); return this.applyWithFunction(sourceFile, walk, { avoidEscape: hasArg(OPTION_AVOID_ESCAPE), avoidTemplate: hasArg(OPTION_AVOID_TEMPLATE), - jsxQuoteMark, - quoteMark, + jsxQuotemark, + quotemark, }); function hasArg(name: string): boolean { @@ -114,19 +115,18 @@ function walk(ctx: Lint.WalkContext) { node.parent.kind !== ts.SyntaxKind.TaggedTemplateExpression && isSameLine(sourceFile, node.getStart(sourceFile), node.end)) ) { - const expectedQuoteMark = + const expectedQuotemark = node.parent.kind === ts.SyntaxKind.JsxAttribute - ? options.jsxQuoteMark - : options.quoteMark; - const actualQuoteMark = sourceFile.text[node.end - 1]; + ? options.jsxQuotemark + : options.quotemark; + const actualQuotemark = sourceFile.text[node.end - 1]; - if (actualQuoteMark === expectedQuoteMark) { + if (actualQuotemark === expectedQuotemark) { return; } - let fixQuoteMark = expectedQuoteMark; - - const needsQuoteEscapes = node.text.includes(expectedQuoteMark); + let fixQuotemark = expectedQuotemark; + const needsQuoteEscapes = node.text.includes(expectedQuotemark); // This string requires escapes to use the expected quote mark, but `avoid-escape` was passed if (needsQuoteEscapes && options.avoidEscape) { @@ -134,24 +134,25 @@ function walk(ctx: Lint.WalkContext) { return; } - // If we are expecting double quotes, use single quotes to avoid - // escaping. Otherwise, just use double quotes. - fixQuoteMark = expectedQuoteMark === '"' ? "'" : '"'; - - // It also includes the fixQuoteMark. 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. What is the desired priority - // here? - if (node.text.includes(fixQuoteMark)) { - fixQuoteMark = expectedQuoteMark === "'" ? "`" : "'"; - - // It contains all of the other kinds of quotes. Escaping is - // unavoidable, sadly. - if (node.text.includes(fixQuoteMark)) { + // If we are expecting double quotes, use single quotes to avoid escaping. + // Otherwise, just use double quotes. + const alternativeFixQuotemark = expectedQuotemark === '"' ? "'" : '"'; + + if (node.text.includes(alternativeFixQuotemark)) { + // It also includes the alternative fix quote mark. 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 === "'" ? "`" : "'"; + + if (fixQuotemark === actualQuotemark) { + // We were already using the best quote mark for this scenario + return; + } else if (node.text.includes(fixQuotemark)) { + // It contains all of the other kinds of quotes. Escaping is unavoidable, sadly. return; } + } else { + fixQuotemark = alternativeFixQuotemark; } } @@ -159,37 +160,33 @@ function walk(ctx: Lint.WalkContext) { let text = sourceFile.text.substring(start + 1, node.end - 1); if (needsQuoteEscapes) { - text = text.replace(new RegExp(fixQuoteMark, "g"), `\\${fixQuoteMark}`); + text = text.replace(new RegExp(fixQuotemark, "g"), `\\${fixQuotemark}`); } - text = text.replace(new RegExp(`\\\\${actualQuoteMark}`, "g"), actualQuoteMark); + text = text.replace(new RegExp(`\\\\${actualQuotemark}`, "g"), actualQuotemark); return ctx.addFailure( start, node.end, - Rule.FAILURE_STRING(actualQuoteMark, fixQuoteMark), - new Lint.Replacement(start, node.end - start, fixQuoteMark + text + fixQuoteMark), + Rule.FAILURE_STRING(actualQuotemark, fixQuotemark), + new Lint.Replacement(start, node.end - start, fixQuotemark + text + fixQuotemark), ); } ts.forEachChild(node, cb); }); } -function getQuotemarkPreference(args: any[]): QUOTE_MARK { - type QUOTE_PREF = typeof OPTION_SINGLE | typeof OPTION_DOUBLE | typeof OPTION_BACKTICK; - - const quoteFromOption = { - [OPTION_SINGLE]: "'", - [OPTION_DOUBLE]: '"', - [OPTION_BACKTICK]: "`", - }; - - for (const arg of args) { +function getQuotemarkPreference(ruleArguments: any[]): QUOTEMARK { + for (const arg of ruleArguments) { switch (arg) { case OPTION_SINGLE: + return "'"; case OPTION_DOUBLE: + return '"'; case OPTION_BACKTICK: - return quoteFromOption[arg as QUOTE_PREF] as QUOTE_MARK; + return "`"; + default: + continue; } } @@ -197,25 +194,19 @@ function getQuotemarkPreference(args: any[]): QUOTE_MARK { return '"'; } -function getJSXQuotemarkPreference(args: any[]): JSX_QUOTE_MARK { - type JSX_QUOTE_PREF = typeof OPTION_JSX_SINGLE | typeof OPTION_JSX_DOUBLE; - - const jsxQuoteFromOption = { - [OPTION_JSX_SINGLE]: "'", - [OPTION_JSX_DOUBLE]: '"', - }; - - for (const arg of args) { +function getJSXQuotemarkPreference(ruleArguments: any[], regularQuotemarkPreference: QUOTEMARK): JSX_QUOTEMARK { + for (const arg of ruleArguments) { switch (arg) { case OPTION_JSX_SINGLE: + return "'"; case OPTION_JSX_DOUBLE: - return jsxQuoteFromOption[arg as JSX_QUOTE_PREF] as JSX_QUOTE_MARK; + return '"'; + default: + continue; } } // The JSX preference was not found, so try to use the regular preference. // If the regular pref is backtick, use double quotes instead. - const regularQuotemark = getQuotemarkPreference(args); - - return regularQuotemark !== "`" ? regularQuotemark : '"'; + return regularQuotemarkPreference !== "`" ? regularQuotemarkPreference : '"'; } diff --git a/test/rules/quotemark/avoid-template/tslint.json b/test/rules/quotemark/avoid-template/tslint.json deleted file mode 100644 index 11c3fd965d7..00000000000 --- a/test/rules/quotemark/avoid-template/tslint.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "rules": { - "quotemark": [true, "avoid-escape", "double", "avoid-template"] - } -} diff --git a/test/rules/quotemark/avoid-template/test.ts.fix b/test/rules/quotemark/double-avoid-template/test.ts.fix similarity index 75% rename from test/rules/quotemark/avoid-template/test.ts.fix rename to test/rules/quotemark/double-avoid-template/test.ts.fix index 2c73dc1dd0c..df3dfeacb23 100644 --- a/test/rules/quotemark/avoid-template/test.ts.fix +++ b/test/rules/quotemark/double-avoid-template/test.ts.fix @@ -16,3 +16,5 @@ bar foo``; `${foo}`; +this.toastCtrl.present('Please tick "Yes" to confirm'); + diff --git a/test/rules/quotemark/avoid-template/test.ts.lint b/test/rules/quotemark/double-avoid-template/test.ts.lint similarity index 69% rename from test/rules/quotemark/avoid-template/test.ts.lint rename to test/rules/quotemark/double-avoid-template/test.ts.lint index ff3d80dbcb2..eb0d6f9481f 100644 --- a/test/rules/quotemark/avoid-template/test.ts.lint +++ b/test/rules/quotemark/double-avoid-template/test.ts.lint @@ -19,5 +19,8 @@ bar foo``; `${foo}`; +this.toastCtrl.present(`Please tick "Yes" to confirm`); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1] + [0]: ` should be " [1]: ` should be ' diff --git a/test/rules/quotemark/double-avoid-template/tslint.json b/test/rules/quotemark/double-avoid-template/tslint.json new file mode 100644 index 00000000000..dbeffb1d574 --- /dev/null +++ b/test/rules/quotemark/double-avoid-template/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "quotemark": [true, "double", "avoid-escape", "avoid-template"] + } +} diff --git a/test/rules/quotemark/single-avoid-escape/tslint.json b/test/rules/quotemark/single-avoid-escape/tslint.json index 03b9fbbe5bb..c360887f9d7 100644 --- a/test/rules/quotemark/single-avoid-escape/tslint.json +++ b/test/rules/quotemark/single-avoid-escape/tslint.json @@ -1,5 +1,5 @@ { "rules": { - "quotemark": [true, "avoid-escape", "single"] + "quotemark": [true, "single", "avoid-escape"] } } diff --git a/test/rules/quotemark/single-avoid-template/test.ts.fix b/test/rules/quotemark/single-avoid-template/test.ts.fix new file mode 100644 index 00000000000..5f26f410553 --- /dev/null +++ b/test/rules/quotemark/single-avoid-template/test.ts.fix @@ -0,0 +1,20 @@ +'fo`o'; + +"a 'quote'"; + +'a "quote"'; + +`a "quote" 'quote'`; + +// Allow multi-line templates +` +foo +bar +`; + +// Allow tagged templates and templates with substitutions +foo``; +`${foo}`; + +this.toastCtrl.present('Please tick "Yes" to confirm'); + diff --git a/test/rules/quotemark/single-avoid-template/test.ts.lint b/test/rules/quotemark/single-avoid-template/test.ts.lint new file mode 100644 index 00000000000..8bf33978e84 --- /dev/null +++ b/test/rules/quotemark/single-avoid-template/test.ts.lint @@ -0,0 +1,26 @@ +`fo\`o`; +~~~~~~~ [0] + +`a 'quote'`; +~~~~~~~~~~~ [1] + +`a "quote"`; +~~~~~~~~~~~ [0] + +`a "quote" 'quote'`; + +// Allow multi-line templates +` +foo +bar +`; + +// Allow tagged templates and templates with substitutions +foo``; +`${foo}`; + +this.toastCtrl.present(`Please tick "Yes" to confirm`); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +[0]: ` should be ' +[1]: ` should be " diff --git a/test/rules/quotemark/single-avoid-template/tslint.json b/test/rules/quotemark/single-avoid-template/tslint.json new file mode 100644 index 00000000000..f5eb55e4f0c --- /dev/null +++ b/test/rules/quotemark/single-avoid-template/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "quotemark": [true, "single", "avoid-escape", "avoid-template"] + } +}