Skip to content

Commit

Permalink
Make TYEPOF resolution lookup qualified names by the parent object type
Browse files Browse the repository at this point in the history
This also starts erroring on `typeof x` where x is declared with the unknown type.

This moves some logic to do this from TypedScope into StaticTypedScope.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=239900690
  • Loading branch information
lauraharker authored and EatingW committed Mar 25, 2019
1 parent 0393c80 commit b60e0d4
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 42 deletions.
26 changes: 0 additions & 26 deletions src/com/google/javascript/jscomp/TypedScope.java
Expand Up @@ -21,12 +21,10 @@
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.QualifiedName;
import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.ObjectType; import com.google.javascript.rhino.jstype.ObjectType;
import com.google.javascript.rhino.jstype.StaticTypedScope; import com.google.javascript.rhino.jstype.StaticTypedScope;
import com.google.javascript.rhino.jstype.StaticTypedSlot; import com.google.javascript.rhino.jstype.StaticTypedSlot;
import javax.annotation.Nullable;


/** /**
* TypedScope contains information about variables and their types. Scopes can be nested, a scope * TypedScope contains information about variables and their types. Scopes can be nested, a scope
Expand Down Expand Up @@ -190,28 +188,4 @@ public JSDocInfo getJsdocOfTypeDeclaration(String typeName) {
StaticTypedSlot slot = getSlot(typeName); StaticTypedSlot slot = getSlot(typeName);
return slot == null ? null : slot.getJSDocInfo(); return slot == null ? null : slot.getJSDocInfo();
} }

/**
* Looks up a given qualified name in the scope. if that fails, looks up the component properties
* off of any owner types that are in scope.
*
* <p>This is always more or equally expensive as calling getVar(String name), so should only be
* used when necessary.
*/
@Nullable
public JSType lookupQualifiedName(QualifiedName qname) {
TypedVar slot = getVar(qname.join());
if (slot != null && !slot.isTypeInferred()) {
JSType type = slot.getType();
if (type != null && !type.isUnknownType()) {
return type;
}
} else if (!qname.isSimple()) {
JSType type = lookupQualifiedName(qname.getOwner());
if (type != null) {
return type.findPropertyType(qname.getComponent());
}
}
return null;
}
} }
19 changes: 10 additions & 9 deletions src/com/google/javascript/rhino/jstype/JSTypeRegistry.java
Expand Up @@ -65,6 +65,7 @@
import com.google.javascript.rhino.JSTypeExpression; import com.google.javascript.rhino.JSTypeExpression;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.PMap; import com.google.javascript.rhino.PMap;
import com.google.javascript.rhino.QualifiedName;
import com.google.javascript.rhino.SimpleErrorReporter; import com.google.javascript.rhino.SimpleErrorReporter;
import com.google.javascript.rhino.StaticScope; import com.google.javascript.rhino.StaticScope;
import com.google.javascript.rhino.StaticSlot; import com.google.javascript.rhino.StaticSlot;
Expand Down Expand Up @@ -2023,21 +2024,21 @@ private JSType createFromTypeNodesInternal(
case TYPEOF: case TYPEOF:
{ {
String name = n.getFirstChild().getString(); String name = n.getFirstChild().getString();
StaticTypedSlot slot = scope.getSlot(name);
if (slot == null) {
reporter.warning("Not in scope: " + name, sourceName, n.getLineno(), n.getCharno());
return getNativeType(UNKNOWN_TYPE);
}
// TODO(sdh): require var to be const? // TODO(sdh): require var to be const?
JSType type = slot.getType(); JSType type = scope.lookupQualifiedName(QualifiedName.of(name));
if (type == null) { if (type == null || type.isUnknownType()) {
reporter.warning("No type for: " + name, sourceName, n.getLineno(), n.getCharno()); reporter.warning(
"Missing type for `typeof` value. The value must be declared and const.",
sourceName,
n.getLineno(),
n.getCharno());
return getNativeType(UNKNOWN_TYPE); return getNativeType(UNKNOWN_TYPE);
} }
if (type.isLiteralObject()) { if (type.isLiteralObject()) {
JSType scopeType = type;
type = type =
createNamedType(scope, "typeof " + name, sourceName, n.getLineno(), n.getCharno()); createNamedType(scope, "typeof " + name, sourceName, n.getLineno(), n.getCharno());
((NamedType) type).setReferencedType(slot.getType()); ((NamedType) type).setReferencedType(scopeType);
} }
return type; return type;
} }
Expand Down
26 changes: 26 additions & 0 deletions src/com/google/javascript/rhino/jstype/StaticTypedScope.java
Expand Up @@ -39,7 +39,9 @@


package com.google.javascript.rhino.jstype; package com.google.javascript.rhino.jstype;


import com.google.javascript.rhino.QualifiedName;
import com.google.javascript.rhino.StaticScope; import com.google.javascript.rhino.StaticScope;
import javax.annotation.Nullable;


/** /**
* The {@code StaticTypedScope} interface must be implemented by any object that defines variables * The {@code StaticTypedScope} interface must be implemented by any object that defines variables
Expand Down Expand Up @@ -68,4 +70,28 @@ public interface StaticTypedScope extends StaticScope {


/** Returns the expected type of {@code this} in the current scope. */ /** Returns the expected type of {@code this} in the current scope. */
JSType getTypeOfThis(); JSType getTypeOfThis();

/**
* Looks up a given qualified name in the scope. if that fails, looks up the component properties
* off of any owner types that are in scope.
*
* <p>This is always more or equally expensive as calling getSlot(String name), so should only be
* used when necessary.
*/
@Nullable
default JSType lookupQualifiedName(QualifiedName qname) {
StaticTypedSlot slot = getSlot(qname.join());
if (slot != null && !slot.isTypeInferred()) {
JSType type = slot.getType();
if (type != null && !type.isUnknownType()) {
return type;
}
} else if (!qname.isSimple()) {
JSType type = lookupQualifiedName(qname.getOwner());
if (type != null) {
return type.findPropertyType(qname.getComponent());
}
}
return null;
}
} }
54 changes: 47 additions & 7 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
Expand Up @@ -22289,7 +22289,9 @@ public void testRefinedTypeInNestedShortCircuitingAndOr() {


@Test @Test
public void testTypeofType_notInScope() { public void testTypeofType_notInScope() {
testTypes("var /** typeof ns */ x;", "Parse error. Not in scope: ns"); testTypes(
"var /** typeof ns */ x;",
"Parse error. Missing type for `typeof` value. The value must be declared and const.");
} }


@Test @Test
Expand Down Expand Up @@ -22445,6 +22447,38 @@ public void testTypeofType_namespacedTypeNameResolves() {
"required: ns1.Foo")); "required: ns1.Foo"));
} }


