Increase consistency between formatting of NTI and OTI.

 * NTI unions are now always parenthesized
 * NTI now omits bangs for objects unioned with null
 * NTI functions now have a space after their colon
 * OTI functions no longer have a space before the lparen

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

src/com/google/javascript/jscomp/newtypes/FunctionType.java

@@ -1505,7 +1505,7 @@ public StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
     }
     builder.append(')');
     if (returnType != null) {
-      builder.append(':');
+      builder.append(": ");
       returnType.appendTo(builder, ctx);
     }
     if (isLoose()) {

src/com/google/javascript/jscomp/newtypes/JSType.java

@@ -1612,7 +1612,8 @@ public final String toString(ToStringContext ctx) {
    * code generation.
    */
   StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
-    // TODO(sdh): checkState(!forAnnotations) all
+    // TODO(sdh): checkState(!forAnnotations) all cases that don't work for annotations.
+    boolean isUnion = isUnion();
     switch (getMask()) {
       case BOTTOM_MASK:
         return builder.append("bottom");
@@ -1621,6 +1622,9 @@ StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
       case UNKNOWN_MASK:
         return builder.append("?");
       default:
+        if (isUnion) {
+          builder.append("(");
+        }
         int tags = getMask();
         boolean firstIteration = true;
         for (int tag = 1; tag != END_MASK; tag <<= 1) {
@@ -1661,12 +1665,17 @@ StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
                 tags &= ~TYPEVAR_MASK;
                 continue;
               case NON_SCALAR_MASK: {
-                if (getObjs().size() == 1) {
+                boolean isNullable = isNullable();
+                if (getObjs().size() == 1 && !isNullable) {
                   Iterables.getOnlyElement(getObjs()).appendTo(builder, ctx);
                 } else {
                   Set<String> strReps = new TreeSet<>();
                   for (ObjectType obj : getObjs()) {
-                    strReps.add(obj.toString(ctx));
+                    String rep = obj.toString(ctx);
+                    if (isNullable && rep.charAt(0) == '!') {
+                      rep = rep.substring(1);
+                    }
+                    strReps.add(rep);
                   }
                   PIPE_JOINER.appendTo(builder, strReps);
                 }
@@ -1692,14 +1701,18 @@ StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
           }
         }
         if (tags == 0) { // Found all types in the union
-          return builder;
+          // nothing else to do
         } else if (tags == TRUTHY_MASK) {
-          return builder.append("truthy");
+          builder.append("truthy");
         } else if (tags == FALSY_MASK) {
-          return builder.append("falsy");
+          builder.append("falsy");
         } else {
-          return builder.append("Unrecognized type: ").append(tags);
+          builder.append("Unrecognized type: ").append(tags);
+        }
+        if (isUnion) {
+          builder.append(")");
         }
+        return builder;
     }
   }
 

src/com/google/javascript/rhino/jstype/FunctionType.java

@@ -1031,7 +1031,7 @@ StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
 
     setPrettyPrint(false);
 
-    sb.append("function (");
+    sb.append("function(");
     int paramNum = call.parameters.getChildCount();
     boolean hasKnownTypeOfThis = !(typeOfThis instanceof UnknownType);
     if (hasKnownTypeOfThis) {

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

@@ -1309,19 +1309,10 @@ public void testCustomBanUnknownProp2() {
         " /** @param {ObjectWithNoProps} a */",
         "function f(a) { alert(a.foobar); };");
 
-    this.mode = TypeInferenceMode.OTI_ONLY;
     testWarning(
         js,
         CheckConformance.CONFORMANCE_VIOLATION,
         "Violation: My rule message\nThe property \"foobar\" on type \"(ObjectWithNoProps|null)\"");
-
-    // TODO(aravindpg): Only difference is we don't add parens at the ends of our union type
-    // string reprs in NTI. Fix them to be the same if possible.
-    this.mode = TypeInferenceMode.NTI_ONLY;
-    testWarning(
-        js,
-        CheckConformance.CONFORMANCE_VIOLATION,
-        "Violation: My rule message\nThe property \"foobar\" on type \"ObjectWithNoProps|null\"");
   }
 
   public void testCustomBanUnknownProp3() {

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

@@ -1448,19 +1448,6 @@ public void testClosureLibraryTypeAnnotationExamples() {
             " * @param {(Object|null)} p0",
             " * @return {undefined}",
             " */",
-            "goog.removeHashCode = goog.removeUid;\n"),
-        LINE_JOINER.join(
-            "/** @const */ var goog = goog || {};",
-            "/**",
-            " * @param {!Object|null} obj",
-            " * @return {undefined}",
-            " */",
-            "goog.removeUid = function(obj) {",
-            "};",
-            "/**",
-            " * @param {!Object|null} p0",
-            " * @return {undefined}",
-            " */",
             "goog.removeHashCode = goog.removeUid;\n"));
   }
 

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

@@ -552,21 +552,21 @@ public void testStaticProperty() {
     output = ""
         + "/** @constructor */ function Foo(){}"
         + "/** @constructor */ function Bar(){}"
-        + "Foo.function__new_Foo___undefined$a = 0;"
-        + "Bar.function__new_Bar___undefined$a = 0;";
+        + "Foo.function_new_Foo___undefined$a = 0;"
+        + "Bar.function_new_Bar___undefined$a = 0;";
 
-    testSets(js, output, "{a=[[function (new:Bar): undefined]," +
-    " [function (new:Foo): undefined]]}");
+    testSets(js, output, "{a=[[function(new:Bar): undefined]," +
+    " [function(new:Foo): undefined]]}");
 
     this.mode = TypeInferenceMode.NTI_ONLY;
     output = ""
         + "/** @constructor */ function Foo(){}"
         + "/** @constructor */ function Bar(){}"
-        + "Foo.Foo__function_new_Foo__undefined__$a = 0;"
-        + "Bar.Bar__function_new_Bar__undefined__$a = 0;";
+        + "Foo.Foo__function_new_Foo___undefined__$a = 0;"
+        + "Bar.Bar__function_new_Bar___undefined__$a = 0;";
 
     testSets(js, output,
-        "{a=[[Bar<|function(new:Bar):undefined|>], [Foo<|function(new:Foo):undefined|>]]}");
+        "{a=[[Bar<|function(new:Bar): undefined|>], [Foo<|function(new:Foo): undefined|>]]}");
   }
 
   public void testSupertypeWithSameField() {
@@ -686,7 +686,7 @@ public void testUntypedCodeWrongDisambiguation1() {
         LINE_JOINER.join(
         "The right side in the assignment is not a subtype of the left side.",
         "Expected : Foo",
-        "Found    : Bar|Foo",
+        "Found    : (Bar|Foo)",
         "More details:",
         "The found type is a union that includes an unexpected type: Bar"));
   }
