From e3b6df64b37826bd2f669be0c62647ddd803e94c Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Fri, 11 Aug 2017 14:26:25 -0700 Subject: [PATCH 1/2] Add support to infer the quote style for import quick fixes --- src/compiler/types.ts | 1 + src/compiler/utilities.ts | 32 +++++++++++++------ src/services/codefixes/importFixes.ts | 21 +++++++++++- .../importNameCodeFixNewImportFile6.ts | 19 +++++++++++ 4 files changed, 62 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportFile6.ts diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 1f4b47f982..0a09bb5b6e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -999,6 +999,7 @@ namespace ts { export interface StringLiteral extends LiteralExpression { kind: SyntaxKind.StringLiteral; /* @internal */ textSourceNode?: Identifier | StringLiteral | NumericLiteral; // Allows a StringLiteral to get its text from another node (used by transforms). + /* @internal */ singleQuote?: boolean; } // Note: 'brands' in our syntax nodes serve to give us a small amount of nominal typing. diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 7a382b7b57..c77d267471 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -344,15 +344,20 @@ namespace ts { // or a (possibly escaped) quoted form of the original text if it's string-like. switch (node.kind) { case SyntaxKind.StringLiteral: - return '"' + escapeText(node.text) + '"'; + if ((node).singleQuote) { + return "'" + escapeText(node.text, CharacterCodes.singleQuote) + "'"; + } + else { + return '"' + escapeText(node.text, CharacterCodes.doubleQuote) + '"'; + } case SyntaxKind.NoSubstitutionTemplateLiteral: - return "`" + escapeText(node.text) + "`"; + return "`" + escapeText(node.text, CharacterCodes.backtick) + "`"; case SyntaxKind.TemplateHead: - return "`" + escapeText(node.text) + "${"; + return "`" + escapeText(node.text, CharacterCodes.backtick) + "${"; case SyntaxKind.TemplateMiddle: - return "}" + escapeText(node.text) + "${"; + return "}" + escapeText(node.text, CharacterCodes.backtick) + "${"; case SyntaxKind.TemplateTail: - return "}" + escapeText(node.text) + "`"; + return "}" + escapeText(node.text, CharacterCodes.backtick) + "`"; case SyntaxKind.NumericLiteral: return node.text; } @@ -2356,7 +2361,9 @@ namespace ts { // the language service. These characters should be escaped when printing, and if any characters are added, // the map below must be updated. Note that this regexp *does not* include the 'delete' character. // There is no reason for this other than that JSON.stringify does not handle it either. - const escapedCharsRegExp = /[\\\"\u0000-\u001f\t\v\f\b\r\n\u2028\u2029\u0085]/g; + const doubleQuoteEscapedCharsRegExp = /[\\\"\u0000-\u001f\t\v\f\b\r\n\u2028\u2029\u0085]/g; + const singleQuoteEscapedCharsRegExp = /[\\\'\u0000-\u001f\t\v\f\b\r\n\u2028\u2029\u0085]/g; + const backtickQuoteEscapedCharsRegExp = /[\\\`\u0000-\u001f\t\v\f\b\r\n\u2028\u2029\u0085]/g; const escapedCharsMap = createMapFromTemplate({ "\0": "\\0", "\t": "\\t", @@ -2367,18 +2374,23 @@ namespace ts { "\n": "\\n", "\\": "\\\\", "\"": "\\\"", + "\'": "\\\'", + "\`": "\\\`", "\u2028": "\\u2028", // lineSeparator "\u2029": "\\u2029", // paragraphSeparator "\u0085": "\\u0085" // nextLine }); - /** * Based heavily on the abstract 'Quote'/'QuoteJSONString' operation from ECMA-262 (24.3.2.2), * but augmented for a few select characters (e.g. lineSeparator, paragraphSeparator, nextLine) * Note that this doesn't actually wrap the input in double quotes. */ - export function escapeString(s: string): string { + export function escapeString(s: string, quoteChar?: CharacterCodes.doubleQuote | CharacterCodes.singleQuote | CharacterCodes.backtick): string { + const escapedCharsRegExp = + quoteChar === CharacterCodes.backtick ? backtickQuoteEscapedCharsRegExp : + quoteChar === CharacterCodes.singleQuote ? singleQuoteEscapedCharsRegExp : + doubleQuoteEscapedCharsRegExp; return s.replace(escapedCharsRegExp, getReplacement); } @@ -2400,8 +2412,8 @@ namespace ts { } const nonAsciiCharacters = /[^\u0000-\u007F]/g; - export function escapeNonAsciiString(s: string): string { - s = escapeString(s); + export function escapeNonAsciiString(s: string, quoteChar?: CharacterCodes.doubleQuote | CharacterCodes.singleQuote | CharacterCodes.backtick): string { + s = escapeString(s, quoteChar); // Replace non-ASCII characters with '\uNNNN' escapes if any exist. // Otherwise just return the original string. return nonAsciiCharacters.test(s) ? diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index fcd02664ef..59355e2bec 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -394,7 +394,9 @@ namespace ts.codefix { : isNamespaceImport ? createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(symbolName))) : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))])); - const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, createLiteral(moduleSpecifierWithoutQuotes)); + const moduleSpecifierLiteral = createLiteral(moduleSpecifierWithoutQuotes); + moduleSpecifierLiteral.singleQuote = getSingleQuoteStyleFromExistingImports(); + const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, moduleSpecifierLiteral); if (!lastImportDeclaration) { changeTracker.insertNodeAt(sourceFile, getSourceFileImportLocation(sourceFile), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` }); } @@ -435,6 +437,23 @@ namespace ts.codefix { return position; } + function getSingleQuoteStyleFromExistingImports() { + const firstModuleSpecifier = forEach(sourceFile.statements, node => { + switch (node.kind) { + case SyntaxKind.ImportDeclaration: + return (node).moduleSpecifier; + case SyntaxKind.ImportEqualsDeclaration: + const moduleReference = (node).moduleReference; + return moduleReference.kind === SyntaxKind.ExternalModuleReference ? moduleReference.expression : undefined; + case SyntaxKind.ExportDeclaration: + return (node).moduleSpecifier; + } + }); + if (firstModuleSpecifier && isStringLiteral(firstModuleSpecifier)) { + return sourceFile.text.charCodeAt(skipTrivia(sourceFile.text, firstModuleSpecifier.pos)) === CharacterCodes.singleQuote; + } + } + function getModuleSpecifierForNewImport() { const fileName = sourceFile.fileName; const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; diff --git a/tests/cases/fourslash/importNameCodeFixNewImportFile6.ts b/tests/cases/fourslash/importNameCodeFixNewImportFile6.ts new file mode 100644 index 0000000000..421161e656 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportFile6.ts @@ -0,0 +1,19 @@ +/// + +//// [|import { v2 } from './module2'; +//// +//// f1/*0*/();|] + +// @Filename: module.ts +//// export function f1() {} +//// export var v1 = 5; + +// @Filename: module2.ts +//// export var v2 = 6; + +verify.importFixAtPosition([ +`import { v2 } from './module2'; +import { f1 } from './module'; + +f1();` +]); From 09487b8a1dab2e625af8cf22b44ac97c4285dfab Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Fri, 11 Aug 2017 15:31:09 -0700 Subject: [PATCH 2/2] Added tests, pr feedback --- src/services/codefixes/importFixes.ts | 21 +++++++++-------- ...ortNameCodeFixNewImportFileQuoteStyle0.ts} | 5 ++-- ...portNameCodeFixNewImportFileQuoteStyle1.ts | 18 +++++++++++++++ ...portNameCodeFixNewImportFileQuoteStyle2.ts | 18 +++++++++++++++ ...portNameCodeFixNewImportFileQuoteStyle3.ts | 19 +++++++++++++++ ...ameCodeFixNewImportFileQuoteStyleMixed0.ts | 23 +++++++++++++++++++ ...ameCodeFixNewImportFileQuoteStyleMixed1.ts | 23 +++++++++++++++++++ 7 files changed, 114 insertions(+), 13 deletions(-) rename tests/cases/fourslash/{importNameCodeFixNewImportFile6.ts => importNameCodeFixNewImportFileQuoteStyle0.ts} (72%) create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle1.ts create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle2.ts create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle3.ts create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyleMixed0.ts create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyleMixed1.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 59355e2bec..d0b5218403 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -439,18 +439,19 @@ namespace ts.codefix { function getSingleQuoteStyleFromExistingImports() { const firstModuleSpecifier = forEach(sourceFile.statements, node => { - switch (node.kind) { - case SyntaxKind.ImportDeclaration: - return (node).moduleSpecifier; - case SyntaxKind.ImportEqualsDeclaration: - const moduleReference = (node).moduleReference; - return moduleReference.kind === SyntaxKind.ExternalModuleReference ? moduleReference.expression : undefined; - case SyntaxKind.ExportDeclaration: - return (node).moduleSpecifier; + if (isImportDeclaration(node) || isExportDeclaration(node)) { + if (node.moduleSpecifier && isStringLiteral(node.moduleSpecifier)) { + return node.moduleSpecifier; + } + } + else if (isImportEqualsDeclaration(node)) { + if (isExternalModuleReference(node.moduleReference) && isStringLiteral(node.moduleReference.expression)) { + return node.moduleReference.expression; + } } }); - if (firstModuleSpecifier && isStringLiteral(firstModuleSpecifier)) { - return sourceFile.text.charCodeAt(skipTrivia(sourceFile.text, firstModuleSpecifier.pos)) === CharacterCodes.singleQuote; + if (firstModuleSpecifier) { + return sourceFile.text.charCodeAt(firstModuleSpecifier.getStart()) === CharacterCodes.singleQuote; } } diff --git a/tests/cases/fourslash/importNameCodeFixNewImportFile6.ts b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle0.ts similarity index 72% rename from tests/cases/fourslash/importNameCodeFixNewImportFile6.ts rename to tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle0.ts index 421161e656..6b5ef958e0 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportFile6.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle0.ts @@ -4,16 +4,15 @@ //// //// f1/*0*/();|] -// @Filename: module.ts +// @Filename: module1.ts //// export function f1() {} -//// export var v1 = 5; // @Filename: module2.ts //// export var v2 = 6; verify.importFixAtPosition([ `import { v2 } from './module2'; -import { f1 } from './module'; +import { f1 } from './module1'; f1();` ]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle1.ts b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle1.ts new file mode 100644 index 0000000000..a9a12c4195 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle1.ts @@ -0,0 +1,18 @@ +/// + +//// [|import { v2 } from "./module2"; +//// +//// f1/*0*/();|] + +// @Filename: module1.ts +//// export function f1() {} + +// @Filename: module2.ts +//// export var v2 = 6; + +verify.importFixAtPosition([ +`import { v2 } from "./module2"; +import { f1 } from "./module1"; + +f1();` +]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle2.ts b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle2.ts new file mode 100644 index 0000000000..c356e239ee --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle2.ts @@ -0,0 +1,18 @@ +/// + +//// [|import m2 = require('./module2'); +//// +//// f1/*0*/();|] + +// @Filename: module1.ts +//// export function f1() {} + +// @Filename: module2.ts +//// export var v2 = 6; + +verify.importFixAtPosition([ +`import m2 = require('./module2'); +import { f1 } from './module1'; + +f1();` +]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle3.ts b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle3.ts new file mode 100644 index 0000000000..5fa9f6e2c9 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyle3.ts @@ -0,0 +1,19 @@ +/// + +//// [|export { v2 } from './module2'; +//// +//// f1/*0*/();|] + +// @Filename: module1.ts +//// export function f1() {} + +// @Filename: module2.ts +//// export var v2 = 6; + +verify.importFixAtPosition([ +`import { f1 } from './module1'; + +export { v2 } from './module2'; + +f1();` +]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyleMixed0.ts b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyleMixed0.ts new file mode 100644 index 0000000000..2f17780df7 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyleMixed0.ts @@ -0,0 +1,23 @@ +/// + +//// [|import { v2 } from "./module2"; +//// import { v3 } from './module3'; +//// +//// f1/*0*/();|] + +// @Filename: module1.ts +//// export function f1() {} + +// @Filename: module2.ts +//// export var v2 = 6; + +// @Filename: module3.ts +//// export var v3 = 6; + +verify.importFixAtPosition([ +`import { v2 } from "./module2"; +import { v3 } from './module3'; +import { f1 } from "./module1"; + +f1();` +]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyleMixed1.ts b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyleMixed1.ts new file mode 100644 index 0000000000..ec68b3be0d --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportFileQuoteStyleMixed1.ts @@ -0,0 +1,23 @@ +/// + +//// [|import { v2 } from './module2'; +//// import { v3 } from "./module3"; +//// +//// f1/*0*/();|] + +// @Filename: module1.ts +//// export function f1() {} + +// @Filename: module2.ts +//// export var v2 = 6; + +// @Filename: module3.ts +//// export var v3 = 6; + +verify.importFixAtPosition([ +`import { v2 } from './module2'; +import { v3 } from "./module3"; +import { f1 } from './module1'; + +f1();` +]);