Skip to content

Commit dac0948

Browse files
concavelenzblickly
authored andcommitted
Don't propagate loose types to declared property slots. This generally improves type checking around property assignments in local scope. This also revealed a problem with @implictCast property handling in local scope, and that is fixed here as well.
------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=88979120
1 parent f5f6eeb commit dac0948

File tree

8 files changed

+80
-20
lines changed

8 files changed

+80
-20
lines changed

Diff for: src/com/google/javascript/jscomp/LinkedFlowScope.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -109,20 +109,21 @@ public void inferSlotType(String symbol, JSType type) {
109109

110110
@Override
111111
public void inferQualifiedSlot(Node node, String symbol, JSType bottomType,
112-
JSType inferredType) {
112+
JSType inferredType, boolean declared) {
113113
TypedScope functionScope = getFunctionScope();
114114
if (functionScope.isLocal()) {
115115
TypedVar v = functionScope.getVar(symbol);
116116
if (v == null && !functionScope.isBottom()) {
117-
functionScope.declare(symbol, node, bottomType, null);
117+
v = functionScope.declare(symbol, node, bottomType, null, !declared);
118118
}
119119

120120
if (v != null && !v.isTypeInferred()) {
121121
JSType declaredType = v.getType();
122122
// Use the inferred type over the declared type only if the
123123
// inferred type is a strict subtype of the declared type.
124124
if (declaredType != null && inferredType.isSubtype(declaredType)
125-
&& !declaredType.isSubtype(inferredType)) {
125+
&& !declaredType.isSubtype(inferredType)
126+
&& !inferredType.isEquivalentTo(declaredType)) {
126127
inferSlotType(symbol, inferredType);
127128
}
128129
} else {

Diff for: src/com/google/javascript/jscomp/TypeCheck.java

+8-7
Original file line numberDiff line numberDiff line change
@@ -944,15 +944,16 @@ private void visitAssign(NodeTraversal t, Node assign) {
944944
objectJsType.restrictByNotNullOrUndefined());
945945
if (type != null) {
946946
if (type.hasProperty(pname) &&
947-
!type.isPropertyTypeInferred(pname) &&
948-
!propertyIsImplicitCast(type, pname)) {
947+
!type.isPropertyTypeInferred(pname)) {
949948
JSType expectedType = type.getPropertyType(pname);
950949
if (!expectedType.isUnknownType()) {
951-
validator.expectCanAssignToPropertyOf(
952-
t, assign, getJSType(rvalue),
953-
expectedType, object, pname);
954-
checkPropertyInheritanceOnGetpropAssign(
955-
t, assign, object, pname, info, expectedType);
950+
if (!propertyIsImplicitCast(type, pname)) {
951+
validator.expectCanAssignToPropertyOf(
952+
t, assign, getJSType(rvalue),
953+
expectedType, object, pname);
954+
checkPropertyInheritanceOnGetpropAssign(
955+
t, assign, object, pname, info, expectedType);
956+
}
956957
return;
957958
}
958959
}

Diff for: src/com/google/javascript/jscomp/TypeInference.java

+14-5
Original file line numberDiff line numberDiff line change
@@ -595,9 +595,18 @@ private void updateScopeForTypeChange(
595595
case Token.GETPROP:
596596
String qualifiedName = left.getQualifiedName();
597597
if (qualifiedName != null) {
598-
scope.inferQualifiedSlot(left, qualifiedName,
599-
leftType == null ? unknownType : leftType,
600-
resultType);
598+
boolean declaredSlotType = false;
599+
JSType rawObjType = left.getFirstChild().getJSType();
600+
if (rawObjType != null) {
601+
ObjectType objType = ObjectType.cast(
602+
rawObjType.restrictByNotNullOrUndefined());
603+
if (objType != null) {
604+
String propName = left.getLastChild().getString();
605+
declaredSlotType = objType.isPropertyTypeDeclared(propName);
606+
}
607+
}
608+
JSType safeLeftType = leftType == null ? unknownType : leftType;
609+
scope.inferQualifiedSlot(left, qualifiedName, safeLeftType, resultType, declaredSlotType);
601610
}
602611

603612
left.setJSType(resultType);
@@ -825,7 +834,7 @@ private FlowScope traverseObjectLiteral(Node n, FlowScope scope) {
825834

826835
scope.inferQualifiedSlot(name, qKeyName,
827836
oldType == null ? unknownType : oldType,
828-
valueType);
837+
valueType, false);
829838
}
830839
} else {
831840
n.setJSType(unknownType);
@@ -976,7 +985,7 @@ private FlowScope narrowScope(FlowScope scope, Node node, JSType narrowed) {
976985
scope = scope.createChildFlowScope();
977986
if (node.isGetProp()) {
978987
scope.inferQualifiedSlot(
979-
node, node.getQualifiedName(), getJSType(node), narrowed);
988+
node, node.getQualifiedName(), getJSType(node), narrowed, false);
980989
} else {
981990
redeclareSimpleVar(scope, node, narrowed);
982991
}

Diff for: src/com/google/javascript/jscomp/type/ChainableReverseAbstractInterpreter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ protected void declareNameInScope(FlowScope scope, Node node, JSType type) {
164164

165165
JSType origType = node.getJSType();
166166
origType = origType == null ? getNativeType(UNKNOWN_TYPE) : origType;
167-
scope.inferQualifiedSlot(node, qualifiedName, origType, type);
167+
scope.inferQualifiedSlot(node, qualifiedName, origType, type, false);
168168
break;
169169

170170
case Token.THIS:

Diff for: src/com/google/javascript/jscomp/type/FlowScope.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public interface FlowScope extends StaticTypedScope<JSType>, LatticeElement {
5757
* inferred.
5858
*/
5959
void inferQualifiedSlot(Node node, String symbol, JSType bottomType,
60-
JSType inferredType);
60+
JSType inferredType, boolean declare);
6161

6262
/**
6363
* Optimize this scope and return a new FlowScope with faster lookup.

Diff for: src/com/google/javascript/jscomp/type/SemanticReverseAbstractInterpreter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ private FlowScope caseIn(Node object, String propertyName, FlowScope blindScope)
490490
JSType unknownType = typeRegistry.getNativeType(
491491
JSTypeNative.UNKNOWN_TYPE);
492492
informed.inferQualifiedSlot(
493-
object, propertyQualifiedName, unknownType, unknownType);
493+
object, propertyQualifiedName, unknownType, unknownType, false);
494494
return informed;
495495
}
496496
}

Diff for: test/com/google/javascript/jscomp/TypeCheckTest.java

+50-1
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,39 @@ public void testQualifiedNameReduction10() throws Exception {
825825
"required: string");
826826
}
827827

828+
public void testUnknownsDontOverrideDeclaredTypesInLocalScope1() throws Exception {
829+
testTypes(
830+
"/** @constructor */ var C = function() {\n"
831+
+ " /** @type {string} */ this.a = 'str'};\n"
832+
+ "/** @param {?} a\n @return {number} */\n"
833+
+ "C.prototype.f = function(a) {\n"
834+
+ " this.a = a;\n"
835+
+ " return this.a;\n"
836+
+ "}\n",
837+
838+
"inconsistent return type\n"
839+
+ "found : string\n"
840+
+ "required: number");
841+
}
842+
843+
public void testUnknownsDontOverrideDeclaredTypesInLocalScope2() throws Exception {
844+
testTypes(
845+
"/** @constructor */ var C = function() {\n"
846+
+ " /** @type {string} */ this.a = 'str';\n"
847+
+ "};\n"
848+
+ "/** @type {C} */ var x = new C();"
849+
+ "/** @param {?} a\n @return {number} */\n"
850+
+ "C.prototype.f = function(a) {\n"
851+
+ " x.a = a;\n"
852+
+ " return x.a;\n"
853+
+ "}\n",
854+
855+
"inconsistent return type\n"
856+
+ "found : string\n"
857+
+ "required: number");
858+
}
859+
860+
828861
public void testObjLitDef1a() throws Exception {
829862
testTypes(
830863
"var x = {/** @type {number} */ a:12 };\n" +
@@ -6340,14 +6373,30 @@ public void testSwitchCase8() throws Exception {
63406373
"required: number");
63416374
}
63426375

6343-
public void testImplicitCast() throws Exception {
6376+
public void testImplicitCast1() throws Exception {
63446377
testTypesWithExterns("/** @constructor */ function Element() {};\n" +
63456378
"/** @type {string}\n" +
63466379
" * @implicitCast */" +
63476380
"Element.prototype.innerHTML;",
63486381
"(new Element).innerHTML = new Array();");
63496382
}
63506383

6384+
public void testImplicitCast2() throws Exception {
6385+
testTypesWithExterns(
6386+
"/** @constructor */ function Element() {};\n" +
6387+
"/**\n" +
6388+
" * @type {string}\n" +
6389+
" * @implicitCast\n" +
6390+
" */\n" +
6391+
"Element.prototype.innerHTML;\n",
6392+
"/** @constructor */ function C(e) {\n" +
6393+
" /** @type {Element} */ this.el = e;\n" +
6394+
"}\n" +
6395+
"C.prototype.method = function() {\n" +
6396+
" this.el.innerHTML = new Array();\n" +
6397+
"};\n");
6398+
}
6399+
63516400
public void testImplicitCastSubclassAccess() throws Exception {
63526401
testTypesWithExterns("/** @constructor */ function Element() {};\n" +
63536402
"/** @type {string}\n" +

Diff for: test/com/google/javascript/jscomp/TypeInferenceTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ public void testPropertyInference3() {
275275
createUndefinableType(STRING_TYPE), null);
276276
assumingThisType(thisType);
277277
inFunction("var y = 1; this.foo = x; y = this.foo;");
278-
verify("y", CHECKED_UNKNOWN_TYPE);
278+
verify("y", createUndefinableType(STRING_TYPE));
279279
}
280280

281281
public void testAssert1() {

0 commit comments

Comments
 (0)