Skylint: add flag to disable certain checkers.

RELNOTES: none
PiperOrigin-RevId: 169518230

2 files changed, 61 insertions, 21 deletions

diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Linter.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Linter.java
index da87f930fa..e6049762d4 100644
--- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Linter.java
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Linter.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.skylark.skylint;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
 import java.io.IOException;
@@ -21,7 +22,10 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map.Entry;
+import java.util.Set;
 
 /**
  * Main class of the linter library.
@@ -29,26 +33,36 @@ import java.util.List;
  * <p>Most users of the linter library should only need to use this class.
  */
 public class Linter {
-
-  /** List of all checkers that the linter runs. */
-  private static final Checker[] checkers = {
-    ControlFlowChecker::check,
-    DocstringChecker::check,
-    NamingConventionsChecker::check,
-    StatementWithoutEffectChecker::check,
-    UsageChecker::check
-  };
+  /** Map of all checks and their names. */
+  private static final ImmutableMap<String, Check> nameToCheck =
+      ImmutableMap.<String, Check>builder()
+          .put("control-flow", ControlFlowChecker::check)
+          .put("docstring", DocstringChecker::check)
+          .put("naming", NamingConventionsChecker::check)
+          .put("no-effect", StatementWithoutEffectChecker::check)
+          .put("usage", UsageChecker::check)
+          .build();
 
   /** Function to read files (can be changed for testing). */
   private FileContentsReader fileReader = Files::readAllBytes;
 
+  private final Set<String> disabledChecks = new LinkedHashSet<>();
+
   public Linter setFileContentsReader(FileContentsReader reader) {
     this.fileReader = reader;
     return this;
   }
 
+  public Linter disable(String checkName) {
+    if (!nameToCheck.containsKey(checkName)) {
+      throw new IllegalArgumentException("Unknown check '" + checkName + "' cannot be disabled.");
+    }
+    disabledChecks.add(checkName);
+    return this;
+  }
+
   /**
-   * Runs all checkers on the given file.
+   * Runs all checks on the given file.
    *
    * @param path path of the file
    * @return list of issues found in that file
@@ -64,8 +78,11 @@ public class Linter {
               }
             },
             content);
-    for (Checker checker : checkers) {
-      issues.addAll(checker.check(ast));
+    for (Entry<String, Check> entry : nameToCheck.entrySet()) {
+      if (disabledChecks.contains(entry.getKey())) {
+        continue;
+      }
+      issues.addAll(entry.getValue().check(ast));
     }
     issues.sort(Issue::compare);
     return issues;
@@ -81,9 +98,9 @@ public class Linter {
     byte[] read(Path path) throws IOException;
   }
 
-  /** A checker analyzes an AST and produces a list of issues. */
+  /** Allows to invoke a check. */
   @FunctionalInterface
-  public interface Checker {
+  public interface Check {
     List<Issue> check(BuildFileAST ast);
   }
 }
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint.java
index 98a5025188..61b0a6210b 100644
--- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint.java
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint.java
@@ -17,19 +17,42 @@ package com.google.devtools.skylark.skylint;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
 import java.util.List;
 
 /** The main class for the skylint binary. */
 public class Skylint {
   public static void main(String[] args) throws IOException {
-    Path path = Paths.get(args[0]).toAbsolutePath();
-    List<Issue> issues = new Linter().lint(path);
-    if (!issues.isEmpty()) {
-      System.out.println(path);
-      for (Issue issue : issues) {
-        System.out.println(issue);
+    List<Path> paths = new ArrayList<>();
+    List<String> disabledChecks = new ArrayList<>();
+    for (String arg : args) {
+      if (arg.startsWith("--disable=")) {
+        String[] checks = arg.substring("--disable=".length()).split(",");
+        for (String check : checks) {
+          if (check.isEmpty()) {
+            continue;
+          }
+          disabledChecks.add(check);
+        }
+      } else {
+        paths.add(Paths.get(arg));
       }
-      System.exit(1);
     }
+    boolean issuesFound = false;
+    Linter linter = new Linter();
+    for (String checkerName : disabledChecks) {
+      linter.disable(checkerName);
+    }
+    for (Path path : paths) {
+      List<Issue> issues = linter.lint(path);
+      if (!issues.isEmpty()) {
+        issuesFound = true;
+        System.out.println(path);
+        for (Issue issue : issues) {
+          System.out.println(issue);
+        }
+      }
+    }
+    System.exit(issuesFound ? 1 : 0);
   }
 }