Skip to content

Commit

Permalink
[NTI] Migrate TypedCodePrinter to NTI.
Browse files Browse the repository at this point in the history 
In performing this migration, a number of bugs on OTI's typed code printing surfaced (primarily around nullability), so I fixed them and added a few regression tests.

One omission was leaving out the `@this` annotation from functions.  Fixing this unfortunately introduces a few redundant `@this` annotations where they shouldn't be necessary (we currently do a bit of work to try to eliminate these, but it depends on type information in the AST which isn't always available - see a few test cases that have been changed; potentially it's not worth doing this at all and we could eliminate the extra logic and simply always emit `@this`).

A number of new TypeI methods were required to support this.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=157625955
  • Loading branch information
@shicks
shicks authored and Tyler Breisacher committed Jun 1, 2017
1 parent 5087f47 commit 23e2bf3
Show file tree
Hide file tree
Showing 14 changed files with 547 additions and 266 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/GlobalTypeInfo.java
View file
Expand Up @@ -410,7 +410,7 @@ String getFunInternalName(Node n) {
public void process(Node externs, Node root) {
Preconditions.checkNotNull(warnings, "Cannot rerun GlobalTypeInfo.process");
Preconditions.checkArgument(externs == null || externs.isRoot());
Preconditions.checkArgument(root.isRoot());
Preconditions.checkArgument(root.isRoot(), "Root must be ROOT, but is %s", root.getToken());

this.compiler.setMostRecentTypechecker(MostRecentTypechecker.NTI);

Expand Down
186 changes: 111 additions & 75 deletions src/com/google/javascript/jscomp/TypedCodeGenerator.java
View file
Expand Up @@ -18,13 +18,16 @@

import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.javascript.rhino.FunctionTypeI;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.ObjectTypeI;
import com.google.javascript.rhino.TypeI;
import com.google.javascript.rhino.TypeIRegistry;
import com.google.javascript.rhino.jstype.FunctionType;
import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.JSTypeNative;
import com.google.javascript.rhino.jstype.ObjectType;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;

Expand Down Expand Up @@ -79,20 +82,20 @@ private String getTypeAnnotation(Node node) {
return "";
}

JSType type = node.getJSType();
TypeI type = node.getTypeI();
if (type == null) {
return "";
} else if (type.isFunctionType()) {
return getFunctionAnnotation(node);
} else if (type.isEnumType()) {
return "/** @enum {" +
type.toMaybeEnumType().getElementsType().toAnnotationString() +
"} */\n";
} else if (type.isEnumObject()) {
return "/** @enum {"
+ type.toMaybeObjectType().getElementsType().toNonNullAnnotationString()
+ "} */\n";
} else if (!type.isUnknownType()
&& !type.isEmptyType()
&& !type.isBottom()
&& !type.isVoidType()
&& !type.isFunctionPrototypeType()) {
return "/** @type {" + node.getJSType().toAnnotationString() + "} */\n";
&& !type.isPrototypeObject()) {
return "/** @type {" + node.getTypeI().toNonNullAnnotationString() + "} */\n";
} else {
return "";
}
Expand All @@ -102,17 +105,16 @@ private String getTypeAnnotation(Node node) {
* @param fnNode A node for a function for which to generate a type annotation
*/
private String getFunctionAnnotation(Node fnNode) {
JSType type = fnNode.getJSType();
TypeI type = fnNode.getTypeI();
Preconditions.checkState(fnNode.isFunction() || type.isFunctionType());

if (type == null || type.isUnknownType()) {
return "";
}

FunctionType funType = type.toMaybeFunctionType();
FunctionTypeI funType = type.toMaybeFunctionType();

if (JSType.isEquivalent(
type, (JSType) registry.getNativeType(JSTypeNative.FUNCTION_INSTANCE_TYPE))) {
if (type.equals(registry.getNativeType(JSTypeNative.FUNCTION_INSTANCE_TYPE))) {
return "/** @type {!Function} */\n";
}

Expand All @@ -132,26 +134,27 @@ private String getFunctionAnnotation(Node fnNode) {
}

// Param types
int i = 0;
for (Node n : funType.getParameters()) {
int minArgs = funType.getMinArguments();
int maxArgs = funType.getMaxArguments();
List<TypeI> args = ImmutableList.copyOf(funType.getParameterTypes());
for (int i = 0; i < args.size(); i++) {
sb.append(" * ");
appendAnnotation(sb, "param", getParameterNodeJSDocType(n));
appendAnnotation(sb, "param", getParameterJSDocType(args, i, minArgs, maxArgs));
sb.append(" ")
.append(paramNode == null ? "p" + i : paramNode.getString())
.append("\n");
if (paramNode != null) {
paramNode = paramNode.getNext();
} else {
i++;
}
}

// Return type
JSType retType = funType.getReturnType();
if (retType != null &&
!retType.isEmptyType() && // There is no annotation for the empty type.
!funType.isInterface() && // Interfaces never return a value.
!(funType.isConstructor() && retType.isVoidType())) {
TypeI retType = funType.getReturnType();
if (retType != null
&& !retType.isBottom() // There is no annotation for the empty type.
&& !funType.isInterface() // Interfaces never return a value.
&& !(funType.isConstructor()
&& retType.isVoidType())) {
sb.append(" * ");
appendAnnotation(sb, "return", retType.toNonNullAnnotationString());
sb.append("\n");
Expand All @@ -160,87 +163,120 @@ private String getFunctionAnnotation(Node fnNode) {
// Constructor/interface
if (funType.isConstructor() || funType.isInterface()) {

FunctionType superConstructor = funType.getSuperClassConstructor();

if (superConstructor != null) {
ObjectType superInstance =
funType.getSuperClassConstructor().getInstanceType();
if (!superInstance.toString().equals("Object")) {
if (funType.isInterface()) {
Set<String> interfaces = new TreeSet<>();
for (ObjectTypeI interfaceType : funType.getAncestorInterfaces()) {
interfaces.add(interfaceType.toAnnotationString());
}
for (String interfaze : interfaces) {
sb.append(" * ");
appendAnnotation(sb, "extends", superInstance.toAnnotationString());
appendAnnotation(sb, "extends", interfaze);
sb.append("\n");
}
}

if (funType.isInterface()) {
for (ObjectType interfaceType : funType.getExtendedInterfaces()) {
if (funType.isConstructor()) {
FunctionTypeI superConstructor = funType.getInstanceType().getSuperClassConstructor();
if (superConstructor != null) {
ObjectTypeI superInstance = superConstructor.getInstanceType();
if (!superInstance.toString().equals("Object")) {
sb.append(" * ");
appendAnnotation(sb, "extends", superInstance.toAnnotationString());
sb.append("\n");
}
}
// Avoid duplicates, add implemented type to a set first
Set<String> interfaces = new TreeSet<>();
for (ObjectTypeI interfaze : funType.getAncestorInterfaces()) {
interfaces.add(interfaze.toAnnotationString());
}
for (String interfaze : interfaces) {
sb.append(" * ");
appendAnnotation(sb, "extends", interfaceType.toAnnotationString());
appendAnnotation(sb, "implements", interfaze);
sb.append("\n");
}
}

// Avoid duplicates, add implemented type to a set first
Set<String> interfaces = new TreeSet<>();
for (ObjectType interfaze : funType.getImplementedInterfaces()) {
interfaces.add(interfaze.toAnnotationString());
}
for (String interfaze : interfaces) {
sb.append(" * ");
appendAnnotation(sb, "implements", interfaze);
sb.append("\n");
}

if (funType.isConstructor()) {
sb.append(" * @constructor\n");
} else if (funType.isStructuralInterface()) {
sb.append(" * @record\n");
} else if (funType.isInterface()) {
sb.append(" * @interface\n");
}
} else {
TypeI thisType = funType.getTypeOfThis();
if (thisType != null && !thisType.isUnknownType() && !thisType.isVoidType()) {
if (fnNode == null || !thisType.equals(findMethodOwner(fnNode))) {
sb.append(" * ");
appendAnnotation(sb, "this", thisType.toNonNullAnnotationString());
sb.append("\n");
}
}
}

if (!funType.getTemplateTypeMap().getTemplateKeys().isEmpty()) {
Collection<String> typeParams = funType.getTypeParameters();
if (!typeParams.isEmpty()) {
sb.append(" * @template ");
Joiner.on(",").appendTo(sb, funType.getTemplateTypeMap().getTemplateKeys());
Joiner.on(",").appendTo(sb, typeParams);
sb.append("\n");
}

sb.append(" */\n");
return sb.toString();
}

// TODO(sdh): This whole method could be deleted if we don't mind adding
// additional @this annotations where they're not actually necessary.
/**
* Given a method definition node, returns the {@link ObjectTypeI} corresponding
* to the class the method is defined on, or null if it is not a prototype method.
*/
private static ObjectTypeI findMethodOwner(Node n) {
if (n == null) {
return null;
}
Node parent = n.getParent();
FunctionTypeI ctor = null;
if (parent.isAssign()) {
Node target = parent.getFirstChild();
if (NodeUtil.isPrototypeProperty(target)) {
TypeI type = target.getFirstFirstChild().getTypeI();
ctor = type != null ? type.toMaybeFunctionType() : null;
}
} else if (parent.isClass()) {
// TODO(sdh): test this case once NTI understands ES6 classes
ctor = parent.getTypeI().toMaybeFunctionType();
}
return ctor != null ? ctor.getInstanceType() : null;
}

private static void appendAnnotation(StringBuilder sb, String name, String type) {
sb.append("@").append(name).append(" {").append(type).append("}");
}

/**
* Creates a JSDoc-suitable String representation the type of a parameter.
*
* @param parameterNode The parameter node.
*/
private String getParameterNodeJSDocType(Node parameterNode) {
JSType parameterType = parameterNode.getJSType();
String typeString;

if (parameterNode.isOptionalArg()) {
typeString = restrictByUndefined(parameterType).toNonNullAnnotationString() +
"=";
} else if (parameterNode.isVarArgs()) {
typeString = "..." +
restrictByUndefined(parameterType).toNonNullAnnotationString();
} else {
typeString = parameterType.toNonNullAnnotationString();
/** Creates a JSDoc-suitable String representation the type of a parameter. */
private String getParameterJSDocType(List<TypeI> types, int index, int minArgs, int maxArgs) {
TypeI type = types.get(index);
if (index >= minArgs) {
if (maxArgs < Integer.MAX_VALUE || index < types.size() - 1) {
return restrictByUndefined(type).toNonNullAnnotationString() + "=";
}
return "..." + restrictByUndefined(type).toNonNullAnnotationString();
}

return typeString;
return type.toNonNullAnnotationString();
}

private JSType restrictByUndefined(JSType type) {
if (type.isUnionType()) {
return type.toMaybeUnionType().getRestrictedUnion(
(JSType) registry.getNativeType(JSTypeNative.VOID_TYPE));
/** Removes undefined from a union type. */
private TypeI restrictByUndefined(TypeI type) {
// If not voidable, there's nothing to do. If not nullable then the easiest
// thing is to simply remove both null and undefined. If nullable, then add
// null back into the union after removing null and undefined.
if (!type.isVoidable()) {
return type;
}
TypeI restricted = type.restrictByNotNullOrUndefined();
if (!type.isNullable()) {
return restricted.isBottom() ? type : restricted;
}
return type;
TypeI nullType = registry.getNativeType(JSTypeNative.NULL_TYPE);
return registry.createUnionType(ImmutableList.of(restricted, nullType));
}
}
41 changes: 40 additions & 1 deletion src/com/google/javascript/jscomp/newtypes/JSType.java
View file
Expand Up @@ -501,6 +501,16 @@ public final boolean isEnumObject() {
return obj != null && obj.isEnumObject();
}

@Override
public final TypeI getElementsType() {
ObjectType obj = getObjTypeIfSingletonObj();
if (obj != null) {
EnumType e = obj.getEnumType();
return e != null ? e.getEnumeratedType() : null;
}
return null;
}

public final boolean isUnion() {
if (isBottom() || isTop() || isUnknown()
|| isScalar() || isTypeVariable() || isEnumElement()
Expand Down Expand Up @@ -1542,7 +1552,7 @@ public final String toString() {
* code generation.
*/
StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
// TODO(sdh): Add checkState(!forAnnotations) calls in places where annotations are nonsense.
// TODO(sdh): checkState(!forAnnotations) all
switch (getMask()) {
case BOTTOM_MASK:
return builder.append("bottom");
Expand Down Expand Up @@ -1633,6 +1643,17 @@ StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
}
}

@Override
public final String toNonNullAnnotationString() {
return appendTo(new StringBuilder(), ToStringContext.FOR_ANNOTATION).toString();
}

@Override
public final String toAnnotationString() {
String s = toNonNullAnnotationString();
return s.startsWith("!") ? s.substring(1) : s;
}

@Override
public final boolean isConstructor() {
FunctionType ft = getFunTypeIfSingletonObj();
Expand Down Expand Up @@ -1830,6 +1851,24 @@ public final boolean acceptsArguments(List<? extends TypeI> argumentTypes) {
return true;
}

@Override
public final int getMinArguments() {
Preconditions.checkState(this.isFunctionType());
return this.getFunTypeIfSingletonObj().getMinArity();
}

@Override
public final int getMaxArguments() {
Preconditions.checkState(this.isFunctionType());
return this.getFunTypeIfSingletonObj().getMaxArity();
}

@Override
public final List<String> getTypeParameters() {
Preconditions.checkState(this.isFunctionType());
return this.getFunTypeIfSingletonObj().getTypeParameters();
}

@Override
public final boolean hasProperties() {
throw new UnsupportedOperationException("hasProperties not implemented yet");
Expand Down
19 changes: 11 additions & 8 deletions src/com/google/javascript/jscomp/newtypes/ObjectType.java
View file
Expand Up @@ -1027,22 +1027,25 @@ private boolean isBottomObject() {
}

/**
* Returns true if this is the type of an enum namespace (as opposed to the
* type of the individual elements).
* If this is an enum object (i.e. the namespace itself, not the individual
* elements), then returns the associated EnumType; otherwise returns null.
*/
boolean isEnumObject() {
EnumType getEnumType() {
if (!this.nominalType.isLiteralObject()) {
return false;
return null;
}
EnumType e = null;
for (Property p : this.props.values()) {
JSType t = p.getType();
if (t.isEnumElement()) {
e = Iterables.getOnlyElement(t.getEnums());
break;
EnumType e = Iterables.getOnlyElement(t.getEnums());
return this.equals(e.toJSType().getObjTypeIfSingletonObj()) ? e : null;
}
}
return e != null && this.equals(e.toJSType().getObjTypeIfSingletonObj());
return null;
}

boolean isEnumObject() {
return this.getEnumType() != null;
}

/**
Expand Down

0 comments on commit 23e2bf3

Please sign in to comment.