While trying to figure out how to handle both MemberExpression comments and IfStatement comments, I ended up doing this one as well... Sorry @yamafaktory :(
The logic is a bit annoying but works.
Fixes#487
There are currently three issues related to suboptimal rendering of MemberExpression chains. The previous implementation was trying to flatten only a single group at the same time, but it didn't work well because we didn't have the full context to be able to make decisions.
In this implementation, I'm going through the entire chain at the same time and group it into logical units and make decisions based on this. It solves all the problems I can think of and if we need to tweak it in the future, it should be easy.
Fixes#268Fixes#212Fixes#21
If there's a break inside of a call, we want to force it in the group, otherwise it may get the indentation wrong. See the real-world use case in #513Fixes#513
It turns that our hasNextLine logic needs to be tuned to skip all the trailing comments. The code is not pretty but it does the job. It looks like it fixes a bunch of things in the test cases :)
I made sure that nested inline comments are NOT valid JavaScript
```js
/* /* a */ */
Uncaught SyntaxError: Unexpected token *
```
so it is okay to do a dumb search for */ when you are in a comment
* Print \x and \u escapes in strings and regexes lowercase
Theoretically, we would want to do this for escapes int identifiers as
well. However, neither flow nor babylon preserves escapes in
identifiers. For example, `\u0061.\u{0061}` cannot be distinguished from
`a.a`. Nobody uses such escapes in real code anyway. It could also be
considered a feature that such escapes are converted to real unicode
characters.
* Update snapshots
* Normalize escapes in template literals
* Update snapshots
* [Failing test] Comments in call expression
```js
foo(
// Hi
)
```
prints
```js
foo
// Hi();
```
* add one more failing case
* Don't group last args that has comments attached
* Update snapshot
It turns out that the range is not inclusive. It incorrectly included a \n after and would expand way more objects than we intended to.
I found it while running prettier on the codebase.
I'll make 0.14.1 for this.
* Print numbers in a uniform way
- Still preserve the radix (binary, octal, hexadecimal or decimal) used
in the original source code.
- Still preserve scientific notation.
- Add 0 to fractions. `.1` -> `0.1`
- Remove trailing dots from integers. `1.` -> `1`
- Always print the radix letters lowercase. `0b`, `0o`, `0x`
- Always print scientific notation lowercase. `1e1`
- Always print hexadecimal digits uppercase. `0x123ABCDEF`
- Remove unneeded plus in scientific notation. `1e+1` -> `1e1`
- Remove unneeded zeroes in scientific notation. `1e-001` -> `1e-1`
- Preserve leading zeroes in non-decimal radix. This can be useful when
working in binary, and having both `0b111000` and `0b000111` for
example.
* Always print numbers lowercase
* Remove trailing dot in scientific notation
* Update snapshots
Another attempt at solving the issue where objects are not expanded the way people expect. If there's any new line in the original source, it's going to expand it. This gives more control to the user in how the objects should be formatted.
Fixes#74
It turns out that in an unlikely turn of event, the inner group can be inline and not print the opening paren but the outer group breaks and outputs the closing paren which generates invalid JavaScript.
I tried removing the group altogether and no tests failed, so I'm assuming the group wasn't needed in the first place. If it was, we should add tests to cover this.
Fixes#501
* Proper support for dangling comments
In one code path, the dangling comment case is not properly handled. So I added the dangling comment, but it turns out that we only print manually in two node types: Program and BlockStatement. I made the generic printComment function print it everywhere but those two nodes. I tried to get rid of those special cases but unfortunately we need them there otherwise they are not printed at the right place.
Fixes#20
* Output dangling comments in specific places
The logic was already working, it was just special-cased to the first comment of the file! Presumably because the new line detection logic used to be broken ;)
I manually checked the first 10 snapshots and they are all legit, so I assume that all of them are.
Fixes#356
We don't call the generic print on the BinaryExpression itself, so we need to manually print those comments. It's going to be useful for my work on the MemberExpression :)
It's actually not needed to use conditionalGroup as we can use ifBreak for it. I was able to do it just for cleanup and found out that it also fixed two of the bugs we have with comments. That's great :p
Fixes#485Fixes#486
Conditionals are very common in JSX and it is unfortunate that they take up so much vertical space in the current prettier.
This pull request does a few tweaks:
- It hugs ConditionalExpression (ternary) and LogicalExpression (&&) inside of `{}` in a jsx child, not an attribute
- It doesn't output parenthesis if your parent is a LogicalExpression (&&)
Fixes#317
Mobx is the only popular JavaScript library that I know about which uses decorators. They put things on the same line so we should follow their conventions.
The logic implemented here is the following: if there is one decorator, it's on the same line. If there is more than one, they are each on their own line.
Fixes#325
This was introduced by #314 where `line` should have been `softline`. By the way, I was going to propose renaming `line` to `line_or_space` and `softline` to `line_or_nothing` which should make it more explicit what is going on.
Fixes#461
Given the discussion on #296, it seems like there's debate between spaces around `{}` but no one puts spaces around `[]`. So changing the behavior to respect this.
The original intent of it was for `if then else` and `try catch` as they aren't likely to be empty, but it accidentally caught function bodys, which have many valid reasons to be empty. Let's special case those out.
- Introduce findInDocs that avoids some annoying boilerplate when you want to get a value while traversing the docs
- Implement a hasStopped function that stops the traversal when you found the value you needed
* Validate user-provided config with jest-validate
* Don't use object shorthand
* Update jest-validate to 18.2
* Fix Node4 destructuring from require
* Make use of jest-validate for deprecation message
* Remove chalk reference
* Use babel-code-frame for syntax errors
* Support the `--color` option more explicitly
* Update rollup config to handle babel-code-frame
* Use exact dependencies
This one is really weird
```js
if (() => {} ? 1 : 0) {}
```
parses fine but
```js
if (() => {
} ? 1 : 0) {}
```
is a syntax error. Let's always add a parenthesis.
Note that no one is every going to write this in practice, this is really useless :p
We actually need this `;` for EmptyStatement, otherwise it applies to the next block of code. (Creating a label with an empty statement is completely useless, but it triggers a lot in the fuzz testing tool)
Fixes#376
We avoid adding a `;` for a variable declaration in for loop but this is only meant if the var declaration is inside of the `()` part of the for loop. Not if the body part.
Fixes#385
* Add tests for quotes
* Update test snapshots
* Output strings with the minimum amount of escaped quotes
* Update test snapshots
* Move tests/prettier/quotes.js into tests/quotes/strings.js
* Update test snapshots
- During the first iteration, we printed the unescaped values which let to printing invalid JavaScript characters and bad things like invisible characters.
- During the second iteration, we escaped everything, which generated valid JavaScript but you lost your emojis and chinese/cyrillic characters
In this iteration, which I hope will be the last one, we maintain the string exactly as encoded and only swap quotes. The swap quotes implementation is a bit convoluted but I think it works.
This PR adds two things:
`--debug-print-doc` command that prints the formatted doc
```js
echo "<div><</div>" | ./bin/prettier.js --stdin --debug-print-doc
[
groupConditional(
group([
group([ "<", "div", group([ indent(2, []), softline() ]), ">" ]),
"<",
"</",
"div",
">"
]),
[
group([
group([ "<", "div", group([ indent(2, []), softline() ]), ">" ]),
"<",
"</",
"div",
">"
]),
group([
group([ "<", "div", group([ indent(2, []), softline() ]), ">" ]),
indent(2, groupBreak([ hardline(), "<" ])),
hardline(),
"</",
"div",
">"
])
]
),
";",
hardline()
];
```
The ability to view the IR in real time on the browser display:
The way it works is pretty cool, we take the doc IR and print a valid JavaScript string out of it, that we then send to prettier in order to make it look good :)
The previous API was inconsistent. The new one is
```js
--parser flow
--parser babylon
{parser: 'flow'}
{parser: 'babylon'}
```
if we ever want to add new parsers in the future it'll allow that more easily.
I put a console.log in parser.js in both functions and tested that the test suite worked both with and without the change in run_spec. I also tested that both the previous and new command line options are working.
At some point in the future we'll likely want to get rid of the old api but might as well keep supporting it so we don't break anyone for now.
Make all the files have a single module.exports call at the end of the file.
I added a missing `"use strict";` and removed a few unused functions from utils at the same time.
- doc-printer.js is now the direct implementation of the Wadler paper
- doc-builders.js are a lot of utils to generate the IR the above file needs
- doc-utils.js are small utils to traverse list of docs.
It's annoying that there's a bug inside of the flow parser, I raised it internally. While this is getting fixed, we can workaround it. This now makes babylon properly escape JSXText.
I thought I didn't need to check the length but forgot that the rest argument is not in the list for class declaration. Now it doesn't crash anymore and there's a test...
The current output of
```js
[...a, ...b]
```
is
```js
[...a, , ...b]
```
because flow parses it as
```
ArrayExpression(SpreadExpression, null, SpreadExpression)
```
This is a bug in the flow parser. Until it gets fixed, we can workaround it by deleting the `null` after a `SpreadExpression`.
* Remove +1 from newline detection
All the changes are related to spurious `;`. Sometimes the logic is more correct, sometimes less. Since `;` are going to be removed after the first save, I don't think it matters that much.
* Handle inconsistent `end` node locations when looking for newlines
I copy and pasted the code for arrays which doesn't have this problem. Would be nice to come up with an abstraction for a list of stuff separated by commas. It happens a lot of time and right now it's duplicated everywhere.
Fixes#255
According to @mroch, "Flow is using CESU-8, not UTF-8. http://www.unicode.org/reports/tr26/ ". While this is being fixed in flow, we can easily work around it inside of prettier. The downside of this approach is that we can't convert those strings to single or double quotes anymore.
```js
for (;;);
function f() {}
```
The `;` was dropped meaning that the line right after was executed within the for loop which is not correct.
I tried to return `;` but it looks like
```js
for (;;)
;
```
which looks super weird so I ended up printing `{}` which looks like
```js
for (;;) {}
```
The current implementation with `JSON.stringify()` is clever but unfortunately generates incorrect JavaScript. Using `jsesc` seems like a better and safer option. https://github.com/mathiasbynens/jsesc It doesn't have any dependencies and is pretty small.
I opted for escaping all the non ascii characters, so we don't display emojis anymore. I don't think that the world is ready yet for having random unicode characters inside of source files, there still are so many parts of the toolchain that breaks with them. If we want to revert back on this decision, there's a `minimal` option on jsesc which only escapes values that need to in order to generate valid JavaScript file (assuming the encoding of the file is set to utf8).
Also, while working on React Native, we've seen that there is an optimization inside of jsc for js files that are all ascii: it doesn't do a copy for the conversion to ucs16.
Fixes#163
There's a handful of files inside of Nuclide that throw exceptions because an assertion is raised.
```
{ AssertionError: ']' === '`'
at fixTemplateLiteral (/Users/vjeux/random/prettier/src/util.js:105:10)
at Object.util.fixFaultyLocations (/Users/vjeux/random/prettier/src/util.js:45:5)
at getSortedChildNodes (/Users/vjeux/random/prettier/src/comments.js:25:8)
at getSortedChildNodes (/Users/vjeux/random/prettier/src/comments.js:61:5)
at decorateComment (/Users/vjeux/random/prettier/src/comments.js:71:20)
at decorateComment (/Users/vjeux/random/prettier/src/comments.js:85:7)
at decorateComment (/Users/vjeux/random/prettier/src/comments.js:85:7)
at decorateComment (/Users/vjeux/random/prettier/src/comments.js:85:7)
at decorateComment (/Users/vjeux/random/prettier/src/comments.js:85:7)
at /Users/vjeux/random/prettier/src/comments.js:129:5
```
When trying https://github.com/facebook/nuclide/blob/master/pkg/nuclide-task-runner/lib/main.js#L174
It throws in the fixTemplateLiteral method.
That method was added to fix https://github.com/benjamn/recast/issues/216 more than a year ago
```js
var x = {
y: () => Relay.QL`
query {
${foo},
field,
}
`
};
```
I've checked (and added a test) and it now parses and prints correctly without that method. So it should be safe to remove.
We were not printing the directives if the body of the function was empty in babylon. Also, we were printing way too many \n
```js
echo "function fn() { 'use strict'; }" | ./bin/prettier.js --stdin
function fn() {
"use strict";
}
```
```js
echo "function fn() { 'use strict'; }" | ./bin/prettier.js --stdin --flow-parser
function fn() {
"use strict";
}
```
DeclareInterface (flow) and InterfaceDeclaration (babylon) are the same type so should behave the same way. I am using the same `declare` trick where I only add it if you are inside of a `declare module` block.
Flow doesn't have a different ast node for `type` and `declare type`. Let's always use the heuristic to be inside of a `declare module` for both ast. This way more snapshot tests are passing between the two parsers.
The search for an empty line incorrectly does +1 which happens to be skipping a `\n`, but in case of windows line endings it skips the `\r` but sees a `\n` afterwards and incorrectly assumes that it is a empty line.
This doesn't change the behavior of doing +1 when there's not a line ending. Making it correct actually triggers a bunch of changes, where half of them are better and half of them regressions. So I'm going to send another pull request to fix that case.
Another issue where babylon and flow ast are different. In babylon, it is NumericLiteral but flow is Literal. All the tests are running on flow so were working correctly, but the default in the command line is to use babylon, so people report bugs with it.
This is working on the flow parser but not babylon
```js
echo 'class C<T> { submit<T>() { } }' | ./bin/prettier.js --stdin
class C<T> {
submit<T>() {}
}
```
If there you are opting in for double quote but there's a string with a double quote in it, it's better to swap to a single quote to avoid having too many `\`. Note that if there are both single and double quotes in the string, we should use the default string instead.
Fixes#139
This is working on the flow parser but not babylon
```js
echo 'function f(...flags: Array<boolean>) {}' | ./bin/prettier.js --stdin
function f(...flags: Array<boolean>) {}
```
Christopher Chedeau
