Skip to content

Commit

Permalink
[NTI] Treat Object<> as an alias of IObject.
Browse files Browse the repository at this point in the history 
This fixes spurious warnings about dot accesses on Object<>, and also we now typecheck the parameterization of Object<>.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=144054003
  • Loading branch information
@dimvar @blickly
dimvar authored and blickly committed Jan 10, 2017
1 parent 132a882 commit 6944ab8
Show file tree
Hide file tree
Showing 8 changed files with 142 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/DiagnosticGroups.java
View file
Expand Up @@ -355,7 +355,7 @@ public DiagnosticGroup forName(String name) {
// NewTypeInference.INEXISTENT_PROPERTY, // NewTypeInference.INEXISTENT_PROPERTY,
// NewTypeInference.INVALID_ARGUMENT_TYPE, // NewTypeInference.INVALID_ARGUMENT_TYPE,
// NewTypeInference.INVALID_CAST, // NewTypeInference.INVALID_CAST,
NewTypeInference.INVALID_INDEX_TYPE, // NewTypeInference.INVALID_INDEX_TYPE,
NewTypeInference.INVALID_INFERRED_RETURN_TYPE, NewTypeInference.INVALID_INFERRED_RETURN_TYPE,
NewTypeInference.INVALID_OBJLIT_PROPERTY_TYPE, NewTypeInference.INVALID_OBJLIT_PROPERTY_TYPE,
// NewTypeInference.INVALID_OPERAND_TYPE, // NewTypeInference.INVALID_OPERAND_TYPE,
Expand Down
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/NewTypeInference.java
View file
Expand Up @@ -2978,7 +2978,8 @@ private boolean mayWarnAboutInexistentProp(Node propAccessNode,
} }


