Skip to content

Preserve source newlines all the things #42303

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 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
f814a25
Merge branch 'FixOrganizeImports' into preserveSourceNewlinesAllTheTh…
andrewbranch Jan 12, 2021
c4cc176
Set preserveSourceNewlines to true in emitter
andrewbranch Jan 12, 2021
e5cba05
Update baselines
andrewbranch Jan 12, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
The diff you're trying to view is too large. We only load the first 3000 changed files.
29 changes: 26 additions & 3 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ namespace ts {
const printerOptions: PrinterOptions = {
removeComments: compilerOptions.removeComments,
newLine: compilerOptions.newLine,
preserveSourceNewlines: true,
noEmitHelpers: compilerOptions.noEmitHelpers,
module: compilerOptions.module,
target: compilerOptions.target,
Expand Down Expand Up @@ -920,7 +921,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;
let lastNode: Node | undefined;
Expand Down Expand Up @@ -4490,7 +4491,7 @@ 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) {
else if (siblingNodePositionsAreComparable(previousNode, nextNode)) {
if (preserveSourceNewlines) {
return getEffectiveLines(
includeComments => getLinesBetweenRangeEndAndRangeStart(
Expand All @@ -4511,6 +4512,28 @@ namespace ts {
return format & ListFormat.MultiLine ? 1 : 0;
}

function siblingNodePositionsAreComparable(previousNode: Node, nextNode: Node) {
if (nodeIsSynthesized(previousNode) || nodeIsSynthesized(nextNode) || previousNode.parent !== nextNode.parent) {
return false;
}

if (nextNode.pos < previousNode.end) {
return false;
}

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

// Get the next specifier and compare against nextNode. If they are not equal, nodes have been rearranged and positions cannot be compared.
if (!nodeIsFirstNodeAtOrAfterPosition(currentSourceFile!, getOriginalNode(nextNode), previousNode.end)) {
return false;
}

return true;
}

function getClosingLineTerminatorCount(parentNode: TextRange, children: readonly Node[], format: ListFormat): number {
if (format & ListFormat.PreserveLines || preserveSourceNewlines) {
if (format & ListFormat.PreferNewLine) {
Expand Down Expand Up @@ -4661,7 +4684,7 @@ namespace ts {
const text = isNumericLiteral(textSourceNode) ? textSourceNode.text : getTextOfNode(textSourceNode);
return jsxAttributeEscape ? `"${escapeJsxAttributeString(text)}"` :
neverAsciiEscape || (getEmitFlags(node) & EmitFlags.NoAsciiEscaping) ? `"${escapeString(text)}"` :
`"${escapeNonAsciiString(text)}"`;
`"${escapeNonAsciiString(text)}"`;
}
else {
return getLiteralTextOfNode(textSourceNode, neverAsciiEscape, jsxAttributeEscape);
Expand Down
16 changes: 0 additions & 16 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2355,22 +2355,6 @@ namespace ts {
}
}

/** Returns a token if position is in [start-of-leading-trivia, end), includes JSDoc only in JS files */
function getNodeAtPosition(sourceFile: SourceFile, position: number): Node {
let current: Node = sourceFile;
const getContainingChild = (child: Node) => {
if (child.pos <= position && (position < child.end || (position === child.end && (child.kind === SyntaxKind.EndOfFileToken)))) {
return child;
}
};
while (true) {
const child = isJavaScriptFile && hasJSDocNodes(current) && forEach(current.jsDoc, getContainingChild) || forEachChild(current, getContainingChild);
if (!child) {
return current;
}
current = child;
}
}
}

function getLibFileFromReference(ref: FileReference) {
Expand Down
50 changes: 50 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7066,4 +7066,54 @@ namespace ts {
return false;
}
}

/** Returns a token if position is in [start-of-leading-trivia, end), includes JSDoc only in JS files */
export function getNodeAtPosition(sourceFile: SourceFile, position: number): Node {
let current: Node = sourceFile;
const getContainingChild = (child: Node) => {
if (child.pos <= position && (position < child.end || (position === child.end && (child.kind === SyntaxKind.EndOfFileToken)))) {
return child;
}
};
while (true) {
const child = isSourceFileJS(sourceFile) && hasJSDocNodes(current) && forEach(current.jsDoc, getContainingChild) || forEachChild(current, getContainingChild);
if (!child) {
return current;
}
current = child;
}
}

export function nodeIsFirstNodeAtOrAfterPosition(sourceFile: SourceFile, node: Node, position: number): boolean {
if (node.pos === position) return true;
if (node.pos < position) return false;
let current: Node = sourceFile;
let next: Node | undefined;
const getContainingChild = (child: Node) => {
if (child.pos <= position && (position < child.end || (position === child.end && (child.kind === SyntaxKind.EndOfFileToken)))) {
return child;
}
else if (!next && child.pos > position) {
next = child;
}
};
while (true) {
const child = isSourceFileJS(sourceFile) && hasJSDocNodes(current) && forEach(current.jsDoc, getContainingChild) || forEachChild(current, getContainingChild);
if (child === node || next === node) {
return true;
}
if (!child) {
if (next) {
// 'position' fell between two nodes (e.g., the comma between
// two ImportSpecifiers). Instead of stopping at the parent node,
// shift forward to the next node and continue searching there.
position = next.pos;
next = undefined;
continue;
}
return false;
}
current = child;
}
}
}
38 changes: 38 additions & 0 deletions tests/baselines/reference/1.0lib-noErrors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1158,4 +1158,42 @@ MERCHANTABLITY OR NON-INFRINGEMENT.
See the Apache Version 2.0 License for specific language governing permissions
and limitations under the License.
***************************************************************************** */

/// <reference no-default-lib="true"/>





































Expand Down
17 changes: 15 additions & 2 deletions tests/baselines/reference/APISample_Watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,15 @@ watchMain();
* Please log a "breaking change" issue for any API breaking change affecting this issue
*/
exports.__esModule = true;

var ts = require("typescript");

var formatHost = {
getCanonicalFileName: function (path) { return path; },
getCurrentDirectory: ts.sys.getCurrentDirectory,
getNewLine: function () { return ts.sys.newLine; }
};

function watchMain() {
var configPath = ts.findConfigFile(/*searchPath*/ "./", ts.sys.fileExists, "tsconfig.json");
if (!configPath) {
Expand All @@ -111,7 +114,12 @@ function watchMain() {
// Between `createEmitAndSemanticDiagnosticsBuilderProgram` and `createSemanticDiagnosticsBuilderProgram`, the only difference is emit.
// For pure type-checking scenarios, or when another tool/process handles emit, using `createSemanticDiagnosticsBuilderProgram` may be more desirable.
// Note that there is another overload for `createWatchCompilerHost` that takes a set of root files.
var host = ts.createWatchCompilerHost(configPath, {}, ts.sys, ts.createSemanticDiagnosticsBuilderProgram, reportDiagnostic, reportWatchStatusChanged);
var host = ts.createWatchCompilerHost(configPath, {}, ts.sys,
ts.createSemanticDiagnosticsBuilderProgram,
reportDiagnostic,
reportWatchStatusChanged
);

// You can technically override any given hook on the host, though you probably don't need to.
// Note that we're assuming `origCreateProgram` and `origPostProgramCreate` doesn't use `this` at all.
var origCreateProgram = host.createProgram;
Expand All @@ -120,16 +128,21 @@ function watchMain() {
return origCreateProgram(rootNames, options, host, oldProgram);
};
var origPostProgramCreate = host.afterProgramCreate;

host.afterProgramCreate = function (program) {
console.log("** We finished making the program! **");
origPostProgramCreate(program);
};
// `createWatchProgram` creates an initial program, watches files, and updates the program over time.
ts.createWatchProgram(host);
}

function reportDiagnostic(diagnostic) {
console.error("Error", diagnostic.code, ":", ts.flattenDiagnosticMessageText(diagnostic.messageText, formatHost.getNewLine()));
console.error("Error", diagnostic.code, ":",
ts.flattenDiagnosticMessageText(diagnostic.messageText, formatHost.getNewLine())
);
}

/**
* Prints a diagnostic every time the watch status changes.
* This is mainly for messages like "Starting compilation" or "Compilation completed".
Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/APISample_WatchWithDefaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ watchMain();
* Please log a "breaking change" issue for any API breaking change affecting this issue
*/
exports.__esModule = true;

var ts = require("typescript");

function watchMain() {
var configPath = ts.findConfigFile(/*searchPath*/ "./", ts.sys.fileExists, "tsconfig.json");
if (!configPath) {
Expand All @@ -79,6 +81,7 @@ function watchMain() {
// For pure type-checking scenarios, or when another tool/process handles emit, using `createSemanticDiagnosticsBuilderProgram` may be more desirable.
// Note that there is another overload for `createWatchCompilerHost` that takes a set of root files.
var host = ts.createWatchCompilerHost(configPath, {}, ts.sys);

// You can technically override any given hook on the host, though you probably don't need to.
// Note that we're assuming `origCreateProgram` and `origPostProgramCreate` doesn't use `this` at all.
var origCreateProgram = host.createProgram;
Expand All @@ -87,6 +90,7 @@ function watchMain() {
return origCreateProgram(rootNames, options, host, oldProgram);
};
var origPostProgramCreate = host.afterProgramCreate;

host.afterProgramCreate = function (program) {
console.log("** We finished making the program! **");
origPostProgramCreate(program);
Expand Down
5 changes: 5 additions & 0 deletions tests/baselines/reference/APISample_WatchWithOwnWatchHost.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,14 @@ watchMain();
* Please log a "breaking change" issue for any API breaking change affecting this issue
*/
exports.__esModule = true;

var ts = require("typescript");

function watchMain() {
// get list of files and compiler options somehow
var files = [];
var options = {};

var host = {
rootFiles: files,
options: options,
Expand All @@ -91,6 +94,7 @@ function watchMain() {
watchDirectory: ts.sys.watchDirectory,
createProgram: ts.createAbstractBuilder
};

// You can technically override any given hook on the host, though you probably don't need to.
// Note that we're assuming `origCreateProgram` and `origPostProgramCreate` doesn't use `this` at all.
var origCreateProgram = host.createProgram;
Expand All @@ -99,6 +103,7 @@ function watchMain() {
return origCreateProgram(rootNames, options, host, oldProgram);
};
var origPostProgramCreate = host.afterProgramCreate;

host.afterProgramCreate = function (program) {
console.log("** We finished making the program! **");
origPostProgramCreate(program);
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/APISample_compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,13 @@ compile(process.argv.slice(2), {
*/
exports.__esModule = true;
exports.compile = void 0;

var ts = require("typescript");

function compile(fileNames, options) {
var program = ts.createProgram(fileNames, options);
var emitResult = program.emit();

var allDiagnostics = ts.getPreEmitDiagnostics(program);
allDiagnostics.forEach(function (diagnostic) {
var message = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n');
Expand Down
19 changes: 16 additions & 3 deletions tests/baselines/reference/APISample_jsdoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,33 +129,41 @@ function getSomeOtherTags(node: ts.Node) {
* Please log a "breaking change" issue for any API breaking change affecting this issue
*/
exports.__esModule = true;

var ts = require("typescript");

// excerpted from https://github.com/YousefED/typescript-json-schema
// (converted from a method and modified; for example, `this: any` to compensate, among other changes)
function parseCommentsIntoDefinition(symbol, definition, otherAnnotations) {
function parseCommentsIntoDefinition(
symbol,
definition,
otherAnnotations) {
var _this = this;
if (!symbol) {
return;
}
// the comments for a symbol
var comments = symbol.getDocumentationComment(undefined);

if (comments.length) {
definition.description = comments.map(function (comment) { return comment.kind === "lineBreak" ? comment.text : comment.text.trim().replace(/\r\n/g, "\n"); }).join("");
}

// jsdocs are separate from comments
var jsdocs = symbol.getJsDocTags();
jsdocs.forEach(function (doc) {
// if we have @TJS-... annotations, we have to parse them
var name = doc.name, text = doc.text;
if (_this.userValidationKeywords[name]) {
definition[name] = _this.parseValue(text);
}
else {
} else {
// special annotations
otherAnnotations[doc.name] = true;
}
});
}


function getAnnotations(node) {
var _this = this;
var symbol = node.symbol;
Expand All @@ -171,10 +179,12 @@ function getAnnotations(node) {
if (value !== undefined) {
result[jsDocTag.name] = value;
}

return result;
}, {});
return Object.keys(annotations).length ? annotations : undefined;
}

// these examples are artificial and mostly nonsensical
function parseSpecificTags(node) {
if (node.kind === ts.SyntaxKind.Parameter) {
Expand All @@ -193,6 +203,7 @@ function parseSpecificTags(node) {
}
}
}

function getReturnTypeFromJSDoc(node) {
if (node.kind === ts.SyntaxKind.FunctionDeclaration) {
return ts.getJSDocReturnType(node);
Expand All @@ -202,9 +213,11 @@ function getReturnTypeFromJSDoc(node) {
return type.type;
}
}

function getAllTags(node) {
ts.getJSDocTags(node);
}

function getSomeOtherTags(node) {
var tags = [];
tags.push(ts.getJSDocAugmentsTag(node));
Expand Down
Loading