From 1d0ff341fd161315cda239642400566d2b39c9a5 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 15:17:46 -0700 Subject: [PATCH 01/19] Skip mutating the 'ignore' flag when printing. --- packages/babel-generator/src/printer.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 6ecd82530d..26daff2f36 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -20,6 +20,7 @@ export default class Printer { _printedCommentStarts: Object; _parenPushNewlineState: ?Object; + _printedComments: WeakSet = new WeakSet(); /** * Increment indent size. @@ -484,8 +485,12 @@ export default class Printer { _printComment(comment) { if (!this._shouldPrintComment(comment)) return; + // Some plugins use this to mark comments as removed using the AST-root 'comments' property, + // where they can't manually mutate the AST node comment lists. if (comment.ignore) return; - comment.ignore = true; + + if (this._printedComments.has(comment)) return; + this._printedComments.add(comment); if (comment.start != null) { if (this._printedCommentStarts[comment.start]) return; From ca1d60103774673c61fb09eddddcb04fd8370d42 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Thu, 14 Jul 2016 23:07:28 -0700 Subject: [PATCH 02/19] Move property definitions to class props. --- packages/babel-generator/src/printer.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 26daff2f36..be79f0f1be 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -9,17 +9,17 @@ export default class Printer { constructor(format, map) { this.format = format || {}; this._buf = new Buffer(map); - this.insideAux = false; - this._printAuxAfterOnNextUserNode = false; - this._printStack = []; - this._printedCommentStarts = {}; - this._parenPushNewlineState = null; - this._indent = 0; - this.inForStatementInitCounter = 0; } - _printedCommentStarts: Object; - _parenPushNewlineState: ?Object; + insideAux: boolean = false; + inForStatementInitCounter: number = 0; + + _buf: Buffer; + _printStack: Array = []; + _indent: number = 0; + _printedCommentStarts: Object = {}; + _parenPushNewlineState: ?Object = null; + _printAuxAfterOnNextUserNode: boolean = false; _printedComments: WeakSet = new WeakSet(); /** From 5e730b18bb56eca7b06485afd0191316e9a4d49c Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Thu, 14 Jul 2016 23:26:13 -0700 Subject: [PATCH 03/19] Instantiate Whitespace in the printer. --- packages/babel-generator/src/index.js | 14 ++++---------- packages/babel-generator/src/printer.js | 5 ++++- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/babel-generator/src/index.js b/packages/babel-generator/src/index.js index f660233c6e..5e443c1df6 100644 --- a/packages/babel-generator/src/index.js +++ b/packages/babel-generator/src/index.js @@ -1,5 +1,4 @@ import detectIndent from "detect-indent"; -import Whitespace from "./whitespace"; import SourceMap from "./source-map"; import * as messages from "babel-messages"; import Printer from "./printer"; @@ -14,15 +13,11 @@ class Generator extends Printer { opts = opts || {}; const tokens = ast.tokens || []; - let format = Generator.normalizeOptions(code, opts, tokens); + let format = Generator.normalizeOptions(code, opts, tokens); + let map = opts.sourceMaps ? new SourceMap(opts, code) : null; + super(format, map, tokens); - let map = opts.sourceMaps ? new SourceMap(opts, code) : null; - - super(format, map); - - this.ast = ast; - - this._whitespace = tokens.length > 0 ? new Whitespace(tokens) : null; + this.ast = ast; } format: { @@ -42,7 +37,6 @@ class Generator extends Printer { } }; - _whitespace: Whitespace; ast: Object; /** diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index be79f0f1be..fd74a6da14 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -3,18 +3,21 @@ import repeat from "lodash/repeat"; import Buffer from "./buffer"; import * as n from "./node"; +import Whitespace from "./whitespace"; import * as t from "babel-types"; export default class Printer { - constructor(format, map) { + constructor(format, map, tokens) { this.format = format || {}; this._buf = new Buffer(map); + this._whitespace = tokens.length > 0 ? new Whitespace(tokens) : null; } insideAux: boolean = false; inForStatementInitCounter: number = 0; _buf: Buffer; + _whitespace: Whitespace; _printStack: Array = []; _indent: number = 0; _printedCommentStarts: Object = {}; From 0d5cbe610204c8ddabde69460409c59920b5c3ff Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Thu, 14 Jul 2016 23:28:08 -0700 Subject: [PATCH 04/19] Move class static helpers off class. --- packages/babel-generator/src/index.js | 165 +++++++++++++------------- 1 file changed, 82 insertions(+), 83 deletions(-) diff --git a/packages/babel-generator/src/index.js b/packages/babel-generator/src/index.js index 5e443c1df6..d16a06e299 100644 --- a/packages/babel-generator/src/index.js +++ b/packages/babel-generator/src/index.js @@ -13,7 +13,7 @@ class Generator extends Printer { opts = opts || {}; const tokens = ast.tokens || []; - let format = Generator.normalizeOptions(code, opts, tokens); + let format = normalizeOptions(code, opts, tokens); let map = opts.sourceMaps ? new SourceMap(opts, code) : null; super(format, map, tokens); @@ -39,88 +39,6 @@ class Generator extends Printer { ast: Object; - /** - * Normalize generator options, setting defaults. - * - * - Detects code indentation. - * - If `opts.compact = "auto"` and the code is over 100KB, `compact` will be set to `true`. - */ - - static normalizeOptions(code, opts, tokens) { - let style = " "; - if (code && typeof code === "string") { - let indent = detectIndent(code).indent; - if (indent && indent !== " ") style = indent; - } - - let format = { - auxiliaryCommentBefore: opts.auxiliaryCommentBefore, - auxiliaryCommentAfter: opts.auxiliaryCommentAfter, - shouldPrintComment: opts.shouldPrintComment, - retainLines: opts.retainLines, - comments: opts.comments == null || opts.comments, - compact: opts.compact, - minified: opts.minified, - concise: opts.concise, - quotes: opts.quotes || Generator.findCommonStringDelimiter(code, tokens), - indent: { - adjustMultilineComment: true, - style: style, - base: 0 - } - }; - - if (format.minified) { - format.compact = true; - } - - if (format.compact === "auto") { - format.compact = code.length > 100000; // 100KB - - if (format.compact) { - console.error("[BABEL] " + messages.get("codeGeneratorDeopt", opts.filename, "100KB")); - } - } - - if (format.compact) { - format.indent.adjustMultilineComment = false; - } - - return format; - } - - /** - * Determine if input code uses more single or double quotes. - */ - static findCommonStringDelimiter(code, tokens) { - let occurences = { - single: 0, - double: 0 - }; - - let checked = 0; - - for (let i = 0; i < tokens.length; i++) { - let token = tokens[i]; - if (token.type.label !== "string") continue; - - let raw = code.slice(token.start, token.end); - if (raw[0] === "'") { - occurences.single++; - } else { - occurences.double++; - } - - checked++; - if (checked >= 3) break; - } - if (occurences.single > occurences.double) { - return "single"; - } else { - return "double"; - } - } - /** * Generate code and sourcemap from ast. * @@ -135,6 +53,87 @@ class Generator extends Printer { } } +/** + * Normalize generator options, setting defaults. + * + * - Detects code indentation. + * - If `opts.compact = "auto"` and the code is over 100KB, `compact` will be set to `true`. + */ + +function normalizeOptions(code, opts, tokens) { + let style = " "; + if (code && typeof code === "string") { + let indent = detectIndent(code).indent; + if (indent && indent !== " ") style = indent; + } + + let format = { + auxiliaryCommentBefore: opts.auxiliaryCommentBefore, + auxiliaryCommentAfter: opts.auxiliaryCommentAfter, + shouldPrintComment: opts.shouldPrintComment, + retainLines: opts.retainLines, + comments: opts.comments == null || opts.comments, + compact: opts.compact, + minified: opts.minified, + concise: opts.concise, + quotes: opts.quotes || findCommonStringDelimiter(code, tokens), + indent: { + adjustMultilineComment: true, + style: style, + base: 0 + } + }; + + if (format.minified) { + format.compact = true; + } + + if (format.compact === "auto") { + format.compact = code.length > 100000; // 100KB + + if (format.compact) { + console.error("[BABEL] " + messages.get("codeGeneratorDeopt", opts.filename, "100KB")); + } + } + + if (format.compact) { + format.indent.adjustMultilineComment = false; + } + + return format; +} + +/** + * Determine if input code uses more single or double quotes. + */ +function findCommonStringDelimiter(code, tokens) { + let occurences = { + single: 0, + double: 0 + }; + + let checked = 0; + + for (let i = 0; i < tokens.length; i++) { + let token = tokens[i]; + if (token.type.label !== "string") continue; + + let raw = code.slice(token.start, token.end); + if (raw[0] === "'") { + occurences.single++; + } else { + occurences.double++; + } + + checked++; + if (checked >= 3) break; + } + if (occurences.single > occurences.double) { + return "single"; + } else { + return "double"; + } +} /** * We originally exported the Generator class above, but to make it extra clear that it is a private API, From a3c99278ba0617341e620666c96a82f8ba250e6d Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Thu, 14 Jul 2016 23:42:11 -0700 Subject: [PATCH 05/19] Move format definition into Printer. --- packages/babel-generator/src/index.js | 20 ++------------------ packages/babel-generator/src/printer.js | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/babel-generator/src/index.js b/packages/babel-generator/src/index.js index d16a06e299..711a335675 100644 --- a/packages/babel-generator/src/index.js +++ b/packages/babel-generator/src/index.js @@ -2,6 +2,7 @@ import detectIndent from "detect-indent"; import SourceMap from "./source-map"; import * as messages from "babel-messages"; import Printer from "./printer"; +import type {Format} from "./printer"; /** * Babel's code generator, turns an ast into code, maintaining sourcemaps, @@ -20,23 +21,6 @@ class Generator extends Printer { this.ast = ast; } - format: { - shouldPrintComment: (comment: string) => boolean; - retainLines: boolean; - comments: boolean; - auxiliaryCommentBefore: string; - auxiliaryCommentAfter: string; - compact: boolean | "auto"; - minified: boolean; - quotes: "single" | "double"; - concise: boolean; - indent: { - adjustMultilineComment: boolean; - style: string; - base: number; - } - }; - ast: Object; /** @@ -60,7 +44,7 @@ class Generator extends Printer { * - If `opts.compact = "auto"` and the code is over 100KB, `compact` will be set to `true`. */ -function normalizeOptions(code, opts, tokens) { +function normalizeOptions(code, opts, tokens): Format { let style = " "; if (code && typeof code === "string") { let indent = detectIndent(code).indent; diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index fd74a6da14..323342e54d 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -6,6 +6,23 @@ import * as n from "./node"; import Whitespace from "./whitespace"; import * as t from "babel-types"; +export type Format = { + shouldPrintComment: (comment: string) => boolean; + retainLines: boolean; + comments: boolean; + auxiliaryCommentBefore: string; + auxiliaryCommentAfter: string; + compact: boolean | "auto"; + minified: boolean; + quotes: "single" | "double"; + concise: boolean; + indent: { + adjustMultilineComment: boolean; + style: string; + base: number; + } +}; + export default class Printer { constructor(format, map, tokens) { this.format = format || {}; @@ -13,6 +30,7 @@ export default class Printer { this._whitespace = tokens.length > 0 ? new Whitespace(tokens) : null; } + format: Format; insideAux: boolean = false; inForStatementInitCounter: number = 0; From 68bc3d7dfbbffc45bc72ae39e7c8f08f870940c4 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Fri, 15 Jul 2016 00:14:42 -0700 Subject: [PATCH 06/19] Standardize on the comment format function. --- packages/babel-generator/src/index.js | 5 +++++ packages/babel-generator/src/printer.js | 15 +-------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/packages/babel-generator/src/index.js b/packages/babel-generator/src/index.js index 711a335675..124789ef28 100644 --- a/packages/babel-generator/src/index.js +++ b/packages/babel-generator/src/index.js @@ -70,6 +70,11 @@ function normalizeOptions(code, opts, tokens): Format { if (format.minified) { format.compact = true; + + format.shouldPrintComment = format.shouldPrintComment || (() => format.comments); + } else { + format.shouldPrintComment = format.shouldPrintComment || ((value) => format.comments || + (value.indexOf("@license") >= 0 || value.indexOf("@preserve") >= 0)); } if (format.compact === "auto") { diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 323342e54d..a03c4f0f31 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -490,21 +490,8 @@ export default class Printer { return (node && (leading ? node.leadingComments : node.trailingComments)) || []; } - _shouldPrintComment(comment) { - if (this.format.shouldPrintComment) { - return this.format.shouldPrintComment(comment.value); - } else { - if (!this.format.minified && - (comment.value.indexOf("@license") >= 0 || comment.value.indexOf("@preserve") >= 0)) { - return true; - } else { - return this.format.comments; - } - } - } - _printComment(comment) { - if (!this._shouldPrintComment(comment)) return; + if (!this.format.shouldPrintComment(comment.value)) return; // Some plugins use this to mark comments as removed using the AST-root 'comments' property, // where they can't manually mutate the AST node comment lists. From 0e05e9f2162036c283f1b69105f942ff64c37536 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 13:18:15 -0700 Subject: [PATCH 07/19] Rely on .space and .newline behavior instead of explicit format checks. --- packages/babel-generator/src/generators/base.js | 2 +- .../babel-generator/src/generators/expressions.js | 4 +++- .../babel-generator/src/generators/statements.js | 6 +++--- packages/babel-generator/src/printer.js | 14 ++++---------- .../simple-a-lot-of-line-comment/expected.js | 1 + 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/babel-generator/src/generators/base.js b/packages/babel-generator/src/generators/base.js index bceaf00521..d291683b6b 100644 --- a/packages/babel-generator/src/generators/base.js +++ b/packages/babel-generator/src/generators/base.js @@ -21,7 +21,7 @@ export function BlockStatement(node: Object) { if (node.directives && node.directives.length) this.newline(); this.printSequence(node.body, node, { indent: true }); - if (!this.format.retainLines && !this.format.concise) this.removeTrailingNewline(); + this.removeTrailingNewline(); this.source("end", node.loc); this.rightBrace(); diff --git a/packages/babel-generator/src/generators/expressions.js b/packages/babel-generator/src/generators/expressions.js index 38f4973665..72e7f4ac9c 100644 --- a/packages/babel-generator/src/generators/expressions.js +++ b/packages/babel-generator/src/generators/expressions.js @@ -89,6 +89,8 @@ export function Decorator(node: Object) { function commaSeparatorNewline() { this.token(","); this.newline(); + + if (!this.endsWith("\n")) this.space(); } export function CallExpression(node: Object) { @@ -97,7 +99,7 @@ export function CallExpression(node: Object) { this.token("("); - let isPrettyCall = node._prettyCall && !this.format.retainLines && !this.format.compact; + let isPrettyCall = node._prettyCall; let separator; if (isPrettyCall) { diff --git a/packages/babel-generator/src/generators/statements.js b/packages/babel-generator/src/generators/statements.js index c3229db035..3b062d06a8 100644 --- a/packages/babel-generator/src/generators/statements.js +++ b/packages/babel-generator/src/generators/statements.js @@ -209,14 +209,14 @@ function variableDeclarationIdent() { // "let " or "var " indentation. this.token(","); this.newline(); - for (let i = 0; i < 4; i++) this.space(true); + if (this.endsWith("\n")) for (let i = 0; i < 4; i++) this.space(true); } function constDeclarationIdent() { // "const " indentation. this.token(","); this.newline(); - for (let i = 0; i < 6; i++) this.space(true); + if (this.endsWith("\n")) for (let i = 0; i < 6; i++) this.space(true); } export function VariableDeclaration(node: Object, parent: Object) { @@ -247,7 +247,7 @@ export function VariableDeclaration(node: Object, parent: Object) { // let separator; - if (!this.format.compact && !this.format.concise && hasInits && !this.format.retainLines) { + if (hasInits) { separator = node.kind === "const" ? constDeclarationIdent : variableDeclarationIdent; } diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index a03c4f0f31..14139c2d16 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -512,7 +512,7 @@ export default class Printer { if (!this.endsWith("[") && !this.endsWith("{")) this.space(); - let val = comment.type === "CommentLine" ? `//${comment.value}` : `/*${comment.value}*/`; + let val = comment.type === "CommentLine" ? `//${comment.value}\n` : `/*${comment.value}*/`; // if (comment.type === "CommentBlock" && this.format.indent.adjustMultilineComment) { @@ -526,19 +526,13 @@ export default class Printer { val = val.replace(/\n(?!$)/g, `\n${repeat(" ", indentSize)}`); } - // force a newline for line comments when retainLines is set in case the next printed node - // doesn't catch up - if ((this.format.compact || this.format.concise || this.format.retainLines) && - comment.type === "CommentLine") { - val += "\n"; - } - // this.token(val); // whitespace after - this.newline((this._whitespace ? this._whitespace.getNewlinesAfter(comment) : 0) || - (comment.type === "CommentLine" ? 1 : 0)); + this.newline((this._whitespace ? this._whitespace.getNewlinesAfter(comment) : 0) + + // Subtract one to account for the line force-added above. + (comment.type === "CommentLine" ? -1 : 0)); }); } diff --git a/packages/babel-generator/test/fixtures/comments/simple-a-lot-of-line-comment/expected.js b/packages/babel-generator/test/fixtures/comments/simple-a-lot-of-line-comment/expected.js index 47869b2606..45443e8c59 100644 --- a/packages/babel-generator/test/fixtures/comments/simple-a-lot-of-line-comment/expected.js +++ b/packages/babel-generator/test/fixtures/comments/simple-a-lot-of-line-comment/expected.js @@ -20,6 +20,7 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF // THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + function test() {} // Copyright (C) 2012 Yusuke Suzuki From 6bf52b74c06f08f81da4739c64e6eeb366c42c80 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 13:41:54 -0700 Subject: [PATCH 08/19] Include newline insertion in the call sites of rightBrace. --- packages/babel-generator/src/generators/base.js | 3 +++ packages/babel-generator/src/generators/classes.js | 2 ++ packages/babel-generator/src/printer.js | 2 -- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/babel-generator/src/generators/base.js b/packages/babel-generator/src/generators/base.js index d291683b6b..a7d1bd35b2 100644 --- a/packages/babel-generator/src/generators/base.js +++ b/packages/babel-generator/src/generators/base.js @@ -24,6 +24,9 @@ export function BlockStatement(node: Object) { this.removeTrailingNewline(); this.source("end", node.loc); + + if (!this.endsWith("\n")) this.newline(); + this.rightBrace(); } else { this.source("end", node.loc); diff --git a/packages/babel-generator/src/generators/classes.js b/packages/babel-generator/src/generators/classes.js index f7b75a321e..841473324a 100644 --- a/packages/babel-generator/src/generators/classes.js +++ b/packages/babel-generator/src/generators/classes.js @@ -42,6 +42,8 @@ export function ClassBody(node: Object) { this.printSequence(node.body, node); this.dedent(); + if (!this.endsWith("\n")) this.newline(); + this.rightBrace(); } } diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 14139c2d16..facf0fb4c0 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -76,8 +76,6 @@ export default class Printer { */ rightBrace(): void { - if (!this.endsWith("\n")) this.newline(); - if (this.format.minified) { this._buf.removeLastSemicolon(); } From 38b91235cc095be4402e4db5ce9d3c7ddd2e2d53 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 14:31:46 -0700 Subject: [PATCH 09/19] Print inter-node newlines before other node items. --- packages/babel-generator/src/printer.js | 18 ++++++++++++++---- .../auxiliary-comment/overview/expected.js | 10 ++++------ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index facf0fb4c0..82f285ef44 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -1,5 +1,7 @@ /* eslint max-len: 0 */ +import find from "lodash/find"; +import findLast from "lodash/findLast"; import repeat from "lodash/repeat"; import Buffer from "./buffer"; import * as n from "./node"; @@ -282,6 +284,8 @@ export default class Printer { print(node, parent, opts = {}) { if (!node) return; + this._printNewline(true, node, parent, opts); + let oldConcise = this.format.concise; if (node._compact) { this.format.concise = true; @@ -304,8 +308,6 @@ export default class Printer { this._printLeadingComments(node, parent); - this._printNewline(true, node, parent, opts); - let loc = (t.isProgram(node) || t.isFile(node)) ? null : node.loc; this.withSource("start", loc, () => { this[node.type](node, parent); @@ -462,9 +464,17 @@ export default class Printer { if (node.start != null && !node._ignoreUserWhitespace && this._whitespace) { // user node if (leading) { - lines = this._whitespace.getNewlinesBefore(node); + const comments = node.leadingComments; + const comment = comments && find(comments, (comment) => + !!comment.loc && this.format.shouldPrintComment(comment.value)); + + lines = this._whitespace.getNewlinesBefore(comment || node); } else { - lines = this._whitespace.getNewlinesAfter(node); + const comments = node.trailingComments; + const comment = comments && findLast(comments, (comment) => + !!comment.loc && this.format.shouldPrintComment(comment.value)); + + lines = this._whitespace.getNewlinesAfter(comment || node); } } else { // generated node diff --git a/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/auxiliary-comment/overview/expected.js b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/auxiliary-comment/overview/expected.js index 10c5029307..7c0eb1860c 100644 --- a/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/auxiliary-comment/overview/expected.js +++ b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/auxiliary-comment/overview/expected.js @@ -4,8 +4,8 @@ Object.defineProperty(exports, "__esModule", { value: true }); exports.test = undefined; -/*after*/ -/*before*/require("foo"); /*after*/ + +/*after*/ /*before*/require("foo"); /*after*/ /*before*/require("foo-bar"); /*after*/ @@ -13,11 +13,9 @@ exports.test = undefined; var /*before*/_foo = require("foo2") /*after*/; -/*before*/ -var _foo2 = babelHelpers.interopRequireDefault(_foo); +/*before*/var _foo2 = babelHelpers.interopRequireDefault(_foo); -/*after*/ -var /*before*/_foo3 = require("foo3") /*after*/; +/*after*/var /*before*/_foo3 = require("foo3") /*after*/; /*before*/var /*after*/foo2 = babelHelpers.interopRequireWildcard(_foo3); From 5de7433147d1fe43476c8441b0da97662441c436 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 15:20:00 -0700 Subject: [PATCH 10/19] Avoid calls to isUserWhitespacable by explicitly marking statement lists. --- packages/babel-generator/src/generators/flow.js | 1 + packages/babel-generator/src/generators/types.js | 2 +- packages/babel-generator/src/node/index.js | 5 ----- packages/babel-generator/src/printer.js | 2 +- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/babel-generator/src/generators/flow.js b/packages/babel-generator/src/generators/flow.js index eb4845ce1c..bd3748e51f 100644 --- a/packages/babel-generator/src/generators/flow.js +++ b/packages/babel-generator/src/generators/flow.js @@ -251,6 +251,7 @@ export function ObjectTypeAnnotation(node: Object) { this.printJoin(props, node, { indent: true, + statement: true, iterator: () => { if (props.length !== 1) { this.semicolon(); diff --git a/packages/babel-generator/src/generators/types.js b/packages/babel-generator/src/generators/types.js index ccca3ba6dc..c1497250b3 100644 --- a/packages/babel-generator/src/generators/types.js +++ b/packages/babel-generator/src/generators/types.js @@ -38,7 +38,7 @@ export function ObjectExpression(node: Object) { if (props.length) { this.space(); - this.printList(props, node, { indent: true }); + this.printList(props, node, { indent: true, statement: true }); this.space(); } diff --git a/packages/babel-generator/src/node/index.js b/packages/babel-generator/src/node/index.js index 4f1d48c42b..971f478a3d 100644 --- a/packages/babel-generator/src/node/index.js +++ b/packages/babel-generator/src/node/index.js @@ -53,11 +53,6 @@ function isOrHasCallExpression(node) { } } - -export function isUserWhitespacable(node) { - return t.isUserWhitespacable(node); -} - export function needsWhitespace(node, parent, type) { if (!node) return 0; diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 82f285ef44..6608e4b419 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -448,7 +448,7 @@ export default class Printer { // Fast path since 'this.newline' does nothing when not tracking lines. if (this.format.retainLines || this.format.compact) return; - if (!opts.statement && !n.isUserWhitespacable(node, parent)) { + if (!opts.statement) { return; } From 59c19454938646221c7574872ee0d034d6dd68e8 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 15:29:14 -0700 Subject: [PATCH 11/19] Move whitespace handling into statement list printing. --- packages/babel-generator/src/printer.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 6608e4b419..8697cf8813 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -284,8 +284,6 @@ export default class Printer { print(node, parent, opts = {}) { if (!node) return; - this._printNewline(true, node, parent, opts); - let oldConcise = this.format.concise; if (node._compact) { this.format.concise = true; @@ -326,8 +324,6 @@ export default class Printer { this.format.concise = oldConcise; this.insideAux = oldInAux; - - this._printNewline(false, node, parent, opts); } _printAuxBeforeComment() { @@ -374,8 +370,6 @@ export default class Printer { if (opts.indent) this.indent(); let printOpts = { - statement: opts.statement, - addNewlines: opts.addNewlines, after: () => { if (opts.iterator) { opts.iterator(node, i); @@ -391,9 +385,17 @@ export default class Printer { } }; + const newlineOpts = { + addNewlines: opts.addNewlines, + }; + for (i = 0; i < nodes.length; i++) { node = nodes[i]; + if (!node) continue; + + if (opts.statement) this._printNewline(true, node, parent, newlineOpts); this.print(node, parent, printOpts); + if (opts.statement) this._printNewline(false, node, parent, newlineOpts); } if (opts.indent) this.dedent(); @@ -448,10 +450,6 @@ export default class Printer { // Fast path since 'this.newline' does nothing when not tracking lines. if (this.format.retainLines || this.format.compact) return; - if (!opts.statement) { - return; - } - // Fast path for concise since 'this.newline' just inserts a space when // concise formatting is in use. if (this.format.concise) { From 11ee8642fdc20034fa3e3382151d738b064a30e3 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 15:45:25 -0700 Subject: [PATCH 12/19] Drop the 'after' callback from 'print'. --- packages/babel-generator/src/printer.js | 42 +++++++++++-------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 8697cf8813..da81052b05 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -281,7 +281,7 @@ export default class Printer { } } - print(node, parent, opts = {}) { + print(node, parent) { if (!node) return; let oldConcise = this.format.concise; @@ -320,7 +320,6 @@ export default class Printer { // end this._printStack.pop(); - if (opts.after) opts.after(); this.format.concise = oldConcise; this.insideAux = oldInAux; @@ -364,37 +363,32 @@ export default class Printer { printJoin(nodes: ?Array, parent: Object, opts = {}) { if (!nodes || !nodes.length) return; - let len = nodes.length; - let node, i; - if (opts.indent) this.indent(); - let printOpts = { - after: () => { - if (opts.iterator) { - opts.iterator(node, i); - } - - if (opts.separator && parent.loc) { - this.printAuxAfterComment(); - } - - if (opts.separator && i < len - 1) { - opts.separator.call(this); - } - } - }; - const newlineOpts = { addNewlines: opts.addNewlines, }; - for (i = 0; i < nodes.length; i++) { - node = nodes[i]; + for (let i = 0; i < nodes.length; i++) { + const node = nodes[i]; if (!node) continue; if (opts.statement) this._printNewline(true, node, parent, newlineOpts); - this.print(node, parent, printOpts); + + this.print(node, parent); + + if (opts.iterator) { + opts.iterator(node, i); + } + + if (opts.separator && parent.loc) { + this.printAuxAfterComment(); + } + + if (opts.separator && i < nodes.length - 1) { + opts.separator.call(this); + } + if (opts.statement) this._printNewline(false, node, parent, newlineOpts); } From 1bbf109e8eb6ba6520d28ac6762e354d5ee46311 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 17:34:15 -0700 Subject: [PATCH 13/19] Move aux comments entirely into printer. --- .../src/generators/expressions.js | 1 - packages/babel-generator/src/index.js | 5 +-- packages/babel-generator/src/printer.js | 37 +++++++++++-------- .../auxiliary-comment/overview/expected.js | 10 ++--- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/babel-generator/src/generators/expressions.js b/packages/babel-generator/src/generators/expressions.js index 72e7f4ac9c..0c2ca52cbd 100644 --- a/packages/babel-generator/src/generators/expressions.js +++ b/packages/babel-generator/src/generators/expressions.js @@ -95,7 +95,6 @@ function commaSeparatorNewline() { export function CallExpression(node: Object) { this.print(node.callee, node); - if (node.loc) this.printAuxAfterComment(); this.token("("); diff --git a/packages/babel-generator/src/index.js b/packages/babel-generator/src/index.js index 124789ef28..5b106a401b 100644 --- a/packages/babel-generator/src/index.js +++ b/packages/babel-generator/src/index.js @@ -30,10 +30,7 @@ class Generator extends Printer { */ generate() { - this.print(this.ast); - this.printAuxAfterComment(); - - return this._buf.get(); + return super.generate(this.ast); } } diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index da81052b05..0ab1998be4 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -33,18 +33,25 @@ export default class Printer { } format: Format; - insideAux: boolean = false; inForStatementInitCounter: number = 0; _buf: Buffer; _whitespace: Whitespace; _printStack: Array = []; _indent: number = 0; + _insideAux: boolean = false; _printedCommentStarts: Object = {}; _parenPushNewlineState: ?Object = null; _printAuxAfterOnNextUserNode: boolean = false; _printedComments: WeakSet = new WeakSet(); + generate(ast) { + this.print(ast); + this._maybeAddAuxComment(); + + return this._buf.get(); + } + /** * Increment indent size. */ @@ -70,6 +77,7 @@ export default class Printer { */ semicolon(force: boolean = false): void { + this._maybeAddAuxComment(); this._append(";", !force /* queue */); } @@ -112,6 +120,7 @@ export default class Printer { word(str: string): void { if (this._endsWithWord) this._space(); + this._maybeAddAuxComment(); this._append(str); this._endsWithWord = true; @@ -133,6 +142,7 @@ export default class Printer { this._space(); } + this._maybeAddAuxComment(); this._append(str); } @@ -296,10 +306,9 @@ export default class Printer { this._printStack.push(node); - let oldInAux = this.insideAux; - this.insideAux = !node.loc; - if (!this.insideAux) this.printAuxAfterComment(); - else if (!oldInAux) this._printAuxBeforeComment(); + let oldInAux = this._insideAux; + this._insideAux = !node.loc; + this._maybeAddAuxComment(this._insideAux && !oldInAux); let needsParens = n.needsParens(node, parent, this._printStack); if (needsParens) this.token("("); @@ -311,9 +320,6 @@ export default class Printer { this[node.type](node, parent); }); - // Check again if any of our children may have left an aux comment on the stack - if (!this.insideAux) this.printAuxAfterComment(); - this._printTrailingComments(node, parent); if (needsParens) this.token(")"); @@ -322,7 +328,12 @@ export default class Printer { this._printStack.pop(); this.format.concise = oldConcise; - this.insideAux = oldInAux; + this._insideAux = oldInAux; + } + + _maybeAddAuxComment(enteredPositionlessNode) { + if (enteredPositionlessNode) this._printAuxBeforeComment(); + if (!this._insideAux) this._printAuxAfterComment(); } _printAuxBeforeComment() { @@ -338,7 +349,7 @@ export default class Printer { } } - printAuxAfterComment() { + _printAuxAfterComment() { if (!this._printAuxAfterOnNextUserNode) return; this._printAuxAfterOnNextUserNode = false; @@ -381,10 +392,6 @@ export default class Printer { opts.iterator(node, i); } - if (opts.separator && parent.loc) { - this.printAuxAfterComment(); - } - if (opts.separator && i < nodes.length - 1) { opts.separator.call(this); } @@ -527,7 +534,7 @@ export default class Printer { } // - this.token(val); + this._append(val); // whitespace after this.newline((this._whitespace ? this._whitespace.getNewlinesAfter(comment) : 0) + diff --git a/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/auxiliary-comment/overview/expected.js b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/auxiliary-comment/overview/expected.js index 7c0eb1860c..3ea207e748 100644 --- a/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/auxiliary-comment/overview/expected.js +++ b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/auxiliary-comment/overview/expected.js @@ -5,11 +5,11 @@ Object.defineProperty(exports, "__esModule", { }); exports.test = undefined; -/*after*/ /*before*/require("foo"); /*after*/ +/*after*/ /*before*/require("foo") /*after*/; -/*before*/require("foo-bar"); /*after*/ +/*before*/require("foo-bar") /*after*/; -/*before*/require("./directory/foo-bar"); /*after*/ +/*before*/require("./directory/foo-bar") /*after*/; var /*before*/_foo = require("foo2") /*after*/; @@ -29,5 +29,5 @@ var test = /*before*/exports. /*after*/test = 5; /*before*/(0, _foo4.bar) /*after*/( /*before*/_foo2.default /*after*/, /*before*/_foo5.foo /*after*/); /* my comment */ -/*before*/_foo5.foo; /*after*/ -/*before*/_foo2.default; /*after*/ +/*before*/_foo5.foo /*after*/; +/*before*/_foo2.default /*after*/; From 59fe72ee7da73fbe2be1d18fa428a76fdf2f3411 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sat, 16 Jul 2016 23:12:20 -0700 Subject: [PATCH 14/19] Avoid recalculating the current line. --- packages/babel-generator/src/printer.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 0ab1998be4..40a2880603 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -243,7 +243,9 @@ export default class Printer { // catch up to this nodes newline if we're behind const pos = loc ? loc[prop] : null; if (pos && pos.line !== null) { - while (this._buf.getCurrentLine() < pos.line) { + const count = pos.line - this._buf.getCurrentLine(); + + for (let i = 0; i < count; i++) { this._newline(); } } From 4dcec860bd885e57019683e87063da9bb7ec3503 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sun, 17 Jul 2016 10:14:29 -0700 Subject: [PATCH 15/19] Drop trailing whitespace after all newlines. --- packages/babel-generator/src/buffer.js | 7 +++---- packages/babel-generator/src/printer.js | 1 - .../edgecase/return-with-retainlines-option/expected.js | 2 +- .../test/fixtures/parentheses/terminator-break/expected.js | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/babel-generator/src/buffer.js b/packages/babel-generator/src/buffer.js index 14bd4d2e07..ed294897c0 100644 --- a/packages/babel-generator/src/buffer.js +++ b/packages/babel-generator/src/buffer.js @@ -58,6 +58,9 @@ export default class Buffer { */ queue(str: string): void { + // Drop trailing spaces when a newline is inserted. + if (str === "\n") while (this._queue.length > 0 && SPACES_RE.test(this._queue[0][0])) this._queue.shift(); + const { line, column, filename } = this._sourcePosition; this._queue.unshift([str, line, column, filename]); } @@ -86,10 +89,6 @@ export default class Buffer { } } - removeTrailingSpaces(): void { - while (this._queue.length > 0 && SPACES_RE.test(this._queue[0][0])) this._queue.shift(); - } - removeTrailingNewline(): void { if (this._queue.length > 0 && this._queue[0][0] === "\n") this._queue.shift(); } diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 40a2880603..8f1601adca 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -167,7 +167,6 @@ export default class Printer { if (this.endsWith("{\n") || this.endsWith(":\n")) i--; if (i <= 0) return; - this._buf.removeTrailingSpaces(); for (let j = 0; j < i; j++) { this._newline(); } diff --git a/packages/babel-generator/test/fixtures/edgecase/return-with-retainlines-option/expected.js b/packages/babel-generator/test/fixtures/edgecase/return-with-retainlines-option/expected.js index fd1cba7b14..809ec9aa05 100644 --- a/packages/babel-generator/test/fixtures/edgecase/return-with-retainlines-option/expected.js +++ b/packages/babel-generator/test/fixtures/edgecase/return-with-retainlines-option/expected.js @@ -6,6 +6,6 @@ function foo(l) { function foo() { return ( - 1 && 2 || + 1 && 2 || 3); } diff --git a/packages/babel-generator/test/fixtures/parentheses/terminator-break/expected.js b/packages/babel-generator/test/fixtures/parentheses/terminator-break/expected.js index e6604a1cf0..750b0e8c70 100644 --- a/packages/babel-generator/test/fixtures/parentheses/terminator-break/expected.js +++ b/packages/babel-generator/test/fixtures/parentheses/terminator-break/expected.js @@ -5,7 +5,7 @@ function foo() { } function foo() { - return( + return ( // foobar "bar" ); From fdc5b7cb5d2aff04e6bb094eb3ac3aa662d7dec9 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sun, 17 Jul 2016 10:23:51 -0700 Subject: [PATCH 16/19] Only set the source location when inserting the comment text. --- packages/babel-generator/src/printer.js | 42 ++++++++++++------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index 8f1601adca..f0f791871a 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -513,35 +513,33 @@ export default class Printer { this._printedCommentStarts[comment.start] = true; } - // Exclude comments from source mappings since they will only clutter things. - this.withSource("start", comment.loc, () => { - // whitespace before - this.newline(this._whitespace ? this._whitespace.getNewlinesBefore(comment) : 0); + // whitespace before + this.newline(this._whitespace ? this._whitespace.getNewlinesBefore(comment) : 0); - if (!this.endsWith("[") && !this.endsWith("{")) this.space(); + if (!this.endsWith("[") && !this.endsWith("{")) this.space(); - let val = comment.type === "CommentLine" ? `//${comment.value}\n` : `/*${comment.value}*/`; + let val = comment.type === "CommentLine" ? `//${comment.value}\n` : `/*${comment.value}*/`; - // - if (comment.type === "CommentBlock" && this.format.indent.adjustMultilineComment) { - let offset = comment.loc && comment.loc.start.column; - if (offset) { - let newlineRegex = new RegExp("\\n\\s{1," + offset + "}", "g"); - val = val.replace(newlineRegex, "\n"); - } - - let indentSize = Math.max(this._getIndent().length, this._buf.getCurrentColumn()); - val = val.replace(/\n(?!$)/g, `\n${repeat(" ", indentSize)}`); + // + if (comment.type === "CommentBlock" && this.format.indent.adjustMultilineComment) { + let offset = comment.loc && comment.loc.start.column; + if (offset) { + let newlineRegex = new RegExp("\\n\\s{1," + offset + "}", "g"); + val = val.replace(newlineRegex, "\n"); } - // - this._append(val); + let indentSize = Math.max(this._getIndent().length, this._buf.getCurrentColumn()); + val = val.replace(/\n(?!$)/g, `\n${repeat(" ", indentSize)}`); + } - // whitespace after - this.newline((this._whitespace ? this._whitespace.getNewlinesAfter(comment) : 0) + - // Subtract one to account for the line force-added above. - (comment.type === "CommentLine" ? -1 : 0)); + this.withSource("start", comment.loc, () => { + this._append(val); }); + + // whitespace after + this.newline((this._whitespace ? this._whitespace.getNewlinesAfter(comment) : 0) + + // Subtract one to account for the line force-added above. + (comment.type === "CommentLine" ? -1 : 0)); } _printComments(comments?: Array) { From 9f49c99774db2941e007a28513846336ecc863df Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sun, 17 Jul 2016 10:32:09 -0700 Subject: [PATCH 17/19] Drop .getLast(). --- packages/babel-generator/src/buffer.js | 28 ++++++++++++++----------- packages/babel-generator/src/printer.js | 7 +++---- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/babel-generator/src/buffer.js b/packages/babel-generator/src/buffer.js index ed294897c0..a1390c1762 100644 --- a/packages/babel-generator/src/buffer.js +++ b/packages/babel-generator/src/buffer.js @@ -97,10 +97,23 @@ export default class Buffer { if (this._queue.length > 0 && this._queue[0][0] === ";") this._queue.shift(); } - endsWith(str: string): boolean { + endsWith(suffix: string): boolean { + // Fast path to avoid iterating over this._queue. + if (suffix.length === 1) { + let last; + if (this._queue.length > 0) { + const str = this._queue[0][0]; + last = str[str.length - 1]; + } else { + last = this._last; + } + + return last === suffix; + } + const end = this._last + this._queue.reduce((acc, item) => item[0] + acc, ""); - if (str.length <= end.length) { - return end.slice(-str.length) === str; + if (suffix.length <= end.length) { + return end.slice(-suffix.length) === suffix; } // We assume that everything being matched is at most a single token plus some whitespace, @@ -108,15 +121,6 @@ export default class Buffer { return false; } - getLast(): string { - if (this._queue.length > 0) { - const last = this._queue[0][0]; - return last[last.length - 1]; - } - - return this._last; - } - hasContent(): boolean { return this._queue.length > 0 || !!this._last; } diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index f0f791871a..ad3884e617 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -131,14 +131,13 @@ export default class Printer { */ token(str: string): void { - const last = this._buf.getLast(); // space is mandatory to avoid outputting