From 9242ff3a85b4ce153b00007fb7b52c733d47f3da Mon Sep 17 00:00:00 2001 From: John Wiseheart Date: Sat, 22 Dec 2018 10:41:07 +0000 Subject: [PATCH 1/4] Fix quotemark --- src/rules/quotemarkRule.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index ddc0312f189..89df3bf69c3 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -155,6 +155,10 @@ function walk(ctx: Lint.WalkContext) { } } + if (actualQuoteMark === fixQuoteMark) { + return; + } + const start = node.getStart(sourceFile); let text = sourceFile.text.substring(start + 1, node.end - 1); From 475f6098e741c6c56b152c30de13032e6c7f6c56 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 10 Jan 2019 15:23:30 -0500 Subject: [PATCH 2/4] More test cases --- .../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 ++++ 8 files changed, 62 insertions(+), 6 deletions(-) delete mode 100644 test/rules/quotemark/avoid-template/tslint.json rename test/rules/quotemark/{avoid-template => double-avoid-template}/test.ts.fix (72%) rename test/rules/quotemark/{avoid-template => double-avoid-template}/test.ts.lint (65%) 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/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 72% rename from test/rules/quotemark/avoid-template/test.ts.fix rename to test/rules/quotemark/double-avoid-template/test.ts.fix index 2c73dc1dd0c..fc62157e51d 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, I'm sure" 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 65% rename from test/rules/quotemark/avoid-template/test.ts.lint rename to test/rules/quotemark/double-avoid-template/test.ts.lint index ff3d80dbcb2..e99350b6841 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, I'm sure" 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..f530953a5b3 --- /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, I'm sure" 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..d3c6eccc080 --- /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, I'm sure" 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"] + } +} From 0d4100b2f1d0a871bbf45c3024b494ba9f3b35ae Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 10 Jan 2019 16:44:22 -0500 Subject: [PATCH 3/4] Minor fixes, small refactoring --- src/rules/quotemarkRule.ts | 121 ++++++++---------- .../single-avoid-template/test.ts.fix | 2 +- .../single-avoid-template/test.ts.lint | 4 +- 3 files changed, 57 insertions(+), 70 deletions(-) diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index 89df3bf69c3..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,66 +134,59 @@ 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; } } - if (actualQuoteMark === fixQuoteMark) { - return; - } - const start = node.getStart(sourceFile); 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; } } @@ -201,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/single-avoid-template/test.ts.fix b/test/rules/quotemark/single-avoid-template/test.ts.fix index f530953a5b3..5f26f410553 100644 --- a/test/rules/quotemark/single-avoid-template/test.ts.fix +++ b/test/rules/quotemark/single-avoid-template/test.ts.fix @@ -16,5 +16,5 @@ bar foo``; `${foo}`; -this.toastCtrl.present('Please tick "Yes, I'm sure" to confirm'); +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 index d3c6eccc080..8bf33978e84 100644 --- a/test/rules/quotemark/single-avoid-template/test.ts.lint +++ b/test/rules/quotemark/single-avoid-template/test.ts.lint @@ -19,8 +19,8 @@ bar foo``; `${foo}`; -this.toastCtrl.present(`Please tick "Yes, I'm sure" to confirm`); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +this.toastCtrl.present(`Please tick "Yes" to confirm`); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] [0]: ` should be ' [1]: ` should be " From 5a6e7ba3ee128a5a5d4383039414b0d0329a7817 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 10 Jan 2019 16:48:06 -0500 Subject: [PATCH 4/4] fix double-avoid-template test case --- test/rules/quotemark/double-avoid-template/test.ts.fix | 2 +- test/rules/quotemark/double-avoid-template/test.ts.lint | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/rules/quotemark/double-avoid-template/test.ts.fix b/test/rules/quotemark/double-avoid-template/test.ts.fix index fc62157e51d..df3dfeacb23 100644 --- a/test/rules/quotemark/double-avoid-template/test.ts.fix +++ b/test/rules/quotemark/double-avoid-template/test.ts.fix @@ -16,5 +16,5 @@ bar foo``; `${foo}`; -this.toastCtrl.present('Please tick "Yes, I'm sure" to confirm'); +this.toastCtrl.present('Please tick "Yes" to confirm'); diff --git a/test/rules/quotemark/double-avoid-template/test.ts.lint b/test/rules/quotemark/double-avoid-template/test.ts.lint index e99350b6841..eb0d6f9481f 100644 --- a/test/rules/quotemark/double-avoid-template/test.ts.lint +++ b/test/rules/quotemark/double-avoid-template/test.ts.lint @@ -19,8 +19,8 @@ bar foo``; `${foo}`; -this.toastCtrl.present(`Please tick "Yes, I'm sure" to confirm`); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1] +this.toastCtrl.present(`Please tick "Yes" to confirm`); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1] [0]: ` should be " [1]: ` should be '