@Test
public void testTypeofType_unknownType() {
testTypes(
lines("var /** ? */ x;", "/** @type {typeof x} */ var y;"),
"Parse error. Missing type for `typeof` value. The value must be declared and const.");
}

@Test
public void testTypeofType_namespacedTypeOnIndirectAccessNameResolves() {
testTypes(
lines(
"/** @const */ var ns1 = {};",
"/** @constructor */ ns1.Foo = function() {};",
"const ns2 = ns1;",
// Verify this works although we have never seen any assignments to `ns2.Foo`
"/** @const {typeof ns2.Foo} */ var Foo2 = /** @type {?} */ (x);",
"/** @type {null} */ var x = new Foo2();"),
lines(
"initializing variable", //
"found : ns1.Foo",
"required: null"));
}

@Test
public void testTypeofType_namespacedTypeMissing() {
testTypes(
lines(
"/** @const */ var ns = {};",
"/** @const {typeof ns.Foo} */ var Foo = /** @type {?} */ (x);"),
"Parse error. Missing type for `typeof` value. The value must be declared and const.");
}

@Test @Test
public void testTypeofType_withGlobalDeclaredVariableWithHoistedFunction() { public void testTypeofType_withGlobalDeclaredVariableWithHoistedFunction() {
// TODO(sdh): fix this. This is another form of problems with partially constructed scopes. // TODO(sdh): fix this. This is another form of problems with partially constructed scopes.
Expand All @@ -22457,7 +22491,8 @@ public void testTypeofType_withGlobalDeclaredVariableWithHoistedFunction() {
"function g(/** typeof x */ a) {}", "function g(/** typeof x */ a) {}",
"x = 'str';", "x = 'str';",
"g(null);"), "g(null);"),
lines("Parse error. Not in scope: x")); lines(
"Parse error. Missing type for `typeof` value. The value must be declared and const."));
} }


@Test @Test
Expand All @@ -22483,7 +22518,8 @@ public void testTypeofType_withGlobalInferredVariableWithFunctionExpression() {
"var g = function (/** typeof x */ a) {}", "var g = function (/** typeof x */ a) {}",
"x = 'str';", "x = 'str';",
"g(null);"), "g(null);"),
lines("Parse error. No type for: x")); lines(
"Parse error. Missing type for `typeof` value. The value must be declared and const."));
} }


@Test @Test
Expand All @@ -22499,7 +22535,8 @@ public void testTypeofType_withLocalInferredVariableInHoistedFunction() {
" x = 'str';", " x = 'str';",
" g(null);", " g(null);",
"}"), "}"),
lines("Parse error. Not in scope: x")); lines(
"Parse error. Missing type for `typeof` value. The value must be declared and const."));
} }


@Test @Test
Expand All @@ -22516,7 +22553,8 @@ public void testTypeofType_withLocalDeclaredVariableWithHoistedFunction() {
" x = 'str';", " x = 'str';",
" g(null);", " g(null);",
"}"), "}"),
lines("Parse error. Not in scope: x")); lines(
"Parse error. Missing type for `typeof` value. The value must be declared and const."));
} }


@Test @Test
Expand All @@ -22529,7 +22567,8 @@ public void testTypeofType_withLocalInferredVariableWithFunctionExpression() {
" x = 'str';", " x = 'str';",
" g(null);", " g(null);",
"}"), "}"),
lines("Parse error. No type for: x")); lines(
"Parse error. Missing type for `typeof` value. The value must be declared and const."));
} }


@Test @Test
Expand Down Expand Up @@ -22559,7 +22598,8 @@ public void testTypeofType_withLocalInferredVariable() {
" var g = null", " var g = null",
" x = 'str';", " x = 'str';",
"}"), "}"),
lines("Parse error. No type for: x")); lines(
"Parse error. Missing type for `typeof` value. The value must be declared and const."));
} }


@Test @Test
Expand Down

0 comments on commit b60e0d4

Please sign in to comment.