From e9db5c87398390d53f04738751b8aa51b2324c2c Mon Sep 17 00:00:00 2001 From: blickly Date: Tue, 11 Jul 2017 08:46:57 -0700 Subject: [PATCH] Make sure that the UNKNOWN_DEFINE warning is produced in checks_only mode ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=161533593 --- .../javascript/jscomp/DefaultPassConfig.java | 2 +- .../google/javascript/jscomp/J2clPass.java | 2 +- .../javascript/jscomp/ProcessDefines.java | 42 +++++++++---------- .../javascript/jscomp/ProcessDefinesTest.java | 15 ++++++- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index df08f3dfe43..a7d69135f63 100644 --- a/src/com/google/javascript/jscomp/DefaultPassConfig.java +++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java @@ -2128,7 +2128,7 @@ public void process(Node externs, Node jsRoot) { replacements.putAll(compiler.getDefaultDefineValues()); replacements.putAll(getAdditionalReplacements(options)); replacements.putAll(options.getDefineReplacements()); - new ProcessDefines(compiler, ImmutableMap.copyOf(replacements), options.checksOnly) + new ProcessDefines(compiler, ImmutableMap.copyOf(replacements), !options.checksOnly) .injectNamespace(namespaceForChecks).process(externs, jsRoot); } }; diff --git a/src/com/google/javascript/jscomp/J2clPass.java b/src/com/google/javascript/jscomp/J2clPass.java index 5ff22a8c1d7..b4ad5523c52 100644 --- a/src/com/google/javascript/jscomp/J2clPass.java +++ b/src/com/google/javascript/jscomp/J2clPass.java @@ -235,7 +235,7 @@ public void process(Node externs, Node root) { * Re-writes Util.getDefine to make it work for compiled mode. */ Set defines = - new ProcessDefines(compiler, null, false).collectDefines(externs, root).keySet(); + new ProcessDefines(compiler, null, true).collectDefines(externs, root).keySet(); NodeTraversal.traverseEs6(compiler, root, new GetDefineRewriter(defines)); /* diff --git a/src/com/google/javascript/jscomp/ProcessDefines.java b/src/com/google/javascript/jscomp/ProcessDefines.java index aae2bc86993..ded7879ccea 100644 --- a/src/com/google/javascript/jscomp/ProcessDefines.java +++ b/src/com/google/javascript/jscomp/ProcessDefines.java @@ -56,7 +56,7 @@ class ProcessDefines implements CompilerPass { private final AbstractCompiler compiler; private final Map dominantReplacements; - private final boolean checksOnly; + private final boolean doReplacements; private GlobalNamespace namespace = null; @@ -98,10 +98,11 @@ class ProcessDefines implements CompilerPass { * @param replacements A hash table of names of defines to their replacements. * All replacements must be literals. */ - ProcessDefines(AbstractCompiler compiler, Map replacements, boolean checksOnly) { + ProcessDefines( + AbstractCompiler compiler, Map replacements, boolean doReplacements) { this.compiler = compiler; this.dominantReplacements = replacements; - this.checksOnly = checksOnly; + this.doReplacements = doReplacements; } /** @@ -120,25 +121,24 @@ public void process(Node externs, Node root) { } private void overrideDefines(Map allDefines) { - if (checksOnly) { - return; - } - for (Map.Entry def : allDefines.entrySet()) { - String defineName = def.getKey(); - DefineInfo info = def.getValue(); - Node inputValue = dominantReplacements.get(defineName); - Node finalValue = inputValue != null ? - inputValue : info.getLastValue(); - if (finalValue != info.initialValue) { - compiler.addToDebugLog("Overriding @define variable " + defineName); - boolean changed = - finalValue.getToken() != info.initialValue.getToken() - || !finalValue.isEquivalentTo(info.initialValue); - if (changed) { - info.initialValueParent.replaceChild( - info.initialValue, finalValue.cloneTree()); + if (doReplacements) { + for (Map.Entry def : allDefines.entrySet()) { + String defineName = def.getKey(); + DefineInfo info = def.getValue(); + Node inputValue = dominantReplacements.get(defineName); + Node finalValue = inputValue != null ? + inputValue : info.getLastValue(); + if (finalValue != info.initialValue) { + compiler.addToDebugLog("Overriding @define variable " + defineName); + boolean changed = + finalValue.getToken() != info.initialValue.getToken() + || !finalValue.isEquivalentTo(info.initialValue); if (changed) { - compiler.reportChangeToEnclosingScope(info.initialValueParent); + info.initialValueParent.replaceChild( + info.initialValue, finalValue.cloneTree()); + if (changed) { + compiler.reportChangeToEnclosingScope(info.initialValueParent); + } } } } diff --git a/test/com/google/javascript/jscomp/ProcessDefinesTest.java b/test/com/google/javascript/jscomp/ProcessDefinesTest.java index 5ac0d401a96..0e1ff5b184d 100644 --- a/test/com/google/javascript/jscomp/ProcessDefinesTest.java +++ b/test/com/google/javascript/jscomp/ProcessDefinesTest.java @@ -35,11 +35,13 @@ public ProcessDefinesTest() { private Map overrides = new HashMap<>(); private GlobalNamespace namespace; + private boolean doReplacements; @Override protected void setUp() throws Exception { super.setUp(); overrides.clear(); + doReplacements = true; // ProcessDefines emits warnings if the user tries to re-define a constant, // but the constant is not defined anywhere in the binary. @@ -82,6 +84,17 @@ public void testDefineBadType() { testError("/** @define {Object} */ var DEF = {}", ProcessDefines.INVALID_DEFINE_TYPE_ERROR); } + public void testChecksOnlyProducesErrors() { + doReplacements = false; + testError("/** @define {Object} */ var DEF = {}", ProcessDefines.INVALID_DEFINE_TYPE_ERROR); + } + + public void testChecksOnlyProducesUnknownDefineWarning() { + doReplacements = false; + overrides.put("a.B", new Node(Token.TRUE)); + test("var a = {};", "var a = {};", warning(ProcessDefines.UNKNOWN_DEFINE_WARNING)); + } + public void testDefineWithBadValue1() { testError("/** @define {boolean} */ var DEF = new Boolean(true);", ProcessDefines.INVALID_DEFINE_INIT_ERROR); @@ -388,7 +401,7 @@ public ProcessDefinesWithInjectedNamespace(Compiler compiler) { @Override public void process(Node externs, Node js) { namespace = new GlobalNamespace(compiler, externs, js); - new ProcessDefines(compiler, overrides, false) + new ProcessDefines(compiler, overrides, doReplacements) .injectNamespace(namespace) .process(externs, js); }