From 7eae1e6448aa8917f6f209df6e1fa22b210e9f1e Mon Sep 17 00:00:00 2001 From: lharker Date: Wed, 20 Mar 2019 16:32:58 -0700 Subject: [PATCH] Treat early references to aliases of non-nullable names as non-nullable This still doesn't handle destructuring aliases. The drawback of this CL is that IdentifyGlobalEnumsAndTypedefsAsNonnullable duplicates work done in regular typed scope creation. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=239500615 --- .../javascript/jscomp/TypedScopeCreator.java | 53 ++++++++++++------- .../rhino/jstype/JSTypeRegistry.java | 8 +++ .../jscomp/TypeCheckNoTranspileTest.java | 24 +++++++-- 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index d1890ab0435..0f5413a4b6c 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/src/com/google/javascript/jscomp/TypedScopeCreator.java @@ -419,14 +419,16 @@ void patchGlobalScope(TypedScope globalScope, Node scriptRoot) { } /** - * Create the outermost scope. This scope contains native binding such as - * {@code Object}, {@code Date}, etc. + * Create the outermost scope. This scope contains native binding such as {@code Object}, {@code + * Date}, etc. */ @VisibleForTesting TypedScope createInitialScope(Node root) { NodeTraversal.traverse( - compiler, root, new IdentifyGlobalEnumsAndTypedefsAsNonNullable(typeRegistry)); + compiler, + root, + new IdentifyGlobalEnumsAndTypedefsAsNonNullable(typeRegistry, codingConvention)); TypedScope s = TypedScope.createGlobalScope(root); declareNativeFunctionType(s, ARRAY_FUNCTION_TYPE); @@ -489,14 +491,17 @@ void resolveTypes() { /** * Adds all globally-defined enums and typedefs to the registry's list of non-nullable types. * - * TODO(bradfordcsmith): We should also make locally-defined enums and typedefs non-nullable. + *

TODO(b/123710194): We should also make locally-defined enums and typedefs non-nullable. */ private static class IdentifyGlobalEnumsAndTypedefsAsNonNullable extends AbstractShallowStatementCallback { private final JSTypeRegistry registry; + private final CodingConvention codingConvention; - IdentifyGlobalEnumsAndTypedefsAsNonNullable(JSTypeRegistry registry) { + IdentifyGlobalEnumsAndTypedefsAsNonNullable( + JSTypeRegistry registry, CodingConvention codingConvention) { this.registry = registry; + this.codingConvention = codingConvention; } @Override @@ -510,20 +515,22 @@ public void visit(NodeTraversal t, Node node, Node parent) { Scope scope = t.getScope(); checkState(scope.isGlobal() || scope.getClosestHoistScope().isGlobal()); if (node.isVar() || scope.isGlobal()) { - for (Node child = node.getFirstChild(); - child != null; child = child.getNext()) { - identifyNameNode(child, NodeUtil.getBestJSDocInfo(child)); + for (Node child = node.getFirstChild(); child != null; child = child.getNext()) { + // TODO(b/116853368): make this work for destructuring aliases as well. + identifyEnumOrTypedefDeclaration( + child, child.getFirstChild(), NodeUtil.getBestJSDocInfo(child)); } } break; case EXPR_RESULT: Node firstChild = node.getFirstChild(); if (firstChild.isAssign()) { - identifyNameNode( - firstChild.getFirstChild(), firstChild.getJSDocInfo()); + Node assign = firstChild; + identifyEnumOrTypedefDeclaration( + assign.getFirstChild(), assign.getSecondChild(), assign.getJSDocInfo()); } else { - identifyNameNode( - firstChild, firstChild.getJSDocInfo()); + identifyEnumOrTypedefDeclaration( + firstChild, /* rvalue= */ null, firstChild.getJSDocInfo()); } break; default: @@ -531,14 +538,20 @@ public void visit(NodeTraversal t, Node node, Node parent) { } } - private void identifyNameNode( - Node nameNode, JSDocInfo info) { - if (info != null && nameNode.isQualifiedName()) { - if (info.hasEnumParameterType()) { - registry.identifyNonNullableName(nameNode.getQualifiedName()); - } else if (info.hasTypedefType()) { - registry.identifyNonNullableName(nameNode.getQualifiedName()); - } + private void identifyEnumOrTypedefDeclaration( + Node nameNode, @Nullable Node rvalue, JSDocInfo info) { + if (!nameNode.isQualifiedName()) { + return; + } + if (info != null && info.hasEnumParameterType()) { + registry.identifyNonNullableName(nameNode.getQualifiedName()); + } else if (info != null && info.hasTypedefType()) { + registry.identifyNonNullableName(nameNode.getQualifiedName()); + } else if (rvalue != null + && rvalue.isQualifiedName() + && registry.isNonNullableName(rvalue.getQualifiedName()) + && NodeUtil.isConstantDeclaration(codingConvention, info, nameNode)) { + registry.identifyNonNullableName(nameNode.getQualifiedName()); } } } diff --git a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java index 8a5f976ea80..a8a712cf157 100644 --- a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java +++ b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java @@ -1943,6 +1943,14 @@ public void identifyNonNullableName(String name) { nonNullableTypeNames.add(name); } + /** + * Identifies the name of a typedef or enum before we actually declare it. + */ + public boolean isNonNullableName(String name) { + checkNotNull(name); + return nonNullableTypeNames.contains(name); + } + public JSType evaluateTypeExpression(JSTypeExpression expr, StaticTypedScope scope) { return createTypeFromCommentNode(expr.getRoot(), expr.getSourceName(), scope); } diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index d25a41ae61e..06b88b881ec 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -5726,14 +5726,32 @@ public void testTypeNameAliasOnAliasedClassSideNamespace() { } @Test - public void testForwardDeclaredGlobalAliasOfEnumIsNonNullable() { - // TODO(b/116853368): this should be non-nullable and warn + public void testForwardDeclaredGlobalAliasOfEnumIsNonNullable_constDeclaration() { testTypes( lines( "/** @enum {string} */", "const Colors = {RED: 'red', YELLOW: 'yellow'};", "const /** ColorsAlias */ c = null", - "const ColorsAlias = Colors;")); + "const ColorsAlias = Colors;"), + lines( + "initializing variable", // + "found : null", + "required: Colors")); + } + + @Test + public void testForwardDeclaredGlobalAliasOfEnumIsNonNullable_constJSDoc() { + testTypes( + lines( + "/** @enum {string} */", + "const Colors = {RED: 'red', YELLOW: 'yellow'};", + "const /** ns.ColorsAlias */ c = null", + "const ns = {};", + "/** @const */ ns.ColorsAlias = Colors;"), + lines( + "initializing variable", // + "found : null", + "required: Colors")); } @Test