Skip to content

Commit

Permalink
Clean up a bunch of formatting that got messed up in earlier CLs.
Browse files Browse the repository at this point in the history
Also cleans up/refactors the logic TypedScopeCreator#isQualifiedNameInferred to be easier to follow.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=191521889
  • Loading branch information
shicks authored and lauraharker committed Apr 4, 2018
1 parent 769e4a5 commit 213e41f
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 55 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/AbstractVar.java
Expand Up @@ -79,7 +79,7 @@ final CompilerInput getInput() {
}

@Override
public StaticSourceFile getSourceFile() {
public final StaticSourceFile getSourceFile() {
return (nameNode != null ? nameNode : scope.getRootNode()).getStaticSourceFile();
}

Expand Down
11 changes: 11 additions & 0 deletions src/com/google/javascript/jscomp/LinkedFlowScope.java
Expand Up @@ -314,6 +314,17 @@ public TypedScope getDeclarationScope() {

/** Join the two FlowScopes. */
static class FlowScopeJoinOp extends JoinOp.BinaryJoinOp<FlowScope> {
// NOTE(sdh): When joining flow scopes with different syntactic scopes,
// we do not attempt to recover the correct syntactic scope. This is
// okay because joins only occur in two situations: (1) performed by
// the DataFlowAnalysis class automatically between CFG nodes, and (2)
// requested manually while traversing a single expression within a CFG
// node. The syntactic scope is always set at the beginning of flowing
// through a CFG node. In the case of (1), the join result's syntactic
// scope is immediately replaced with the correct one when we flow through
// the next node. In the case of (2), both inputs will always have the
// same syntactic scope. So simply propagating either input's scope is
// perfectly fine.
@SuppressWarnings("ReferenceEquality")
@Override
public FlowScope apply(FlowScope a, FlowScope b) {
Expand Down
16 changes: 10 additions & 6 deletions src/com/google/javascript/jscomp/TypeInference.java
Expand Up @@ -1088,18 +1088,22 @@ private FlowScope narrowScope(FlowScope scope, Node node, JSType narrowed) {
/**
* We only do forward type inference. We do not do full backwards type inference.
*
* <p>In other words, if we have, <code>
* In other words, if we have,
* <code>
* var x = f();
* g(x);
* </code> a forward type-inference engine would try to figure out the type of "x" from the return
* type of "f". A backwards type-inference engine would try to figure out the type of "x" from the
* parameter type of "g".
* </code>
* a forward type-inference engine would try to figure out the type
* of "x" from the return type of "f". A backwards type-inference engine
* would try to figure out the type of "x" from the parameter type of "g".
*
* <p>However, there are a few special syntactic forms where we do some some half-assed backwards
* type-inference, because programmers expect it in this day and age. To take an example from
* Java, <code>
* Java,
* <code>
* List<String> x = Lists.newArrayList();
* </code> The Java compiler will be able to infer the generic type of the List returned by
* </code>
* The Java compiler will be able to infer the generic type of the List returned by
* newArrayList().
*
* <p>In much the same way, we do some special-case backwards inference for JS. Those cases are
Expand Down
105 changes: 65 additions & 40 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
Expand Up @@ -839,8 +839,7 @@ private JSType getDeclaredTypeInAnnotation(Node node, JSDocInfo info) {
}
} else if (FunctionTypeBuilder.isFunctionTypeDeclaration(info)) {
String fnName = node.getQualifiedName();
jsType = createFunctionTypeFromNodes(
null, fnName, info, node);
jsType = createFunctionTypeFromNodes(null, fnName, info, node);
}
}
return jsType;
Expand All @@ -861,9 +860,7 @@ void assertDefinitionNode(Node n, Token type) {
void defineCatch(Node n) {
assertDefinitionNode(n, Token.CATCH);
Node catchName = n.getFirstChild();
defineSlot(catchName, n,
getDeclaredType(
catchName.getJSDocInfo(), catchName, null));
defineSlot(catchName, n, getDeclaredType(catchName.getJSDocInfo(), catchName, null));
}

/**
Expand Down Expand Up @@ -1493,8 +1490,7 @@ && shouldUseFunctionLiteralType(
rValue, lValue.getQualifiedName(), info, isLValueRootedInGlobalScope(lValue));
}
} else if (info.isConstructorOrInterface()) {
return createFunctionTypeFromNodes(
rValue, lValue.getQualifiedName(), info, lValue);
return createFunctionTypeFromNodes(rValue, lValue.getQualifiedName(), info, lValue);
}
}

Expand Down Expand Up @@ -1866,8 +1862,7 @@ void maybeDeclareQualifiedName(NodeTraversal t, JSDocInfo info,
* or a function literal with a name we haven't seen before.
*/
private boolean isQualifiedNameInferred(
String qName, Node n, JSDocInfo info,
Node rhsValue, JSType valueType) {
String qName, Node n, JSDocInfo info, Node rhsValue, JSType valueType) {
if (valueType == null) {
return true;
}
Expand All @@ -1877,46 +1872,76 @@ private boolean isQualifiedNameInferred(
String className = qName.substring(0, qName.lastIndexOf(".prototype"));
TypedVar slot = currentScope.getVar(className);
JSType classType = slot == null ? null : slot.getType();
if (classType != null
&& (classType.isConstructor() || classType.isInterface())) {
if (classType != null && (classType.isConstructor() || classType.isInterface())) {
return false;
}
}

boolean inferred = true;
if (info != null) {
inferred = !(info.hasType()
|| info.hasEnumParameterType()
|| (NodeUtil.isConstantDeclaration(
compiler.getCodingConvention(), info, n)
&& valueType != null
&& !valueType.isUnknownType())
|| FunctionTypeBuilder.isFunctionTypeDeclaration(info));
// If the jsdoc or RHS specifies a concrete type, it's not inferred.
if (info != null
&& (info.hasType()
|| info.hasEnumParameterType()
|| isConstantDeclarationWithKnownType(info, n, valueType)
|| FunctionTypeBuilder.isFunctionTypeDeclaration(info)
|| rhsValue != null && rhsValue.isFunction())) {
return false;
}

if (inferred && rhsValue != null && rhsValue.isFunction()) {
if (info != null) {
return false;
} else if (!currentScope.hasOwnSlot(qName) && n.isUnscopedQualifiedName()) {

// Check if this is in a conditional block.
// Functions assigned in conditional blocks are inferred.
for (Node current = n.getParent();
!(current.isScript() || current.isFunction());
current = current.getParent()) {
if (NodeUtil.isControlStructure(current)) {
return true;
}
}
// At this point, we're pretty sure it's inferred, since there's neither
// useful jsdoc info, nor a useful const or doc'd function RHS. But
// there's still one case where it may still not be: if the RHS is a
// function that is not
// (1) a scoped qualified name (i.e. this.b.c or super.b.c),
// (2) already declared in a scope,
// (3) assigned in a conditional block, or
// (4) escaped to a closure,
// then we treat it as if it is declared, rather than inferred.
// Stubs and non-functions are always considered inferred at this point.
if (rhsValue == null || !rhsValue.isFunction()) {
return true;
}

// Check if this is assigned in an inner scope.
// Functions assigned in inner scopes are inferred.
if (!escapedVarNames.contains(ScopedName.of(qName, currentScope.getRootNode()))) {
return false;
}
// "Scoped" qualified names (e.g. this.b.c or super.d) are inferred.
if (!n.isUnscopedQualifiedName()) {
return true;
}

// If this qname is already declared then treat this definition as inferred.
TypedScope ownerScope = getLValueRootScope(n);
if (ownerScope != null && ownerScope.hasOwnSlot(qName)) {
return true;
}

// Check if this is in a conditional block.
// Functions assigned in conditional blocks are inferred.
if (hasControlStructureAncestor(n.getParent())) {
return true;
}

// Check if this is assigned in an inner scope.
// Functions assigned in inner scopes are inferred.
if (ownerScope != null
&& escapedVarNames.contains(ScopedName.of(qName, ownerScope.getRootNode()))) {
return true;
}

return false;
}

private boolean isConstantDeclarationWithKnownType(JSDocInfo info, Node n, JSType valueType) {
return NodeUtil.isConstantDeclaration(compiler.getCodingConvention(), info, n)
&& valueType != null
&& !valueType.isUnknownType();
}

private boolean hasControlStructureAncestor(Node n) {
while (!(n.isScript() || n.isFunction())) {
if (NodeUtil.isControlStructure(n)) {
return true;
}
n = n.getParent();
}
return inferred;
return false;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/TypedVar.java
Expand Up @@ -71,7 +71,7 @@ void setType(JSType type) {

void resolveType(ErrorReporter errorReporter) {
if (type != null) {
type = type.resolve(errorReporter, scope);
this.type = type.resolve(errorReporter, scope);
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/com/google/javascript/jscomp/VariableReferenceCheck.java
Expand Up @@ -182,9 +182,11 @@ private void checkDefaultParam(
compiler,
param.getParentNode().getSecondChild(),
/**
* Do a shallow check since cases like: {@code function f(y = () => x, x = 5) { return
* y(); } } is legal. We are going to miss cases like: {@code function f(y = (() => x)(),
* x = 5) { return y(); } } but this should be rare.
* Do a shallow check since cases like: {@code
* function f(y = () => x, x = 5) { return y(); }
* } is legal. We are going to miss cases like: {@code
* function f(y = (() => x)(), x = 5) { return y(); }
* } but this should be rare.
*/
new AbstractShallowCallback() {
@Override
Expand Down
20 changes: 16 additions & 4 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
Expand Up @@ -6010,7 +6010,10 @@ public void testOr6() {
" var x = opt_x || [];",
" var /** undefined */ y = x;",
"}"),
lines("initializing variable", "found : Array", "required: undefined"));
lines(
"initializing variable",
"found : Array",
"required: undefined"));
}

public void testAnd1() {
Expand Down Expand Up @@ -8829,7 +8832,10 @@ public void testQualifiedNameInference14() {
" x.onload = null;",
" };",
"}"),
lines("assignment", "found : null", "required: function(): undefined"));
lines(
"assignment",
"found : null",
"required: function(): undefined"));
}

public void testScopeQualifiedNamesOnThis() {
Expand Down Expand Up @@ -19428,7 +19434,10 @@ public void testSuperclassDefinedInBlockUsingVar() {
"Foo.prototype.baz = function(x) {",
" var /** string */ y = x;",
"};"),
lines("initializing variable", "found : number", "required: string"));
lines(
"initializing variable",
"found : number",
"required: string"));
}

public void testSuperclassDefinedInBlockOnNamespace() {
Expand All @@ -19448,7 +19457,10 @@ public void testSuperclassDefinedInBlockOnNamespace() {
"ns.Foo.prototype.baz = function(x) {",
" var /** string */ y = x;",
"};"),
lines("initializing variable", "found : number", "required: string"));
lines(
"initializing variable",
"found : number",
"required: string"));
}

private void testTypes(String js) {
Expand Down
2 changes: 2 additions & 0 deletions test/com/google/javascript/jscomp/TypeInferenceTest.java
Expand Up @@ -16,6 +16,7 @@

package com.google.javascript.jscomp;

import static com.google.common.base.Preconditions.checkState;
import static com.google.javascript.jscomp.CompilerTypeTestCase.lines;
import static com.google.javascript.rhino.jstype.JSTypeNative.ALL_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.ARRAY_TYPE;
Expand Down Expand Up @@ -102,6 +103,7 @@ private void inFunction(String js) {
}

private void inGenerator(String js) {
checkState(assumedThisType == null);
parseAndRunTypeInference("(function *() {" + js + "});");
}

Expand Down

0 comments on commit 213e41f

Please sign in to comment.