Skip to content

Commit

Permalink
Preserve blank lines inside of objects (#606)
Browse files Browse the repository at this point in the history
We already preserve blank lines inside of classes but not objects. It's still very common to have objects that represent classes (React.createClass) and having the same behavior there seems like a good idea. It does make the snapshot tests look better!

Fixes #554
  • Loading branch information
vjeux authored and jlongster committed Feb 9, 2017
1 parent 0287cad commit 6632b95
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/printer.js
Expand Up @@ -600,13 +600,19 @@ function genericPrintNoParens(path, options, print) {

fields.push("properties");

var i = 0;
var props = [];
let separatorParts = [];

fields.forEach(function(field) {
path.each(
function(childPath) {
props.push(concat(separatorParts));
props.push(group(print(childPath)));

separatorParts = [separator, line];
if (util.isNextLineEmpty(options.originalText, childPath.getValue())) {
separatorParts.push(hardline);
}
},
field
);
Expand Down Expand Up @@ -639,7 +645,7 @@ function genericPrintNoParens(path, options, print) {
options.tabWidth,
concat([
options.bracketSpacing ? line : softline,
join(concat([separator, line]), props)
concat(props)
])
),
ifBreak(canHaveTrailingComma && options.trailingComma ? "," : ""),
Expand Down
2 changes: 1 addition & 1 deletion src/util.js
Expand Up @@ -124,7 +124,7 @@ function skip(chars) {

const skipWhitespace = skip(/\s/);
const skipSpaces = skip(" \t");
const skipToLineEnd = skip("; \t");
const skipToLineEnd = skip(",; \t");
const skipEverythingButNewLine = skip(/[^\r\n]/);

function skipInlineComment(text, index) {
Expand Down
1 change: 1 addition & 0 deletions tests/flow/get-def/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -81,6 +81,7 @@ module.exports = {
iTakeAString: function(name: string): number {
return 42;
},
bar: function(): number {
return 42;
}
Expand Down
Expand Up @@ -210,29 +210,35 @@ var obj = {
get goodGetterWithAnnotation(): number {
return 4;
},
set goodSetterNoAnnotation(x) {
z = x;
},
set goodSetterWithAnnotation(x: number) {
z = x;
},
get propWithMatchingGetterAndSetter(): number {
return 4;
},
set propWithMatchingGetterAndSetter(x: number) {},
// The getter and setter need not have the same type
get propWithSubtypingGetterAndSetter(): ?number {
return 4;
}, // OK
set propWithSubtypingGetterAndSetter(x: number) {},
set propWithSubtypingGetterAndSetterReordered(x: number) {}, // OK
get propWithSubtypingGetterAndSetterReordered(): ?number {
return 4;
},
get exampleOfOrderOfGetterAndSetter(): A {
return new A();
},
set exampleOfOrderOfGetterAndSetter(x: B) {},
set exampleOfOrderOfGetterAndSetterReordered(x: B) {},
get exampleOfOrderOfGetterAndSetterReordered(): A {
return new A();
Expand Down
Expand Up @@ -95,18 +95,21 @@ var Foo = {
// missing arg annotation
return arg;
},
b: function(arg) {
// missing arg annotation
return {
bar: arg
};
},
c: function(arg: string) {
// no return annotation required
return {
bar: arg
};
},
d: function(
arg: string
): {
Expand All @@ -116,6 +119,7 @@ var Foo = {
bar: arg
};
},
// return type annotation may be omitted, but if so, input positions on
// observed return type (e.g. param types in a function type) must come
// from annotations
Expand All @@ -125,6 +129,7 @@ var Foo = {
return x;
};
},
// ...if the return type is annotated explicitly, this is unnecessary
f: function(arg: string): (x: number) => number {
return function(x) {
Expand All @@ -136,10 +141,13 @@ var Foo = {
var Bar = {
a: Foo.a(\"Foo\"), // no annotation required
// object property types are inferred, so make sure that this doesn\'t cause
// us to also infer the parameter\'s type.
b: Foo.b(\"bar\"), // no annotation required
c: Foo.c(\"bar\"), // no annotation required
d: Foo.d(\"bar\") // no annotation required
};
Expand Down
3 changes: 3 additions & 0 deletions tests/flow/more_react/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -79,12 +79,15 @@ var App = React.createClass({
getDefaultProps: function(): { y: string } {
return { y: \"\" }; // infer props.y: string
},
getInitialState: function() {
return { z: 0 }; // infer state.z: number
},
handler: function() {
this.setState({ z: 42 }); // ok
},
render: function() {
var x = this.props.x;
var y = this.props.y;
Expand Down
18 changes: 18 additions & 0 deletions tests/flow/new_react/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -67,6 +67,7 @@ var FeedUFI = React.createClass({
<UFILikeCount className=\"\" key=\"\" feedback={feedback} permalink=\"\" />
);
},
render: function(): ?React.Element<any> {
return <div />;
}
Expand Down Expand Up @@ -130,6 +131,7 @@ var UFILikeCount = React.createClass({
permalink: React.PropTypes.string,
feedback: React.PropTypes.object.isRequired
},
render: function(): ?React.Element<any> {
return <div />;
}
Expand Down Expand Up @@ -337,22 +339,29 @@ var FooLegacy = React.createClass({
propTypes: {
x: React.PropTypes.number.isRequired
},
getDefaultProps(): DefaultProps {
return {};
},
statics: {
bar(): void {}
},
qux(): void {
var _: string = this.props.x;
},
getInitialState(): { y: string } {
return { y: \"\" };
},
setState(o: { y_: string }): void {},
componentDidMount(): void {
this.is_mounted = true;
},
componentWillReceiveProps(nextProps: Object, nextContext: any): void {
this.qux();
}
Expand Down Expand Up @@ -452,6 +461,7 @@ var C = React.createClass({
z: React.PropTypes.number
},
replaceProps(props: {}) {},
getDefaultProps(): { z: number } {
return { z: 0 };
},
Expand Down Expand Up @@ -558,9 +568,11 @@ var TestProps = React.createClass({
x: React.PropTypes.string,
z: React.PropTypes.number
},
getDefaultProps: function() {
return { x: \"\", y: 0 };
},
test: function() {
var a: number = this.props.x; // error
var b: string = this.props.y; // error
Expand Down Expand Up @@ -688,12 +700,14 @@ var TestProps = React.createClass({
object_rec: React.PropTypes.object.isRequired,
string: React.PropTypes.string,
string_rec: React.PropTypes.string.isRequired,
any: React.PropTypes.any,
any_rec: React.PropTypes.any.isRequired,
element: React.PropTypes.element,
element_rec: React.PropTypes.element.isRequired,
node: React.PropTypes.node,
node_rec: React.PropTypes.node.isRequired,
arrayOf: React.PropTypes.arrayOf(React.PropTypes.string),
arrayOf_rec: React.PropTypes.arrayOf(React.PropTypes.string).isRequired,
instanceOf: React.PropTypes.instanceOf(Object),
Expand All @@ -718,6 +732,7 @@ var TestProps = React.createClass({
foo: React.PropTypes.string,
bar: React.PropTypes.number
}).isRequired,
// And do something bad here
bad_one: React.PropTypes.imaginaryType,
bad_two: React.PropTypes.string.inRequired
Expand Down Expand Up @@ -840,6 +855,7 @@ var ReactClass = React.createClass({
getInitialState: function(): State {
return { bar: null };
},
render: function(): any {
// Any state access here seems to make state any
this.state;
Expand Down Expand Up @@ -919,6 +935,7 @@ var TestState = React.createClass({
x: \"\"
};
},
test: function() {
var a: number = this.state.x; // error
Expand Down Expand Up @@ -951,6 +968,7 @@ var C = React.createClass({
getInitialState: function() {
return { x: 0 };
},
render() {
this.setState({ y: 0 });
return <div>{this.state.z}</div>;
Expand Down
1 change: 1 addition & 0 deletions tests/flow/overload/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -65,6 +65,7 @@ var x4: number = \"\".split(/pattern/)[0];
declare class C {
foo(x: number): number,
foo(x: string): string,
bar(x: { a: number }): number,
bar(x: { a: string }): string
}
Expand Down
1 change: 1 addition & 0 deletions tests/flow/react/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -271,6 +271,7 @@ var React = React.createClass({
return \"Alice\";
}
},
render() {
// But this never errored
return <div id={this.props.name} />;
Expand Down
2 changes: 2 additions & 0 deletions tests/flow/react_modules/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -34,6 +34,7 @@ var HelloLocal = React.createClass({
propTypes: {
name: React.PropTypes.string.isRequired
},
render: function(): React.Element<*> {
return <div>{this.props.name}</div>;
}
Expand Down Expand Up @@ -77,6 +78,7 @@ var Hello = React.createClass({
propTypes: {
name: React.PropTypes.string.isRequired
},
render: function(): React.Element<*> {
return <div>{this.props.name}</div>;
}
Expand Down
1 change: 1 addition & 0 deletions tests/flow/type-at-pos/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -33,6 +33,7 @@ export const X = {
returnsATuple: function(): [number, number] {
return [1, 2];
},
test: function() {
let [a, b] = this.returnsATuple();
}
Expand Down
Expand Up @@ -63,22 +63,29 @@ declare class Promise<R> {
reject: (error?: any) => void
) => void
): void,
then<U>(
onFulfill?: ?(value: R) => Promise<U> | ?U,
onReject?: ?(error: any) => Promise<U> | ?U
): Promise<U>,
done<U>(
onFulfill?: ?(value: R) => void,
onReject?: ?(error: any) => void
): void,
catch<U>(onReject?: (error: any) => ?Promise<U> | U): Promise<U>,
static resolve<T>(object?: Promise<T> | T): Promise<T>,
static reject<T>(error?: any): Promise<T>,
// Non-standard APIs
finally<U>(onSettled?: ?(value: any) => Promise<U> | U): Promise<U>,
static cast<T>(object?: T): Promise<T>,
static all<T>(promises: Array<?Promise<T> | T>): Promise<Array<T>>,
static race<T>(promises: Array<Promise<T>>): Promise<T>,
static allObject<T: Object>(promisesByKey: T): Promise<{
[key: $Keys<T>]: any
}>
Expand Down

0 comments on commit 6632b95

Please sign in to comment.