From 6e9c40343f49658e0fd79da7bbf349997e868efc Mon Sep 17 00:00:00 2001 From: tbreisacher Date: Tue, 28 Jun 2016 14:10:57 -0700 Subject: [PATCH] Move the check for goog.scope aliases into the strictMissingRequires group because there is a lot of existing code that violates it by creating an alias for a prefix of a required name, instead of a required name itself. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=126116204 --- .../jscomp/CheckRequiresForConstructors.java | 7 ++++++ .../javascript/jscomp/DiagnosticGroups.java | 1 + .../javascript/jscomp/MissingRequireTest.java | 25 +++++++++++++++++-- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/com/google/javascript/jscomp/CheckRequiresForConstructors.java b/src/com/google/javascript/jscomp/CheckRequiresForConstructors.java index 5e3742021a6..7f9973cb04a 100644 --- a/src/com/google/javascript/jscomp/CheckRequiresForConstructors.java +++ b/src/com/google/javascript/jscomp/CheckRequiresForConstructors.java @@ -92,6 +92,10 @@ public static enum Mode { DiagnosticType.disabled( "JSC_MISSING_REQUIRE_WARNING", "missing require: ''{0}''"); + static final DiagnosticType MISSING_REQUIRE_FOR_GOOG_SCOPE = + DiagnosticType.disabled( + "JSC_MISSING_REQUIRE_FOR_GOOG_SCOPE", "missing require: ''{0}''"); + static final DiagnosticType MISSING_REQUIRE_CALL_WARNING = DiagnosticType.disabled( "JSC_MISSING_REQUIRE_CALL_WARNING", "missing require: ''{0}''"); @@ -315,6 +319,9 @@ private void visitScriptNode(NodeTraversal t) { String defaultName = parentNamespace != null ? parentNamespace : namespace; String nameToReport = Iterables.getFirst(classNames, defaultName); compiler.report(t.makeError(node, MISSING_REQUIRE_CALL_WARNING, nameToReport)); + } else if (node.getParent().isName() + && node.getParent().getGrandparent() == googScopeBlock) { + compiler.report(t.makeError(node, MISSING_REQUIRE_FOR_GOOG_SCOPE, namespace)); } else { compiler.report(t.makeError(node, MISSING_REQUIRE_WARNING, namespace)); } diff --git a/src/com/google/javascript/jscomp/DiagnosticGroups.java b/src/com/google/javascript/jscomp/DiagnosticGroups.java index a34754008ac..b3ad64187de 100644 --- a/src/com/google/javascript/jscomp/DiagnosticGroups.java +++ b/src/com/google/javascript/jscomp/DiagnosticGroups.java @@ -412,6 +412,7 @@ public DiagnosticGroup forName(String name) { public static final DiagnosticGroup STRICT_MISSING_REQUIRE = DiagnosticGroups.registerGroup("strictMissingRequire", CheckRequiresForConstructors.MISSING_REQUIRE_WARNING, + CheckRequiresForConstructors.MISSING_REQUIRE_FOR_GOOG_SCOPE, CheckRequiresForConstructors.MISSING_REQUIRE_CALL_WARNING); public static final DiagnosticGroup EXTRA_REQUIRE = diff --git a/test/com/google/javascript/jscomp/MissingRequireTest.java b/test/com/google/javascript/jscomp/MissingRequireTest.java index 22f135ec443..dcc3ec2d74a 100644 --- a/test/com/google/javascript/jscomp/MissingRequireTest.java +++ b/test/com/google/javascript/jscomp/MissingRequireTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_CALL_WARNING; +import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_FOR_GOOG_SCOPE; import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_WARNING; import com.google.common.collect.ImmutableList; @@ -60,6 +61,10 @@ private void testMissingRequire(String[] js, String warningText) { test(js, js, null, MISSING_REQUIRE_WARNING, warningText); } + private void testMissingRequireForScope(String[] js, String warningText) { + test(js, js, null, MISSING_REQUIRE_FOR_GOOG_SCOPE, warningText); + } + public void testPassWithNoNewNodes() { String js = "var str = 'g4'; /* does not use new */"; testSame(js); @@ -813,7 +818,7 @@ public void testAliasConstructorToPrivateVariable() { testSame(js); } - public void testMissingGoogRequireFromGoogScope() { + public void testMissingGoogRequireFromGoogScope1() { String good = "" + "goog.provide('foo.bar.Baz');\n" + "/** @constructor */\n" @@ -827,7 +832,23 @@ public void testMissingGoogRequireFromGoogScope() { + "});\n"; String[] js = new String[] {good, bad}; String warning = "missing require: 'foo.bar.Baz'"; - testMissingRequire(js, warning); + testMissingRequireForScope(js, warning); + } + + public void testMissingGoogRequireFromGoogScope2() { + String good = "" + + "goog.provide('foo.bar.Baz');\n" + + "/** @constructor */\n" + + "foo.bar.Baz = function() {}\n"; + String bad = "" + + "goog.require('foo.bar.Baz');\n" + + "goog.scope(function() {\n" + + " var bar = foo.bar;\n" + + " use(new bar.Baz);\n" + + "});"; + String[] js = new String[] {good, bad}; + String warning = "missing require: 'foo.bar'"; + testMissingRequireForScope(js, warning); } public void testNoMissingGoogRequireFromGoogScope() {