Skip to content

Commit

Permalink
[NTI] Don't warn about inexistent-prop on bracketed accesses.
Browse files Browse the repository at this point in the history 
Also, uncomment some tests that have been passing for a while.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=100423097
  • Loading branch information
@dimvar @blickly
dimvar authored and blickly committed Aug 11, 2015
1 parent 8c959cd commit 1c42120
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 57 deletions.
75 changes: 41 additions & 34 deletions src/com/google/javascript/jscomp/NewTypeInference.java
View file
Expand Up @@ -2546,6 +2546,24 @@ private boolean mayWarnAboutPropCreation(
return false;
}

private boolean mayWarnAboutInexistentProp(
Node propAccessNode, JSType recvType, QualifiedName propQname) {
if (!propAccessNode.isGetProp()) {
return false;
}
String pname = propQname.toString();
if (!recvType.mayHaveProp(propQname)) {
warnings.add(JSError.make(propAccessNode,
TypeCheck.INEXISTENT_PROPERTY, pname, recvType.toString()));
return true;
} else if (!recvType.hasProp(propQname)) {
warnings.add(JSError.make(propAccessNode,
POSSIBLY_INEXISTENT_PROPERTY, pname, recvType.toString()));
return true;
}
return false;
}

private boolean mayWarnAboutConst(Node n) {
Node lhs = n.getFirstChild();
if (lhs.isName() && currentScope.isConstVar(lhs.getString())) {
Expand Down Expand Up @@ -2642,33 +2660,25 @@ private EnvTypePair analyzePropAccessFwd(Node receiver, String pname,
return new EnvTypePair(pair.env, JSType.UNKNOWN);
}
if (!propAccessNode.getParent().isExprResult()
&& !specializedType.isTruthy() && !specializedType.isFalsy()
&& !recvType.mayBeDict()) {
if (!recvType.mayHaveProp(propQname)) {
// TODO(dimvar): maybe don't warn if the getprop is inside a typeof,
// see testMissingProperty8 (who relies on that for prop checking?)
warnings.add(JSError.make(propAccessNode, TypeCheck.INEXISTENT_PROPERTY,
pname, recvType.toString()));
} else if (!recvType.hasProp(propQname)) {
warnings.add(JSError.make(
propAccessNode, POSSIBLY_INEXISTENT_PROPERTY,
pname, recvType.toString()));
} else if (recvType.hasProp(propQname) &&
!resultType.isSubtypeOf(requiredType) &&
tightenTypeAndDontWarn(
receiver.isName() ? receiver.getString() : null,
receiver,
recvType.getDeclaredProp(propQname),
resultType, requiredType)) {
// Tighten the inferred type and don't warn.
// See analyzeNameFwd for explanation about types as lower/upper bounds.
resultType = resultType.specialize(requiredType);
LValueResultFwd lvr =
analyzeLValueFwd(propAccessNode, inEnv, resultType);
TypeEnv updatedEnv =
updateLvalueTypeInEnv(lvr.env, propAccessNode, lvr.ptr, resultType);
return new EnvTypePair(updatedEnv, resultType);
}
&& !specializedType.isTruthy()
&& !specializedType.isFalsy()
&& !recvType.mayBeDict()
&& !mayWarnAboutInexistentProp(propAccessNode, recvType, propQname)
&& recvType.hasProp(propQname)
&& !resultType.isSubtypeOf(requiredType)
&& tightenTypeAndDontWarn(
receiver.isName() ? receiver.getString() : null,
receiver,
recvType.getDeclaredProp(propQname),
resultType, requiredType)) {
// Tighten the inferred type and don't warn.
// See analyzeNameFwd for explanation about types as lower/upper bounds.
resultType = resultType.specialize(requiredType);
LValueResultFwd lvr =
analyzeLValueFwd(propAccessNode, inEnv, resultType);
TypeEnv updatedEnv =
updateLvalueTypeInEnv(lvr.env, propAccessNode, lvr.ptr, resultType);
return new EnvTypePair(updatedEnv, resultType);
}
// We've already warned about missing props, and never want to return null.
if (resultType == null) {
Expand Down Expand Up @@ -3608,13 +3618,10 @@ private LValueResultFwd analyzePropLValFwd(Node obj, QualifiedName pname,
// name, or for assignment ops that won't create a new property.
boolean warnForInexistentProp = insideQualifiedName ||
propAccessNode.getParent().getType() != Token.ASSIGN;
if (warnForInexistentProp &&
!lvalueType.isUnknown() &&
!lvalueType.mayBeDict()) {
DiagnosticType dt = lvalueType.mayHaveProp(pname)
? POSSIBLY_INEXISTENT_PROPERTY : TypeCheck.INEXISTENT_PROPERTY;
warnings.add(
JSError.make(obj, dt, pnameAsString, lvalueType.toString()));
if (warnForInexistentProp
&& !lvalueType.isUnknown()
&& !lvalueType.mayBeDict()) {
mayWarnAboutInexistentProp(propAccessNode, lvalueType, pname);
return new LValueResultFwd(lvalueEnv, type, null, null);
}
}
Expand Down
37 changes: 14 additions & 23 deletions test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java
View file
Expand Up @@ -1866,6 +1866,8 @@ public void testNonexistentProperty() {

typeCheck("var x = {}; var y = x.a;", TypeCheck.INEXISTENT_PROPERTY);

typeCheck("var x = {}; var y = x['a'];");

typeCheck("var x = {}; x.y - 3; x.y = 5;", TypeCheck.INEXISTENT_PROPERTY);
}

Expand Down Expand Up @@ -2943,17 +2945,17 @@ public void testPrototypePropertyAssignments() {
// "function g() { (new Foo()).bar(5); }"),
// NewTypeInference.INVALID_ARGUMENT_TYPE);

// TODO(blickly): Add fancier JSDoc annotation finding to jstypecreator
// typeCheck(Joiner.on('\n').join(
// "/** @constructor */ function Foo() {};",
// "/** @param {string} s */ Foo.prototype.bar = function(s) {};",
// "function g() { (new Foo()).bar(5); }"),
// NewTypeInference.INVALID_ARGUMENT_TYPE);
// typeCheck(Joiner.on('\n').join(
// "/** @constructor */ function Foo() {};",
// "Foo.prototype.bar = function(/** string */ s) {};",
// "function g() { (new Foo()).bar(5); }"),
// NewTypeInference.INVALID_ARGUMENT_TYPE);
typeCheck(Joiner.on('\n').join(
"/** @constructor */ function Foo() {};",
"/** @param {string} s */ Foo.prototype.bar = function(s) {};",
"function g() { (new Foo()).bar(5); }"),
NewTypeInference.INVALID_ARGUMENT_TYPE);

typeCheck(Joiner.on('\n').join(
"/** @constructor */ function Foo() {};",
"Foo.prototype.bar = function(/** string */ s) {};",
"function g() { (new Foo()).bar(5); }"),
NewTypeInference.INVALID_ARGUMENT_TYPE);

typeCheck(Joiner.on('\n').join(
"function f() {}",
Expand All @@ -2962,15 +2964,6 @@ public void testPrototypePropertyAssignments() {
typeCheck(Joiner.on('\n').join(
"/** @param {!Function} f */",
"function foo(f) { f.prototype.bar = function(x) {}; }"));

typeCheck(Joiner.on('\n').join(
"/** @constructor */",
"function Foo() {}",
"Foo.prototype.method = function() {};",
"/** @type {number} */",
"Foo.prototype.method.pnum = 123;",
"var /** number */ n = Foo.prototype['method.pnum'];"),
TypeCheck.INEXISTENT_PROPERTY);
}

public void testPrototypeAssignment() {
Expand Down Expand Up @@ -4633,7 +4626,6 @@ public void testNamespaces() {
"/** @type {number} */ ns.foo = 123;",
"/** @type {string} */ ns.foo = '';"));

// TODO(dimvar): warn about redeclared property
typeCheck(Joiner.on('\n').join(
"/** @const */ var ns = {};",
"ns.x = 5;",
Expand Down Expand Up @@ -8924,8 +8916,7 @@ public void testStructWithIn() {
typeCheck(Joiner.on('\n').join(
"function f(p1, /** !Object */ obj) {",
" if (p1 in obj['asdf']) {}",
"}"),
TypeCheck.INEXISTENT_PROPERTY);
"}"));
}

public void testStructDictSubtyping() {
Expand Down

0 comments on commit 1c42120

Please sign in to comment.