Implement a CheckProvidesSorted linter check, which together with Che…

…ckRequiresSorted will replace the existing CheckRequiresAndProvidesSorted.

As with CheckRequiresSorted, the intent behind this class is to share logic between the linter and ErrorToFixMapper.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=236695265

src/com/google/javascript/jscomp/lint/CheckProvidesSorted.java

@@ -0,0 +1,144 @@
+/*
+ * Copyright 2019 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp.lint;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted.PROVIDES_NOT_SORTED;
+
+import com.google.common.collect.Iterables;
+import com.google.javascript.jscomp.NodeTraversal;
+import com.google.javascript.rhino.Node;
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
+ * Checks that goog.provide statements are sorted and deduplicated, exposing the necessary
+ * information to produce a suggested fix.
+ */
+public final class CheckProvidesSorted implements NodeTraversal.Callback {
+
+  /** Operation modes. */
+  public enum Mode {
+    /** Collect information to determine whether a fix is required, but do not report a warning. */
+    COLLECT_ONLY,
+    /** Additionally report a warning. */
+    COLLECT_AND_REPORT
+  };
+
+  private final Mode mode;
+
+  // The provided namespaces in the order they appear.
+  private final List<String> originalProvides = new ArrayList<>();
+
+  // The provided namespaces in canonical order.
+  @Nullable private List<String> canonicalProvides = null;
+
+  @Nullable private Node firstNode = null;
+  @Nullable private Node lastNode = null;
+  private boolean finished = false;
+
+  @Nullable private String replacement = null;
+  private boolean needsFix = false;
+
+  public CheckProvidesSorted(Mode mode) {
+    this.mode = mode;
+  }
+
+  /** Returns the node for the first recognized provide statement. */
+  public Node getFirstNode() {
+    return firstNode;
+  }
+
+  /** Returns the node for the last recognized provide statement. */
+  public Node getLastNode() {
+    return lastNode;
+  }
+
+  /** Returns a textual replacement yielding a canonical version of the provides. */
+  public String getReplacement() {
+    return replacement;
+  }
+
+  /**
+   * Returns whether the provides need to be fixed, i.e., whether they are *not* already canonical.
+   */
+  public boolean needsFix() {
+    return needsFix;
+  }
+
+  @Override
+  public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
+    // Traverse top-level statements until a block of contiguous provides is found.
+    return !finished
+        && (parent == null || parent.isRoot() || parent.isScript() || parent.isModuleBody());
+  }
+
+  @Override
+  public void visit(NodeTraversal t, Node n, Node parent) {
+    if (n.isScript()) {
+      checkCanonical(t);
+      return;
+    }
+
+    if (n.isExprResult() && isValidProvideCall(n.getFirstChild())) {
+      originalProvides.add(getNamespace(n));
+      if (firstNode == null) {
+        firstNode = lastNode = n;
+      } else {
+        lastNode = n;
+      }
+    } else if (!originalProvides.isEmpty()) {
+      finished = true;
+    }
+  }
+
+  private static boolean isValidProvideCall(Node n) {
+    return n.isCall()
+        && n.hasTwoChildren()
+        && n.getFirstChild().matchesQualifiedName("goog.provide")
+        && n.getSecondChild().isString();
+  }
+
+  private static String getNamespace(Node n) {
+    return n.getFirstChild().getSecondChild().getString();
+  }
+
+  /** Returns the code for a correctly formatted provide call. */
+  private static String formatProvide(String namespace) {
+    StringBuilder sb = new StringBuilder();
+
+    sb.append("goog.provide('");
+    sb.append(namespace);
+    sb.append("');");
+
+    return sb.toString();
+  }
+
+  private void checkCanonical(NodeTraversal t) {
+    canonicalProvides = originalProvides.stream().distinct().sorted().collect(toImmutableList());
+    if (!originalProvides.equals(canonicalProvides)) {
+      needsFix = true;
+      replacement =
+          String.join(
+              "\n", Iterables.transform(canonicalProvides, CheckProvidesSorted::formatProvide));
+      if (mode == Mode.COLLECT_AND_REPORT) {
+        t.report(firstNode, PROVIDES_NOT_SORTED, replacement);
+      }
+    }
+  }
+}

