Skip to content

Commit

Permalink
Change the type/property map optimization to only recognize object-li…
Browse files Browse the repository at this point in the history
…teral types, and not other built-in types such as Function.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=141814784
  • Loading branch information
dimvar authored and blickly committed Dec 14, 2016
1 parent 2de29d4 commit a7aca28
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 12 deletions.
28 changes: 19 additions & 9 deletions src/com/google/javascript/rhino/jstype/JSTypeRegistry.java
Expand Up @@ -148,7 +148,7 @@ public class JSTypeRegistry implements TypeIRegistry, Serializable {
private final Map<String, UnionTypeBuilder> typesIndexedByProperty =
new HashMap<>();

private final JSType sentinelObjectLiteral;
private JSType sentinelObjectLiteral;
private boolean optimizePropertyIndex = false;

// A map of properties to each reference type on which those
Expand Down Expand Up @@ -195,10 +195,16 @@ public JSTypeRegistry(ErrorReporter reporter, Set<String> forwardDeclaredTypes)
nativeTypes = new JSType[JSTypeNative.values().length];
namesToTypes = new HashMap<>();
resetForTypeCheck();
this.sentinelObjectLiteral = createAnonymousObjectType(null);
}

public void setOptimizePropertyIndex(boolean optimizePropIndex) {
private JSType getSentinelObjectLiteral() {
if (this.sentinelObjectLiteral == null) {
this.sentinelObjectLiteral = createAnonymousObjectType(null);
}
return this.sentinelObjectLiteral;
}

public void setOptimizePropertyIndex_TRANSITIONAL_METHOD(boolean optimizePropIndex) {
this.optimizePropertyIndex = optimizePropIndex;
}

Expand Down Expand Up @@ -715,11 +721,14 @@ private void registerNativeType(JSTypeNative typeId, JSType type) {
// we don't need to store these properties in the propertyIndex separately.
private static boolean isObjectLiteralThatCanBeSkipped(JSType t) {
t = t.restrictByNotNullOrUndefined();
if (t.isObject() && !t.isUnionType()) {
ObjectType tObj = t.toObjectType();
ObjectType proto = tObj.getImplicitPrototype();
return !tObj.isNativeObjectType() && !tObj.isPrototypeObject()
&& proto != null && proto.isNativeObjectType();
// Inline-record type declaration
if (t.toMaybeRecordType() != null) {
return true;
}
// Type of an object-literal value
else if (t instanceof PrototypeObjectType) {
PrototypeObjectType tObj = (PrototypeObjectType) t;
return tObj.isAnonymous();
}
return false;
}
Expand All @@ -744,8 +753,9 @@ public void registerPropertyOnType(String propertyName, JSType type) {
}

if (this.optimizePropertyIndex && isObjectLiteralThatCanBeSkipped(type)) {
type = this.sentinelObjectLiteral;
type = getSentinelObjectLiteral();
}

typeSet.addAlternate(type);
addReferenceTypeIndexedByProperty(propertyName, type);

Expand Down
Expand Up @@ -120,6 +120,7 @@ protected void setUp() throws Exception {
super.setUp();
errorReporter = new TestErrorReporter(null, null);
registry = new JSTypeRegistry(errorReporter, ImmutableSet.of("forwardDeclared"));
registry.setOptimizePropertyIndex_TRANSITIONAL_METHOD(true);
initTypes();
}

Expand Down
Expand Up @@ -21,7 +21,6 @@

import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
import com.google.javascript.rhino.testing.BaseJSTypeTestCase;

import java.util.Arrays;

/**
Expand Down Expand Up @@ -111,6 +110,7 @@ protected void initializeNewCompiler(CompilerOptions options) {
compiler = new Compiler();
compiler.initOptions(options);
registry = compiler.getTypeRegistry();
registry.setOptimizePropertyIndex_TRANSITIONAL_METHOD(true);
initTypes();
}
}
125 changes: 125 additions & 0 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
Expand Up @@ -16682,6 +16682,131 @@ public void testCovarianceForRecordType20_2() throws Exception {
"var x = b.prop2"));
}

public void testOptimizePropertyMap1() throws Exception {
// For non object-literal types such as Function, the behavior doesn't change.
// The stray property is added as unknown.
testTypes(LINE_JOINER.join(
"/** @return {!Function} */",
"function f() {",
" var x = function() {};",
" /** @type {number} */",
" x.prop = 123;",
" return x;",
"}",
"function g(/** !Function */ x) {",
" var /** null */ n = x.prop;",
"}"));
}

