Skip to content

Commit

Permalink
Refactors CheckAccessControls to work with an abstraction of a "pro…
Browse files Browse the repository at this point in the history
…perty reference" rather than a GETROP node directly.

This change is a step towards adding support for ES6 class syntax. It is necessary because class syntax creates new ways of referencing properties that the existing code didn't model well. Refactoring to this abstraction allows us to reuse the existing logic, which is large, complex, and fragile.

A number of latent errors in access control checking were discovered during this change (unchecked l-value syntaxes, ambiguity between override/overload/access). These issues are left as is since this is intended to have minimal impact on behaviour. Fixes will come later.

A bunch of code is forked from `AccessControlUtils` into `CheckAccessControls` to support the new abstraction. A tracking bug exists for reconciliation.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=207583478
  • Loading branch information
nreid260 authored and tjgq committed Aug 7, 2018
1 parent 2de240a commit dc78c0b
Show file tree
Hide file tree
Showing 4 changed files with 380 additions and 231 deletions.
26 changes: 10 additions & 16 deletions src/com/google/javascript/jscomp/AccessControlUtils.java
Expand Up @@ -118,19 +118,17 @@ static Visibility getEffectivePropertyVisibility(
}
}

/**
* Returns the source file in which the given property is defined,
* or null if it is not known.
*/
@Nullable static StaticSourceFile getDefiningSource(
Node getprop, @Nullable ObjectType referenceType, String propertyName) {
/** Returns the source file in which the given property is defined, or null if it is not known. */
@Nullable
static StaticSourceFile getDefiningSource(
Node node, @Nullable ObjectType referenceType, String propertyName) {
if (referenceType != null) {
Node propDefNode = referenceType.getPropertyDefSite(propertyName);
if (propDefNode != null) {
return propDefNode.getStaticSourceFile();
}
}
return getprop.getStaticSourceFile();
return node.getStaticSourceFile();
}

/**
Expand All @@ -155,22 +153,18 @@ static Visibility getEffectivePropertyVisibility(
return null;
}

/**
* Returns the original visibility of an overridden property.
*/
private static Visibility getOverriddenPropertyVisibility(
ObjectType objectType, String propertyName) {
/** Returns the original visibility of an overridden property. */
static Visibility getOverriddenPropertyVisibility(ObjectType objectType, String propertyName) {
return objectType != null
? objectType.getOwnPropertyJSDocInfo(propertyName).getVisibility()
: Visibility.INHERITED;
}

/**
* Returns the effective visibility of the given overridden property.
* An overridden property inherits the visibility of the property it
* overrides.
* Returns the effective visibility of the given overridden property. An overridden property
* inherits the visibility of the property it overrides.
*/
private static Visibility getEffectiveVisibilityForOverriddenProperty(
static Visibility getEffectiveVisibilityForOverriddenProperty(
Visibility visibility,
@Nullable Visibility fileOverviewVisibility,
String propertyName,
Expand Down

0 comments on commit dc78c0b

Please sign in to comment.