@@ -1545,15 +1545,15 @@ public void testConstructorsWithTypeErrorsAreNotDisambiguated() {
 
     this.mode = TypeInferenceMode.OTI_ONLY;
     testSets("", js, js, "{}", TypeValidator.TYPE_MISMATCH_WARNING, "assignment\n"
-            + "found   : function (new:Foo): undefined\n"
-            + "required: function (new:Bar): undefined");
+            + "found   : function(new:Foo): undefined\n"
+            + "required: function(new:Bar): undefined");
     this.mode = TypeInferenceMode.NTI_ONLY;
     testSets("", js, js, "{}",
         NewTypeInference.MISTYPED_ASSIGN_RHS,
         LINE_JOINER.join(
             "The right side in the assignment is not a subtype of the left side.",
-            "Expected : Bar<|function(new:Bar):?|>",
-            "Found    : Foo<|function(new:Foo):undefined|>",
+            "Expected : Bar<|function(new:Bar): ?|>",
+            "Found    : Foo<|function(new:Foo): undefined|>",
             "More details:",
             "Incompatible types for property prototype.",
             "Expected : Bar.prototype",
@@ -1979,7 +1979,7 @@ public void testMismatchInvalidation() {
     testSets("", js, js, "{}", NewTypeInference.MISTYPED_ASSIGN_RHS,
         LINE_JOINER.join(
             "The right side in the assignment is not a subtype of the left side.",
-            "Expected : Foo|null",
+            "Expected : (Foo|null)",
             "Found    : Bar\n"));
   }
 
@@ -2239,8 +2239,8 @@ public void testUnsafeTypingOfThis() {
         NewTypeInference.INVALID_ARGUMENT_TYPE,
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function myArrayPrototypeMap.",
-            "Expected : function(this:Bar|Foo):?",
-            "Found    : function(this:Foo):?\n"));
+            "Expected : function(this:(Bar|Foo)): ?",
+            "Found    : function(this:Foo): ?\n"));
 
     js = LINE_JOINER.join(
         "/** @constructor */",
@@ -2279,8 +2279,8 @@ public void testUnsafeTypingOfThis() {
         NewTypeInference.INVALID_ARGUMENT_TYPE,
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function f.",
-            "Expected : function(this:Bar|Foo):?",
-            "Found    : function(this:Foo):?\n"));
+            "Expected : function(this:(Bar|Foo)): ?",
+            "Found    : function(this:Foo): ?\n"));
   }
 
   public void testErrorOnProtectedProperty() {

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

@@ -164,7 +164,7 @@ public void testExportMultiple2() throws Exception {
             "goog.exportProperty(a.b, 'c', a.b.c);",
             "goog.exportProperty(a.b.prototype, 'c', a.b.prototype.c);"),
         LINE_JOINER.join(
-            "/** @type {{b: function (?): undefined}} */",
+            "/** @type {{b: function(?): undefined}} */",
             "var hello = {};",
             "hello.b;",
             "/**",
@@ -878,8 +878,8 @@ public void testNullabilityInFunctionTypes() throws Exception {
             "goog.exportSymbol('x', x);"),
         LINE_JOINER.join(
             "/**",
-            " * @param {function ((Object|null)): ?} takesNullable",
-            " * @param {function (!Object): ?} takesNonNullable",
+            " * @param {function((Object|null)): ?} takesNullable",
+            " * @param {function(!Object): ?} takesNonNullable",
             " * @return {undefined}",
             " */",
             "var x = function(takesNullable, takesNonNullable) {",

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

@@ -66,8 +66,8 @@ public void testBuiltInTypeDifferentReturnType() throws Exception {
         + "function String(opt_str) {}\n",
         "", FunctionTypeBuilder.TYPE_REDEFINITION,
         "attempted re-definition of type String\n"
-        + "found   : function (new:String, *=): number\n"
-        + "expected: function (new:String, *=): string");
+        + "found   : function(new:String, *=): number\n"
+        + "expected: function(new:String, *=): string");
   }
 
   public void testBuiltInTypeDifferentNumParams() throws Exception {
@@ -79,8 +79,8 @@ public void testBuiltInTypeDifferentNumParams() throws Exception {
         + "function String() {}\n",
         "", FunctionTypeBuilder.TYPE_REDEFINITION,
         "attempted re-definition of type String\n"
-        + "found   : function (new:String): string\n"
-        + "expected: function (new:String, *=): string");
+        + "found   : function(new:String): string\n"
+        + "expected: function(new:String, *=): string");
   }
 
   public void testBuiltInTypeDifferentNumParams2() throws Exception {
@@ -92,8 +92,8 @@ public void testBuiltInTypeDifferentNumParams2() throws Exception {
         + "function String(opt_str, opt_nothing) {}\n",
         "", FunctionTypeBuilder.TYPE_REDEFINITION,
         "attempted re-definition of type String\n"
-        + "found   : function (new:String, ?=, ?=): string\n"
-        + "expected: function (new:String, *=): string");
+        + "found   : function(new:String, ?=, ?=): string\n"
+        + "expected: function(new:String, *=): string");
   }
 
   public void testBuiltInTypeDifferentParamType() throws Exception {
@@ -105,17 +105,17 @@ public void testBuiltInTypeDifferentParamType() throws Exception {
         + "function String(opt_str) {}\n",
         "", FunctionTypeBuilder.TYPE_REDEFINITION,
         "attempted re-definition of type String\n"
-        + "found   : function (new:String, ?=): string\n"
-        + "expected: function (new:String, *=): string");
+        + "found   : function(new:String, ?=): string\n"
+        + "expected: function(new:String, *=): string");
   }
 
   public void testBadFunctionTypeDefinition() throws Exception {
     testSame(
         "/** @constructor */function Function(opt_str) {}\n",
         "", FunctionTypeBuilder.TYPE_REDEFINITION,
         "attempted re-definition of type Function\n"
-        + "found   : function (new:Function, ?=): ?\n"
-        + "expected: function (new:Function, ...*): ?");
+        + "found   : function(new:Function, ?=): ?\n"
+        + "expected: function(new:Function, ...*): ?");
   }
 
   public void testInlineJsDoc() throws Exception {

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

@@ -17957,11 +17957,11 @@ public void testPinpointTypeDiffWhenMismatch() {
         NewTypeInference.INVALID_ARGUMENT_TYPE,
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function f.",
-            "Expected : {a: number, b: string|undefined=}",
+            "Expected : {a: number, b: (string|undefined)=}",
             "Found    : {a: number, b: number}",
             "More details:",
             "Incompatible types for property b.",
-            "Expected : string|undefined",
+            "Expected : (string|undefined)",
             "Found    : number"));
 
     typeCheckMessageContents(LINE_JOINER.join(
@@ -17982,7 +17982,7 @@ public void testPinpointTypeDiffWhenMismatch() {
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function f.",
             "Expected : {a: number, b: string}",
-            "Found    : {a: number, b: string|undefined=}",
+            "Found    : {a: number, b: (string|undefined)=}",
             "More details:",
             "In found type, property b is optional but should be required."));
 
@@ -17993,12 +17993,12 @@ public void testPinpointTypeDiffWhenMismatch() {
         NewTypeInference.INVALID_ARGUMENT_TYPE,
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function f.",
-            "Expected : function(number|string):?",
-            "Found    : function(number):undefined",
+            "Expected : function((number|string)): ?",
+            "Found    : function(number): undefined",
             "More details:",
             "The expected and found types are functions which have incompatible"
             + " types for argument 1.",
-            "Expected a supertype of : number|string",
+            "Expected a supertype of : (number|string)",
             "but found               : number"));
 
     typeCheckMessageContents(LINE_JOINER.join(
@@ -18008,8 +18008,8 @@ public void testPinpointTypeDiffWhenMismatch() {
         NewTypeInference.INVALID_ARGUMENT_TYPE,
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function f.",
-            "Expected : function():number",
-            "Found    : function():string",
+            "Expected : function(): number",
+            "Found    : function(): string",
             "More details:",
             "The expected and found types are functions which have incompatible"
             + " return types.",
@@ -18027,7 +18027,7 @@ public void testPinpointTypeDiffWhenMismatch() {
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function f.",
             "Expected : Foo",
-            "Found    : Bar|Foo",
+            "Found    : (Bar|Foo)",
             "More details:",
             "The found type is a union that includes an unexpected type: Bar"));
 
@@ -18042,7 +18042,7 @@ public void testPinpointTypeDiffWhenMismatch() {
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function f.",
             "Expected : Foo",
-            "Found    : Bar|null",
+            "Found    : (Bar|null)",
             "More details:",
             "The found type is a union that "
             + "includes an unexpected type: null"));
@@ -18056,7 +18056,7 @@ public void testPinpointTypeDiffWhenMismatch() {
         LINE_JOINER.join(
             "Invalid type for parameter 1 of function f.",
             "Expected : number",
-            "Found    : number|string",
+            "Found    : (number|string)",
             "More details:",
             "The found type is a union that "
             + "includes an unexpected type: string"));
@@ -21113,7 +21113,7 @@ public void testReanalyzeLoopConditionAfterLoopBody() {
         LINE_JOINER.join(
             "Invalid type(s) for operator SUB.",
             "Expected : number",
-            "Found    : number|string",
+            "Found    : (number|string)",
             "More details:",
             "The found type is a union that includes an unexpected type: string"));
   }
@@ -21156,8 +21156,8 @@ public void testPrintTypevarIDs() {
         NewTypeInference.MISTYPED_ASSIGN_RHS,
         LINE_JOINER.join(
             "The right side in the assignment is not a subtype of the left side.",
-            "Expected : function(this:?,T#1,W):?",
-            "Found    : function(this:Foo,T#2,?):?",
+            "Expected : function(this:?,T#1,W): ?",
+            "Found    : function(this:Foo,T#2,?): ?",
             "More details:",
             "The expected and found types are functions which have incompatible types"
                 + " for argument 1.",