Various fixes for small bugs

CHANGELOG.md

@@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
 
 ## [Unreleased]
 
+### Changed
+
+- [@nathan-beam] - Gem does not work with CMD/Powershell.
+- [@blampe], [@kddeisz] - Comments inside keyword parameters in method declarations are not printed.
+- [@blampe], [@kddeisz] - `command_call` nodes with unary operators incorrectly parse their operator.
+- [@blampe], [@kddeisz] - Returning multiple values where the first has parentheses was incorrectly removing the remaining values.
+- [@johncsnyder], [@kddeisz] - Call chains whose left-most receiver is a no-indent expression should not indent their entire chain.
+
 ## [1.2.1] - 2020-12-27
 
 ### Changed
@@ -1051,6 +1059,7 @@ would previously result in `array[]`, but now prints properly.
 [@andyw8]: https://github.com/andyw8
 [@ashfurrow]: https://github.com/ashfurrow
 [@awinograd]: https://github.com/awinograd
+[@blampe]: https://github.com/blampe
 [@bugthing]: https://github.com/bugthing
 [@cbothner]: https://github.com/cbothner
 [@christoomey]: https://github.com/christoomey
@@ -1090,6 +1099,7 @@ would previously result in `array[]`, but now prints properly.
 [@meleyal]: https://github.com/meleyal
 [@mmainz]: https://github.com/mmainz
 [@mmcnl]: https://github.com/mmcnl
+[@nathan-beam]: https://github.com/nathan-beam
 [@noahtheduke]: https://github.com/NoahTheDuke
 [@overload119]: https://github.com/Overload119
 [@petevk]: https://github.com/petevk

lib/prettier.rb

@@ -9,8 +9,8 @@ module Prettier
 
   class << self
     def run(args)
-      quoted = args.map { |arg| arg.start_with?('-') ? arg : "'#{arg}'" }
-      command = "node #{BINARY} --plugin '#{PLUGIN}' #{quoted.join(' ')}"
+      quoted = args.map { |arg| arg.start_with?('-') ? arg : "\"#{arg}\"" }
+      command = "node #{BINARY} --plugin \"#{PLUGIN}\" #{quoted.join(' ')}"
 
       system({ 'RBPRETTIER' => '1' }, command)
     end

src/nodes/calls.js

@@ -41,15 +41,18 @@ function printCall(path, opts, print) {
     parentNode.chain = (node.chain || 0) + 1;
     parentNode.callChain = (node.callChain || 0) + 1;
     parentNode.breakDoc = (node.breakDoc || [receiverDoc]).concat(rightSideDoc);
+    parentNode.firstReceiverType = node.firstReceiverType || receiverNode.type;
   }
 
   // If we're at the top of a chain, then we're going to print out a nice
   // multi-line layout if this doesn't break into multiple lines.
   if (!chained.includes(parentNode.type) && (node.chain || 0) >= 3) {
-    return ifBreak(
-      group(indent(concat(node.breakDoc.concat(rightSideDoc)))),
-      concat([receiverDoc, group(rightSideDoc)])
-    );
+    let breakDoc = concat(node.breakDoc.concat(rightSideDoc));
+    if (!noIndent.includes(node.firstReceiverType)) {
+      breakDoc = indent(breakDoc);
+    }
+
+    return ifBreak(group(breakDoc), concat([receiverDoc, group(rightSideDoc)]));
   }
 
   // For certain left sides of the call nodes, we want to attach directly to
@@ -105,6 +108,7 @@ function printMethodAddArg(path, opts, print) {
   if (chained.includes(parentNode.type)) {
     parentNode.chain = (node.chain || 0) + 1;
     parentNode.breakDoc = (node.breakDoc || [methodDoc]).concat(argsDoc);
+    parentNode.firstReceiverType = node.firstReceiverType;
   }
 
   // If we're at the top of a chain, then we're going to print out a nice
@@ -152,7 +156,7 @@ function isSorbetTypeAnnotation(node) {
     callNode.body[0].body[0].body === "sig" &&
     callNode.body[1].type === "args" &&
     callNode.body[1].body.length === 0 &&
-    blockNode.type === "brace_block"
+    blockNode
   );
 }
 
