Skip to content

Commit

Permalink
Add support for class members to `CheckAccessControls::bestInstanceTy…
Browse files Browse the repository at this point in the history 
…peForMethodOrCtor`.

This is not currently intended to have visible effects but is a step toward separating constructor and class access controls.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=209613691
  • Loading branch information
@nreid260 @blickly
nreid260 authored and blickly committed Aug 27, 2018
1 parent 34493b4 commit 47572a6
Showing 1 changed file with 79 additions and 63 deletions.
142 changes: 79 additions & 63 deletions src/com/google/javascript/jscomp/CheckAccessControls.java
View file
Expand Up @@ -141,7 +141,7 @@ class CheckAccessControls extends AbstractPostOrderCallback
// NOTE: LinkedList is almost always the wrong choice, but in this case we have at most a small
// handful of elements, it provides the smoothest API (push, pop, and a peek that doesn't throw
// on empty), and (unlike ArrayDeque) is null-permissive. No other option meets all these needs.
private final Deque<JSType> currentClassStack = new LinkedList<JSType>();
private final Deque<ObjectType> currentClassStack = new LinkedList<>();

private ImmutableMap<StaticSourceFile, Visibility> defaultVisibilityForFiles;
private final Multimap<JSType, String> initializedConstantProperties;
Expand Down Expand Up @@ -185,7 +185,7 @@ public void enterScope(NodeTraversal t) {
}

if (isFunctionOrClass(n)) {
currentClassStack.push(bestInstanceTypeForMethodOrCtor(n, n.getParent()));
currentClassStack.push(bestInstanceTypeForMethodOrCtor(n));
}
}

