Stop printing unnecessary parens around single element arrow function…

… param lists (e.g. `(x) => 2 * x;` becomes `x => 2 * x`)

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=235014974

src/com/google/javascript/jscomp/CodeGenerator.java

@@ -21,6 +21,8 @@
 
 import com.google.common.base.Preconditions;
 import com.google.debugging.sourcemap.Util;
+import com.google.javascript.jscomp.parsing.parser.FeatureSet;
+import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
 import com.google.javascript.rhino.JSDocInfo.Visibility;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.Token;
@@ -49,6 +51,7 @@ public class CodeGenerator {
   private final boolean trustedStrings;
   private final boolean quoteKeywordProperties;
   private final boolean useOriginalName;
+  private final FeatureSet outputFeatureSet;
   private final JSDocInfoPrinter jsDocInfoPrinter;
 
   private CodeGenerator(CodeConsumer consumer) {
@@ -59,6 +62,7 @@ private CodeGenerator(CodeConsumer consumer) {
     preserveTypeAnnotations = false;
     quoteKeywordProperties = false;
     useOriginalName = false;
+    this.outputFeatureSet = FeatureSet.BARE_MINIMUM;
     this.jsDocInfoPrinter = new JSDocInfoPrinter(false);
   }
 
@@ -71,6 +75,7 @@ protected CodeGenerator(CodeConsumer consumer, CompilerOptions options) {
     this.preserveTypeAnnotations = options.preserveTypeAnnotations;
     this.quoteKeywordProperties = options.shouldQuoteKeywordProperties();
     this.useOriginalName = options.getUseOriginalNamesInOutput();
+    this.outputFeatureSet = options.getOutputFeatureSet();
     this.jsDocInfoPrinter = new JSDocInfoPrinter(useOriginalName);
   }
 
@@ -297,9 +302,17 @@ protected void add(Node n, Context context) {
         break;
 
       case PARAM_LIST:
-        add("(");
-        addList(first);
-        add(")");
+        // If this is the list for a non-TypeScript arrow function with one simple name param.
+        if (n.getParent().isArrowFunction()
+            && n.hasOneChild()
+            && first.isName()
+            && !outputFeatureSet.has(Feature.TYPE_ANNOTATION)) {
+          add(first);
+        } else {
+          add("(");
+          addList(first);
+          add(")");
+        }
         break;
 
       case DEFAULT_VALUE:

test/com/google/javascript/jscomp/CodePrinterTest.java

@@ -1149,7 +1149,7 @@ public void testPrettyPrinter4() {
 
   @Test
   public void testPrettyPrinter_arrow() {
-    assertPrettyPrint("(a)=>123;", "(a) => 123;\n");
+    assertPrettyPrint("(a)=>123;", "a => 123;\n");
   }
 
   @Test
@@ -2751,18 +2751,33 @@ public void testMemberGeneratorYield1() {
   }
 
   @Test
-  public void testArrowFunction() {
+  public void testArrowFunction_zeroParams() {
     assertPrintSame("()=>1");
     assertPrint("(()=>1)", "()=>1");
     assertPrintSame("()=>{}");
-    assertPrint("a=>b", "(a)=>b");
-    assertPrint("(a=>b)(1)", "((a)=>b)(1)");
-    assertPrintSame("var z={x:(a)=>1}");
-    assertPrint("(a,b)=>b", "(a,b)=>b");
-    assertPrintSame("()=>(a,b)");
     assertPrint("(()=>a),b", "()=>a,b");
     assertPrint("()=>(a=b)", "()=>a=b");
-    assertPrintSame("[1,2].forEach((x)=>y)");
+  }
+
+  @Test
+  public void testArrowFunction_oneParam() {
+    assertPrintSame("a=>b");
+    assertPrintSame("([a])=>b");
+    assertPrintSame("(...a)=>b");
+    assertPrintSame("(a=0)=>b");
+    assertPrintSame("(a=>b)(1)");
+    assertPrintSame("var z={x:a=>1}");
+    assertPrintSame("[1,2].forEach(x=>y)");
+  }
+
+  @Test
+  public void testArrowFunction_manyParams() {
+    assertPrintSame("(a,b)=>b");
+  }
+
+  @Test
+  public void testArrowFunction_bodyEdgeCases() {
+    assertPrintSame("()=>(a,b)");
     assertPrintSame("()=>({a:1})");
     assertPrintSame("()=>{return 1}");
   }
@@ -2793,6 +2808,8 @@ public void testAsyncGeneratorFunction() {
   public void testAsyncArrowFunction() {
     languageMode = LanguageMode.ECMASCRIPT_NEXT;
     assertPrintSame("async()=>1");
+    assertPrint("async (a) => 1", "async a=>1");
+
     // implicit semicolon prevents async being treated as a keyword
     assertPrint("f=async\n()=>1", "f=async;()=>1");
   }
@@ -2826,36 +2843,41 @@ public void testAwaitExpression() {
     assertPrintSame("pwait=async function(promise){return await promise}");
     assertPrintSame("class C{async pwait(promise){await promise}}");
     assertPrintSame("o={async pwait(promise){await promise}}");
-    assertPrintSame("pwait=async(promise)=>await promise");
+    assertPrintSame("pwait=async promise=>await promise");
   }
 
-  /** Regression test for b/28633247 - necessary parens dropped around arrow functions. */
+  /**
+   * Regression test for b/28633247 - necessary parens dropped around arrow functions.
+   *
+   * <p>Many of these cases use single param arrows because their PARAM_LIST parens should also be
+   * dropped, which can make this harder to parse for humans.
+   */
   @Test
   public void testParensAroundArrow() {
     // Parens required for non-assignment binary operator
-    assertPrintSame("x||((_)=>true)");
+    assertPrint("x||((_)=>true)", "x||(_=>true)");
     // Parens required for unary operator
-    assertPrintSame("void((e)=>e*5)");
+    assertPrint("void((e)=>e*5)", "void(e=>e*5)");
     // Parens not required for comma operator
-    assertPrint("((_) => true), ((_) => false)", "(_)=>true,(_)=>false");
+    assertPrint("((_) => true), ((_) => false)", "_=>true,_=>false");
     // Parens not required for right side of assignment operator
     // NOTE: An arrow function on the left side would be a parse error.
-    assertPrint("x = ((_) => _ + 1)", "x=(_)=>_+1");
+    assertPrint("x = ((_) => _ + 1)", "x=_=>_+1");
     // Parens required for template tag
-    assertPrintSame("((_)=>\"\")`template`");
+    assertPrint("((_)=>\"\")`template`", "(_=>\"\")`template`");
     // Parens required to reference a property
     assertPrintSame("((a,b,c)=>a+b+c).length");
     assertPrintSame("((a,b,c)=>a+b+c)[\"length\"]");
     // Parens not required when evaluating property name.
     // (It doesn't make much sense to do it, though.)
-    assertPrint("x[((_)=>0)]", "x[(_)=>0]");
+    assertPrint("x[((_)=>0)]", "x[_=>0]");
     // Parens required to call the arrow function immediately
-    assertPrintSame("((x)=>x*5)(10)");
+    assertPrint("((x)=>x*5)(10)", "(x=>x*5)(10)");
     // Parens not required for function call arguments
-    assertPrint("x(((_) => true), ((_) => false))", "x((_)=>true,(_)=>false)");
+    assertPrint("x(((_) => true), ((_) => false))", "x(_=>true,_=>false)");
     // Parens required for first operand to a conditional, but not the rest.
-    assertPrintSame("((x)=>1)?a:b");
-    assertPrint("x?((x)=>0):((x)=>1)", "x?(x)=>0:(x)=>1");
+    assertPrint("((x)=>1)?a:b", "(x=>1)?a:b");
+    assertPrint("x?((x)=>0):((x)=>1)", "x?x=>0:x=>1");
   }
 
   @Test

test/com/google/javascript/refactoring/examples/GoogBindToArrowTest.java

@@ -16,12 +16,10 @@
 package com.google.javascript.refactoring.examples;
 
 import com.google.javascript.refactoring.Scanner;
-
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-
 /**
  * Test case for {@link GoogBindToArrow}.
  */
@@ -128,10 +126,10 @@ public void testSimpleReturn() {
 
     assertChanges(
         LINE_JOINER.join(
-            "[1,2,3].forEach(goog.bind(function(x) {",
+            "[1,2,3].forEach(goog.bind(function(x) {", //
             "  return y;",
             "}, this));"),
-        "[1,2,3].forEach((x) => y);");
+        "[1,2,3].forEach(x => y);");
   }
 
   /**