@@ -198,6 +202,7 @@ function printMethodAddBlock(path, opts, print) {
   if (chained.includes(parentNode.type)) {
     parentNode.chain = (node.chain || 0) + 1;
     parentNode.breakDoc = (node.breakDoc || [callDoc]).concat(blockDoc);
+    parentNode.firstReceiverType = node.firstReceiverType;
   }
 
   // If we're at the top of a chain, then we're going to print out a nice

src/nodes/return.js

@@ -13,10 +13,13 @@ const { literal } = require("../utils");
 // because they have low enough operator precedence that you need to explicitly
 // keep them in there.
 const canSkipParens = (args) => {
-  const statement = args.body[0].body[0].body[0];
-  return (
-    statement.type !== "binary" || !["and", "or"].includes(statement.body[1])
-  );
+  const stmts = args.body[0].body[0];
+  if (stmts.comments) {
+    return false;
+  }
+
+  const stmt = stmts.body[0];
+  return stmt.type !== "binary" || !["and", "or"].includes(stmt.body[1]);
 };
 
 const printReturn = (path, opts, print) => {
@@ -27,25 +30,27 @@ const printReturn = (path, opts, print) => {
     return "return";
   }
 
-  // If the body of the return contains parens, then just skip directly to the
-  // content of the parens so that we can skip printing parens if we don't
-  // want them.
-  if (args.body[0] && args.body[0].type === "paren" && canSkipParens(args)) {
-    args = args.body[0].body[0];
-    steps = steps.concat("body", 0, "body", 0);
-  }
+  if (args.body.length === 1) {
+    // If the body of the return contains parens, then just skip directly to the
+    // content of the parens so that we can skip printing parens if we don't
+    // want them.
+    if (args.body[0] && args.body[0].type === "paren" && canSkipParens(args)) {
+      args = args.body[0].body[0];
+      steps = steps.concat("body", 0, "body", 0);
+    }
 
-  // If we're returning an array literal that isn't a special array, single
-  // element array, or an empty array, then we want to grab the arguments so
-  // that we can print them out as if they were normal return arguments.
-  if (
-    args.body[0] &&
-    args.body[0].type === "array" &&
-    args.body[0].body[0] &&
-    args.body[0].body[0].body.length > 1 &&
-    ["args", "args_add_star"].includes(args.body[0].body[0].type)
-  ) {
-    steps = steps.concat("body", 0, "body", 0);
+    // If we're returning an array literal that isn't a special array, single
+    // element array, or an empty array, then we want to grab the arguments so
+    // that we can print them out as if they were normal return arguments.
+    if (
+      args.body[0] &&
+      args.body[0].type === "array" &&
+      args.body[0].body[0] &&
+      args.body[0].body[0].body.length > 1 &&
+      ["args", "args_add_star"].includes(args.body[0].body[0].type)
+    ) {
+      steps = steps.concat("body", 0, "body", 0);
+    }
   }
 
   // Now that we've established which actual node is the arguments to return,
@@ -56,6 +61,12 @@ const printReturn = (path, opts, print) => {
   // be a singular doc as opposed to an array.
   const value = Array.isArray(parts) ? join(concat([",", line]), parts) : parts;
 
+  // We only get here if we have comments somewhere that would prevent us from
+  // skipping the parentheses.
+  if (args.body.length === 1 && args.body[0].type === "paren") {
+    return concat(["return", value]);
+  }
+
   return group(
     concat([
       "return",

src/parser.rb

@@ -753,6 +753,11 @@ def on_command(ident, args)
   # of the method, the operator being used to send the method, the name of
   # the method, and the arguments being passed to the method.
   def on_command_call(receiver, oper, ident, args)
+    # Make sure we take the operator out of the scanner events so that it
+    # doesn't get confused for a unary operator later.
+    scanner_events.delete(oper)
+
+    # Grab the ending from either the arguments or the method being sent
     ending = args || ident
 
     {

src/printer.js

@@ -67,6 +67,38 @@ function getCommentChildNodes(node) {
 
       return [node.body[0]].concat(values).concat(node.body[2]);
     }
+    case "params": {
+      const [reqs, optls, rest, post, kwargs, kwargRest, block] = node.body;
+      let parts = reqs || [];
+
+      (optls || []).forEach((optl) => {
+        parts = parts.concat(optl);
+      });
+
+      if (rest) {
+        parts.push(rest);
+      }
+
+      parts = parts.concat(post || []);
+
+      (kwargs || []).forEach((kwarg) => {
+        if (kwarg[1]) {
+          parts = parts.concat(kwarg);
+        } else {
+          parts.push(kwarg[0]);
+        }
+      });
+
+      if (kwargRest && kwargRest !== "nil") {
+        parts.push(kwargRest);
+      }
+
+      if (block) {
+        parts.push(block);
+      }
+
+      return parts;
+    }
     default:
       return node.body;
   }

test/js/nodes/calls.test.js

@@ -49,12 +49,34 @@ describe("calls", () => {
   test("explicit call maintains call", () =>
     expect("a.call(1, 2, 3)").toMatchFormat());
 
-  test("double bang with a special operator", () =>
+  test("double bang with a special operator on a call", () =>
     expect("!!object&.topic_list").toMatchFormat());
 
+  test("bang with a special operator on a command_call", () =>
+    expect("!domain&.include? '@'").toMatchFormat());
+
   test("#call shorthand does not eliminate empty parentheses", () =>
     expect("Foo.new.()").toMatchFormat());
 
   test("methods that look like constants do not eliminate empty parens", () =>
     expect("Foo()").toMatchFormat());
+
+  test("call chains with no indent on the first receiver", () => {
+    const item = long.slice(0, 30);
+    const content = `result = [${item}, ${item}, ${item}].map(&:foo?).bbb.ccc`;
+
+    const expected = ruby(`
+      result =
+        [
+          ${item},
+          ${item},
+          ${item}
+        ]
+        .map(&:foo?)
+        .bbb
+        .ccc
+    `);
+
+    return expect(content).toChangeFormat(expected);
+  });
 });

test/js/nodes/method.test.js

@@ -135,6 +135,19 @@ describe("method", () => {
 
       return expect(content).toChangeFormat(expected);
     });
+
+    test("comments on kwargs", () => {
+      const content = ruby(`
+        def foo(
+          bar:, # hello
+          baz:
+        )
+          bar
+        end
+      `);
+
+      return expect(content).toMatchFormat();
+    });
   });
 
   describe("method calls", () => {

test/js/nodes/return.test.js

@@ -40,4 +40,7 @@ describe("return", () => {
     expect(`return foo, ${long}`).toChangeFormat(
       `return [\n  foo,\n  ${long}\n]`
     ));
+
+  test("returning two arguments, the first with parentheses", () =>
+    expect("return (1), 2").toMatchFormat());
 });