Skip to content

Commit

Permalink
Change FunctionType to print as "(typeof Foo)" when the type is ass…
Browse files Browse the repository at this point in the history
…ociated with the definition of an instance type.

This condition corresponds to constructors or interfaces with a known source location and reflects the use of these functions as namespaces. It makes specific distinction between "the Foo constructor" and "some constructor of Foo's". The latter still prints as "function(new:Foo): undefined".

One of the goals is to make the printed types of static methods less verbose and more understandable. Example "function(this:(typeof Foo), string): number".

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=245086325
  • Loading branch information
nreid260 authored and blickly committed Apr 25, 2019
1 parent 4db9c16 commit a42292f
Show file tree
Hide file tree
Showing 8 changed files with 175 additions and 132 deletions.
10 changes: 10 additions & 0 deletions src/com/google/javascript/rhino/jstype/FunctionType.java
Expand Up @@ -953,6 +953,16 @@ StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
return sb.append(forAnnotations ? "!Function" : "Function");
}

if (hasInstanceType() && getSource() != null) {
// Render function types known to be type definitions as "(typeof Foo)". This includes types
// defined like "/** @constructor */ function Foo() { }" but not to those defined like "@param
// {function(new:Foo)}". Only the former will have a source node.
sb.append("(typeof ");
getInstanceType().appendTo(sb, forAnnotations);
sb.append(")");
return sb;
}

setPrettyPrint(false);

sb.append("function(");
Expand Down
80 changes: 42 additions & 38 deletions test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java
Expand Up @@ -613,14 +613,14 @@ public void testStaticProperty() {
+ "Bar.a = 0;";
String output;

output = ""
+ "/** @constructor */ function Foo(){}"
+ "/** @constructor */ function Bar(){}"
+ "Foo.function_new_Foo___undefined$a = 0;"
+ "Bar.function_new_Bar___undefined$a = 0;";
output =
""
+ "/** @constructor */ function Foo(){}"
+ "/** @constructor */ function Bar(){}"
+ "Foo._typeof_Foo_$a = 0;"
+ "Bar._typeof_Bar_$a = 0;";

testSets(js, output, "{a=[[function(new:Bar): undefined]," +
" [function(new:Foo): undefined]]}");
testSets(js, output, "{a=[[(typeof Bar)]," + " [(typeof Foo)]]}");
}

@Test
Expand Down Expand Up @@ -1629,9 +1629,13 @@ public void testConstructorsWithTypeErrorsAreNotDisambiguated() {
"",
"(new Bar()).alias();");

testSets("", js, js, "{}", TypeValidator.TYPE_MISMATCH_WARNING, "assignment\n"
+ "found : function(new:Foo): undefined\n"
+ "required: function(new:Bar): undefined");
testSets(
"",
js,
js,
"{}",
TypeValidator.TYPE_MISMATCH_WARNING,
"assignment\n" + "found : (typeof Foo)\n" + "required: (typeof Bar)");
}

@Test
Expand Down Expand Up @@ -2968,13 +2972,13 @@ public void testDisambiguateEs6ClassStaticMethods() {
"Bar.method();"),
lines(
"class Foo {",
" function_new_Foo___undefined$method() {}",
" _typeof_Foo_$method() {}",
"}",
"class Bar {",
" function_new_Bar___undefined$method() {}",
" _typeof_Bar_$method() {}",
"}",
"Foo.function_new_Foo___undefined$method();",
"Bar.function_new_Bar___undefined$method();"));
"Foo._typeof_Foo_$method();",
"Bar._typeof_Bar_$method();"));
}

@Test
Expand All @@ -2995,17 +2999,17 @@ public void testEs6ClassSideInheritedMethods_areDisambiguated() {
"Bar.method();"),
lines(
"class Foo {",
" function_new_Foo___undefined$method() {}",
" _typeof_Foo_$method() {}",
"}",
"class SubFoo extends Foo {",
" static function_new_Foo___undefined$method() {}",
" static _typeof_Foo_$method() {}",
"}",
"class Bar {",
" function_new_Bar___undefined$method() {}",
" _typeof_Bar_$method() {}",
"}",
"Foo.function_new_Foo___undefined$method();",
"SubFoo.function_new_Foo___undefined$method();",
"Bar.function_new_Bar___undefined$method();"));
"Foo._typeof_Foo_$method();",
"SubFoo._typeof_Foo_$method();",
"Bar._typeof_Bar_$method();"));
}