src/com/google/javascript/refactoring/ErrorToFixMapper.java

@@ -19,21 +19,16 @@
 import static com.google.javascript.refactoring.SuggestedFix.getShortNameForRequire;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.javascript.jscomp.AbstractCompiler;
 import com.google.javascript.jscomp.JSError;
 import com.google.javascript.jscomp.NodeTraversal;
 import com.google.javascript.jscomp.NodeUtil;
-import com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted;
+import com.google.javascript.jscomp.lint.CheckProvidesSorted;
 import com.google.javascript.jscomp.lint.CheckRequiresSorted;
 import com.google.javascript.rhino.IR;
-import com.google.javascript.rhino.JSDocInfo;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.Token;
 import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -370,27 +365,20 @@ private static SuggestedFix getFixForUnsortedRequires(JSError error, AbstractCom
   }
 
   private static SuggestedFix getFixForUnsortedProvides(JSError error, AbstractCompiler compiler) {
-    SuggestedFix.Builder fix = new SuggestedFix.Builder();
-    fix.attachMatchedNodeInfo(error.node, compiler);
+    // TODO(tjgq): Encode enough information in the error to avoid the need to run a traversal in
+    // order to produce the fix.
     Node script = NodeUtil.getEnclosingScript(error.node);
-    RequireProvideSorter cb = new RequireProvideSorter("goog.provide");
-    NodeTraversal.traverse(compiler, script, cb);
-    Node first = cb.calls.get(0);
-    Node last = Iterables.getLast(cb.calls);
-
-    cb.sortCallsAlphabetically();
-    StringBuilder sb = new StringBuilder();
-    for (Node n : cb.calls) {
-      String statement = fix.generateCode(compiler, n);
-      JSDocInfo jsDoc = NodeUtil.getBestJSDocInfo(n);
-      if (jsDoc != null) {
-        statement = jsDoc.getOriginalCommentString() + "\n" + statement;
-      }
-      sb.append(statement);
+    CheckProvidesSorted callback = new CheckProvidesSorted(CheckProvidesSorted.Mode.COLLECT_ONLY);
+    NodeTraversal.traverse(compiler, script, callback);
+
+    if (!callback.needsFix()) {
+      return null;
     }
-    // Trim to remove the newline after the last goog.require/provide.
-    String newContent = sb.toString().trim();
-    return fix.replaceRange(first, last, newContent).build();
+
+    return new SuggestedFix.Builder()
+        .attachMatchedNodeInfo(callback.getFirstNode(), compiler)
+        .replaceRange(callback.getFirstNode(), callback.getLastNode(), callback.getReplacement())
+        .build();
   }
 
   private static SuggestedFix getFixForRedundantNullabilityModifierJsDoc(
@@ -400,58 +388,4 @@ private static SuggestedFix getFixForRedundantNullabilityModifierJsDoc(
         .replaceText(error.node, 1, "")
         .build();
   }
-
-  // TODO(tjgq): Rewrite this into a simpler ProvideFixer now that RequireFixer has been split off.
-  private static class RequireProvideSorter implements NodeTraversal.Callback, Comparator<Node> {
-    private final ImmutableSet<String> closureFunctions;
-    private final List<Node> calls = new ArrayList<>();
-    private boolean finished = false;
-
-    RequireProvideSorter(String... closureFunctions) {
-      this.closureFunctions = ImmutableSet.copyOf(closureFunctions);
-    }
-
-    @Override
-    public final boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
-      return !finished;
-    }
-
-    @Override
-    public final void visit(NodeTraversal nodeTraversal, Node n, Node parent) {
-      if (n.isCall()
-          && parent.isExprResult()
-          && matchName(n.getFirstChild())) {
-        calls.add(parent);
-      } else if (NodeUtil.isNameDeclaration(parent)
-          && n.hasChildren()
-          && n.getLastChild().isCall()
-          && matchName(n.getLastChild().getFirstChild())) {
-        checkState(n.isName() || n.isDestructuringLhs(), n);
-        calls.add(parent);
-      } else if (!calls.isEmpty() && parent != null && NodeUtil.isStatement(parent)) {
-        // Reached a non-goog.(require|provide|forwardDeclare) statement, so stop.
-        finished = true;
-      }
-    }
-
-    private boolean matchName(Node n) {
-      for (String closureFn : closureFunctions) {
-        if (n.matchesQualifiedName(closureFn)) {
-          return true;
-        }
-      }
-      return false;
-    }
-
-    public void sortCallsAlphabetically() {
-      Collections.sort(calls, this);
-    }
-
-    @Override
-    public int compare(Node n1, Node n2) {
-      String namespace1 = CheckRequiresAndProvidesSorted.getSortKey(n1);
-      String namespace2 = CheckRequiresAndProvidesSorted.getSortKey(n2);
-      return namespace1.compareTo(namespace2);
-    }
-  }
 }

