Skip to content

Commit

Permalink
[NTI] Add defsites to Namespaces.
Browse files Browse the repository at this point in the history
CheckAccessControls benefits from this. Also includes a related bugfix in AccessControlUtils.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=128645778
  • Loading branch information
aravind-pg authored and blickly committed Jul 28, 2016
1 parent d3abf7e commit 9ab961d
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/AccessControlUtils.java
Expand Up @@ -204,7 +204,7 @@ private static Visibility getEffectiveVisibilityForNonOverriddenProperty(
if (objectType != null) { if (objectType != null) {
raw = objectType.getOwnPropertyJSDocInfo(propertyName).getVisibility(); raw = objectType.getOwnPropertyJSDocInfo(propertyName).getVisibility();
} }
TypeI type = getprop.getTypeIIfOld(); TypeI type = getprop.getTypeI();
boolean createdFromGoogProvide = (type != null && type.isInstanceofObject()); boolean createdFromGoogProvide = (type != null && type.isInstanceofObject());
// Ignore @fileoverview visibility when computing the effective visibility // Ignore @fileoverview visibility when computing the effective visibility
// for properties created by goog.provide. // for properties created by goog.provide.
Expand Down
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/GlobalTypeInfo.java
Expand Up @@ -938,6 +938,7 @@ private void visitEnum(Node qnameNode) {
EnumType.make( EnumType.make(
commonTypes, commonTypes,
qnameNode.getQualifiedName(), qnameNode.getQualifiedName(),
qnameNode,
jsdoc.getEnumParameterType(), ImmutableSet.copyOf(propNames))); jsdoc.getEnumParameterType(), ImmutableSet.copyOf(propNames)));
} }


Expand Down Expand Up @@ -1148,7 +1149,7 @@ private void maybeAddFunctionToNamespace(Node funQname) {
QualifiedName pname = new QualifiedName(funQname.getLastChild().getString()); QualifiedName pname = new QualifiedName(funQname.getLastChild().getString());
if (!ns.isDefined(pname)) { if (!ns.isDefined(pname)) {
ns.addNamespace(pname, ns.addNamespace(pname,
new FunctionNamespace(commonTypes, funQname.getQualifiedName(), s)); new FunctionNamespace(commonTypes, funQname.getQualifiedName(), s, funQname));
} }
} }


Expand Down
6 changes: 3 additions & 3 deletions src/com/google/javascript/jscomp/NTIScope.java
Expand Up @@ -487,20 +487,20 @@ void addFunNamespace(Node qnameNode) {
Preconditions.checkState(!this.localNamespaces.containsKey(varName)); Preconditions.checkState(!this.localNamespaces.containsKey(varName));
NTIScope s = Preconditions.checkNotNull(this.localFunDefs.get(varName)); NTIScope s = Preconditions.checkNotNull(this.localFunDefs.get(varName));
this.localNamespaces.put(varName, this.localNamespaces.put(varName,
new FunctionNamespace(getCommonTypes(), varName, s)); new FunctionNamespace(getCommonTypes(), varName, s, qnameNode));
} else { } else {
Preconditions.checkArgument(!isNamespace(qnameNode)); Preconditions.checkArgument(!isNamespace(qnameNode));
QualifiedName qname = QualifiedName.fromNode(qnameNode); QualifiedName qname = QualifiedName.fromNode(qnameNode);
Namespace ns = getNamespace(qname.getLeftmostName()); Namespace ns = getNamespace(qname.getLeftmostName());
NTIScope s = (NTIScope) ns.getDeclaration(qname).getFunctionScope(); NTIScope s = (NTIScope) ns.getDeclaration(qname).getFunctionScope();
ns.addNamespace(qname.getAllButLeftmost(), ns.addNamespace(qname.getAllButLeftmost(),
new FunctionNamespace(getCommonTypes(), qname.toString(), s)); new FunctionNamespace(getCommonTypes(), qname.toString(), s, qnameNode));
} }
} }


void addNamespaceLit(Node qnameNode) { void addNamespaceLit(Node qnameNode) {
addNamespace(qnameNode, addNamespace(qnameNode,
new NamespaceLit(getCommonTypes(), qnameNode.getQualifiedName())); new NamespaceLit(getCommonTypes(), qnameNode.getQualifiedName(), qnameNode));
} }


