We break on `class` before `extends` but didn't for `interface`. Now we do in the same way. Also TIL that `interface` was an actual keyword in JavaScript :P
* Verify parsers against same snapshot
- Reworked run_spec, now accepts 3th optional array argument for
additional parsers to verify against
- Merged duplicate run_spec configs
- Removed duplicate snapshot data
* Formatted run_spec.js with prettier
* Fixed node4 incompatibility
If you write your code in a functional way where you have an arrow function for each argument, it looks better for them to be inline. I can't imagine any case where it would be used in a different way if we limit to a single argument.
Fixes#1065
We started using the same logic for union and intersection but then added a special case for `a & {}`. It turns out that they should be handled completely differently in practice.
The heuristic i'm using is if you go from object to non-object or vis-versa, then inline, otherwise go to the next line and indent (like &&).
Fixes#864Fixes#1017
This is very common for stateful react functional components. It also matches last object expansion. I think that we should only do that for a single argument in function definitions though, unlike the last for function calls.
Fixes#1019
* Revert "Remove mutation in `printBinaryishExpressions` (#1067)"
This reverts commit e7312ad7b2.
* Revert "Make it clear what parser was used in each snapshot (#1068)"
This reverts commit 4f7ae4815b.
The root cause is that we're calling `printComments` with an empty string, meaning that the leading/trailing comments are not correctly inserted at the right location.
So, the way to fix it is to call `p => concat(parts)` but because we're mutating the array in place, it doesn't work. We need to mutate it and create a copy. But, the root call is actually checking the shape of the parts array which our code is now breaking...
```js
// Don't include the initial expression in the indentation
// level. The first item is guaranteed to be the first
// left-most expression.
parts.length > 0 ? parts[0] : "",
```
The consequence is that binary expressions are no longer indented if the first expression has a comment (but now it places the comment properly!), which seems like a good trade-off.
I'm not sure if we should merge this one or instead refactor this code such that it doesn't rely on mutation and looking at the shape of the printed tree. `printMemberChain` is a good thing to reference for inspiration.
Fixes#946
Right now it's only doing one test for begin/end, but in the issue #1037, we have two nodes that have the same start but different end. The current implementation incorrectly sorts them and the identifier ends up being before the container and therefore the comment search doesn't recurse into it.
Fixes#1037
ast-types was missing a type, I sent a PR on ast-type ( https://github.com/benjamn/ast-types/pull/211 ) which was merged and released (thanks @benjamn for being super quick!). So I'm upgrading ast-types and add the test in this PR.
Fixes#1007
While looking at an instability on the React codebase ( ba1299acc2 (diff-2550aa3d377452ae29361f5e53c51c10) ), I realized that we don't let them break which makes the code look weird.
So I added the ability for them to break :)
We already had a special case for it for a group of 2 elements but now that we are also going in this codepath for 3 if we merge, then we need to do the same here.
Fixes#930
* Add some typescript tests
* fix: Remove extraneous colon in type parameter constraint
* style: Add missing newline at EOF in TS tests
* feat: Pretty print typescript object type annotations
* feat: Pretty print TSFunctionType
* fix: Type annotations was missing on class properties
* Add a new batch of tests
* Bump typescript-eslint-parser
* Add a new batch of tests and fix a syntax error in previous
* Bump typescript-eslint-parser
* Group first arg for inline functions
* Group first arg. Unless there are comments.
* Group first arg. Allow second arg to be empty object/array.
This implementation has a side effect of disallowing empty objects/arrays in the should group last arg heuristic.
* Add handleImportDeclarationComments function.
* Fix indent.
* Update test cases accordingly.
* Implement better heuristic for adding comments.
* Make the test case more exhaustive.
* Refactor condition in printStatementSequence, add new helper function.
* Add new test cases.
* Move logic in SwitchCase case.
* Revert unrelated changes, remove unecessary variables.
* Use util.getLast helper function.
* Move variables out of the loop.
* Simplify code.
* Fix mapping with mutated path.
* Implement checking for not printing the parens in this case.
* Add test case.
* Remove parent type checking.
* Fix test accordingly.
* Refactor to a better heuristic.
* Add new test cases.
* Fix implementation.
* Fix unnecessary function call.
* Add new test cases 🚀.
* Use isObjectTypePropertyAFunction and create isTypeAnnotationAFunction.
* Add missing space.
* Add new test cases.
Printing the first line of a binary expression next to the `=` leads to weird cases where the first expression is parenthesised and doens't read well with chained conditionals as they don't align well. This makes it behave the same was as `if` tests.
Fixes#863
This was intended for object expressions. I tried to remove it for ObjectTypeAnnotation but it changes a ton of stuff as it's used all over the place in many different contexts. We should clean it up but in a later PR :)
Fixes part of #975
In practice, trying to allow calls to be inlined is causing a lot of code to look very weird and unstable as seen by the four issues this is fixing. It also requires us to add a conditional group and do hackery around it.
Fixes#959Fixes#760Fixes#615Fixes#625
* WIP immediate feedback
* typescript parser is drop-in replacement for flow parser
* Add new TypeScript Parser snapshots where drop-in replacement possible
* Snapshot updates after rebasing
* Remove unnecessary stripping of properties on TypeScript parser AST
* Remove annotated issues
* Move TS dependencies to dev for now
* Implement new logic for wrapping binary op in arrowFunctionExpression.
* Add new test cases.
* Reuse new helper function in order to fix#917.
* Add new test case.
* Extend heuristic to dive deeper into mixed types.
* Add new test.
* Enhance logic to cover more cases.
* Add new test cases.
* Disable Flow as it gets BindExpression as an unexpected token.
* Simplify getCombinedDeepest function.
* Add missing case.
* Extract all conditions in switch cases to one top level condition.
* Refactor implementation to make it cleaner and also handle ExpressionStatement.
* Update related test cases.
* Add new test case.
* Make condition less expensive.
* Clean up unecessary conditions, simplify condition involving startsWithOpenCurlyBrace.
* Update and add new test cases for better coverage.
* Remove unecessary condition, refactor canBeFirstInStatement to drop some useless parens.
* Update test cases accordingly 🚀.
* Add new handleOnlyComments function.
* Update tests.
* Update test as the printer forces a trailing newline if there were any contents.
* Implement a different heuristic.
* Update tests.
* Add directives checking in handleOnlyComments function.
* Add directives checking in handleOnlyComments function (amend to retrigger CI).
* Remove duplicate.
* Add new helper function to convert comments as blocks in ExportNamedDeclaration.
* Add new test.
* Switch to a leading position.
* Update test accordingly.
The comments infra has been architected with trailing operators and fails for leading `|`. Instead of having leading comments, we can put trailing comments on the previous one and use the same technique as assignment to deal with the indentation.
Fixes#849
This happens very frequently that naming a test makes the entire line go > 80 columns and requires to indent everything which takes a lot more space. We've had this being reported multiple times and it also affects a lot of tests inside of fb.
Fixes#159
(Note, this is built on-top of #841 but github doesn't handle stacked pull requests well...)
This is a leftover from the recast prototype, it hasn't been touched since then. I have never seen anyone not put the label on the same line.
Fixes#859
This is a neat trick from the React codebase. It helps highlight the fact that this is an assignment and not a comparison which is subtle to realize.
Fixes#861
In #605 I restricted it to binary operations as I didn't know how it would affect boolean operations but it turns out the same technique solves problems that people are reporting. So doesn't make sense to restrict it.
Fixes#824
It turns out that my fix was not correct. We should not add parenthesis for FunctionDeclaration instead of checking if the function expression is named.
Fixes#819
In #596, I fixed a bunch of jsx expression comment edge cases and happened to add a softline there. But it turns out that it's not needed and is actually harmful :)
Fixes#712
Since this is extremely rare, I just took the easy way out and if you are adding a comment inside the "as" node, I just move it outside. We could be more fancy but that works.
Fixes#620
This has come up a couple times on the issue tracker on the react-native-web discussion about Twitter internals. It seems like there's a concensus that people don't break long require calls and they'd rather have it go > 80 columns.
Fixes#303Fixes#752
I'm not really sure what a general rule is for those, but starting with LogicalExpressions should be good. Outside of identifiers, function calls and logical expressions, there aren't a lot of things you'd put there in real code anyway.
Fixes#822
Instead of trying to figure out a complicated way to preserve their correct position, it's easier to just migrate those comments before the class.
Fixes#693Fixes#694
Printing a merged group indented was actually not the right fix. The right fix was to print them in a single line. It used to have this behavior when I was mutating the first group but now that I don't anymore I need to reproduce this condition.
Fixes#823
We need to add parenthesis around function expressions if they are named otherwise the name leak, but if the function is not named then it's just superfluous.
The conditional change is due to a bad use of switch case where it would fall through the next one. We don't want parenthesis there.
* Add new handleFunctionDeclarationComments function.
* Add dangling comments printing for function params.
* Add new test case.
* Update tests.
* Refactor handleFunctionDeclarationComments to only addDanglingComment when no params.
* Remove unecessary helper function, only attach dangling comments when no params.
* Reset flow tests, no more regression.
It seems like precedence for this combination of operators is very unclear to people (myself included) and consistently adding parenthesis there would be good.
Fixes#773
The idea is that if you reach the end of the `}` inside of a template literal, we have to flush the trailing comma, otherwise it would generate invalid code. We also need the same special case for JSX.
I don't like adding yet another type of document but it seems like the most elegant way to solve the problem.
Fixes#623
* Add handleTemplateLiteralComment helper function.
* Fix handleTemplateLiteralComment function.
* Extend handleTemplateLiteralComment to deal with trailing comments 🚀.
* Add test.
* Make handle comments function naming more consistent, fix merge conflicts.
* Update tests.
* Add better comment injection in Template Literal.
* Pass options to attach function.
* Update tests to match new implementation.
* Fix let -> var in findExpressionIndexForComment for NodeJS v4.
* Reorder after merge conflicts.
* Drop old tests for dangling arrays.
* Replace redundant conditional by a boolean 🚀.
* Refactor implementation.
Eric Clemmons