mirror of
https://github.com/microsoft/TypeScript.git
synced 2026-02-05 08:11:30 -06:00
Fix preserveSourceNewlines sibling node comparison (fixes extra newlines in organize imports) (#42630)
* Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * Update package-lock.json * More sophisticated check for source position comparability * Fix organize imports by looking at the nodes positions * Rollback formatting changes * Added tests, fixed organizeImports algorithm * Fix autoformatting again * Make sibling node comparison work for all lists * Don’t run siblingNodePositionsAreComparable at all unless `preserveSourceNewlines` is true * Move getNodeAtPosition back * Optimize * Use node array index check instead of tree walk * Revert unneeded change Co-authored-by: TypeScript Bot <typescriptbot@microsoft.com> Co-authored-by: Armando Aguirre <armanio123@outlook.com>
This commit is contained in:
Andrew Branch
GitHub
parent
68b0323b72
commit
3c32f6e154
@ -910,7 +910,7 @@ namespace ts {
|
||||
let containerEnd = -1;
|
||||
let declarationListContainerEnd = -1;
|
||||
let currentLineMap: readonly number[] | undefined;
|
||||
let detachedCommentsInfo: { nodePos: number, detachedCommentEndPos: number}[] | undefined;
|
||||
let detachedCommentsInfo: { nodePos: number, detachedCommentEndPos: number }[] | undefined;
|
||||
let hasWrittenComment = false;
|
||||
let commentsDisabled = !!printerOptions.removeComments;
|
||||
const { enter: enterComment, exit: exitComment } = performance.createTimerIf(extendedDiagnostics, "commentTime", "beforeComment", "afterComment");
|
||||
@ -4469,15 +4469,15 @@ namespace ts {
|
||||
// JsxText will be written with its leading whitespace, so don't add more manually.
|
||||
return 0;
|
||||
}
|
||||
else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode) && (!previousNode.parent || !nextNode.parent || previousNode.parent === nextNode.parent)) {
|
||||
if (preserveSourceNewlines && previousNode.parent && nextNode.parent) {
|
||||
return getEffectiveLines(
|
||||
includeComments => getLinesBetweenRangeEndAndRangeStart(
|
||||
previousNode,
|
||||
nextNode,
|
||||
currentSourceFile!,
|
||||
includeComments));
|
||||
}
|
||||
else if (preserveSourceNewlines && siblingNodePositionsAreComparable(previousNode, nextNode)) {
|
||||
return getEffectiveLines(
|
||||
includeComments => getLinesBetweenRangeEndAndRangeStart(
|
||||
previousNode,
|
||||
nextNode,
|
||||
currentSourceFile!,
|
||||
includeComments));
|
||||
}
|
||||
else if (!preserveSourceNewlines && !nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode)) {
|
||||
return rangeEndIsOnSameLineAsRangeStart(previousNode, nextNode, currentSourceFile!) ? 0 : 1;
|
||||
}
|
||||
else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) {
|
||||
@ -5173,6 +5173,27 @@ namespace ts {
|
||||
|
||||
}
|
||||
|
||||
function siblingNodePositionsAreComparable(previousNode: Node, nextNode: Node) {
|
||||
if (nodeIsSynthesized(previousNode) || nodeIsSynthesized(nextNode)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (nextNode.pos < previousNode.end) {
|
||||
return false;
|
||||
}
|
||||
|
||||
previousNode = getOriginalNode(previousNode);
|
||||
nextNode = getOriginalNode(nextNode);
|
||||
const parent = previousNode.parent;
|
||||
if (!parent || parent !== nextNode.parent) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const parentNodeArray = getContainingNodeArray(previousNode);
|
||||
const prevNodeIndex = parentNodeArray?.indexOf(previousNode);
|
||||
return prevNodeIndex !== undefined && prevNodeIndex > -1 && parentNodeArray!.indexOf(nextNode) === prevNodeIndex + 1;
|
||||
}
|
||||
|
||||
function emitLeadingComments(pos: number, isEmittedNode: boolean) {
|
||||
hasWrittenComment = false;
|
||||
|
||||
|
||||
@ -7097,4 +7097,71 @@ namespace ts {
|
||||
export function containsIgnoredPath(path: string) {
|
||||
return some(ignoredPaths, p => stringContains(path, p));
|
||||
}
|
||||
|
||||
export function getContainingNodeArray(node: Node): NodeArray<Node> | undefined {
|
||||
if (!node.parent) return undefined;
|
||||
switch (node.kind) {
|
||||
case SyntaxKind.TypeParameter:
|
||||
const { parent } = node as TypeParameterDeclaration;
|
||||
return parent.kind === SyntaxKind.InferType ? undefined : parent.typeParameters;
|
||||
case SyntaxKind.Parameter:
|
||||
return (node as ParameterDeclaration).parent.parameters;
|
||||
case SyntaxKind.TemplateLiteralTypeSpan:
|
||||
return (node as TemplateLiteralTypeSpan).parent.templateSpans;
|
||||
case SyntaxKind.TemplateSpan:
|
||||
return (node as TemplateSpan).parent.templateSpans;
|
||||
case SyntaxKind.Decorator:
|
||||
return (node as Decorator).parent.decorators;
|
||||
case SyntaxKind.HeritageClause:
|
||||
return (node as HeritageClause).parent.heritageClauses;
|
||||
}
|
||||
|
||||
const { parent } = node;
|
||||
if (isJSDocTag(node)) {
|
||||
return isJSDocTypeLiteral(node.parent) ? undefined : node.parent.tags;
|
||||
}
|
||||
|
||||
switch (parent.kind) {
|
||||
case SyntaxKind.TypeLiteral:
|
||||
case SyntaxKind.InterfaceDeclaration:
|
||||
return isTypeElement(node) ? (parent as TypeLiteralNode | InterfaceDeclaration).members : undefined;
|
||||
case SyntaxKind.UnionType:
|
||||
case SyntaxKind.IntersectionType:
|
||||
return (parent as UnionOrIntersectionTypeNode).types;
|
||||
case SyntaxKind.TupleType:
|
||||
case SyntaxKind.ArrayLiteralExpression:
|
||||
case SyntaxKind.CommaListExpression:
|
||||
case SyntaxKind.NamedImports:
|
||||
case SyntaxKind.NamedExports:
|
||||
return (parent as TupleTypeNode | ArrayLiteralExpression | CommaListExpression | NamedImports | NamedExports).elements;
|
||||
case SyntaxKind.ObjectLiteralExpression:
|
||||
case SyntaxKind.JsxAttributes:
|
||||
return (parent as ObjectLiteralExpressionBase<ObjectLiteralElement>).properties;
|
||||
case SyntaxKind.CallExpression:
|
||||
case SyntaxKind.NewExpression:
|
||||
return isTypeNode(node) ? (parent as CallExpression | NewExpression).typeArguments :
|
||||
(parent as CallExpression | NewExpression).expression === node ? undefined :
|
||||
(parent as CallExpression | NewExpression).arguments;
|
||||
case SyntaxKind.JsxElement:
|
||||
case SyntaxKind.JsxFragment:
|
||||
return isJsxChild(node) ? (parent as JsxElement | JsxFragment).children : undefined;
|
||||
case SyntaxKind.JsxOpeningElement:
|
||||
case SyntaxKind.JsxSelfClosingElement:
|
||||
return isTypeNode(node) ? (parent as JsxOpeningElement | JsxSelfClosingElement).typeArguments : undefined;
|
||||
case SyntaxKind.Block:
|
||||
case SyntaxKind.CaseClause:
|
||||
case SyntaxKind.DefaultClause:
|
||||
case SyntaxKind.ModuleBlock:
|
||||
return (parent as Block | CaseOrDefaultClause | ModuleBlock).statements;
|
||||
case SyntaxKind.CaseBlock:
|
||||
return (parent as CaseBlock).clauses;
|
||||
case SyntaxKind.ClassDeclaration:
|
||||
case SyntaxKind.ClassExpression:
|
||||
return isClassElement(node) ? (parent as ClassLikeDeclaration).members : undefined;
|
||||
case SyntaxKind.EnumDeclaration:
|
||||
return isEnumMember(node) ? (parent as EnumDeclaration).members : undefined;
|
||||
case SyntaxKind.SourceFile:
|
||||
return (parent as SourceFile).statements;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
32
tests/cases/fourslash/organizeImports1.ts
Normal file
32
tests/cases/fourslash/organizeImports1.ts
Normal file
@ -0,0 +1,32 @@
|
||||
/// <reference path="fourslash.ts" />
|
||||
|
||||
// Regression test for bug #41417
|
||||
|
||||
//// import {
|
||||
//// d, d as D,
|
||||
//// c,
|
||||
//// c as C, b,
|
||||
//// b as B, a
|
||||
//// } from './foo';
|
||||
//// import {
|
||||
//// h, h as H,
|
||||
//// g,
|
||||
//// g as G, f,
|
||||
//// f as F, e
|
||||
//// } from './foo';
|
||||
////
|
||||
//// console.log(a, B, b, c, C, d, D);
|
||||
//// console.log(e, f, F, g, G, H, h);
|
||||
|
||||
verify.organizeImports(
|
||||
`import {
|
||||
a, b,
|
||||
b as B, c,
|
||||
c as C, d, d as D, e, f,
|
||||
f as F, g,
|
||||
g as G, h, h as H
|
||||
} from './foo';
|
||||
|
||||
console.log(a, B, b, c, C, d, D);
|
||||
console.log(e, f, F, g, G, H, h);`
|
||||
);
|
||||
16
tests/cases/fourslash/organizeImports2.ts
Normal file
16
tests/cases/fourslash/organizeImports2.ts
Normal file
@ -0,0 +1,16 @@
|
||||
/// <reference path="fourslash.ts" />
|
||||
|
||||
// Regression test for bug #41417
|
||||
|
||||
//// import {
|
||||
//// Foo
|
||||
//// , Bar
|
||||
//// } from "foo"
|
||||
////
|
||||
//// console.log(Foo, Bar);
|
||||
|
||||
verify.organizeImports(
|
||||
`import { Bar, Foo } from "foo";
|
||||
|
||||
console.log(Foo, Bar);`
|
||||
);
|
||||
18
tests/cases/fourslash/organizeImports3.ts
Normal file
18
tests/cases/fourslash/organizeImports3.ts
Normal file
@ -0,0 +1,18 @@
|
||||
/// <reference path="fourslash.ts" />
|
||||
|
||||
// Regression test for bug #41417
|
||||
|
||||
//// import {
|
||||
//// Bar
|
||||
//// , Foo
|
||||
//// } from "foo"
|
||||
////
|
||||
//// console.log(Foo, Bar);
|
||||
|
||||
verify.organizeImports(
|
||||
`import {
|
||||
Bar,
|
||||
Foo
|
||||
} from "foo";
|
||||
|
||||
console.log(Foo, Bar);`);
|
||||
Loading…
x
Reference in New Issue
Block a user