Expand All @@ -212,78 +212,96 @@ public void exitScope(NodeTraversal t) {
* <li>Prototype methods => The instance type having that prototype
* <li>Instance methods => The type the method is attached to
* <li>Constructors => The type that constructor instantiates
* <li>Object literal members => {@code null}
* <li>Non-prototype object-literal members => {@code null}
* </ul>
*
* TODO(nickreid): Remove the {@code parent} parameter.
*/
@Nullable
private JSType bestInstanceTypeForMethodOrCtor(Node n, Node parent) {
private ObjectType bestInstanceTypeForMethodOrCtor(Node n) {
checkState(isFunctionOrClass(n), n);
Node parent = n.getParent();

if (parent.isAssign()) {
Node lValue = parent.getFirstChild();
if (NodeUtil.isGet(lValue)) {
// We have an assignment of the form `a.b = ...`.
JSType lValueType = lValue.getJSType();
if (lValueType != null && (lValueType.isConstructor() || lValueType.isInterface())) {
// Case `a.B = ...`
return normalizeClassType(lValueType);
} else if (NodeUtil.isPrototypeProperty(lValue)) {
// Case `a.B.prototype = ...`
return normalizeClassType(NodeUtil.getPrototypeClassName(lValue).getJSType());
} else {
// Case `a.b = ...`
return normalizeClassType(lValue.getFirstChild().getJSType());
// We need to handle declaration syntaxes separately in a way that we can't determine based on
// the type of just one node.
// TODO(nickreid): Determine if these can be replaced with FUNCTION and CLASS cases below.
if (NodeUtil.isFunctionDeclaration(n) || NodeUtil.isClassDeclaration(n)) {
return instanceTypeFor(n.getJSType());
}

// All the remaining cases can be isolated based on `parent`.
switch (parent.getToken()) {
case NAME:
return instanceTypeFor(n.getJSType());

case ASSIGN:
{
Node lValue = parent.getFirstChild();
if (NodeUtil.isGet(lValue)) {
// We have an assignment of the form `a.b = ...`.
JSType lValueType = lValue.getJSType();
if (lValueType != null && (lValueType.isConstructor() || lValueType.isInterface())) {
// Case `a.B = ...`
return instanceTypeFor(lValueType);
} else if (NodeUtil.isPrototypeProperty(lValue)) {
// Case `a.B.prototype = ...`
return instanceTypeFor(NodeUtil.getPrototypeClassName(lValue).getJSType());
} else {
// Case `a.b = ...`
return instanceTypeFor(lValue.getFirstChild().getJSType());
}
} else {
// We have an assignment of the form "a = ...", so pull the type off the "a".
return instanceTypeFor(lValue.getJSType());
}
}
} else {
// We have an assignment of the form "a = ...", so pull the
// type off the "a".
return normalizeClassType(lValue.getJSType());
}
} else if (NodeUtil.isFunctionDeclaration(n)
|| NodeUtil.isClassDeclaration(n)
|| parent.isName()) {
return normalizeClassType(n.getJSType());
} else if (parent.isStringKey()
|| parent.isGetterDef()
|| parent.isSetterDef()
|| parent.isMemberFunctionDef()
|| parent.isComputedProp()) {
Node objectLitParent = parent.getGrandparent();
if (!objectLitParent.isAssign()) {

case STRING_KEY:
case GETTER_DEF:
case SETTER_DEF:
case MEMBER_FUNCTION_DEF:
case COMPUTED_PROP:
{
Node grandparent = parent.getParent();
Node greatGrandparent = grandparent.getParent();

if (grandparent.isObjectLit()) {
return grandparent.getJSType().isFunctionPrototypeType()
// Case: `grandparent` is an object-literal prototype.
// Example: `Foo.prototype = { a: function() {} };` where `parent` is "a".
? instanceTypeFor(grandparent.getJSType())
: null;
} else if (greatGrandparent.isClass()) {
// Case: `n` is a class member definition.
// Example: `class Foo { a() {} }` where `parent` is "a".
return instanceTypeFor(greatGrandparent.getJSType());
} else {
// This would indicate the AST is malformed.
throw new AssertionError(greatGrandparent);
}
}

default:
return null;
}
Node className = NodeUtil.getPrototypeClassName(objectLitParent.getFirstChild());
if (className != null) {
return normalizeClassType(className.getJSType());
}
}

return null;
}

/**
* Normalize the type of a constructor, its instance, and its prototype
* all down to the same type (the instance type).
* Normalize the type of a constructor, its instance, and its prototype all down to the same type
* (the instance type).
*/
private static JSType normalizeClassType(JSType type) {
private static ObjectType instanceTypeFor(JSType type) {
if (type == null || type.isUnknownType()) {
return type;
return (ObjectType) type;
} else if (type.isConstructor() || type.isInterface()) {
return type.toMaybeFunctionType().getInstanceType();
} else if (type.isFunctionPrototypeType()) {
return normalizePrototypeObject(type.toMaybeObjectType());
FunctionType ctorType = type.toMaybeObjectType().getOwnerFunction();
if (ctorType.hasInstanceType()) {
return ctorType.getInstanceType();
}
}
return type;
}

private static ObjectType normalizePrototypeObject(ObjectType type) {
FunctionType owner = type.getOwnerFunction();
if (owner.hasInstanceType()) {
return owner.getInstanceType();
}
return type;
// Sometimes unions get to here. A union has no meaningful instance type.
return type.toMaybeObjectType();
}

@Override
Expand Down Expand Up @@ -311,7 +329,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
break;
case FUNCTION:
case CLASS:
checkFinalClassOverrides(t, n, parent);
checkFinalClassOverrides(t, n);
break;
default:
break;
Expand Down Expand Up @@ -569,13 +587,12 @@ private static Visibility getOverridingPropertyVisibility(PropertyReference prop
}

/** Checks if a constructor is trying to override a final class. */
private void checkFinalClassOverrides(NodeTraversal t, Node ctor, Node parent) {
private void checkFinalClassOverrides(NodeTraversal t, Node ctor) {
checkArgument(isFunctionOrClass(ctor), ctor);

JSType type = ctor.getJSType().toMaybeFunctionType();
if (type != null && type.isConstructor()) {
JSType finalParentClass =
getSuperClassInstanceIfFinal(bestInstanceTypeForMethodOrCtor(ctor, parent));
JSType finalParentClass = getSuperClassInstanceIfFinal(bestInstanceTypeForMethodOrCtor(ctor));
if (finalParentClass != null) {
compiler.report(
t.makeError(
Expand Down Expand Up @@ -809,7 +826,7 @@ private void checkNonOverriddenPropertyVisibility(
return;
}

JSType ownerType = normalizeClassType(objectType);
JSType ownerType = instanceTypeFor(objectType);

switch (visibility) {
case PACKAGE:
Expand Down Expand Up @@ -983,8 +1000,7 @@ private boolean canAccessDeprecatedTypes(NodeTraversal t) {
// Case #3
|| (scopeRootParent != null
&& scopeRootParent.isAssign()
&& getTypeDeprecationInfo(bestInstanceTypeForMethodOrCtor(scopeRoot, scopeRootParent))
!= null);
&& getTypeDeprecationInfo(bestInstanceTypeForMethodOrCtor(scopeRoot)) != null);
}

/**
Expand Down

0 comments on commit 47572a6

Please sign in to comment.