if (recvType.isUnknown() || recvType.isTrueOrTruthy() || recvType.isLoose() if (recvType.isUnknown() || recvType.isTrueOrTruthy() || recvType.isLoose()
|| allowPropertyOnSubtypes && recvType.mayContainUnknownObject()) { || (allowPropertyOnSubtypes
&& (recvType.mayContainUnknownObject() || recvType.isIObject()))) {
if (symbolTable.isPropertyDefined(pname)) { if (symbolTable.isPropertyDefined(pname)) {
return false; return false;
} }
Expand Down
5 changes: 5 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/JSType.java
View file
Expand Up @@ -1282,6 +1282,11 @@ public boolean isNonClassyObject() {
return nt != null && !nt.isClassy(); return nt != null && !nt.isClassy();
} }


public boolean isIObject() {
NominalType nt = getNominalTypeIfSingletonObj();
return nt != null && nt.isIObject();
}

public boolean isInterfaceDefinition() { public boolean isInterfaceDefinition() {
FunctionType ft = getFunTypeIfSingletonObj(); FunctionType ft = getFunTypeIfSingletonObj();
return ft != null && ft.isInterfaceDefinition(); return ft != null && ft.isInterfaceDefinition();
Expand Down
40 changes: 24 additions & 16 deletions src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java
View file
Expand Up @@ -414,12 +414,22 @@ private JSType getNamedTypeHelper(
case "Function": case "Function":
checkInvalidGenericsInstantiation(n); checkInvalidGenericsInstantiation(n);
return maybeMakeNullable(this.commonTypes.qmarkFunction()); return maybeMakeNullable(this.commonTypes.qmarkFunction());
case "Object": case "Object": {
// We don't generally handle parameterized Object<...>, but we want to JSType result;
// at least not warn about inexistent properties on it, so we type it if (n.hasChildren()) {
// as @dict. // We consider a parameterized Object<...> as an alias of IObject.
return maybeMakeNullable(n.hasChildren() NominalType iobject = this.commonTypes.getIObjectType();
? this.commonTypes.getTopDict() : this.commonTypes.getTopObject()); if (iobject == null) {
// Can happen when using old externs.
return this.commonTypes.UNKNOWN;
}
result = getNominalTypeHelper(
iobject.getRawNominalType(), n, registry, outerTypeParameters);
} else {
result = this.commonTypes.getTopObject();
}
return maybeMakeNullable(result);
}
default: default:
return lookupTypeByName(typeName, n, registry, outerTypeParameters); return lookupTypeByName(typeName, n, registry, outerTypeParameters);
} }
Expand Down Expand Up @@ -534,14 +544,13 @@ private JSType getNominalTypeHelper(RawNominalType rawType, Node n,
ImmutableList.Builder<JSType> typeList = ImmutableList.builder(); ImmutableList.Builder<JSType> typeList = ImmutableList.builder();
if (n.hasChildren()) { if (n.hasChildren()) {
// Compute instantiation of polymorphic class/interface. // Compute instantiation of polymorphic class/interface.
Preconditions.checkState(n.getFirstChild().isBlock(), n); Preconditions.checkState(n.getFirstChild().isNormalBlock(), n);
for (Node child : n.getFirstChild().children()) { for (Node child : n.getFirstChild().children()) {
typeList.add( typeList.add(getTypeFromCommentHelper(child, registry, outerTypeParameters));
getTypeFromCommentHelper(child, registry, outerTypeParameters));
} }
} }
ImmutableList<JSType> typeArguments = typeList.build(); List<JSType> typeArguments = typeList.build();
ImmutableList<String> typeParameters = rawType.getTypeParameters(); List<String> typeParameters = rawType.getTypeParameters();
int typeArgsSize = typeArguments.size(); int typeArgsSize = typeArguments.size();
int typeParamsSize = typeParameters.size(); int typeParamsSize = typeParameters.size();
if (typeArgsSize != typeParamsSize) { if (typeArgsSize != typeParamsSize) {
Expand All @@ -555,16 +564,15 @@ private JSType getNominalTypeHelper(RawNominalType rawType, Node n,
String.valueOf(typeParamsSize), String.valueOf(typeParamsSize),
String.valueOf(typeArgsSize))); String.valueOf(typeArgsSize)));
} }
return maybeMakeNullable(JSType.fromObjectType(ObjectType.fromNominalType( typeArguments = fixLengthOfTypeList(typeParameters.size(), typeArguments);
uninstantiated.instantiateGenerics( NominalType instantiated = uninstantiated.instantiateGenerics(typeArguments);
fixLengthOfTypeList(typeParameters.size(), typeArguments))))); return maybeMakeNullable(JSType.fromObjectType(ObjectType.fromNominalType(instantiated)));
} }
return maybeMakeNullable(JSType.fromObjectType(ObjectType.fromNominalType( return maybeMakeNullable(JSType.fromObjectType(ObjectType.fromNominalType(
uninstantiated.instantiateGenerics(typeArguments)))); uninstantiated.instantiateGenerics(typeArguments))));
} }