test/com/google/javascript/jscomp/lint/CheckProvidesSortedTest.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2019 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.javascript.jscomp.lint;
+
+import static com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted.PROVIDES_NOT_SORTED;
+
+import com.google.javascript.jscomp.Compiler;
+import com.google.javascript.jscomp.CompilerOptions;
+import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
+import com.google.javascript.jscomp.CompilerPass;
+import com.google.javascript.jscomp.CompilerTestCase;
+import com.google.javascript.jscomp.NodeTraversal;
+import com.google.javascript.jscomp.parsing.Config.JsDocParsing;
+import com.google.javascript.rhino.Node;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link CheckProvidesSorted}.
+ *
+ * <p>Note that the sort/deduplication logic is extensively tested by {@link ErrorToFixMapperTest}.
+ * These tests are only for asserting that warnings are reported correctly.
+ */
+@RunWith(JUnit4.class)
+public final class CheckProvidesSortedTest extends CompilerTestCase {
+  @Override
+  protected CompilerPass getProcessor(Compiler compiler) {
+    CheckProvidesSorted callback =
+        new CheckProvidesSorted(CheckProvidesSorted.Mode.COLLECT_AND_REPORT);
+    return new CompilerPass() {
+      @Override
+      public void process(Node externs, Node root) {
+        NodeTraversal.traverse(compiler, root, callback);
+      }
+    };
+  }
+
+  @Override
+  protected CompilerOptions getOptions() {
+    CompilerOptions options = super.getOptions();
+    options.setLanguage(LanguageMode.ECMASCRIPT_NEXT);
+    options.setParseJsDocDocumentation(JsDocParsing.INCLUDE_DESCRIPTIONS_WITH_WHITESPACE);
+    options.setPreserveDetailedSourceInfo(true);
+    options.setPrettyPrint(true);
+    options.setPreserveTypeAnnotations(true);
+    options.setPreferSingleQuotes(true);
+    options.setEmitUseStrict(false);
+    return options;
+  }
+
+  @Test
+  public void testNoWarning() {
+    testNoWarning(
+        lines(
+            "/** @fileoverview foo */",
+            "",
+            "goog.provide('a');",
+            "goog.provide('b');",
+            "goog.provide('c');",
+            "",
+            "alert(1);"));
+  }
+
+  @Test
+  public void testNoWarning_noProvides() {
+    testNoWarning(
+        lines("/** @fileoverview foo */", "", "goog.module('m');", "", "goog.require('x');"));
+  }
+
+  @Test
+  public void testWarning() {
+    test(
+        srcs(
+            lines(
+                "/** @fileoverview foo */",
+                "",
+                "goog.provide('b');",
+                "goog.provide('a');",
+                "goog.provide('c');",
+                "",
+                "alert(1);")),
+        warning(PROVIDES_NOT_SORTED)
+            .withMessageContaining(
+                lines(
+                    "The correct order is:",
+                    "",
+                    "goog.provide('a');",
+                    "goog.provide('b');",
+                    "goog.provide('c');")));
+  }
+}