Skip to content

Commit

Permalink
[NTI] Type the global THIS more precisely than ?.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=130691769
  • Loading branch information
dimvar committed Aug 19, 2016
1 parent 699825b commit 474c6b3
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 28 deletions.
9 changes: 9 additions & 0 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
Expand Up @@ -418,6 +418,7 @@ public void process(Node externs, Node root) {
}
checkAndFinalizeNominalType(rawType);
}
JSType globalThisType;
if (win != null) {
// Copy properties from window to Window.prototype, because in rare cases
// people pass window around rather than using it directly.
Expand All @@ -426,7 +427,15 @@ public void process(Node externs, Node root) {
winNs.copyWindowProperties(this.commonTypes, win);
}
checkAndFinalizeNominalType(win);
// Type the global THIS as window
globalThisType = win.getInstanceAsJSType();
} else {
// Type the global THIS as a loose object
globalThisType = JSType.TOP_OBJECT.withLoose();
}
this.globalScope.setDeclaredType(
(new FunctionTypeBuilder(this.commonTypes)).
addReceiverType(globalThisType).buildDeclaration());

nominaltypesByNode = null;
propertyDefs = null;
Expand Down
15 changes: 6 additions & 9 deletions src/com/google/javascript/jscomp/NTIScope.java
Expand Up @@ -78,8 +78,8 @@ final class NTIScope implements DeclaredTypeRegistry {
// The set localEnums is used for enum resolution, and then discarded.
private Set<EnumType> localEnums = new LinkedHashSet<>();

// declaredType is null for top level, but never null for functions,
// even those without jsdoc.
// For top level, the DeclaredFunctionType just includes a type for THIS.
// For functions, the DeclaredFunctionType is never null, even those without jsdoc.
// Any inferred parameters or return will be set to null individually.
private DeclaredFunctionType declaredType;

Expand Down Expand Up @@ -321,11 +321,8 @@ boolean isEscapedVar(String name) {
}

boolean hasThis() {
if (!isFunction()) {
return false;
}
DeclaredFunctionType dft = getDeclaredFunctionType();
// dft is null for function scopes early during GlobalTypeInfo
DeclaredFunctionType dft = this.declaredType;
// dft is null early during GlobalTypeInfo
return dft != null && dft.getThisType() != null;
}

Expand Down Expand Up @@ -657,8 +654,8 @@ void resolveEnums(JSTypeCreatorFromJSDoc typeParser) {
}

void finalizeScope() {
Preconditions.checkState(isTopLevel() || this.declaredType != null,
"No declared type for function-scope: %s", this.root);
Preconditions.checkNotNull(
this.declaredType, "No declared type for scope: %s", this.root);
unknownTypeNames = ImmutableSet.of();
// For now, we put types of namespaces directly into the locals.
// Alternatively, we could move this into NewTypeInference.initEdgeEnvs
Expand Down
37 changes: 18 additions & 19 deletions src/com/google/javascript/jscomp/NewTypeInference.java
Expand Up @@ -550,15 +550,15 @@ private void initEdgeEnvsFwd(TypeEnv entryEnv) {
// For function scopes, add the formal parameters and the free variables
// from outer scopes to the environment.
Set<String> nonLocals = new LinkedHashSet<>();
if (currentScope.hasThis()) {
nonLocals.add(THIS_ID);
}
if (currentScope.isFunction()) {
if (currentScope.getName() != null) {
nonLocals.add(currentScope.getName());
}
nonLocals.addAll(currentScope.getOuterVars());
nonLocals.addAll(currentScope.getFormals());
if (currentScope.hasThis()) {
nonLocals.add(THIS_ID);
}
entryEnv = envPutType(entryEnv, RETVAL_ID, UNDEFINED);
} else {
nonLocals.addAll(currentScope.getExterns());
Expand Down Expand Up @@ -597,14 +597,14 @@ private TypeEnv getTypeEnvFromDeclaredTypes() {
Set<String> locals = currentScope.getLocals();
varNames.addAll(locals);
varNames.addAll(currentScope.getExterns());
if (currentScope.hasThis()) {
varNames.add(THIS_ID);
}
if (currentScope.isFunction()) {
if (currentScope.getName() != null) {
varNames.add(currentScope.getName());
}
varNames.addAll(currentScope.getFormals());
if (currentScope.hasThis()) {
varNames.add(THIS_ID);
}
// In the rare case when there is a local variable named "arguments",
// this entry will be overwritten in the foreach loop below.
JSType argumentsType;
Expand Down Expand Up @@ -2300,8 +2300,8 @@ private EnvTypePair analyzeSpecializedTypeof(Node typeof, Node typeString,

private EnvTypePair analyzeThisFwd(
Node expr, TypeEnv inEnv, JSType requiredType, JSType specializedType) {
mayWarnAboutGlobalThis(expr, currentScope);
if (!currentScope.hasThis()) {
mayWarnAboutGlobalThis(expr, currentScope);
return new EnvTypePair(inEnv, UNKNOWN);
}
// A trimmed-down version of analyzeNameFwd.
Expand Down Expand Up @@ -2935,19 +2935,18 @@ private boolean mayWarnAboutConstProp(
return false;
}

private boolean mayWarnAboutGlobalThis(Node thisExpr, NTIScope currentScope) {
private void mayWarnAboutGlobalThis(Node thisExpr, NTIScope currentScope) {
Preconditions.checkArgument(thisExpr.isThis());
Preconditions.checkState(!currentScope.hasThis());
Node parent = thisExpr.getParent();
if ((parent.isGetProp() || parent.isGetElem())
// Don't warn for callbacks. Most of them are not annotated but THIS is
// bound to a legitimate object at runtime. They do lose typechecking
// for THIS however, but we won't warn.
&& !NodeUtil.isCallOrNewArgument(currentScope.getRoot())) {
warnings.add(JSError.make(thisExpr, GLOBAL_THIS));
return true;
if (currentScope.isTopLevel() || !currentScope.hasThis()) {
Node parent = thisExpr.getParent();
if ((parent.isGetProp() || parent.isGetElem())
// Don't warn for callbacks. Most of them are not annotated but THIS is
// bound to a legitimate object at runtime. They do lose typechecking
// for THIS however, but we won't warn.
&& !NodeUtil.isCallOrNewArgument(currentScope.getRoot())) {
warnings.add(JSError.make(thisExpr, GLOBAL_THIS));
}
}
return false;
}

private boolean mayWarnAboutBadIObjectIndex(Node n, JSType iobjectType,
Expand Down Expand Up @@ -3902,12 +3901,12 @@ private LValueResultFwd analyzeLValueFwd(
LValueResultFwd lvalResult = null;
switch (expr.getToken()) {
case THIS: {
mayWarnAboutGlobalThis(expr, currentScope);
if (currentScope.hasThis()) {
lvalResult = new LValueResultFwd(inEnv, envGetType(inEnv, THIS_ID),
currentScope.getDeclaredTypeOf(THIS_ID),
new QualifiedName(THIS_ID));
} else {
mayWarnAboutGlobalThis(expr, currentScope);
lvalResult = new LValueResultFwd(inEnv, UNKNOWN, null, null);
}
break;
Expand Down
12 changes: 12 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
Expand Up @@ -17838,4 +17838,16 @@ public void testDontCrashWhenDeclaringFunctionOnScalar() {
"}"),
NewTypeInference.ADDING_PROPERTY_TO_NON_OBJECT);
}

public void testDontTypeGlobalThisAsUknown() {
typeCheck(
"var /** null */ x = this;",
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck("var /** !Window */ x = this;");

typeCheck(
"var y = this.toString();",
NewTypeInference.GLOBAL_THIS);
}
}

0 comments on commit 474c6b3

Please sign in to comment.