Skip to content

Commit

Permalink
Defer applying the ! operator on NamedType until after it's resolved.
Browse files Browse the repository at this point in the history
Automated g4 rollback of changelist 254149009.

*** Reason for rollback ***

Roll forward, fixing bugs.

*** Original change description ***

Automated g4 rollback of changelist 254144841.

*** Reason for rollback ***

10 or more targets failed to build

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=255017937
  • Loading branch information
shicks authored and blickly committed Jun 26, 2019
1 parent 49a43ff commit 6ed62f0
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 10 deletions.
16 changes: 13 additions & 3 deletions src/com/google/javascript/rhino/jstype/JSTypeRegistry.java
Expand Up @@ -1943,9 +1943,19 @@ private JSType createFromTypeNodesInternal(
n.getFirstChild(), sourceName, scope);

case BANG: // Not nullable
return createFromTypeNodesInternal(
n.getFirstChild(), sourceName, scope, recordUnresolvedTypes)
.restrictByNotNullOrUndefined();
{
JSType child =
createFromTypeNodesInternal(
n.getFirstChild(), sourceName, scope, recordUnresolvedTypes);
if (child instanceof NamedType) {
JSType type = ((NamedType) child).getBangType();
if (type instanceof NamedType && recordUnresolvedTypes && type != child) {
unresolvedNamedTypes.add((NamedType) type);
}
return type;
}
return child.restrictByNotNullOrUndefined();
}

case QMARK: // Nullable or unknown
Node firstChild = n.getFirstChild();
Expand Down
28 changes: 23 additions & 5 deletions src/com/google/javascript/rhino/jstype/NamedType.java
Expand Up @@ -97,6 +97,7 @@ static int nominalHashCode(ObjectType type) {
private final String sourceName;
private final int lineno;
private final int charno;
private final boolean nonNull;

/**
* Validates the type resolution.
Expand Down Expand Up @@ -135,15 +136,27 @@ static int nominalHashCode(ObjectType type) {
int charno,
ImmutableList<JSType> templateTypes) {
super(registry, registry.getNativeObjectType(JSTypeNative.UNKNOWN_TYPE));
checkNotNull(reference);
this.nonNull = reference.startsWith("!");
this.resolutionScope = scope;
this.reference = reference;
this.reference = nonNull ? reference.substring(1) : reference;
this.sourceName = sourceName;
this.lineno = lineno;
this.charno = charno;
this.templateTypes = templateTypes;
}

/** Returns a new non-null version of this type. */
JSType getBangType() {
if (nonNull) {
return this;
} else if (resolutionScope == null) {
// Already resolved, just restrict.
return getReferencedType().restrictByNotNullOrUndefined();
}
return new NamedType(
resolutionScope, registry, "!" + reference, sourceName, lineno, charno, templateTypes);
}

@Override
public ImmutableList<JSType> getTemplateTypes() {
return templateTypes;
Expand Down Expand Up @@ -291,7 +304,9 @@ private void resolveViaProperties(ErrorReporter reporter) {
return;
}

StaticTypedSlot slot = resolutionScope.getSlot(componentNames[0]);
StaticTypedSlot slot =
checkNotNull(resolutionScope, "resolutionScope")
.getSlot(checkNotNull(componentNames, "componentNames")[0]);
if (slot == null) {
handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true);
return;
Expand Down Expand Up @@ -348,6 +363,9 @@ private void resolveViaProperties(ErrorReporter reporter) {

private void setReferencedAndResolvedType(
JSType type, ErrorReporter reporter) {
if (nonNull) {
type = type.restrictByNotNullOrUndefined();
}
if (validator != null) {
validator.apply(type);
}
Expand Down Expand Up @@ -405,8 +423,8 @@ private void handleUnresolvedType(
}

/**
* Check for an obscure but very confusing error condition where a local variable shadows a
* global namespace.
* Check for an obscure but very confusing error condition where a local variable shadows a global
* namespace.
*/
private boolean localVariableShadowsGlobalNamespace(String root) {
StaticSlot rootVar = resolutionScope.getSlot(root);
Expand Down
3 changes: 1 addition & 2 deletions test/com/google/javascript/jscomp/CodePrinterTest.java
Expand Up @@ -1305,7 +1305,6 @@ public void testTypeAnnotationsTypeDef() {
// TODO(johnlenz): It would be nice if there were some way to preserve
// typedefs but currently they are resolved into the basic types in the
// type registry.
// NOTE(sdh): The type inferrence does not correctly remove null
assertTypeAnnotations(
LINE_JOINER.join(
"/** @const */ var goog = {};",
Expand All @@ -1318,7 +1317,7 @@ public void testTypeAnnotationsTypeDef() {
"/** @const */ goog.java = {};",
"goog.java.Long;",
"/**",
" * @param {(Array<number>|null)} a",
" * @param {!Array<number>} a",
" * @return {undefined}",
" */",
"function f(a) {\n}\n"));
Expand Down
46 changes: 46 additions & 0 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
Expand Up @@ -24016,6 +24016,52 @@ public void testParameterShadowsNamespace() {
+ "the intended global namespace."));
}

@Test
public void testBangOperatorOnForwardReferencedType() {
testTypes(
lines(
"/** @typedef {?number} */",
"var Foo;",
"/** @return {!Foo} */",
"function f() {}",
"/** @const {!Foo} */",
"var x = f();"));
}

@Test
public void testBangOperatorOnForwardReferencedType_mismatch() {
testTypes(
lines(
"/** @type {!Foo} */",
"var x;",
"/** @typedef {?number} */",
"var Foo;",
"/** @type {?Foo} */",
"var y;",
"x = y;"),
lines("assignment", "found : (null|number)", "required: number"));
}

@Test
public void testBangOperatorOnTypedefForShadowedNamespace() {
// NOTE: This is a pattern used to work around the fact that @ngInjected constructors very
// frequently (and unavoidably) shadow global namespaces (i.e. angular.modules) with constructor
// parameters.
testTypes(
lines(
"/** @typedef {!service.Service} */",
"var serviceService;",
"/** @constructor @param {!service.Service} service */",
"var Controller = function(service) {",
" /** @private {!serviceService} */",
" this.service = service;",
"};",
"/** @const */",
"var service = {};",
"/** @constructor */",
"service.Service = function() {};"));
}

private void testClosureTypes(String js, String description) {
testClosureTypesMultipleWarnings(js,
description == null ? null : ImmutableList.of(description));
Expand Down

0 comments on commit 6ed62f0

Please sign in to comment.