Skip to content

Commit

Permalink
Automated g4 rollback
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

Breaks pantheon

*** Original change description ***

Fix aliasing of typedefs

Adds a new AST annotation to keep track of any typedef types defined by a definition site. This is different from the type of the value, in that e.g.

```
/** @typedef {string} */
ns.MyString;
```

calling `getJSType()` on the `ns.MyString` qualified name node returns the undefined type, since `ns.MyString` has a runtime value of undefined, but the type *name* represents a string, which we store in `Node#getTypedefTypeProp`. This is necessary so that typedefs can be dis...

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=219737644
  • Loading branch information
@shicks @EatingW
shicks authored and EatingW committed Nov 2, 2018
1 parent eb519bc commit 9c1603b
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 302 deletions.
85 changes: 15 additions & 70 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
View file
Expand Up @@ -63,7 +63,6 @@
import com.google.javascript.jscomp.NodeTraversal.AbstractScopedCallback;
import com.google.javascript.jscomp.NodeTraversal.AbstractShallowStatementCallback;
import com.google.javascript.rhino.ErrorReporter;
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.InputId;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.Node;
Expand Down Expand Up @@ -225,37 +224,19 @@ void resolve() {
}
}

/**
* Stores the type and qualified name for a destructuring rvalue, which has no actual AST node.
* The {@code qualifiedName} is therefore a detached Node cloned out of the source tree with extra
* getprops added onto it. Cloning should generally be pretty cheap (since it's always just a
* qualified name). Nevertheless, we may pull out a QualifiedName class abstraction at some point
* in the future, which would allow avoiding the clone in the first place. Using a Node here is
* necessary to avoid duplicating logic for non-destructured qualified name aliasing.
*/
/** Stores the type and qualified name for a destructuring rvalue, which has no AST node */
private static class RValueInfo {
@Nullable final JSType type;
@Nullable final Node qualifiedName;
@Nullable final String qualifiedName;

RValueInfo(JSType type, Node qualifiedName) {
private RValueInfo(JSType type, String qualifiedName) {
this.type = type;
this.qualifiedName = qualifiedName;
}

static RValueInfo empty() {
private static RValueInfo empty() {
return new RValueInfo(null, null);
}

/** Returns a new RValueInfo whose qualified name has an extra property. */
RValueInfo forProperty(JSType type, String property) {
if (qualifiedName == null) {
return new RValueInfo(type, null);
} else if (qualifiedName.getParent() == null) {
return new RValueInfo(type, IR.getprop(qualifiedName, property));
}
// qualified name is already in the tree, so we need to clone it first.
return new RValueInfo(type, IR.getprop(qualifiedName.cloneTree(), property));
}
}

TypedScopeCreator(AbstractCompiler compiler) {
Expand Down Expand Up @@ -900,8 +881,7 @@ void defineVarChild(JSDocInfo declarationInfo, Node child, TypedScope scope) {
// for (const {x, y} of data) {
value != null
? new RValueInfo(
getDeclaredRValueType(/* lValue= */ null, value),
/* qualifiedName= */ value)
getDeclaredRValueType(/* lValue= */ null, value), value.getQualifiedName())
: new RValueInfo(unknownType, /* qualifiedName= */ null));
}
}
Expand All @@ -928,14 +908,16 @@ private RValueInfo inferTypeForDestructuredTarget(
RValueInfo rvalue = patternTypeSupplier.get();
JSType patternType = rvalue.type;
String propertyName = target.getStringKey().getString();
String qualifiedName =
rvalue.qualifiedName != null ? rvalue.qualifiedName + "." + propertyName : null;
if (patternType == null || patternType.isUnknownType()) {
return rvalue.forProperty(null, propertyName);
return new RValueInfo(null, qualifiedName);
}
if (patternType.hasProperty(propertyName)) {
JSType type = patternType.findPropertyType(propertyName);
return rvalue.forProperty(type, propertyName);
return new RValueInfo(type, qualifiedName);
}
return rvalue.forProperty(null, propertyName);
return new RValueInfo(null, qualifiedName);
}

