Fix newline issues when adding multiple imports (#38119)

* Add new import declarations in a single TextChanges call

* Refactor
This commit is contained in:
Andrew Branch
2020-04-23 11:59:38 -07:00
committed by GitHub
parent c28bd6579d
commit 9569e8aaa4
7 changed files with 96 additions and 23 deletions

View File

@@ -844,6 +844,28 @@ namespace ts {
return to;
}
/**
* Combines two arrays, values, or undefineds into the smallest container that can accommodate the resulting set:
*
* ```
* undefined -> undefined -> undefined
* T -> undefined -> T
* T -> T -> T[]
* T[] -> undefined -> T[] (no-op)
* T[] -> T -> T[] (append)
* T[] -> T[] -> T[] (concatenate)
* ```
*/
export function combine<T>(xs: T | readonly T[] | undefined, ys: T | readonly T[] | undefined): T | readonly T[] | undefined;
export function combine<T>(xs: T | T[] | undefined, ys: T | T[] | undefined): T | T[] | undefined;
export function combine<T>(xs: T | T[] | undefined, ys: T | T[] | undefined) {
if (xs === undefined) return ys;
if (ys === undefined) return xs;
if (isArray(xs)) return isArray(ys) ? concatenate(xs, ys) : append(xs, ys);
if (isArray(ys)) return append(ys, xs);
return [xs, ys];
}
/**
* Gets the actual offset into an array for a relative offset. Negative offsets indicate a
* position offset from the end of the array.

View File

@@ -45,7 +45,6 @@ namespace ts.codefix {
// Keys are import clause node IDs.
const addToExisting = createMap<{ readonly importClauseOrBindingPattern: ImportClause | ObjectBindingPattern, defaultImport: string | undefined; readonly namedImports: string[], canUseTypeOnlyImport: boolean }>();
const newImports = createMap<Mutable<ImportsCollection & { useRequire: boolean }>>();
let lastModuleSpecifier: string | undefined;
return { addImportFromDiagnostic, addImportFromExportedSymbol, writeFixes };
function addImportFromDiagnostic(diagnostic: DiagnosticWithLocation, context: CodeFixContextBase) {
@@ -97,7 +96,6 @@ namespace ts.codefix {
let entry = newImports.get(moduleSpecifier);
if (!entry) {
newImports.set(moduleSpecifier, entry = { namedImports: [], namespaceLikeImport: undefined, typeOnly, useRequire });
lastModuleSpecifier = moduleSpecifier;
}
else {
// An import clause can only be type-only if every import fix contributing to it can be type-only.
@@ -135,10 +133,15 @@ namespace ts.codefix {
addToExisting.forEach(({ importClauseOrBindingPattern, defaultImport, namedImports, canUseTypeOnlyImport }) => {
doAddExistingFix(changeTracker, sourceFile, importClauseOrBindingPattern, defaultImport, namedImports, canUseTypeOnlyImport);
});
let newDeclarations: Statement | readonly Statement[] | undefined;
newImports.forEach(({ useRequire, ...imports }, moduleSpecifier) => {
const addDeclarations = useRequire ? addNewRequires : addNewImports;
addDeclarations(changeTracker, sourceFile, moduleSpecifier, quotePreference, imports, /*blankLineBetween*/ lastModuleSpecifier === moduleSpecifier);
const getDeclarations = useRequire ? getNewRequires : getNewImports;
newDeclarations = combine(newDeclarations, getDeclarations(moduleSpecifier, quotePreference, imports));
});
if (newDeclarations) {
insertImports(changeTracker, sourceFile, newDeclarations, /*blankLineBetween*/ true);
}
}
}
@@ -631,11 +634,11 @@ namespace ts.codefix {
}
case ImportFixKind.AddNew: {
const { importKind, moduleSpecifier, typeOnly, useRequire } = fix;
const addDeclarations = useRequire ? addNewRequires : addNewImports;
const getDeclarations = useRequire ? getNewRequires : getNewImports;
const importsCollection = importKind === ImportKind.Default ? { defaultImport: symbolName, typeOnly } :
importKind === ImportKind.Named ? { namedImports: [symbolName], typeOnly } :
{ namespaceLikeImport: { importKind, name: symbolName }, typeOnly };
addDeclarations(changes, sourceFile, moduleSpecifier, quotePreference, importsCollection, /*blankLineBetween*/ true);
insertImports(changes, sourceFile, getDeclarations(moduleSpecifier, quotePreference, importsCollection), /*blankLineBetween*/ true);
return [importKind === ImportKind.Default ? Diagnostics.Import_default_0_from_module_1 : Diagnostics.Import_0_from_module_1, symbolName, moduleSpecifier];
}
default:
@@ -717,13 +720,13 @@ namespace ts.codefix {
readonly name: string;
};
}
function addNewImports(changes: textChanges.ChangeTracker, sourceFile: SourceFile, moduleSpecifier: string, quotePreference: QuotePreference, imports: ImportsCollection, blankLineBetween: boolean): void {
function getNewImports(moduleSpecifier: string, quotePreference: QuotePreference, imports: ImportsCollection): Statement | readonly Statement[] {
const quotedModuleSpecifier = makeStringLiteral(moduleSpecifier, quotePreference);
let statements: Statement | readonly Statement[] | undefined;
if (imports.defaultImport !== undefined || imports.namedImports?.length) {
insertImport(changes, sourceFile,
makeImport(
imports.defaultImport === undefined ? undefined : createIdentifier(imports.defaultImport),
imports.namedImports?.map(n => createImportSpecifier(/*propertyName*/ undefined, createIdentifier(n))), moduleSpecifier, quotePreference, imports.typeOnly), /*blankLineBetween*/ blankLineBetween);
statements = combine(statements, makeImport(
imports.defaultImport === undefined ? undefined : createIdentifier(imports.defaultImport),
imports.namedImports?.map(n => createImportSpecifier(/*propertyName*/ undefined, createIdentifier(n))), moduleSpecifier, quotePreference, imports.typeOnly));
}
const { namespaceLikeImport, typeOnly } = imports;
if (namespaceLikeImport) {
@@ -741,12 +744,14 @@ namespace ts.codefix {
createNamespaceImport(createIdentifier(namespaceLikeImport.name)),
typeOnly),
quotedModuleSpecifier);
insertImport(changes, sourceFile, declaration, /*blankLineBetween*/ blankLineBetween);
statements = combine(statements, declaration);
}
return Debug.checkDefined(statements);
}
function addNewRequires(changes: textChanges.ChangeTracker, sourceFile: SourceFile, moduleSpecifier: string, quotePreference: QuotePreference, imports: ImportsCollection, blankLineBetween: boolean) {
function getNewRequires(moduleSpecifier: string, quotePreference: QuotePreference, imports: ImportsCollection): Statement | readonly Statement[] {
const quotedModuleSpecifier = makeStringLiteral(moduleSpecifier, quotePreference);
let statements: Statement | readonly Statement[] | undefined;
// const { default: foo, bar, etc } = require('./mod');
if (imports.defaultImport || imports.namedImports?.length) {
const bindingElements = imports.namedImports?.map(name => createBindingElement(/*dotDotDotToken*/ undefined, /*propertyName*/ undefined, name)) || [];
@@ -754,13 +759,14 @@ namespace ts.codefix {
bindingElements.unshift(createBindingElement(/*dotDotDotToken*/ undefined, "default", imports.defaultImport));
}
const declaration = createConstEqualsRequireDeclaration(createObjectBindingPattern(bindingElements), quotedModuleSpecifier);
insertImport(changes, sourceFile, declaration, blankLineBetween);
statements = combine(statements, declaration);
}
// const foo = require('./mod');
if (imports.namespaceLikeImport) {
const declaration = createConstEqualsRequireDeclaration(imports.namespaceLikeImport.name, quotedModuleSpecifier);
insertImport(changes, sourceFile, declaration, blankLineBetween);
statements = combine(statements, declaration);
}
return Debug.checkDefined(statements);
}
function createConstEqualsRequireDeclaration(name: string | ObjectBindingPattern, quotedModuleSpecifier: StringLiteral): VariableStatement {

View File

@@ -121,7 +121,7 @@ namespace ts.refactor {
const quotePreference = getQuotePreference(oldFile, preferences);
const importsFromNewFile = createOldFileImportsFromNewFile(usage.oldFileImportsFromNewFile, newModuleName, useEs6ModuleSyntax, quotePreference);
if (importsFromNewFile) {
insertImport(changes, oldFile, importsFromNewFile, /*blankLineBetween*/ true);
insertImports(changes, oldFile, importsFromNewFile, /*blankLineBetween*/ true);
}
deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker);

View File

@@ -349,11 +349,25 @@ namespace ts.textChanges {
}
public insertNodeAtTopOfFile(sourceFile: SourceFile, newNode: Statement, blankLineBetween: boolean): void {
this.insertAtTopOfFile(sourceFile, newNode, blankLineBetween);
}
public insertNodesAtTopOfFile(sourceFile: SourceFile, newNodes: readonly Statement[], blankLineBetween: boolean): void {
this.insertAtTopOfFile(sourceFile, newNodes, blankLineBetween);
}
private insertAtTopOfFile(sourceFile: SourceFile, insert: Statement | readonly Statement[], blankLineBetween: boolean): void {
const pos = getInsertionPositionAtSourceFileTop(sourceFile);
this.insertNodeAt(sourceFile, pos, newNode, {
const options = {
prefix: pos === 0 ? undefined : this.newLineCharacter,
suffix: (isLineBreak(sourceFile.text.charCodeAt(pos)) ? "" : this.newLineCharacter) + (blankLineBetween ? this.newLineCharacter : ""),
});
};
if (isArray(insert)) {
this.insertNodesAt(sourceFile, pos, insert, options);
}
else {
this.insertNodeAt(sourceFile, pos, insert, options);
}
}
public insertFirstParameter(sourceFile: SourceFile, parameters: NodeArray<ParameterDeclaration>, newParam: ParameterDeclaration): void {

View File

@@ -1871,14 +1871,23 @@ namespace ts {
return node.modifiers && find(node.modifiers, m => m.kind === kind);
}
export function insertImport(changes: textChanges.ChangeTracker, sourceFile: SourceFile, importDecl: Statement, blankLineBetween: boolean): void {
const importKindPredicate = importDecl.kind === SyntaxKind.VariableStatement ? isRequireVariableDeclarationStatement : isAnyImportSyntax;
export function insertImports(changes: textChanges.ChangeTracker, sourceFile: SourceFile, imports: Statement | readonly Statement[], blankLineBetween: boolean): void {
const decl = isArray(imports) ? imports[0] : imports;
const importKindPredicate = decl.kind === SyntaxKind.VariableStatement ? isRequireVariableDeclarationStatement : isAnyImportSyntax;
const lastImportDeclaration = findLast(sourceFile.statements, statement => importKindPredicate(statement));
if (lastImportDeclaration) {
changes.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl);
if (isArray(imports)) {
changes.insertNodesAfter(sourceFile, lastImportDeclaration, imports);
}
else {
changes.insertNodeAfter(sourceFile, lastImportDeclaration, imports);
}
}
else if (isArray(imports)) {
changes.insertNodesAtTopOfFile(sourceFile, imports, blankLineBetween);
}
else {
changes.insertNodeAtTopOfFile(sourceFile, importDecl, blankLineBetween);
changes.insertNodeAtTopOfFile(sourceFile, imports, blankLineBetween);
}
}