void updateType(String name, JSType newDeclType) { void updateType(String name, JSType newDeclType) {
Expand Down
9 changes: 5 additions & 4 deletions src/com/google/javascript/jscomp/newtypes/EnumType.java
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.javascript.rhino.JSTypeExpression; import com.google.javascript.rhino.JSTypeExpression;
import com.google.javascript.rhino.Node;


import java.util.Collection; import java.util.Collection;


Expand Down Expand Up @@ -50,19 +51,19 @@ private enum State {
// All properties have the same type, so we only need a set, not a map. // All properties have the same type, so we only need a set, not a map.
private ImmutableSet<String> props; private ImmutableSet<String> props;


private EnumType(JSTypes commonTypes, String name, private EnumType(JSTypes commonTypes, String name, Node defSite,
JSTypeExpression typeExpr, Collection<String> props) { JSTypeExpression typeExpr, Collection<String> props) {
super(commonTypes, name); super(commonTypes, name, defSite);
Preconditions.checkNotNull(typeExpr); Preconditions.checkNotNull(typeExpr);
this.state = State.NOT_RESOLVED; this.state = State.NOT_RESOLVED;
// typeExpr is non-null iff the enum is not resolved // typeExpr is non-null iff the enum is not resolved
this.typeExpr = typeExpr; this.typeExpr = typeExpr;
this.props = ImmutableSet.copyOf(props); this.props = ImmutableSet.copyOf(props);
} }


public static EnumType make(JSTypes commonTypes, String name, public static EnumType make(JSTypes commonTypes, String name, Node defSite,
JSTypeExpression typeExpr, Collection<String> props) { JSTypeExpression typeExpr, Collection<String> props) {
return new EnumType(commonTypes, name, typeExpr, props); return new EnumType(commonTypes, name, defSite, typeExpr, props);
} }


public boolean isResolved() { public boolean isResolved() {
Expand Down
Expand Up @@ -17,6 +17,7 @@
package com.google.javascript.jscomp.newtypes; package com.google.javascript.jscomp.newtypes;


import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.javascript.rhino.Node;


/** /**
* *
Expand All @@ -26,8 +27,8 @@ public final class FunctionNamespace extends Namespace {
private DeclaredTypeRegistry scope; private DeclaredTypeRegistry scope;


public FunctionNamespace( public FunctionNamespace(
JSTypes commonTypes, String name, DeclaredTypeRegistry scope) { JSTypes commonTypes, String name, DeclaredTypeRegistry scope, Node defSite) {
super(commonTypes, name); super(commonTypes, name, defSite);
Preconditions.checkNotNull(name); Preconditions.checkNotNull(name);
Preconditions.checkNotNull(scope); Preconditions.checkNotNull(scope);
this.scope = scope; this.scope = scope;
Expand Down
13 changes: 9 additions & 4 deletions src/com/google/javascript/jscomp/newtypes/Namespace.java
Expand Up @@ -49,10 +49,13 @@ public abstract class Namespace {
protected JSType namespaceType; protected JSType namespaceType;
// Used to detect recursion when computing the type of circular namespaces. // Used to detect recursion when computing the type of circular namespaces.
private boolean duringComputeJSType = false; private boolean duringComputeJSType = false;
// The node that defines this namespace.
protected final Node defSite;


protected Namespace(JSTypes commonTypes, String name) { protected Namespace(JSTypes commonTypes, String name, Node defSite) {
this.name = name; this.name = name;
this.commonTypes = commonTypes; this.commonTypes = commonTypes;
this.defSite = Preconditions.checkNotNull(defSite);
} }


protected abstract JSType computeJSType(); protected abstract JSType computeJSType();
Expand All @@ -61,6 +64,10 @@ public final String getName() {
return name; return name;
} }


public Node getDefSite() {
return this.defSite;
}

private boolean isDefined(String name) { private boolean isDefined(String name) {
return namespaces.containsKey(name) return namespaces.containsKey(name)
|| typedefs.containsKey(name) || typedefs.containsKey(name)
Expand Down Expand Up @@ -199,9 +206,7 @@ final Property getNsProp(String pname) {
if (this.namespaces.containsKey(pname)) { if (this.namespaces.containsKey(pname)) {
Namespace subns = this.namespaces.get(pname); Namespace subns = this.namespaces.get(pname);
Preconditions.checkState(subns.namespaceType != null); Preconditions.checkState(subns.namespaceType != null);
// TODO(aravindpg): we need a defsite here for the test return Property.makeWithDefsite(subns.getDefSite(), subns.namespaceType, subns.namespaceType);
// CheckAccessControlsTest::testFileoverviewVisibilityDoesNotApplyToGoogProvidedNamespace4
return Property.make(subns.namespaceType, subns.namespaceType);
} }
if (this.otherProps.containsKey(pname)) { if (this.otherProps.containsKey(pname)) {
return this.otherProps.get(pname); return this.otherProps.get(pname);
Expand Down
5 changes: 3 additions & 2 deletions src/com/google/javascript/jscomp/newtypes/NamespaceLit.java
Expand Up @@ -17,6 +17,7 @@
package com.google.javascript.jscomp.newtypes; package com.google.javascript.jscomp.newtypes;


import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.javascript.rhino.Node;


/** /**
* *
Expand All @@ -27,8 +28,8 @@ public final class NamespaceLit extends Namespace {
// For when window is used as a namespace // For when window is used as a namespace
private NominalType window = null; private NominalType window = null;


public NamespaceLit(JSTypes commonTypes, String name) { public NamespaceLit(JSTypes commonTypes, String name, Node defSite) {
super(commonTypes, name); super(commonTypes, name, defSite);
} }


NominalType getWindowType() { NominalType getWindowType() {
Expand Down
10 changes: 1 addition & 9 deletions src/com/google/javascript/jscomp/newtypes/RawNominalType.java
Expand Up @@ -38,9 +38,6 @@
* @author dimvar@google.com (Dimitris Vardoulakis) * @author dimvar@google.com (Dimitris Vardoulakis)
*/ */
public final class RawNominalType extends Namespace { public final class RawNominalType extends Namespace {
// The node (if any) that defines the type. Most times it's a function, in
// rare cases it's a call node.
private final Node defSite;
// If true, we can't add more properties to this type. // If true, we can't add more properties to this type.
private boolean isFinalized; private boolean isFinalized;
// Each instance of the class has these properties by default // Each instance of the class has these properties by default
Expand Down Expand Up @@ -82,7 +79,7 @@ private enum Kind {
private RawNominalType( private RawNominalType(
JSTypes commonTypes, Node defSite, JSTypes commonTypes, Node defSite,
String name, ImmutableList<String> typeParameters, Kind kind, ObjectKind objectKind) { String name, ImmutableList<String> typeParameters, Kind kind, ObjectKind objectKind) {
super(commonTypes, name); super(commonTypes, name, defSite);
Preconditions.checkNotNull(objectKind); Preconditions.checkNotNull(objectKind);
Preconditions.checkState( Preconditions.checkState(
defSite == null || defSite.isFunction() || defSite.isCall(), defSite == null || defSite.isFunction() || defSite.isCall(),
Expand All @@ -91,7 +88,6 @@ private RawNominalType(
if (typeParameters == null) { if (typeParameters == null) {
typeParameters = ImmutableList.of(); typeParameters = ImmutableList.of();
} }
this.defSite = defSite;
this.typeParameters = typeParameters; this.typeParameters = typeParameters;
// NTI considers IObject to be a record so that, eg, an object literal can // NTI considers IObject to be a record so that, eg, an object literal can
// be considered to have any IObject type. // be considered to have any IObject type.
Expand Down Expand Up @@ -148,10 +144,6 @@ public static RawNominalType makeStructuralInterface(JSTypes commonTypes,
name, typeParameters, Kind.RECORD, ObjectKind.STRUCT); name, typeParameters, Kind.RECORD, ObjectKind.STRUCT);
} }


public Node getDefSite() {
return this.defSite;
}

private static boolean isBuiltinHelper( private static boolean isBuiltinHelper(
String nameToCheck, String builtinName, Node defSite) { String nameToCheck, String builtinName, Node defSite) {
return defSite != null && defSite.isFromExterns() return defSite != null && defSite.isFromExterns()
Expand Down
Expand Up @@ -1184,11 +1184,6 @@ public void testFileoverviewVisibilityDoesNotApplyToGoogProvidedNamespace3() {
} }


public void testFileoverviewVisibilityDoesNotApplyToGoogProvidedNamespace4() { public void testFileoverviewVisibilityDoesNotApplyToGoogProvidedNamespace4() {
// TODO(aravindpg): the problem here is that we do not store defsites for namespaces within
// namespaces (e.g. for `two` and `three` in `one.two.three`). Looking up "two" in the
// namespace object for "one" yields a property created (without a defsite) in
// Namespace.getNsProp (line 204).
this.mode = TypeInferenceMode.OtiOnly;
test( test(
ImmutableList.of( ImmutableList.of(
SourceFile.fromCode( SourceFile.fromCode(
Expand Down

0 comments on commit 9ab961d

Please sign in to comment.