Skip to content

Commit

Permalink
Add support for computed property names and shorthand method names fo…
Browse files Browse the repository at this point in the history 
…r object

literals in NTI. For computed property names, a property would only be added to
the object's list of properties during type checking if it is a simple string,
number, or boolean without any computation involved.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=162281584
  • Loading branch information
@EatingW @blickly
EatingW authored and blickly committed Jul 18, 2017
1 parent c2b3979 commit 974f135
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 21 deletions.
7 changes: 1 addition & 6 deletions src/com/google/javascript/jscomp/CodeGenerator.java
View file
Expand Up @@ -983,12 +983,7 @@ protected void add(Node n, Context context) {
cc.listSeparator();
}

checkState(
c.isComputedProp()
|| c.isGetterDef()
|| c.isSetterDef()
|| c.isStringKey()
|| c.isMemberFunctionDef());
checkState(NodeUtil.isObjLitProperty(c));
add(c);
}
add("}");
Expand Down
22 changes: 14 additions & 8 deletions src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java
View file
Expand Up @@ -1529,13 +1529,21 @@ private void visitObjectLit(Node objLitNode, Node parent) {
Node maybeLvalue = parent.isAssign() ? parent.getFirstChild() : parent;
if (NodeUtil.isNamespaceDecl(maybeLvalue) && currentScope.isNamespace(maybeLvalue)) {
for (Node prop : objLitNode.children()) {
recordPropertyName(prop);
visitNamespacePropertyDeclaration(prop, maybeLvalue, prop.getString());
Node keyNode = NodeUtil.getObjectLitKeyNode(prop);
if (keyNode != null) {
recordPropertyName(keyNode);
}
if (!prop.isComputedProp()) {
visitNamespacePropertyDeclaration(prop, maybeLvalue, prop.getString());
}
}
} else if (!NodeUtil.isEnumDecl(maybeLvalue)
&& !NodeUtil.isPrototypeAssignment(maybeLvalue)) {
for (Node prop : objLitNode.children()) {
recordPropertyName(prop);
Node keyNode = NodeUtil.getObjectLitKeyNode(prop);
if (keyNode != null) {
recordPropertyName(keyNode);
}
JSDocInfo propJsdoc = prop.getJSDocInfo();
if (propJsdoc != null) {
getDeclaredObjLitProps().put(prop, getDeclaredTypeOfNode(propJsdoc, currentScope));
Expand Down Expand Up @@ -1806,10 +1814,7 @@ private void visitNamespacePropertyDeclaration(Node getProp) {

private void visitNamespacePropertyDeclaration(Node declNode, Node recv, String pname) {
checkArgument(
declNode.isGetProp()
|| declNode.isStringKey()
|| declNode.isGetterDef()
|| declNode.isSetterDef(),
declNode.isGetProp() || NodeUtil.isObjLitProperty(declNode),
declNode);
checkArgument(currentScope.isNamespace(recv));
if (declNode.isGetterDef()) {
Expand Down Expand Up @@ -2682,7 +2687,8 @@ private static Node fromDefsiteToName(Node defSite) {
return defSite.getLastChild();
}
if (defSite.isName() || defSite.isStringKey()
|| defSite.isGetterDef() || defSite.isSetterDef()) {
|| defSite.isGetterDef() || defSite.isSetterDef()
|| defSite.isMemberFunctionDef()) {
return defSite;
}
throw new RuntimeException("Unknown defsite: " + defSite.getToken());
Expand Down
29 changes: 25 additions & 4 deletions src/com/google/javascript/jscomp/NewTypeInference.java
View file
Expand Up @@ -1532,6 +1532,14 @@ private EnvTypePair analyzeExprFwd(
resultPair = analyzeNameFwd(expr, inEnv, requiredType, specializedType);
}
break;
case MEMBER_FUNCTION_DEF:
resultPair = analyzeExprFwd(expr.getFirstChild(), inEnv, requiredType, specializedType);
break;
case COMPUTED_PROP:
resultPair = analyzeExprFwd(expr.getFirstChild(), inEnv, requiredType, specializedType);
resultPair = analyzeExprFwd(
expr.getSecondChild(), resultPair.env, requiredType, specializedType);
break;
default:
throw new RuntimeException("Unhandled expression type: " + expr.getToken());
}
Expand Down Expand Up @@ -2766,13 +2774,13 @@ private EnvTypePair analyzeObjLitFwd(
} else if (isDict && !prop.isQuotedString()) {
warnings.add(JSError.make(prop, ILLEGAL_OBJLIT_KEY, "dict"));
}
String pname = NodeUtil.getObjectLitKeyName(prop);
// We can't assign to a getter to change its value.
// We can't do a prop access on a setter.
// So, we don't associate pname with a getter/setter.
// We add a property with a name that's weird enough to hopefully avoid
// an accidental clash.
if (prop.isGetterDef() || prop.isSetterDef()) {
String pname = NodeUtil.getObjectLitKeyName(prop);
EnvTypePair pair = analyzeExprFwd(prop.getFirstChild(), env);
FunctionType funType = pair.type.getFunType();
checkNotNull(funType);
Expand All @@ -2788,7 +2796,14 @@ private EnvTypePair analyzeObjLitFwd(
result = result.withProperty(new QualifiedName(specialPropName), propType);
env = pair.env;
} else {
QualifiedName qname = new QualifiedName(pname);
Node pnameNode = NodeUtil.getObjectLitKeyNode(prop);
if (pnameNode == null) {
// pnameNode is null when prop is a computed prop does not have a String node key.
// Just type-check the prop, then move on to the next property.
env = analyzeExprFwd(prop, env).env;
continue;
}
QualifiedName qname = new QualifiedName(pnameNode.getString());
JSType jsdocType = symbolTable.getPropDeclaredType(prop);
JSType reqPtype;
JSType specPtype;
Expand Down Expand Up @@ -3665,6 +3680,11 @@ private EnvTypePair analyzeExprBwd(
} else {
return analyzeNameBwd(expr, outEnv, requiredType);
}
case MEMBER_FUNCTION_DEF:
return analyzeExprBwd(expr.getFirstChild(), outEnv, requiredType);
case COMPUTED_PROP:
TypeEnv env = analyzeExprBwd(expr.getSecondChild(), outEnv).env;
return analyzeExprBwd(expr.getFirstChild(), env);
default:
throw new RuntimeException(
"BWD: Unhandled expression type: "
Expand Down Expand Up @@ -3992,11 +4012,12 @@ private EnvTypePair analyzeObjLitBwd(
for (Node prop = objLit.getLastChild();
prop != null;
prop = prop.getPrevious()) {
QualifiedName pname =
new QualifiedName(NodeUtil.getObjectLitKeyName(prop));
if (prop.isGetterDef() || prop.isSetterDef()) {
env = analyzeExprBwd(prop.getFirstChild(), env).env;
} else if (prop.isComputedProp() && !prop.getFirstChild().isString()){
env = analyzeExprBwd(prop, env).env;
} else {
QualifiedName pname = new QualifiedName(NodeUtil.getObjectLitKeyName(prop));
JSType jsdocType = symbolTable.getPropDeclaredType(prop);
JSType reqPtype;
if (jsdocType != null) {
Expand Down
29 changes: 26 additions & 3 deletions src/com/google/javascript/jscomp/NodeUtil.java
View file
Expand Up @@ -3146,12 +3146,27 @@ static boolean isObjectLitKey(Node node) {
* @param key A node
*/
static String getObjectLitKeyName(Node key) {
Node keyNode = getObjectLitKeyNode(key);
if (keyNode != null) {
return keyNode.getString();
}
throw new IllegalStateException("Unexpected node type: " + key);
}

/**
* Get the Node that defines the name of an object literal key.
*
* @param key A node
*/
static Node getObjectLitKeyNode(Node key) {
switch (key.getToken()) {
case STRING_KEY:
case GETTER_DEF:
case SETTER_DEF:
case MEMBER_FUNCTION_DEF:
return key.getString();
return key;
case COMPUTED_PROP:
return key.getFirstChild().isString() ? key.getFirstChild() : null;
default:
break;
}
Expand Down Expand Up @@ -4400,8 +4415,7 @@ static boolean evaluatesToLocalValue(Node value, Predicate<Node> locals) {
return true;
case OBJECTLIT:
for (Node key : value.children()) {
Preconditions.checkState(
key.isGetterDef() || key.isSetterDef() || key.isStringKey() || key.isComputedProp(),
Preconditions.checkState(isObjLitProperty(key),
"Unexpected obj literal key:",
key);

Expand Down Expand Up @@ -5111,4 +5125,13 @@ public void visit(NodeTraversal t, Node n, Node parent) {}
NodeTraversal t = new NodeTraversal(compiler, finder, scopeCreator);
t.traverseAtScope(scope);
}

/** Returns true if the node is a property of an object literal. */
public static boolean isObjLitProperty(Node node) {
return node.isStringKey()
|| node.isGetterDef()
|| node.isSetterDef()
|| node.isMemberFunctionDef()
|| node.isComputedProp();
}
}
65 changes: 65 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
View file
Expand Up @@ -2479,6 +2479,71 @@ public void testObjectLiteralShorthandProperty() {
NewTypeInference.MISTYPED_ASSIGN_RHS);
}

public void testComputedProp() {
typeCheck(LINE_JOINER.join(
"var i = 1;",
"var obj = {",
" ['var' + i]: i,",
"};"));

// Computed prop type checks within
typeCheck(LINE_JOINER.join(
"var i = null;",
"var obj = {",
" ['var' + i++]: i",
"};"),
NewTypeInference.INVALID_OPERAND_TYPE);

// Computed prop does not exist as obj prop
typeCheck(LINE_JOINER.join(
"var i = 1;",
"var obj = {",
" ['var' + i]: i",
"};",
"var x = obj.var1"),
NewTypeInference.INEXISTENT_PROPERTY);

// But if it is a plain string then safe to add it as a property.
typeCheck(LINE_JOINER.join(
"var obj = {",
" ['static']: 1",
"};",
"var /** number */ x = obj.static"));

// Does not yet have support for qualified names
typeCheck(LINE_JOINER.join(
"/** @const */ FOO = 'bar';",
"var obj = {",
" [FOO]: 1",
"};",
"var x = obj.bar"),
NewTypeInference.INEXISTENT_PROPERTY);

// Test recordPropertyName calls in GTICollector
typeCheck(LINE_JOINER.join(
"/** @const */",
"var ns = { ['a']: 123 };",
"var obj = { ['b']: 234 };",
"function f(x) {",
" return x.a + x.b;",
"}"));
}

public void testMemberFunctionDef() {
typeCheck(LINE_JOINER.join(
"var obj = {",
" method (/** number */ n) {}",
"};",
"obj.method(1);"));

typeCheck(LINE_JOINER.join(
"var obj = {",
" method (/** string */ n) {}",
"};",
"obj.method(1);"),
NewTypeInference.INVALID_ARGUMENT_TYPE);
}

public void testDeclaredVersusInferredTypeForVar() {
typeCheck("var /** ? */ x = 'str'; x - 123;");

Expand Down

0 comments on commit 974f135

Please sign in to comment.