private List<JSType> fixLengthOfTypeList( private List<JSType> fixLengthOfTypeList(int desiredLength, List<JSType> typeList) {
int desiredLength, List<JSType> typeList) {
int length = typeList.size(); int length = typeList.size();
if (length == desiredLength) { if (length == desiredLength) {
return typeList; return typeList;
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/newtypes/ObjectType.java
View file
Expand Up @@ -682,7 +682,7 @@ private boolean compareRecordTypeToIObject(
if (keyType.isNumber() && Ints.tryParse(pname) == null) { if (keyType.isNumber() && Ints.tryParse(pname) == null) {
return false; return false;
} }
if (!keyType.isNumber() && !keyType.isString()) { if (!keyType.isNumber() && !keyType.isString() && !keyType.isUnknown()) {
return false; return false;
} }
if (!ptype.isSubtypeOf(valueType, subSuperMap)) { if (!ptype.isSubtypeOf(valueType, subSuperMap)) {
Expand Down
10 changes: 8 additions & 2 deletions src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java
View file
Expand Up @@ -2017,12 +2017,13 @@ private Node parseTopLevelTypeExpression(JsDocToken token) {
* TypeExpressionList := TopLevelTypeExpression * TypeExpressionList := TopLevelTypeExpression
* | TopLevelTypeExpression ',' TypeExpressionList * | TopLevelTypeExpression ',' TypeExpressionList
*/ */
private Node parseTypeExpressionList(JsDocToken token) { private Node parseTypeExpressionList(String typeName, JsDocToken token) {
Node typeExpr = parseTopLevelTypeExpression(token); Node typeExpr = parseTopLevelTypeExpression(token);
if (typeExpr == null) { if (typeExpr == null) {
return null; return null;
} }
Node typeList = IR.block(); Node typeList = IR.block();
int numTypeExprs = 1;
typeList.addChildToBack(typeExpr); typeList.addChildToBack(typeExpr);
while (match(JsDocToken.COMMA)) { while (match(JsDocToken.COMMA)) {
next(); next();
Expand All @@ -2031,8 +2032,13 @@ private Node parseTypeExpressionList(JsDocToken token) {
if (typeExpr == null) { if (typeExpr == null) {
return null; return null;
} }
numTypeExprs++;
typeList.addChildToBack(typeExpr); typeList.addChildToBack(typeExpr);
} }
if (typeName.equals("Object") && numTypeExprs == 1) {
// Unlike other generic types, Object<V> means Object<?, V>, not Object<V, ?>.
typeList.addChildToFront(newNode(Token.QMARK));
}
return typeList; return typeList;
} }


Expand Down Expand Up @@ -2161,7 +2167,7 @@ private Node parseTypeName(JsDocToken token) {
if (match(JsDocToken.LEFT_ANGLE)) { if (match(JsDocToken.LEFT_ANGLE)) {
next(); next();
skipEOLs(); skipEOLs();
Node memberType = parseTypeExpressionList(next()); Node memberType = parseTypeExpressionList(typeName, next());
if (memberType != null) { if (memberType != null) {
typeNameNode.addChildToFront(memberType); typeNameNode.addChildToFront(memberType);


Expand Down
2 changes: 1 addition & 1 deletion test/com/google/javascript/jscomp/Es6InlineTypesNotYetParsedTest.java
View file
Expand Up @@ -115,7 +115,7 @@ public void testParameterizedType() {
assertSource("/** @type {Object.<string, number>} */ var s;") assertSource("/** @type {Object.<string, number>} */ var s;")
.transpilesTo("var s: Object<string, number>;"); .transpilesTo("var s: Object<string, number>;");
assertSource("/** @type {Object.<number>} */ var s;") assertSource("/** @type {Object.<number>} */ var s;")
.transpilesTo("var s: Object<number>;"); .transpilesTo("var s: Object<any, number>;");
} }


public void testParameterizedTypeWithVoid() throws Exception { public void testParameterizedTypeWithVoid() throws Exception {
Expand Down
101 changes: 100 additions & 1 deletion test/com/google/javascript/jscomp/NewTypeInferenceTest.java
View file
Expand Up @@ -16350,7 +16350,7 @@ public void testRecursiveStructuralInterfaces() {
"}")); "}"));
} }


public void testIObjectAccesses() { public void testIObjectBracketAccesses() {
typeCheck(LINE_JOINER.join( typeCheck(LINE_JOINER.join(
"function f(/** !IObject<number,string> */ x) {", "function f(/** !IObject<number,string> */ x) {",
" return x['asdf'];", " return x['asdf'];",
Expand Down Expand Up @@ -16445,6 +16445,37 @@ public void testIObjectAccesses() {
NewTypeInference.MISTYPED_ASSIGN_RHS); NewTypeInference.MISTYPED_ASSIGN_RHS);
} }


// A dot access on IObject<K, V> is not typed as V.
// It is either declared separately with its own type (eg, when Foo implements IObject and
// has some extra properties), or it is a type error.
public void testIObjectDotAccesses() {
typeCheck(LINE_JOINER.join(
"function f(/** !IObject<?, ?> */ x) {",
" return x.hasOwnProperty('test');",
"}"));

typeCheck(LINE_JOINER.join(
"function g(x) {",
" x.foobar = 123;",
"}",
"function f(/** !IObject<?, ?> */ x) {",
" return x.foobar;",
"}"),
NewTypeInference.INEXISTENT_PROPERTY);

typeCheck(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {",
" this.foobar = 123;",
"}",
"function f(/** !IObject<?, ?> */ x) {",
" return x.foobar;",
"}"),
NewTypeInference.INEXISTENT_PROPERTY);

typeCheck("var /** !IObject<?, ?> */ x = { 'abs': '', '%': ''};");
}

public void testIObjectSubtyping() { public void testIObjectSubtyping() {
typeCheck(LINE_JOINER.join( typeCheck(LINE_JOINER.join(
"function f(/** !IObject */ x) {}", "function f(/** !IObject */ x) {}",
Expand Down Expand Up @@ -17930,6 +17961,15 @@ public void testPropertyCheckingCompatibility() {
" this.prop = 123;", " this.prop = 123;",
"}")); "}"));


typeCheck(LINE_JOINER.join(
"function f(/** !IObject<?, ?> */ x) {",
" return x.prop + 1;",
"}",
"/** @constructor */",
"function Bar() {",
" this.prop = 123;",
"}"));

typeCheck(LINE_JOINER.join( typeCheck(LINE_JOINER.join(
"/** @constructor */", "/** @constructor */",
"function Foo() {}", "function Foo() {}",
Expand Down Expand Up @@ -18643,4 +18683,63 @@ public void testConstInferenceDependsOnOrderOfScopeTraversing() {
" }", " }",
"}")); "}"));
} }

// We currently consider Object<K,V> to be the same as IObject<K,V>.
// Since we may change that in the future, they are tested separately.
public void testParameterizedObject() {
typeCheck(LINE_JOINER.join(
"function f(/** !Object<?, ?> */ x) {",
" return x.hasOwnProperty('test');",
"}"));

typeCheck(LINE_JOINER.join(
"function g(x) {",
" x.foobar = 123;",
"}",
"function f(/** !Object<?, ?> */ x) {",
" return x.foobar;",
"}"),
NewTypeInference.INEXISTENT_PROPERTY);

typeCheck(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {",
" this.foobar = 123;",
"}",
"function f(/** !Object<?, ?> */ x) {",
" return x.foobar;",
"}"),
NewTypeInference.INEXISTENT_PROPERTY);

typeCheck("var /** !Object<?, ?> */ x = { 'abs': '', '%': ''};");

typeCheck(LINE_JOINER.join(
"function f(/** !Object<number,string> */ x) {",
" return x['asdf'];",
"}"),
NewTypeInference.INVALID_INDEX_TYPE);

typeCheck(LINE_JOINER.join(
"function f(/** !Object<number, number> */ x) {",
" x['asdf'] = 123;",
"}"),
NewTypeInference.INVALID_INDEX_TYPE);

typeCheck(LINE_JOINER.join(
"function f(/** !Object<number, string> */ x, /** number */ i) {",
" x[i] - 123;",
"}"),
NewTypeInference.INVALID_OPERAND_TYPE);

typeCheck(LINE_JOINER.join(
"function f(/** !Object<boolean> */ x) {",
" var /** string */ s = x['asdf'];",
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck(LINE_JOINER.join(
"function f(/** !Object<number> */ x) {",
" return x[{a: 123}] + x['sadf'];",
"}"));
}
} }

0 comments on commit 6944ab8

Please sign in to comment.