Skip to content

Commit

Permalink
Bugfix: in OTI, a generic type can be partially instantiated. The TTL…
Browse files Browse the repository at this point in the history
… function

isTemplatized should return true for such a type.
Follow-up

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=163531762
  • Loading branch information
dimvar authored and blickly committed Jul 31, 2017
1 parent 34332a8 commit 08b3483
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 46 deletions.
7 changes: 3 additions & 4 deletions src/com/google/javascript/jscomp/TypeTransformation.java
Expand Up @@ -344,7 +344,7 @@ private TypeI evalTypeName(Node ttlAst) {
private TypeI evalTemplatizedType(Node ttlAst, NameResolver nameResolver) { private TypeI evalTemplatizedType(Node ttlAst, NameResolver nameResolver) {
ImmutableList<Node> params = getCallParams(ttlAst); ImmutableList<Node> params = getCallParams(ttlAst);
TypeI firstParam = evalInternal(params.get(0), nameResolver); TypeI firstParam = evalInternal(params.get(0), nameResolver);
if (!firstParam.hasUninstantiatedTypeVariables()) { if (firstParam.isFullyInstantiated()) {
reportWarning(ttlAst, BASETYPE_INVALID, firstParam.toString()); reportWarning(ttlAst, BASETYPE_INVALID, firstParam.toString());
return getUnknownType(); return getUnknownType();
} }
Expand Down Expand Up @@ -428,9 +428,8 @@ private boolean evalTypePredicate(Node ttlAst, NameResolver nameResolver) {
case ISCTOR: case ISCTOR:
return type.isConstructor(); return type.isConstructor();
case ISTEMPLATIZED: case ISTEMPLATIZED:
// TODO(dimvar): here, we also need !type.hasUninstantiatedTypeVariables() return type.isObjectType() && type.toMaybeObjectType().isGenericObjectType()
// Fix after debugging breakage. && type.isPartiallyInstantiated();
return type.isObjectType() && type.toMaybeObjectType().isGenericObjectType();
case ISRECORD: case ISRECORD:
return type.isRecordType(); return type.isRecordType();
case ISUNKNOWN: case ISUNKNOWN:
Expand Down
9 changes: 7 additions & 2 deletions src/com/google/javascript/jscomp/newtypes/JSType.java
Expand Up @@ -2291,9 +2291,14 @@ public boolean isRecordType() {
} }


@Override @Override
public boolean hasUninstantiatedTypeVariables() { public boolean isFullyInstantiated() {
NominalType nt = getNominalTypeIfSingletonObj(); NominalType nt = getNominalTypeIfSingletonObj();
return nt == null ? false : nt.isUninstantiatedGenericType(); return nt == null || !nt.isUninstantiatedGenericType();
}

@Override
public boolean isPartiallyInstantiated() {
return isFullyInstantiated();
} }


@Override @Override
Expand Down
17 changes: 11 additions & 6 deletions src/com/google/javascript/rhino/TypeI.java
Expand Up @@ -186,13 +186,18 @@ public interface TypeI extends Serializable {
boolean isEnumObject(); boolean isEnumObject();


/** /**
* Returns true if this type is a generic object (non function) and some (or all) of its type * Returns true if this type is a generic object (non function) and *all* its type variables
* variables are not instantiated. * are instantiated.
* Note that in OTI it is possible to instantiate only some of the type variables of a
* generic type (bad implementation choice).
* In NTI, a generic type can only be uninstantiated or fully instantiated.
*/ */
boolean hasUninstantiatedTypeVariables(); boolean isFullyInstantiated();

/**
* Returns true if this type is a generic object (non function) and *some* of its type variables
* are instantiated.
* In NTI, this is the same as isFullyInstantiated.
* In OTI, generic types can be partially instantiated (bad implementation choice).
*/
boolean isPartiallyInstantiated();


/** /**
* If this type is a generic nominal type or function, return the names of all type parameters. * If this type is a generic nominal type or function, return the names of all type parameters.
Expand Down
9 changes: 7 additions & 2 deletions src/com/google/javascript/rhino/jstype/JSType.java
Expand Up @@ -292,8 +292,13 @@ public final boolean isUnionType() {
} }


@Override @Override
public boolean hasUninstantiatedTypeVariables() { public boolean isFullyInstantiated() {
return getTemplateTypeMap().hasUnfilledTemplateKeys(); return getTemplateTypeMap().isFull();
}

@Override
public boolean isPartiallyInstantiated() {
return getTemplateTypeMap().isPartiallyFull();
} }


@Override @Override
Expand Down
8 changes: 6 additions & 2 deletions src/com/google/javascript/rhino/jstype/TemplateTypeMap.java
Expand Up @@ -138,8 +138,12 @@ int numUnfilledTemplateKeys() {
return templateKeys.size() - templateValues.size(); return templateKeys.size() - templateValues.size();
} }


boolean hasUnfilledTemplateKeys() { boolean isFull() {
return numUnfilledTemplateKeys() > 0; return numUnfilledTemplateKeys() == 0;
}

boolean isPartiallyFull() {
return !this.templateValues.isEmpty();
} }


/** /**
Expand Down
67 changes: 40 additions & 27 deletions test/com/google/javascript/jscomp/NewTypeInferenceTTLTest.java
Expand Up @@ -706,33 +706,46 @@ public void testMapRecord() {
TypeTransformation.MAPRECORD_BODY_INVALID); TypeTransformation.MAPRECORD_BODY_INVALID);
} }


// TODO(dimvar): fix issue with evalTypePredicate and uncomment public void testIsTemplatized() {
// public void testIsTemplatized() { typeCheck(LINE_JOINER.join(
// typeCheck(LINE_JOINER.join( "/**",
// "/**", " * @template T := cond(isTemplatized('Array'), 'number', 'string') =:",
// " * @template T := cond(isTemplatized('Array'), 'number', 'string') =:", " * @return {T}",
// " * @return {T}", " */",
// " */", "function f() { return 'asdf'; }",
// "function f() { return 'asdf'; }", "var /** string */ s = f();"));
// "var /** string */ s = f();"));
// typeCheck(LINE_JOINER.join(
// typeCheck(LINE_JOINER.join( "/**",
// "/**", " * @template T := cond(isTemplatized(type('Array', 'number')), 'number', 'string') =:",
// " * @template T := cond(isTemplatized(type('Array', 'number')), 'number', 'string') =:", " * @return {T}",
// " * @return {T}", " */",
// " */", "function f() { return 123; }",
// "function f() { return 123; }", "var /** number */ n = f();"));
// "var /** number */ n = f();"));
//
// typeCheck(LINE_JOINER.join(
// typeCheck(LINE_JOINER.join( "/**",
// "/**", " * @template T := cond(isTemplatized('number'), 'number', 'string') =:",
// " * @template T := cond(isTemplatized('number'), 'number', 'string') =:", " * @return {T}",
// " * @return {T}", " */",
// " */", "function f() { return 'asdf'; }",
// "function f() { return 'asdf'; }", "var /** string */ s = f();"));
// "var /** string */ s = f();"));
// } // isTemplatized is true for partially instantiated types
typeCheck(LINE_JOINER.join(
"/**",
" * @constructor",
" * @template T, U",
" */",
"function Foo() {}",
"/**",
" * @template T := cond(isTemplatized(type('Foo', 'number')), 'number', 'string') =:",
" * @return {T}",
" */",
"function f() { return 123; }",
"var /** number */ x = f();"));
}


public void testIsRecord() { public void testIsRecord() {
typeCheck(LINE_JOINER.join( typeCheck(LINE_JOINER.join(
Expand Down
21 changes: 18 additions & 3 deletions test/com/google/javascript/jscomp/TypeInferenceTest.java
Expand Up @@ -46,11 +46,9 @@
import com.google.javascript.rhino.jstype.ObjectType; import com.google.javascript.rhino.jstype.ObjectType;
import com.google.javascript.rhino.jstype.StaticTypedSlot; import com.google.javascript.rhino.jstype.StaticTypedSlot;
import com.google.javascript.rhino.testing.Asserts; import com.google.javascript.rhino.testing.Asserts;

import junit.framework.TestCase;

import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import junit.framework.TestCase;


/** /**
* Tests {@link TypeInference}. * Tests {@link TypeInference}.
Expand Down Expand Up @@ -1496,6 +1494,23 @@ public void testTypeTransformationRecordFromObjectWithTemplatizedType() {
verify("r", getType("e")); verify("r", getType("e"));
} }


public void testTypeTransformationIsTemplatizedPartially() {
inFunction(
Joiner.on('\n').join(
"/**",
" * @constructor",
" * @template T, U",
" */",
"function Foo() {}",
"/**",
" * @template T := cond(isTemplatized(type('Foo', 'number')), 'number', 'string') =:",
" * @return {T}",
" */",
"function f() { return 123; }",
"var x = f();"));
assertTrue(getType("x").isNumber());
}

public void testAssertTypeofProp() { public void testAssertTypeofProp() {
assuming("x", createNullableType(OBJECT_TYPE)); assuming("x", createNullableType(OBJECT_TYPE));
inFunction( inFunction(
Expand Down

0 comments on commit 08b3483

Please sign in to comment.