Skip to content

Commit

Permalink
Remove ast-types from comments traversal (#1744)
Browse files Browse the repository at this point in the history
It's very annoying to have to have a static definition of the ast, we should instead just traverse the objects to discover it. We just need to make sure not to have any cycles, but it's also good for debugging anyway.

This fixed a few comments already :)
  • Loading branch information
vjeux committed May 26, 2017
1 parent 7bd9516 commit ba69e9f
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 27 deletions.
31 changes: 15 additions & 16 deletions src/comments.js
@@ -1,10 +1,6 @@
"use strict";

const assert = require("assert");
const types = require("./ast-types");
const n = types.namedTypes;
const isArray = types.builtInTypes.array;
const isObject = types.builtInTypes.object;
const docBuilders = require("./doc-builders");
const concat = docBuilders.concat;
const hardline = docBuilders.hardline;
Expand All @@ -19,16 +15,19 @@ const locEnd = util.locEnd;
const getNextNonSpaceNonCommentCharacter =
util.getNextNonSpaceNonCommentCharacter;

// TODO Move a non-caching implementation of this function into ast-types,
// and implement a caching wrapper function here.
function getSortedChildNodes(node, text, resultArray) {
if (!node) {
return;
}

if (resultArray) {
if (
n.Node.check(node) &&
node &&
node.type &&
node.type !== "CommentBlock" &&
node.type !== "CommentLine" &&
node.type !== "Line" &&
node.type !== "Block" &&
node.type !== "EmptyStatement" &&
node.type !== "TemplateElement"
) {
Expand All @@ -52,10 +51,11 @@ function getSortedChildNodes(node, text, resultArray) {
}

let names;
if (isArray.check(node)) {
names = Object.keys(node);
} else if (isObject.check(node)) {
names = types.getFieldNames(node);
if (node && typeof node === "object") {
names = Object.keys(node).filter(
n =>
n !== "enclosingNode" && n !== "precedingNode" && n !== "followingNode"
);
} else {
return;
}
Expand Down Expand Up @@ -153,7 +153,7 @@ function decorateComment(node, comment, text) {
}

function attach(comments, ast, text) {
if (!isArray.check(comments)) {
if (!Array.isArray(comments)) {
return;
}

Expand Down Expand Up @@ -928,8 +928,7 @@ function printComments(path, print, options, needsSemi) {
const value = path.getValue();
const parent = path.getParentNode();
const printed = print(path);
const comments =
n.Node.check(value) && types.getFieldValue(value, "comments");
const comments = value && value.comments;

if (!comments || comments.length === 0) {
return printed;
Expand All @@ -940,8 +939,8 @@ function printComments(path, print, options, needsSemi) {

path.each(commentPath => {
const comment = commentPath.getValue();
const leading = types.getFieldValue(comment, "leading");
const trailing = types.getFieldValue(comment, "trailing");
const leading = comment.leading;
const trailing = comment.trailing;

if (leading) {
const contents = printLeadingComment(commentPath, print, options);
Expand Down
13 changes: 10 additions & 3 deletions src/parser.js
Expand Up @@ -81,8 +81,9 @@ function parseWithBabylon(text) {
]
};

let ast;
try {
return babylon.parse(text, babylonOptions);
ast = babylon.parse(text, babylonOptions);
} catch (originalError) {
try {
return babylon.parse(
Expand All @@ -93,21 +94,27 @@ function parseWithBabylon(text) {
throw originalError;
}
}
delete ast.tokens;
return ast;
}

function parseWithTypeScript(text) {
const jsx = isProbablyJsx(text);
let ast;
try {
try {
// Try passing with our best guess first.
return tryParseTypeScript(text, jsx);
ast = tryParseTypeScript(text, jsx);
} catch (e) {
// But if we get it wrong, try the opposite.
return tryParseTypeScript(text, !jsx);
ast = tryParseTypeScript(text, !jsx);
}
} catch (e) {
throw createError(e.message, e.lineNumber, e.column);
}

delete ast.tokens;
return ast;
}

function tryParseTypeScript(text, jsx) {
Expand Down
Expand Up @@ -146,8 +146,7 @@ namespace X {
}
namespace Z {
// 'y' should be a fundule here
// 'y' should be a fundule here
export import y = X.Y;
}
Expand Down
12 changes: 6 additions & 6 deletions tests/typescript/webhost/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -108,18 +108,14 @@ namespace TypeScript.WebTsc {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/// <reference path='..\\..\\src\\compiler\\tsc.ts'/>
namespace TypeScript /*text*/.WebTsc {
// Load file and read the first two bytes into a string with no interpretation
// Position must be at 0 before encoding can be changed
// [0xFF,0xFE] and [0xFE,0xFF] mean utf-16 (little or big endian), otherwise default to utf-8
// ReadText method always strips byte order mark from resulting string
namespace TypeScript.WebTsc {
declare var RealActiveXObject: { new (s: string): any };
function getWScriptSystem() {
const fso = new RealActiveXObject("Scripting.FileSystemObject");
const fileStream = new ActiveXObject("ADODB.Stream");
fileStream.Type = 2;
fileStream.Type = 2 /*text*/;
const args: string[] = [];
for (let i = 0; i < WScript.Arguments.length; i++) {
Expand All @@ -144,16 +140,20 @@ namespace TypeScript /*text*/.WebTsc {
fileStream.Charset = encoding;
fileStream.LoadFromFile(fileName);
} else {
// Load file and read the first two bytes into a string with no interpretation
fileStream.Charset = "x-ansi";
fileStream.LoadFromFile(fileName);
const bom = fileStream.ReadText(2) || "";
// Position must be at 0 before encoding can be changed
fileStream.Position = 0;
// [0xFF,0xFE] and [0xFE,0xFF] mean utf-16 (little or big endian), otherwise default to utf-8
fileStream.Charset = bom.length >= 2 &&
((bom.charCodeAt(0) === 0xff && bom.charCodeAt(1) === 0xfe) ||
(bom.charCodeAt(0) === 0xfe && bom.charCodeAt(1) === 0xff))
? "unicode"
: "utf-8";
}
// ReadText method always strips byte order mark from resulting string
return fileStream.ReadText();
} catch (e) {
throw e;
Expand Down

0 comments on commit ba69e9f

Please sign in to comment.