void defineDestructuringPatternInVarDeclaration(
Expand Down Expand Up @@ -1890,7 +1872,7 @@ && shouldUseFunctionLiteralType(
if (NodeUtil.isConstantDeclaration(compiler.getCodingConvention(), info, lValue)) {
if (rValue != null) {
JSType rValueType = getDeclaredRValueType(lValue, rValue);
maybeDeclareAliasType(lValue, rValue, rValueType);
maybeDeclareAliasType(lValue, rValue.getQualifiedName(), rValueType);
if (rValueType != null) {
return rValueType;
}
Expand Down Expand Up @@ -1919,57 +1901,30 @@ && shouldUseFunctionLiteralType(
*
* @param rvalueName the rvalue's qualified name if it exists, null otherwise
*/
private void maybeDeclareAliasType(
Node lValue, @Nullable Node rValue, @Nullable JSType rValueType) {
private void maybeDeclareAliasType(Node lValue, String rvalueName, JSType rValueType) {
// NOTE: this allows some strange patterns such allowing instance properties
// to be aliases of constructors, and then creating a local alias of that to be
// used as a type name. Consider restricting this.

if (!lValue.isQualifiedName() || (rValue == null)) {
if (!lValue.isQualifiedName() || (rvalueName == null)) {
return;
}

Node definitionNode = getDefinitionNode(rValue);
if (definitionNode != null) {
JSType typedefType = definitionNode.getTypedefTypeProp();
if (typedefType != null) {
// Propagate typedef type to typedef aliases.
lValue.setTypedefTypeProp(typedefType);
String qName = lValue.getQualifiedName();
typeRegistry.identifyNonNullableName(qName);
typeRegistry.declareType(currentScope, qName, typedefType);
return;
}
}

// Treat @const-annotated aliases like @constructor/@interface if RHS has instance type
if (rValueType != null
&& rValueType.isFunctionType()
&& rValueType.toMaybeFunctionType().hasInstanceType()) {
FunctionType functionType = rValueType.toMaybeFunctionType();
typeRegistry.declareType(
currentScope, lValue.getQualifiedName(), functionType.getInstanceType());
} else if (rValue.isQualifiedName()) {
} else {
// Also infer a type name for aliased @typedef
JSType rhsNamedType = typeRegistry.getType(currentScope, rValue.getQualifiedName());
JSType rhsNamedType = typeRegistry.getType(currentScope, rvalueName);
if (rhsNamedType != null) {
typeRegistry.declareType(currentScope, lValue.getQualifiedName(), rhsNamedType);
}
}
}

/** Returns the AST node associated with the definition, if any. */
private Node getDefinitionNode(Node qnameNode) {
if (!qnameNode.isGetProp()) {
TypedVar var = currentScope.getVar(qnameNode.getQualifiedName());
return var != null ? var.getNameNode() : null;
}
ObjectType parent = ObjectType.cast(lookupQualifiedName(qnameNode.getFirstChild()));
return parent != null
? parent.getPropertyDefSite(qnameNode.getLastChild().getString())
: null;
}

/**
* Check for common idioms of a typed R-value assigned to a const L-value.
*
Expand Down Expand Up @@ -2505,8 +2460,6 @@ void checkForTypedef(Node candidate, JSDocInfo info) {
JSType realType = info.getTypedefType().evaluate(currentScope, typeRegistry);
if (realType == null) {
report(JSError.make(candidate, MALFORMED_TYPEDEF, typedef));
} else {
candidate.setTypedefTypeProp(realType);
}

typeRegistry.overwriteDeclaredType(currentScope, typedef, realType);
Expand All @@ -2518,14 +2471,6 @@ void checkForTypedef(Node candidate, JSDocInfo info) {
.withType(getNativeType(NO_TYPE))
.allowLaterTypeInference(false)
.defineSlot();
Node definitionNode = getDefinitionNode(candidate.getFirstChild());
if (definitionNode != null) {
ObjectType parent = ObjectType.cast(definitionNode.getJSType());
if (parent != null) {
JSType type = getNativeType(NO_TYPE);
parent.defineDeclaredProperty(candidate.getLastChild().getString(), type, candidate);
}
}
}
}
} // end AbstractScopeBuilder
Expand Down
17 changes: 1 addition & 16 deletions src/com/google/javascript/rhino/Node.java
View file
Expand Up @@ -166,10 +166,8 @@ public class Node implements Serializable {
MODULE_EXPORT = 97, // Mark a property as a module export so that collase properties
// can act on it.
IS_SHORTHAND_PROPERTY = 98, // Indicates that a property {x:x} was originally parsed as {x}.
ES6_MODULE = 99, // Indicates that a SCRIPT node is or was an ES6 module. Remains set
ES6_MODULE = 99; // Indicates that a SCRIPT node is or was an ES6 module. Remains set
// after the module is rewritten.
TYPEDEF_TYPE = 100; // Record the type associated with a @typedef to enable looking up typedef
// in the AST possible without saving the type scope.

private static final String propToString(byte propType) {
switch (propType) {
Expand Down Expand Up @@ -238,8 +236,6 @@ private static final String propToString(byte propType) {
return "is_shorthand_property";
case ES6_MODULE:
return "es6_module";
case TYPEDEF_TYPE:
return "typedef_type";
default:
throw new IllegalStateException("unexpected prop id " + propType);
}
Expand Down Expand Up @@ -2545,17 +2541,6 @@ public final boolean isDeleted() {
return getBooleanProp(DELETED);
}

/** If this node represents a typedef declaration, the associated JSType */
public final void setTypedefTypeProp(JSType type) {
putProp(TYPEDEF_TYPE, type);
}

/** If this node represents a typedef declaration, the associated JSType */
public final JSType getTypedefTypeProp() {
return (JSType) getProp(TYPEDEF_TYPE);
}

/** @param unused Whether a parameter was function to be unused. Set by RemoveUnusedVars */
public final void setUnusedParameter(boolean unused) {
putBooleanProp(IS_UNUSED_PARAMETER, unused);
}
Expand Down
75 changes: 37 additions & 38 deletions src/com/google/javascript/rhino/jstype/NamedType.java
View file
Expand Up @@ -285,68 +285,67 @@ private boolean resolveViaRegistry(ErrorReporter reporter) {
* as properties. The scope must have been fully parsed and a symbol table constructed.
*/
private void resolveViaProperties(ErrorReporter reporter) {
JSType value = lookupViaProperties();
// last component of the chain
if (value != null && value.isFunctionType() &&
(value.isConstructor() || value.isInterface())) {
FunctionType functionType = value.toMaybeFunctionType();
setReferencedAndResolvedType(functionType.getInstanceType(), reporter);
} else if (value != null && value.isNoObjectType()) {
setReferencedAndResolvedType(
registry.getNativeObjectType(
JSTypeNative.NO_OBJECT_TYPE), reporter);
} else if (value instanceof EnumType) {
setReferencedAndResolvedType(
((EnumType) value).getElementsType(), reporter);
} else {
// We've been running into issues where people forward-declare
// non-named types. (This is legitimate...our dependency management
// code doubles as our forward-declaration code.)
//
// So if the type does resolve to an actual value, but it's not named,
// then don't respect the forward declaration.
handleUnresolvedType(reporter, value == null || value.isUnknownType());
}
}

/**
* Resolves a type by looking up its first component in the scope, and
* subsequent components as properties. The scope must have been fully
* parsed and a symbol table constructed.
* @return The type of the symbol, or null if the type could not be found.
*/
private JSType lookupViaProperties() {
String[] componentNames = reference.split("\\.", -1);
if (componentNames[0].length() == 0) {
handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true);
return;
return null;
}

StaticTypedSlot slot = resolutionScope.getSlot(componentNames[0]);
if (slot == null) {
handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true);
return;
return null;
}

// If the first component has a type of 'Unknown', then any type
// names using it should be regarded as silently 'Unknown' rather than be
// noisy about it.
JSType slotType = slot.getType();
if (slotType == null || slotType.isAllType() || slotType.isNoType()) {
handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true);
return;
return null;
}

// resolving component by component
for (int i = 1; i < componentNames.length; i++) {
ObjectType parentObj = ObjectType.cast(slotType);
if (parentObj == null) {
handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true);
return;
return null;
}
if (componentNames[i].length() == 0) {
handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true);
return;
} else if (i == componentNames.length - 1) {
// Look for a typedefTypeProp on the definition node of the last component.
Node def = parentObj.getPropertyDefSite(componentNames[i]);
JSType typedefType = def != null ? def.getTypedefTypeProp() : null;
if (typedefType != null) {
setReferencedAndResolvedType(typedefType, reporter);
return;
}
return null;
}
slotType = parentObj.getPropertyType(componentNames[i]);
}

// Translate "constructor" types to "instance" types.
if (slotType == null) {
handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true);
} else if (slotType.isFunctionType() && (slotType.isConstructor() || slotType.isInterface())) {
setReferencedAndResolvedType(slotType.toMaybeFunctionType().getInstanceType(), reporter);
} else if (slotType.isNoObjectType()) {
setReferencedAndResolvedType(
registry.getNativeObjectType(JSTypeNative.NO_OBJECT_TYPE), reporter);
} else if (slotType instanceof EnumType) {
setReferencedAndResolvedType(((EnumType) slotType).getElementsType(), reporter);
} else {
// We've been running into issues where people forward-declare
// non-named types. (This is legitimate...our dependency management
// code doubles as our forward-declaration code.)
//
// So if the type does resolve to an actual value, but it's not named,
// then don't respect the forward declaration.
handleUnresolvedType(reporter, slotType.isUnknownType());
}
return slotType;
}

private void setReferencedAndResolvedType(
Expand Down

0 comments on commit 9c1603b

Please sign in to comment.