Never inline decorators, unless they're lone parameter decorators (#4830)

Fixes #2613.

src/common/util.js

@@ -786,6 +786,22 @@ function addTrailingComment(node, comment) {
   addCommentHelper(node, comment);
 }
 
+function isWithinParentArrayProperty(path, propertyName) {
+  const node = path.getValue();
+  const parent = path.getParentNode();
+
+  if (parent == null) {
+    return false;
+  }
+
+  if (!Array.isArray(parent[propertyName])) {
+    return false;
+  }
+
+  const key = path.getName();
+  return parent[propertyName][key] === node;
+}
+
 module.exports = {
   punctuationRegex,
   punctuationCharRange,
@@ -823,5 +839,6 @@ module.exports = {
   matchAncestorTypes,
   addLeadingComment,
   addDanglingComment,
-  addTrailingComment
+  addTrailingComment,
+  isWithinParentArrayProperty
 };

src/language-js/printer-estree.js

@@ -20,7 +20,8 @@ const {
   getPenultimate,
   startsWithNoLookaheadToken,
   getIndentSize,
-  matchAncestorTypes
+  matchAncestorTypes,
+  isWithinParentArrayProperty
 } = require("../common/util");
 const {
   isNextLineEmpty,
@@ -94,7 +95,12 @@ function genericPrint(path, options, printPath, args) {
     // responsible for printing node.decorators.
     !getParentExportDeclaration(path)
   ) {
-    let separator = hardline;
+    const separator =
+      node.decorators.length === 1 &&
+      isWithinParentArrayProperty(path, "params")
+        ? line
+        : hardline;
+
     path.each(decoratorPath => {
       let decorator = decoratorPath.getValue();
       if (decorator.expression) {
@@ -103,27 +109,6 @@ function genericPrint(path, options, printPath, args) {
         decorator = decorator.callee;
       }
 
-      if (
-        node.decorators.length === 1 &&
-        node.type !== "ClassDeclaration" &&
-        node.type !== "MethodDefinition" &&
-        node.type !== "ClassMethod" &&
-        (decorator.type === "Identifier" ||
-          decorator.type === "MemberExpression" ||
-          decorator.type === "OptionalMemberExpression" ||
-          ((decorator.type === "CallExpression" ||
-            decorator.type === "OptionalCallExpression") &&
-            (decorator.arguments.length === 0 ||
-              (decorator.arguments.length === 1 &&
-                (isStringLiteral(decorator.arguments[0]) ||
-                  decorator.arguments[0].type === "Identifier" ||
-                  decorator.arguments[0].type === "MemberExpression" ||
-                  decorator.arguments[0].type ===
-                    "OptionalMemberExpression")))))
-      ) {
-        separator = line;
-      }
-
       decorators.push(printPath(decoratorPath), separator);
     }, "decorators");
   } else if (

tests/decorators-ts/__snapshots__/jsfmt.spec.js.snap

@@ -0,0 +1,223 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`accessor-decorator.ts - typescript-verify 1`] = `
+class Point {
+  private _x: number;
+  private _y: number;
+  constructor(x: number, y: number) {
+    this._x = x;
+    this._y = y;
+  }
+
+  @configurable(false)
+  get x() {
+    return this._x;
+  }
+
+  @configurable(false)
+  get y() {
+    return this._y;
+  }
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+class Point {
+  private _x: number;
+  private _y: number;
+  constructor(x: number, y: number) {
+    this._x = x;
+    this._y = y;
+  }
+
+  @configurable(false)
+  get x() {
+    return this._x;
+  }
+
+  @configurable(false)
+  get y() {
+    return this._y;
+  }
+}
+
+`;
+
+exports[`class-decorator.ts - typescript-verify 1`] = `
+@sealed
+class Greeter {
+  greeting: string;
+  constructor(message: string) {
+    this.greeting = message;
+  }
+  greet() {
+    return "Hello, " + this.greeting;
+  }
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+@sealed
+class Greeter {
+  greeting: string;
+  constructor(message: string) {
+    this.greeting = message;
+  }
+  greet() {
+    return "Hello, " + this.greeting;
+  }
+}
+
+`;
+
+exports[`method-decorator.ts - typescript-verify 1`] = `
+class Greeter {
+  greeting: string;
+  constructor(message: string) {
+    this.greeting = message;
+  }
+
+  @enumerable(false)
+  greet() {
+    return "Hello, " + this.greeting;
+  }
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+class Greeter {
+  greeting: string;
+  constructor(message: string) {
+    this.greeting = message;
+  }
+
+  @enumerable(false)
+  greet() {
+    return "Hello, " + this.greeting;
+  }
+}
+
+`;
+
+exports[`multiple.ts - typescript-verify 1`] = `
+class C {
+  @f()
+  @g()
+  method() {}
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+class C {
+  @f()
+  @g()
+  method() {}
+}
+
+`;
+
+exports[`parameter-decorator.ts - typescript-verify 1`] = `
+class Greeter {
+  greeting: string;
+
+  constructor(message: string) {
+    this.greeting = message;
+  }
+
+  @validate
+  greet(@required name: string) {
+    return "Hello " + name + ", " + this.greeting;
+  }
+
+  @validate
+  destructured(@required { toString }: Object) {
+    return Function.prototype.toString.apply(toString);
+  }
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+class Greeter {
+  greeting: string;
+
+  constructor(message: string) {
+    this.greeting = message;
+  }
+
+  @validate
+  greet(@required name: string) {
+    return "Hello " + name + ", " + this.greeting;
+  }
+
+  @validate
+  destructured(@required { toString }: Object) {
+    return Function.prototype.toString.apply(toString);
+  }
+}
+
+`;
+
+exports[`property-decorator.ts - typescript-verify 1`] = `
+class Greeter {
+  @format("Hello, %s") greeting: string;
+
+  constructor(message: string) {
+    this.greeting = message;
+  }
+  greet() {
+    let formatString = getFormat(this, "greeting");
+    return formatString.replace("%s", this.greeting);
+  }
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+class Greeter {
+  @format("Hello, %s")
+  greeting: string;
+
+  constructor(message: string) {
+    this.greeting = message;
+  }
+  greet() {
+    let formatString = getFormat(this, "greeting");
+    return formatString.replace("%s", this.greeting);
+  }
+}
+
+`;
+
+exports[`typeorm.ts - typescript-verify 1`] = `
+@Entity()
+export class Board {
+
+    @PrimaryGeneratedColumn()
+    id: number;
+
+    @Column()
+    slug: string;
+
+    @Column()
+    name: string;
+
+    @Column()
+    theme: string;
+
+    @Column()
+    description: string;
+
+    @OneToMany(type => Topic, topic => topic.board)
+    topics: Topic[]
+
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+@Entity()
+export class Board {
+  @PrimaryGeneratedColumn()
+  id: number;
+
+  @Column()
+  slug: string;
+
+  @Column()
+  name: string;
+
+  @Column()
+  theme: string;
+
+  @Column()
+  description: string;
+
+  @OneToMany(type => Topic, topic => topic.board)
+  topics: Topic[];
+}
+
+`;

tests/decorators-ts/accessor-decorator.ts

@@ -0,0 +1,18 @@
+class Point {
+  private _x: number;
+  private _y: number;
+  constructor(x: number, y: number) {
+    this._x = x;
+    this._y = y;
+  }
+
+  @configurable(false)
+  get x() {
+    return this._x;
+  }
+
+  @configurable(false)
+  get y() {
+    return this._y;
+  }
+}

tests/decorators-ts/class-decorator.ts

@@ -0,0 +1,10 @@
+@sealed
+class Greeter {
+  greeting: string;
+  constructor(message: string) {
+    this.greeting = message;
+  }
+  greet() {
+    return "Hello, " + this.greeting;
+  }
+}

tests/decorators-ts/jsfmt.spec.js

@@ -0,0 +1 @@
+run_spec(__dirname, ["typescript"]);

tests/decorators-ts/method-decorator.ts

@@ -0,0 +1,11 @@
+class Greeter {
+  greeting: string;
+  constructor(message: string) {
+    this.greeting = message;
+  }
+
+  @enumerable(false)
+  greet() {
+    return "Hello, " + this.greeting;
+  }
+}

tests/decorators-ts/multiple.ts

@@ -0,0 +1,5 @@
+class C {
+  @f()
+  @g()
+  method() {}
+}

tests/decorators-ts/parameter-decorator.ts

@@ -0,0 +1,17 @@
+class Greeter {
+  greeting: string;
+
+  constructor(message: string) {
+    this.greeting = message;
+  }
+
+  @validate
+  greet(@required name: string) {
+    return "Hello " + name + ", " + this.greeting;
+  }
+
+  @validate
+  destructured(@required { toString }: Object) {
+    return Function.prototype.toString.apply(toString);
+  }
+}

tests/decorators-ts/property-decorator.ts

@@ -0,0 +1,11 @@
+class Greeter {
+  @format("Hello, %s") greeting: string;
+
+  constructor(message: string) {
+    this.greeting = message;
+  }
+  greet() {
+    let formatString = getFormat(this, "greeting");
+    return formatString.replace("%s", this.greeting);
+  }
+}

tests/decorators-ts/typeorm.ts

@@ -0,0 +1,22 @@
+@Entity()
+export class Board {
+
+    @PrimaryGeneratedColumn()
+    id: number;
+
+    @Column()
+    slug: string;
+
+    @Column()
+    name: string;
+
+    @Column()
+    theme: string;
+
+    @Column()
+    description: string;
+
+    @OneToMany(type => Topic, topic => topic.board)
+    topics: Topic[]
+
+}

tests/decorators/__snapshots__/jsfmt.spec.js.snap

@@ -108,8 +108,10 @@ import { observable } from "mobx";
 
 @observer
 class OrderLine {
-  @observable price: number = 0;
-  @observable amount: number = 1;
+  @observable
+  price: number = 0;
+  @observable
+  amount: number = 1;
 
   constructor(price) {
     this.price = price;

tests/typescript/conformance/types/decorator/__snapshots__/jsfmt.spec.js.snap

@@ -8,6 +8,7 @@ enum E {}
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 declare function dec<T>(target: T): T;
 
-@dec enum E {}
+@dec
+enum E {}
 
 `;

tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap

@@ -49,7 +49,8 @@ export class TabCompletionController {}
   selector: "angular-component"
 })
 class AngularComponent {
-  @Input() myInput: string;
+  @Input()
+  myInput: string;
 }
 
 `;
@@ -217,10 +218,14 @@ class Class2 {
 }
 
 class Class3 {
-  @d1 fieldA;
-  @d2(foo) fieldB;
-  @d3.bar fieldC;
-  @d4.baz() fieldD;
+  @d1
+  fieldA;
+  @d2(foo)
+  fieldB;
+  @d3.bar
+  fieldC;
+  @d4.baz()
+  fieldD;
 
   constructor(
     @d1 private x: number,