@Test
Expand Down Expand Up @@ -3051,18 +3055,18 @@ public void testEs6ClassSideInheritedMethods_referencedWithSuper_areDisambiguate
"Bar.method();"),
lines(
"class Foo {",
" static function_new_Foo___undefined$method() {}",
" static _typeof_Foo_$method() {}",
"}",
"class SubFoo extends Foo {",
" static method2() {",
" super.function_new_Foo___undefined$method();",
" super._typeof_Foo_$method();",
" }",
"}",
"class Bar {",
" static function_new_Bar___undefined$method() {}",
" static _typeof_Bar_$method() {}",
"}",
"Foo.function_new_Foo___undefined$method();",
"Bar.function_new_Bar___undefined$method();"));
"Foo._typeof_Foo_$method();",
"Bar._typeof_Bar_$method();"));
}

@Test
Expand Down Expand Up @@ -3228,9 +3232,9 @@ public void testDisambiguatePropertyReference_objectPattern_stringKey_inDeclarat
" }",
"}",
"/** @type {string} */",
"Foo.function_new_Foo___undefined$prop = 'static property!';",
"Foo._typeof_Foo_$prop = 'static property!';",
"const {Foo$prop: prop} = (new Foo());"),
"{prop=[[Foo], [function(new:Foo): undefined]]}");
"{prop=[[(typeof Foo)], [Foo]]}");
}

@Test
Expand All @@ -3254,10 +3258,10 @@ public void testDontDisambiguatePropertyReference_objectPattern_withQuotedString
" }",
"}",
"/** @type {string} */",
"Foo.function_new_Foo___undefined$prop = 'static property!';",
"Foo._typeof_Foo_$prop = 'static property!';",
// we still rewrite the other 'prop' references, but ignore the quoted 'prop'
"const {'prop': prop} = {'prop': 3};"),
"{prop=[[Foo], [function(new:Foo): undefined]]}");
"{prop=[[(typeof Foo)], [Foo]]}");
}

@Test
Expand All @@ -3281,9 +3285,9 @@ public void testDisambiguateCtorPropertyReference_objectPattern_stringKey_inDecl
" }",
"}",
"/** @type {string} */",
"Foo.function_new_Foo___undefined$prop = 'static property!';",
"const {function_new_Foo___undefined$prop: prop} = Foo;"),
"{prop=[[Foo], [function(new:Foo): undefined]]}");
"Foo._typeof_Foo_$prop = 'static property!';",
"const {_typeof_Foo_$prop: prop} = Foo;"),
"{prop=[[(typeof Foo)], [Foo]]}");
}

@Test
Expand All @@ -3307,9 +3311,9 @@ public void testDisambiguatePropertyReference_objectPattern_stringKey_withDefaul
" }",
"}",
"/** @type {string} */",
"Foo.function_new_Foo___undefined$prop = 'static property!';",
"Foo._typeof_Foo_$prop = 'static property!';",
"const {Foo$prop: prop = 0} = (new Foo());"),
"{prop=[[Foo], [function(new:Foo): undefined]]}");
"{prop=[[(typeof Foo)], [Foo]]}");
}

@Test
Expand All @@ -3333,9 +3337,9 @@ public void testDisambiguatePropertyReference_objectPattern_stringKey_inParamete
" }",
"}",
"/** @type {string} */",
"Foo.function_new_Foo___undefined$prop = 'static property!';",
"Foo._typeof_Foo_$prop = 'static property!';",
"const fn = (/** !Foo */ {Foo$prop: prop}) => prop;"),
"{prop=[[Foo], [function(new:Foo): undefined]]}");
"{prop=[[(typeof Foo)], [Foo]]}");
}

