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
google · Jul 11, 2017 · e9db5c8 · e9db5c8
1 parent 7bd9db6
commit e9db5c8
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 24 deletions.
diff --git 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
@@ -235,7 +235,7 @@ public void process(Node externs, Node root) {
      * Re-writes Util.getDefine to make it work for compiled mode.
      */
     Set<String> 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
@@ -56,7 +56,7 @@ class ProcessDefines implements CompilerPass {
 
   private final AbstractCompiler compiler;
   private final Map<String, Node> 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 <b>must</b> be literals.
    */
-  ProcessDefines(AbstractCompiler compiler, Map<String, Node> replacements, boolean checksOnly) {
+  ProcessDefines(
+      AbstractCompiler compiler, Map<String, Node> 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<String, DefineInfo> allDefines) {
-    if (checksOnly) {
-      return;
-    }
-    for (Map.Entry<String, DefineInfo> 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<String, DefineInfo> 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
@@ -35,11 +35,13 @@ public ProcessDefinesTest() {
 
   private Map<String, Node> 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);
     }