Skip to content

Fix preserveSourceNewlines sibling node comparison (fixes extra newlines in organize imports) #42630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 40 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
cb05c28
Update package-lock.json
typescript-bot Nov 20, 2020
2f4d4c1
Update package-lock.json
typescript-bot Nov 23, 2020
b94544a
Update package-lock.json
typescript-bot Nov 26, 2020
60d63dd
Update package-lock.json
typescript-bot Nov 27, 2020
3d45ac0
Update package-lock.json
typescript-bot Dec 1, 2020
7dda7e3
Update package-lock.json
typescript-bot Dec 2, 2020
24f0d6b
Update package-lock.json
typescript-bot Dec 3, 2020
a9a8cbb
Update package-lock.json
typescript-bot Dec 4, 2020
9e42a69
Update package-lock.json
typescript-bot Dec 5, 2020
12cc980
Update package-lock.json
typescript-bot Dec 6, 2020
a43ec41
Update package-lock.json
typescript-bot Dec 9, 2020
3a6a36a
Update package-lock.json
typescript-bot Dec 11, 2020
202873a
Update package-lock.json
typescript-bot Dec 13, 2020
78ea60a
Update package-lock.json
typescript-bot Dec 16, 2020
2b0578d
Update package-lock.json
typescript-bot Dec 17, 2020
e7c832a
Update package-lock.json
typescript-bot Dec 19, 2020
62604bc
Update package-lock.json
typescript-bot Dec 21, 2020
4d807de
Update package-lock.json
typescript-bot Dec 23, 2020
dda079d
Update package-lock.json
typescript-bot Dec 24, 2020
9c121aa
Update package-lock.json
typescript-bot Dec 31, 2020
8777dc7
Update package-lock.json
typescript-bot Jan 1, 2021
f1aa0f9
Update package-lock.json
typescript-bot Jan 2, 2021
e4e4cf2
Update package-lock.json
typescript-bot Jan 5, 2021
dd79363
Update package-lock.json
typescript-bot Jan 6, 2021
2e93e31
Merge remote-tracking branch 'upstream/master'
armanio123 Jan 7, 2021
c709f17
More sophisticated check for source position comparability
andrewbranch Apr 8, 2020
67ad6b4
Merge branch 'master' of https://github.com/armanio123/TypeScript
armanio123 Jan 7, 2021
5215977
Fix organize imports by looking at the nodes positions
armanio123 Jan 7, 2021
7cdbdb9
Merge remote-tracking branch 'upstream/master' into FixOrganizeImports
armanio123 Jan 7, 2021
b51345e
Rollback formatting changes
armanio123 Jan 7, 2021
662ee6c
Added tests, fixed organizeImports algorithm
armanio123 Jan 12, 2021
aaf353e
Fix autoformatting again
armanio123 Jan 12, 2021
c40b579
Make sibling node comparison work for all lists
andrewbranch Jan 12, 2021
07930c2
Don’t run siblingNodePositionsAreComparable at all unless `preserveSo…
andrewbranch Feb 3, 2021
a368a48
Merge branch 'master' into FixOrganizeImports
andrewbranch Feb 3, 2021
de698f9
Move getNodeAtPosition back
andrewbranch Feb 3, 2021
77b1902
Optimize
andrewbranch Feb 5, 2021
bcbec24
Use node array index check instead of tree walk
andrewbranch Feb 9, 2021
af14aef
Revert unneeded change
andrewbranch Feb 9, 2021
a0bf137
Merge branch 'master' into FixOrganizeImports
andrewbranch Feb 26, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4520,16 +4520,18 @@ namespace ts {
return false;
}

// If nodes haven't been transformed, they should always be real siblings
if (!previousNode.original && !nextNode.original) {
return true;
}

if (!previousNode.parent || !nextNode.parent) {
const previousParent = getOriginalNode(previousNode).parent;
return previousParent && previousParent === getOriginalNode(nextNode).parent;
}

if (!nodeIsFirstNodeAtOrAfterPosition(currentSourceFile!, getOriginalNode(nextNode), previousNode.end)) {
return false;
}

return true;
// This check is most expensive, so everything preceding is avoiding it when possible
return nodeIsFirstNodeAtOrAfterPosition(getOriginalNode(nextNode), previousNode.end);
}

function getClosingLineTerminatorCount(parentNode: TextRange, children: readonly Node[], format: ListFormat): number {
Expand Down
9 changes: 6 additions & 3 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7119,10 +7119,11 @@ namespace ts {
}
}

export function nodeIsFirstNodeAtOrAfterPosition(sourceFile: SourceFile, node: Node, position: number): boolean {
export function nodeIsFirstNodeAtOrAfterPosition(node: Node, position: number): boolean {
if (node.pos === position) return true;
if (node.pos < position) return false;
let current: Node = sourceFile;
let current = findAncestor(node.parent, p => textRangeContainsPositionInclusive(p, position));
if (!current) return false;
let next: Node | undefined;
const getContainingChild = (child: Node) => {
if (child.pos <= position && (position < child.end || (position === child.end && (child.kind === SyntaxKind.EndOfFileToken)))) {
Expand All @@ -7133,7 +7134,9 @@ namespace ts {
}
};
while (true) {
const child = isSourceFileJS(sourceFile) && hasJSDocNodes(current) && forEach(current.jsDoc, getContainingChild) || forEachChild(current, getContainingChild);
const child: Node | undefined =
isInJSFile(current) && hasJSDocNodes(current) && forEach(current.jsDoc, getContainingChild) ||
forEachChild(current, getContainingChild);
if (child === node || next === node) {
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/utilitiesPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ namespace ts {
// All node tests in the following list should *not* reference parent pointers so that
// they may be used with transformations.
/* @internal */
export function isNode(node: Node) {
export function isNode(node: any): node is Node {
return isNodeKind(node.kind);
}

Expand Down