@Test
Expand All @@ -3359,9 +3363,9 @@ public void testDisambiguatePropertyReference_objectPattern_stringKey_inNestedPa
" }",
"}",
"/** @type {string} */",
"Foo.function_new_Foo___undefined$prop = 'static property!';",
"Foo._typeof_Foo_$prop = 'static property!';",
"const {f: {Foo$prop: prop}} = {f: new Foo()};"),
"{prop=[[Foo], [function(new:Foo): undefined]]}");
"{prop=[[(typeof Foo)], [Foo]]}");
}

@Test
Expand Down
20 changes: 10 additions & 10 deletions test/com/google/javascript/jscomp/FunctionTypeBuilderTest.java
Expand Up @@ -72,8 +72,8 @@ public void testBuiltInTypeDifferentReturnType() {
warning(FunctionTypeBuilder.TYPE_REDEFINITION)
.withMessage(
"attempted re-definition of type String\n"
+ "found : function(new:String, *=): number\n"
+ "expected: function(new:String, *=): string"));
+ "found : (typeof String)\n"
+ "expected: (typeof String)"));
}

@Test
Expand All @@ -89,8 +89,8 @@ public void testBuiltInTypeDifferentNumParams() {
warning(FunctionTypeBuilder.TYPE_REDEFINITION)
.withMessage(
"attempted re-definition of type String\n"
+ "found : function(new:String): string\n"
+ "expected: function(new:String, *=): string"));
+ "found : (typeof String)\n"
+ "expected: (typeof String)"));
}

@Test
Expand All @@ -106,8 +106,8 @@ public void testBuiltInTypeDifferentNumParams2() {
warning(FunctionTypeBuilder.TYPE_REDEFINITION)
.withMessage(
"attempted re-definition of type String\n"
+ "found : function(new:String, ?=, ?=): string\n"
+ "expected: function(new:String, *=): string"));
+ "found : (typeof String)\n"
+ "expected: (typeof String)"));
}

@Test
Expand All @@ -123,8 +123,8 @@ public void testBuiltInTypeDifferentParamType() {
warning(FunctionTypeBuilder.TYPE_REDEFINITION)
.withMessage(
"attempted re-definition of type String\n"
+ "found : function(new:String, ?=): string\n"
+ "expected: function(new:String, *=): string"));
+ "found : (typeof String)\n"
+ "expected: (typeof String)"));
}

@Test
Expand All @@ -135,8 +135,8 @@ public void testBadFunctionTypeDefinition() {
warning(FunctionTypeBuilder.TYPE_REDEFINITION)
.withMessage(
"attempted re-definition of type Function\n"
+ "found : function(new:Function, ?=): ?\n"
+ "expected: function(new:Function, ...*): ?"));
+ "found : (typeof Function)\n"
+ "expected: (typeof Function)"));
}

@Test
Expand Down
6 changes: 3 additions & 3 deletions test/com/google/javascript/jscomp/InferJSDocInfoTest.java
Expand Up @@ -354,7 +354,7 @@ public void testJSDocIsPropagatedToCtors() {
)));

JSType xType = inferredTypeOfName("x");
assertThat(xType.toString()).isEqualTo("function(new:Foo): undefined");
assertThat(xType.toString()).isEqualTo("(typeof Foo)");

// Then
assertThat(xType.getJSDocInfo().getBlockDescription()).isEqualTo("I'm a user class.");
Expand Down Expand Up @@ -919,7 +919,7 @@ public void testJSDocIsPropagatedToStaticProperties_Es5() {
)));

ObjectType xType = (ObjectType) inferredTypeOfName("x");
assertThat(xType.toString()).isEqualTo("function(new:Foo): undefined");
assertThat(xType.toString()).isEqualTo("(typeof Foo)");

// Then
assertThat(xType.getPropertyJSDocInfo("static").getBlockDescription())
Expand All @@ -944,7 +944,7 @@ public void testJSDocIsPropagatedToStaticProperties_Es6Class() {
)));

ObjectType xType = (ObjectType) inferredTypeOfName("x");
assertThat(xType.toString()).isEqualTo("function(new:Foo): undefined");
assertThat(xType.toString()).isEqualTo("(typeof Foo)");

// Then
assertThat(xType.getPropertyJSDocInfo("static").getBlockDescription())
Expand Down

0 comments on commit a42292f

Please sign in to comment.