Skip to content

Commit

Permalink
[NTI] Handle constructor declarations in externs without a function l…
Browse files Browse the repository at this point in the history
…iteral.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=129480744
  • Loading branch information
dimvar authored and blickly committed Aug 5, 2016
1 parent a0b8840 commit ed3e7ef
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 12 deletions.
55 changes: 51 additions & 4 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,8 @@ public void visit(NodeTraversal t, Node n, Node parent) {
visitWindowVar(nameNode);
} else if (isCtorDefinedByCall(nameNode)) {
visitNewCtorDefinedByCall(nameNode);
} else if (isCtorWithoutFunctionLiteral(nameNode)) {
visitNewCtorWithoutFunctionLiteral(nameNode);
}
if (!n.isFromExterns()
&& !this.currentScope.isDefinedLocally(varName, false)) {
Expand Down Expand Up @@ -757,6 +759,10 @@ public void visit(NodeTraversal t, Node n, Node parent) {
expr = lhs;
// fall through
case GETPROP:
if (isCtorWithoutFunctionLiteral(expr)) {
visitNewCtorWithoutFunctionLiteral(expr);
return;
}
if (isPrototypeProperty(expr)
|| NodeUtil.referencesThis(expr)
|| !expr.isQualifiedName()) {
Expand Down Expand Up @@ -1171,6 +1177,12 @@ private void visitNewCtorDefinedByCall(Node qnameNode) {
Node rhs = NodeUtil.getRValueOfLValue(qnameNode);
maybeRecordNominalType(rhs, qnameNode, jsdoc, false);
}

private void visitNewCtorWithoutFunctionLiteral(Node qnameNode) {
Preconditions.checkState(qnameNode.isName() || qnameNode.isGetProp());
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(qnameNode);
maybeRecordNominalType(qnameNode, qnameNode, jsdoc, false);
}
}

private class ProcessScope extends AbstractShallowCallback {
Expand Down Expand Up @@ -1351,6 +1363,11 @@ private void visitVar(Node nameNode, Node parent) {
nameNode.getFirstChild(), null, this.currentScope);
return;
}
if (isCtorWithoutFunctionLiteral(nameNode)) {
computeFnDeclaredType(NodeUtil.getBestJSDocInfo(nameNode), name,
nameNode, null, this.currentScope);
return;
}
if (isDefinedLocally && this.currentScope.isNamespace(name)) {
return;
}
Expand Down Expand Up @@ -1615,6 +1632,13 @@ private void visitNamespacePropertyDeclaration(Node getProp) {
getProp.getNext(), null, this.currentScope);
return;
}
if (isCtorWithoutFunctionLiteral(getProp)) {
computeFnDeclaredType(
NodeUtil.getBestJSDocInfo(getProp),
getProp.getQualifiedName(),
getProp, null, this.currentScope);
return;
}
// Named types have already been crawled in CollectNamedTypes
if (isNamedType(getProp)) {
return;
Expand Down Expand Up @@ -1729,6 +1753,13 @@ private void visitClassPropertyDeclaration(Node getProp) {
private void visitOtherPropertyDeclaration(Node getProp) {
Preconditions.checkArgument(getProp.isGetProp());
Preconditions.checkArgument(getProp.isQualifiedName());
if (isCtorWithoutFunctionLiteral(getProp)) {
computeFnDeclaredType(
NodeUtil.getBestJSDocInfo(getProp),
getProp.getQualifiedName(),
getProp, null, this.currentScope);
return;
}
if (isAnnotatedAsConst(getProp)) {
warnings.add(JSError.make(getProp, MISPLACED_CONST_ANNOTATION));
}
Expand Down Expand Up @@ -2117,7 +2148,7 @@ private DeclaredFunctionType computeFnDeclaredType(
JSDocInfo fnDoc, String functionName, Node declNode,
RawNominalType ownerType, NTIScope parentScope) {
Preconditions.checkArgument(
declNode.isFunction() || declNode.isGetProp() || declNode.isCall());
declNode.isFunction() || declNode.isQualifiedName() || declNode.isCall());

// For an unannotated function, check if we can grab a type signature for
// it from the surrounding code where it appears.
Expand All @@ -2129,12 +2160,11 @@ private DeclaredFunctionType computeFnDeclaredType(
}
}
// TODO(dimvar): warn if multiple jsdocs for a fun
RawNominalType ctorType =
!declNode.isGetProp() ? nominaltypesByNode.get(declNode) : null;
RawNominalType ctorType = nominaltypesByNode.get(declNode);
FunctionAndSlotType result = typeParser.getFunctionType(
fnDoc, functionName, declNode, ctorType, ownerType, parentScope);
Node qnameNode;
if (declNode.isGetProp()) {
if (declNode.isQualifiedName()) {
qnameNode = declNode;
} else if (declNode.isFunction()) {
qnameNode = NodeUtil.getNameNode(declNode);
Expand Down Expand Up @@ -2459,6 +2489,23 @@ private static boolean isCtorDefinedByCall(Node qnameNode) {
return jsdoc != null && jsdoc.isConstructor() && rhs != null && rhs.isCall();
}

private static boolean isCtorWithoutFunctionLiteral(Node qnameNode) {
if (!qnameNode.isFromExterns()) {
return false;
}
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(qnameNode);
if (jsdoc == null || !jsdoc.isConstructor()) {
return false;
}
if (qnameNode.isName()) {
return qnameNode.getParent().isVar() && !qnameNode.hasChildren();
}
if (qnameNode.isGetProp()) {
return qnameNode.getParent().isExprResult();
}
return false;
}

// Utility class when analyzing property declarations
private static class PropertyType {
// The declared type of the property, from the jsdoc.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -923,8 +923,8 @@ private void fillInReturnType(
ImmutableList<String> typeParameters,
DeclaredTypeRegistry registry, FunctionTypeBuilder builder,
boolean ignoreJsdoc /* for when the jsdoc is malformed */) {
JSDocInfo inlineRetJsdoc =
ignoreJsdoc ? null : funNode.getFirstChild().getJSDocInfo();
JSDocInfo inlineRetJsdoc = ignoreJsdoc || !funNode.isFunction()
? null : funNode.getFirstChild().getJSDocInfo();
JSTypeExpression retTypeExp = jsdoc == null ? null : jsdoc.getReturnType();
if (parent.isSetterDef() && retTypeExp == null) {
// inline returns for getters/setters are not parsed
Expand Down
31 changes: 25 additions & 6 deletions src/com/google/javascript/jscomp/newtypes/RawNominalType.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,7 @@ private RawNominalType(
String name, ImmutableList<String> typeParameters, Kind kind, ObjectKind objectKind) {
super(commonTypes, name, defSite);
Preconditions.checkNotNull(objectKind);
Preconditions.checkState(
defSite == null || defSite.isFunction() || defSite.isCall(),
"Expected function or call but found %s",
defSite.getType());
Preconditions.checkState(isValidDefsite(defSite), "Invalid defsite %s", defSite);
if (typeParameters == null) {
typeParameters = ImmutableList.of();
}
Expand Down Expand Up @@ -117,6 +114,26 @@ private RawNominalType(
this.wrappedAsNullableJSType = JSType.join(JSType.NULL, this.wrappedAsJSType);
}

private static boolean isValidDefsite(Node defSite) {
if (defSite == null) {
return false;
}
if (defSite.isFunction()) {
return true;
}
Node parent = defSite.getParent();
if (defSite.isCall()) {
return parent.isName() || parent.isAssign();
}
if (defSite.isName()) {
return parent.isVar() && !defSite.hasChildren();
}
if (defSite.isGetProp()) {
return parent.isExprResult();
}
return false;
}

public static RawNominalType makeUnrestrictedClass(JSTypes commonTypes,
Node defSite, String name, ImmutableList<String> typeParameters) {
return new RawNominalType(commonTypes, defSite,
Expand Down Expand Up @@ -558,8 +575,10 @@ public JSType getCtorPropDeclaredType(String pname) {

@Override
public void finalize() {
Preconditions.checkState(!this.isFinalized);
Preconditions.checkNotNull(this.ctorFn);
Preconditions.checkState(
!this.isFinalized, "Raw type not finalized: %s", this.defSite);
Preconditions.checkNotNull(
this.ctorFn, "Null constructor function for raw type: %s", this.defSite);
if (this.interfaces == null) {
this.interfaces = ImmutableSet.of();
}
Expand Down
27 changes: 27 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17618,4 +17618,31 @@ public void testPropertyCheckingCompatibility() {
"}",
"f(E.A);"));
}

public void testConstructorDeclWithoutFunctionLiteral() {
typeCheckCustomExterns(
DEFAULT_EXTERNS + "/** @constructor */ var Foo;",
"var x = new Foo;");

typeCheckCustomExterns(
DEFAULT_EXTERNS + LINE_JOINER.join(
"/** @const */ var ns = {};",
"/**",
" * @constructor",
" * @param {number} x",
" */",
"ns.Foo;"),
"var x = new ns.Foo('asdf');",
NewTypeInference.INVALID_ARGUMENT_TYPE);

// ns is not declared; don't crash
typeCheckCustomExterns(
DEFAULT_EXTERNS + "/** @constructor */ ns.Foo;",
"");

typeCheck(LINE_JOINER.join(
"/** @constructor */ var Foo;",
"var x = new Foo;"),
NewTypeInference.NOT_CALLABLE);
}
}

0 comments on commit ed3e7ef

Please sign in to comment.