Skip to content

Commit

Permalink
TypedScopeCreator: Handle destructuring in variable declarations and …
Browse files Browse the repository at this point in the history 
…catch clauses.

Destructuring in assignments and function parameters still needs to be done.
Also doesn't yet handle object rest.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202411333
  • Loading branch information
@brad4d
brad4d committed Jun 28, 2018
1 parent d7ac822 commit 43ec254
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 30 deletions.
107 changes: 77 additions & 30 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
View file
Expand Up @@ -729,6 +729,7 @@ void processObjectLitProperties(
new SlotDefiner()
.forDeclarationNode(keyNode)
.forVariableName(qualifiedName)
.inScope(getLValueRootScope(keyNode))
.withType(keyType)
.allowLaterTypeInference(keyType == null)
.defineSlot();
Expand Down Expand Up @@ -801,34 +802,61 @@ void assertDefinitionNode(Node n, Token type) {
*/
void defineCatch(Node n) {
assertDefinitionNode(n, Token.CATCH);
Node catchName = n.getFirstChild();
JSType type = getDeclaredType(catchName.getJSDocInfo(), catchName, null);
new SlotDefiner()
.forDeclarationNode(catchName)
.readVariableNameFromDeclarationNode()
.withType(type)
.allowLaterTypeInference(type == null)
.defineSlot();
// Though almost certainly a terrible idea, it is possible to do destructuring in
// the catch declaration.
// e.g. `} catch ({message, errno}) {`
for (Node catchName : NodeUtil.findLhsNodesInNode(n)) {
JSType type = getDeclaredType(catchName.getJSDocInfo(), catchName, null);
new SlotDefiner()
.forDeclarationNode(catchName)
.forVariableName(catchName.getString())
.inScope(currentScope)
.withType(type)
.allowLaterTypeInference(type == null)
.defineSlot();
}
}

/**
* Defines a VAR initialization.
*/
void defineVar(Node n) {
/** Defines a variable declared with `var`, `let`, or `const`. */
void defineVars(Node n) {
checkState(sourceName != null);
checkState(NodeUtil.isNameDeclaration(n));
JSDocInfo info = n.getJSDocInfo();
if (n.hasMoreThanOneChild()) {
// `var` declarations are hoisted, but `let` and `const` are not.
TypedScope scope = n.isVar() ? currentHoistScope : currentScope;
// Get a list of all of the name nodes for all the variables being declared.
// This handles even complex destructuring cases.
// e.g.
// let x = 3, {y, someProp: [ a1, a2 ]} = someObject;
// varNames will contain x, y, a1, and a2
List<Node> varNames = NodeUtil.findLhsNodesInNode(n);
if (varNames.size() == 1) {
// e.g.
// /** @type {string} */
// let x = 'hi';
Node singleVarName = varNames.get(0);
if (info == null) {
// There might be inline JSDoc on the variable name.
// e.g.
// let /** string */ x = 'hi';
info = singleVarName.getJSDocInfo();
// TODO(bradfordcsmith): Report an error if both the declaration node and the name itself
// have JSDoc.
}
defineName(singleVarName, scope, info);
} else {
if (info != null) {
// multiple children
// We do not allow a single JSDoc to apply to multiple variables in a single
// declaration.
report(JSError.make(n, MULTIPLE_VAR_DEF));
}
for (Node name : n.children()) {
defineName(name, name.getJSDocInfo());
// TODO(bradfordcsmith): Should we report an error if there are actually 0 variable names?
// This can happen with destructuring patterns.
// e.g.
// let {} = foo();
for (Node nameNode : varNames) {
defineName(nameNode, scope, nameNode.getJSDocInfo());
}
} else {
Node name = n.getFirstChild();
defineName(name, (info != null) ? info : name.getJSDocInfo());
}
}

Expand Down Expand Up @@ -859,9 +887,11 @@ void defineClassLiteral(Node n) {
// declaration. Otherwise, the declaration will happen in other
// code paths.
if (NodeUtil.isClassDeclaration(n)) {
checkNotNull(className);
new SlotDefiner()
.forDeclarationNode(n.getFirstChild())
.readVariableNameFromDeclarationNode()
.forVariableName(className)
.inScope(currentScope)
.withType(classType)
.allowLaterTypeInference(classType == null)
.defineSlot();
Expand Down Expand Up @@ -890,7 +920,8 @@ void defineFunctionLiteral(Node n) {
if (NodeUtil.isFunctionDeclaration(n)) {
new SlotDefiner()
.forDeclarationNode(n.getFirstChild())
.readVariableNameFromDeclarationNode()
.forVariableName(functionName)
.inScope(currentScope)
.withType(functionType)
.allowLaterTypeInference(functionType == null)
.defineSlot();
Expand All @@ -899,11 +930,12 @@ void defineFunctionLiteral(Node n) {

/**
* Defines a variable based on the {@link Token#NAME} node passed.
*
* @param name The {@link Token#NAME} node.
* @param info the {@link JSDocInfo} information relating to this
* {@code name} node.
* @param scope
* @param info the {@link JSDocInfo} information relating to this {@code name} node.
*/
private void defineName(Node name, JSDocInfo info) {
private void defineName(Node name, TypedScope scope, JSDocInfo info) {
Node value = name.getFirstChild();

// variable's type
Expand All @@ -914,7 +946,8 @@ private void defineName(Node name, JSDocInfo info) {
}
new SlotDefiner()
.forDeclarationNode(name)
.readVariableNameFromDeclarationNode()
.forVariableName(name.getString())
.inScope(scope)
.withType(type)
.allowLaterTypeInference(type == null)
.defineSlot();
Expand Down Expand Up @@ -1307,6 +1340,7 @@ private EnumType createEnumTypeFromNodes(Node rValue, String name, JSDocInfo inf
class SlotDefiner {
Node declarationNode;
String variableName;
TypedScope scope;
// default is no type and a type may be inferred later
JSType type = null;
boolean allowLaterTypeInference = true;
Expand Down Expand Up @@ -1350,6 +1384,11 @@ SlotDefiner forVariableName(String variableName) {
return this;
}

SlotDefiner inScope(TypedScope scope) {
this.scope = checkNotNull(scope);
return this;
}

SlotDefiner withType(@Nullable JSType type) {
this.type = type;
return this;
Expand All @@ -1369,10 +1408,12 @@ void defineSlot() {
checkNotNull(declarationNode, "declarationNode not set");
checkNotNull(variableName, "variableName not set");
checkState(allowLaterTypeInference || type != null, "null type but inference not allowed");
checkState(!variableName.isEmpty());
checkNotNull(scope);

Node parent = declarationNode.getParent();
checkArgument(!variableName.isEmpty());

TypedScope scopeToDeclareIn = getLValueRootScope(declarationNode);
TypedScope scopeToDeclareIn = scope;

boolean isGlobalVar = declarationNode.isName() && scopeToDeclareIn.isGlobal();
boolean shouldDeclareOnGlobalThis = isGlobalVar && (parent.isVar() || parent.isFunction());
Expand Down Expand Up @@ -1983,6 +2024,7 @@ void maybeDeclareQualifiedName(NodeTraversal t, JSDocInfo info,
new SlotDefiner()
.forDeclarationNode(n)
.readVariableNameFromDeclarationNode()
.inScope(getLValueRootScope(n))
.withType(valueType)
.allowLaterTypeInference(inferred)
.defineSlot();
Expand Down Expand Up @@ -2151,6 +2193,7 @@ void resolveStubDeclaration(Node n, boolean isExtern, String ownerName) {
new SlotDefiner()
.forDeclarationNode(n)
.readVariableNameFromDeclarationNode()
.inScope(getLValueRootScope(n))
.withType(stubType)
.allowLaterTypeInference(true)
.defineSlot();
Expand Down Expand Up @@ -2200,7 +2243,8 @@ void checkForTypedef(Node candidate, JSDocInfo info) {
if (candidate.isGetProp()) {
new SlotDefiner()
.forDeclarationNode(candidate)
.readVariableNameFromDeclarationNode()
.forVariableName(typedef)
.inScope(currentScope)
.withType(getNativeType(NO_TYPE))
.allowLaterTypeInference(false)
.defineSlot();
Expand Down Expand Up @@ -2263,7 +2307,7 @@ void visitPostorder(NodeTraversal t, Node n, Node parent) {
case VAR:
case LET:
case CONST:
defineVar(n);
defineVars(n);
// Handle typedefs.
if (n.hasOneChild()) {
checkForTypedef(n.getFirstChild(), n.getJSDocInfo());
Expand Down Expand Up @@ -2321,7 +2365,8 @@ void handleFunctionInputs() {
&& fnVar.getInitialValue() != fnNode)) {
new SlotDefiner()
.forDeclarationNode(fnNameNode)
.readVariableNameFromDeclarationNode()
.forVariableName(fnName)
.inScope(currentScope)
.withType(fnNode.getJSType())
.allowLaterTypeInference(false)
.defineSlot();
Expand Down Expand Up @@ -2382,6 +2427,7 @@ void declareArguments() {
new SlotDefiner()
.forDeclarationNode(astParameter)
.readVariableNameFromDeclarationNode()
.inScope(currentScope)
.withType(paramType)
.allowLaterTypeInference(inferred)
.defineSlot();
Expand Down Expand Up @@ -2431,6 +2477,7 @@ void visitPostorder(NodeTraversal t, Node n, Node parent) {
new SlotDefiner()
.forDeclarationNode(n)
.readVariableNameFromDeclarationNode()
.inScope(currentScope)
.withType(parent.getJSType())
.allowLaterTypeInference(false)
.defineSlot();
Expand Down
116 changes: 116 additions & 0 deletions test/com/google/javascript/jscomp/TypedScopeCreatorTest.java
View file
Expand Up @@ -143,6 +143,115 @@ public void process(Node externs, Node root) {
};
}

public void testVarDeclarationWithJSDocForObjPatWithOneVariable() {
testSame("/** @type {number} */ const {a} = {a: 1};");
TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationWithJSDocForObjPatWithMultipleVariables() {
// We don't allow statement-level JSDoc when multiple variables are declared.
testWarning("/** @type {number} */ const {a, b} = {a: 1};", TypeCheck.MULTIPLE_VAR_DEF);
}

public void testVarDeclarationObjPatShorthandProp() {
testSame("var {/** number */ a} = {a: 1};");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationObjPatShorthandPropWithDefault() {
testSame("var {/** number */ a = 2} = {a: 1};");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationObjPatNormalProp() {
testSame("var {a: /** number */ a} = {a: 1};");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationObjPatNormalPropWithDefault() {
testSame("var {a: /** number */ a = 2} = {a: 1};");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationObjPatComputedProp() {
testSame("var {['a']: /** number */ a} = {a: 1};");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationObjPatComputedPropWithDefault() {
testSame("var {['a']: /** number */ a = 2} = {a: 1};");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationArrayPat() {
testSame("var [ /** number */ a ] = [1];");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationArrayPatWithDefault() {
testSame("var [ /** number */ a = 2 ] = [1];");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("number");
assertFalse(aVar.isTypeInferred());
}

public void testVarDeclarationArrayPatRest() {
// TODO(bradfordcsmith): Add a TypeCheck test case to ensure rest values are always Arrays
testSame("var [ ... /** !Array<number> */ a ] = [1];");

TypedVar aVar = checkNotNull(globalScope.getVar("a"));
assertType(aVar.getType()).toStringIsEqualTo("Array<number>");
assertFalse(aVar.isTypeInferred());
}

// TODO(bradfordcsmith): Add Object rest test case.

public void testVarDeclarationNestedPatterns() {
testSame(
lines(
"var [",
" {a: /** number */ a1},",
" {a: /** string */ a2},",
" ...{/** number */ length}",
" ] = [{a: 1}, {a: '2'}, 1, 2, 3];"));

TypedVar a1Var = checkNotNull(globalScope.getVar("a1"));
assertType(a1Var.getType()).toStringIsEqualTo("number");
assertFalse(a1Var.isTypeInferred());

TypedVar a2Var = checkNotNull(globalScope.getVar("a2"));
assertType(a2Var.getType()).toStringIsEqualTo("string");
assertFalse(a2Var.isTypeInferred());

TypedVar lengthVar = checkNotNull(globalScope.getVar("length"));
assertType(lengthVar.getType()).toStringIsEqualTo("number");
assertFalse(lengthVar.isTypeInferred());
}

public void testDeclarativelyUnboundVarsWithoutTypes() {
testSame(
lines(
Expand Down Expand Up @@ -2677,6 +2786,13 @@ public void testDeclaredCatchExpression2() {
assertEquals("string", lastLocalScope.getVar("e").getType().toString());
}

public void testDestructuringCatch() {
testSame(
"try {} catch ({/** string */ message, /** number */ errno}) {}");
assertType(lastLocalScope.getVar("message").getType()).toStringIsEqualTo("string");
assertType(lastLocalScope.getVar("errno").getType()).toStringIsEqualTo("number");
}

public void testDuplicateCatchVariableNames() {
testSame(
lines(
Expand Down

0 comments on commit 43ec254

Please sign in to comment.