diff --git a/src/com/google/javascript/jscomp/CheckRequiresForConstructors.java b/src/com/google/javascript/jscomp/CheckRequiresForConstructors.java index a5f94b55392..2c65e0d0fb4 100644 --- a/src/com/google/javascript/jscomp/CheckRequiresForConstructors.java +++ b/src/com/google/javascript/jscomp/CheckRequiresForConstructors.java @@ -263,63 +263,25 @@ private void visitScriptNode(NodeTraversal t) { // For every usage, check that there is a goog.require, and warn if not. for (Map.Entry entry : usages.entrySet()) { String namespace = entry.getKey(); - if (namespace.endsWith(".call") || namespace.endsWith(".apply")) { - namespace = namespace.substring(0, namespace.lastIndexOf('.')); - } - if (namespace.startsWith("goog.global.") - // Most functions in base.js are goog.someName, but - // goog.module.{get,declareLegacyNamespace} are the exceptions, so just check for them - // explicitly. - || namespace.equals("goog.module.get") - || namespace.equals("goog.module.declareLegacyNamespace")) { - continue; - } - Node node = entry.getValue(); - JSDocInfo info = NodeUtil.getBestJSDocInfo(NodeUtil.getEnclosingStatement(node)); - if (info != null && info.getSuppressions().contains("missingRequire")) { - continue; - } - - List classNames = getClassNames(namespace); - // The parent namespace of the outermost class is also checked, so that classes - // used by goog.module are still checked properly. This may cause missing requires - // to be missed but in practice that should happen rarely. - String nonNullClassName = Iterables.getFirst(classNames, namespace); - String parentNamespace = null; - int separatorIndex = nonNullClassName.lastIndexOf('.'); - if (separatorIndex > 0) { - parentNamespace = nonNullClassName.substring(0, separatorIndex); + boolean isMissing = isMissingRequire(namespace, node); + if (isMissing && (namespace.endsWith(".call") || namespace.endsWith(".apply"))) { + // assume that the user is calling the corresponding built in function and only look for + // imports 'above' it. + String namespaceMinusApply = namespace.substring(0, namespace.lastIndexOf('.')); + isMissing = isMissingRequire(namespaceMinusApply, node); } - if ("goog".equals(parentNamespace) - && !isClassName(nonNullClassName.substring(separatorIndex + 1))) { - // This is probably something provided in Closure's base.js so it doesn't need - // to be required. - continue; - } - - boolean providedByConstructors = - providedNames.contains(namespace) || providedNames.contains(parentNamespace); - boolean providedByRequires = - requires.containsKey(namespace) || requires.containsKey(parentNamespace); - - for (String className : classNames) { - if (providedNames.contains(className)) { - providedByConstructors = true; - } - if (requires.containsKey(className)) { - providedByRequires = true; - } - } - - if (!providedByConstructors && !providedByRequires && !namespaces.contains(namespace)) { + if (isMissing && !namespaces.contains(namespace)) { // TODO(mknichel): If the symbol is not explicitly provided, find the next best // symbol from the provides in the same file. String rootName = Splitter.on('.').split(namespace).iterator().next(); if (mode != Mode.SINGLE_FILE || closurizedNamespaces.contains(rootName)) { if (node.isCall()) { - String defaultName = parentNamespace != null ? parentNamespace : namespace; - String nameToReport = Iterables.getFirst(classNames, defaultName); + String defaultName = + namespace.lastIndexOf('.') > 0 + ? namespace.substring(0, namespace.lastIndexOf('.')) + : namespace; + String nameToReport = Iterables.getFirst(getClassNames(namespace), defaultName); compiler.report(t.makeError(node, MISSING_REQUIRE_STRICT_WARNING, nameToReport)); } else if (node.getParent().isName() && node.getParent().getGrandparent() == googScopeBlock) { @@ -347,6 +309,57 @@ private void visitScriptNode(NodeTraversal t) { } } + /** Returns true if the given namespace is not satisfied by any {@code goog.provide}. */ + private boolean isMissingRequire(String namespace, Node node) { + if (namespace.startsWith("goog.global.") + // Most functions in base.js are goog.someName, but + // goog.module.{get,declareLegacyNamespace} are the exceptions, so just check for them + // explicitly. + || namespace.equals("goog.module.get") + || namespace.equals("goog.module.declareLegacyNamespace")) { + return false; + } + + JSDocInfo info = NodeUtil.getBestJSDocInfo(NodeUtil.getEnclosingStatement(node)); + if (info != null && info.getSuppressions().contains("missingRequire")) { + return false; + } + + List classNames = getClassNames(namespace); + // The parent namespace of the outermost class is also checked, so that classes + // used by goog.module are still checked properly. This may cause missing requires + // to be missed but in practice that should happen rarely. + String nonNullClassName = Iterables.getFirst(classNames, namespace); + String parentNamespace = null; + int separatorIndex = nonNullClassName.lastIndexOf('.'); + if (separatorIndex > 0) { + parentNamespace = nonNullClassName.substring(0, separatorIndex); + } + if ("goog".equals(parentNamespace) + && !isClassName(nonNullClassName.substring(separatorIndex + 1))) { + // This is probably something provided in Closure's base.js so it doesn't need + // to be required. + return false; + } + + boolean providedByConstructors = + providedNames.contains(namespace) || providedNames.contains(parentNamespace); + boolean providedByRequires = + requires.containsKey(namespace) || requires.containsKey(parentNamespace); + + for (String className : classNames) { + if (providedNames.contains(className)) { + providedByConstructors = true; + break; + } + if (requires.containsKey(className)) { + providedByRequires = true; + break; + } + } + return !providedByRequires && !providedByConstructors; + } + private void reportExtraRequireWarning(Node call, String require) { if (DEFAULT_EXTRA_NAMESPACES.contains(require)) { return; diff --git a/test/com/google/javascript/jscomp/MissingRequireTest.java b/test/com/google/javascript/jscomp/MissingRequireTest.java index 208a4e66c07..714ad857343 100644 --- a/test/com/google/javascript/jscomp/MissingRequireTest.java +++ b/test/com/google/javascript/jscomp/MissingRequireTest.java @@ -310,24 +310,26 @@ public void testDirectCall() { public void testDotCall() { String js = "foo.bar.baz.call();"; - testMissingRequireStrict(js, "missing require: 'foo.bar'"); + testMissingRequireStrict(js, "missing require: 'foo.bar.baz'"); List externs = ImmutableList.of(SourceFile.fromCode("externs", "var foo;")); test(externs, js, js, null, null, null); + testSame("goog.require('foo.bar.baz.call'); " + js); testSame("goog.require('foo.bar.baz'); " + js); testSame("goog.require('foo.bar'); " + js); } public void testDotApply() { - String js = "foo.bar.baz.call();"; - testMissingRequireStrict(js, "missing require: 'foo.bar'"); + String js = "foo.bar.baz.apply();"; + testMissingRequireStrict(js, "missing require: 'foo.bar.baz'"); List externs = ImmutableList.of(SourceFile.fromCode("externs", "var foo;")); test(externs, js, js, null, null, null); + testSame("goog.require('foo.bar.baz.apply'); " + js); testSame("goog.require('foo.bar.baz'); " + js); testSame("goog.require('foo.bar'); " + js); }