public void testOptimizePropertyMap2() throws Exception {
// Don't add the inferred property to all Foo values.
testTypes(LINE_JOINER.join(
"/** @typedef {{a:number}} */",
"var Foo;",
"function f(/** !Foo */ x) {",
" var y = x;",
" /** @type {number} */",
" y.b = 123;",
"}",
"function g(/** !Foo */ x) {",
" var /** null */ n = x.b;",
"}"),
"Property b never defined on x");
}

public void testOptimizePropertyMap3() throws Exception {
// For @record types, add the stray property to the index as before.
testTypes(LINE_JOINER.join(
"/** @record */",
"function Foo() {}",
"/** @type {number} */",
"Foo.prototype.a;",
"function f(/** !Foo */ x) {",
" var y = x;",
" /** @type {number} */",
" y.b = 123;",
"}",
"function g(/** !Foo */ x) {",
" var /** null */ n = x.b;",
"}"));
}

public void testOptimizePropertyMap4() throws Exception {
testTypes(LINE_JOINER.join(
"function f(x) {",
" var y = { a: 1, b: 2 };",
"}",
"function g(x) {",
" return x.b + 1;",
"}"));
}

public void testOptimizePropertyMap5() throws Exception {
// Tests that we don't declare the properties on Object (so they don't appear on
// all object types).
testTypes(LINE_JOINER.join(
"function f(x) {",
" var y = { a: 1, b: 2 };",
"}",
"function g() {",
" var x = { c: 123 };",
" return x.a + 1;",
"}"),
"Property a never defined on x");
}

public void testOptimizePropertyMap6() throws Exception {
// The stray property doesn't appear on other inline record types.
testTypes(LINE_JOINER.join(
"function f(/** {a:number} */ x) {",
" var y = x;",
" /** @type {number} */",
" y.b = 123;",
"}",
"function g(/** {c:number} */ x) {",
" var /** null */ n = x.b;",
"}"),
"Property b never defined on x");
}

public void testOptimizePropertyMap7() throws Exception {
testTypes(LINE_JOINER.join(
"function f() {",
" var x = {a:1};",
" x.b = 2;",
"}",
"function g() {",
" var y = {a:1};",
" return y.b + 1;",
"}"),
"Property b never defined on y");
}

public void testOptimizePropertyMap8() throws Exception {
testTypes(LINE_JOINER.join(
"function f(/** {a:number, b:number} */ x) {}",
"function g(/** {c:number} */ x) {",
" var /** null */ n = x.b;",
"}"),
"Property b never defined on x");
}

public void testOptimizePropertyMap9() throws Exception {
// Don't add the stray property to all types that meet with {a: number, c: string}.
testTypes(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {",
" this.a = 123;",
"}",
"function f(/** {a: number, c: string} */ x) {",
" x.b = 234;",
"}",
"function g(/** !Foo */ x) {",
" return x.b + 5;",
"}"),
"Property b never defined on Foo");
}

public void testCovarianceForRecordType21() throws Exception {
testTypesWithExtraExterns(
"",
Expand Down
7 changes: 5 additions & 2 deletions test/com/google/javascript/rhino/jstype/JSTypeTest.java
Expand Up @@ -5754,8 +5754,11 @@ public void testRegisterProperty() {
objType.defineDeclaredProperty(propName, UNKNOWN_TYPE, null);
objType.defineDeclaredProperty("allHaz", UNKNOWN_TYPE, null);

assertTypeEquals(type,
registry.getGreatestSubtypeWithProperty(type, propName));
// We exclude {a: number, b: string} because, for inline record types,
// we register their properties on a sentinel object literal in the registry.
if (!type.equals(this.recordType)) {
assertTypeEquals(type, registry.getGreatestSubtypeWithProperty(type, propName));
}

assertTypeEquals(NO_TYPE,
registry.getGreatestSubtypeWithProperty(type, "GRRR"));
Expand Down

0 comments on commit a7aca28

Please sign in to comment.