Add linter check for deprecated ctx methods.

RELNOTES: None.
PiperOrigin-RevId: 186297559

3 files changed, 116 insertions, 1 deletions

diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/DeprecatedApiChecker.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DeprecatedApiChecker.java
new file mode 100644
index 0000000000..c4e849b1ce
--- /dev/null
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DeprecatedApiChecker.java
@@ -0,0 +1,66 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// 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.devtools.skylark.skylint;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.syntax.BuildFileAST;
+import com.google.devtools.build.lib.syntax.DotExpression;
+import com.google.devtools.build.lib.syntax.Identifier;
+import java.util.ArrayList;
+import java.util.List;
+
+/** Checks for operations that are deprecated */
+public class DeprecatedApiChecker extends AstVisitorWithNameResolution {
+  private static final String DEPRECATED_API = "deprecated-api";
+
+  private final List<Issue> issues = new ArrayList<>();
+
+  private DeprecatedApiChecker() {}
+
+  public static List<Issue> check(BuildFileAST ast) {
+    DeprecatedApiChecker checker = new DeprecatedApiChecker();
+    checker.visit(ast);
+    return checker.issues;
+  }
+
+  private static final ImmutableMap<String, String> deprecatedMethods =
+      ImmutableMap.<String, String>builder()
+          .put("ctx.action", "Use ctx.actions.run() or ctx.actions.run_shell().")
+          .put("ctx.default_provider", "Use DefaultInfo.")
+          .put("ctx.empty_action", "Use ctx.actions.do_nothing.")
+          .put("ctx.expand_make_variables", "Use ctx.var to access the variables.")
+          .put("ctx.file_action", "Use ctx.actions.write.")
+          .put("ctx.new_file", "Use ctx.actions.declare_file.")
+          .put("ctx.template_action", "Use ctx.actions.expand_template().")
+          .build();
+
+  @Override
+  public void visit(DotExpression node) {
+    super.visit(node);
+
+    if (!(node.getObject() instanceof Identifier)) {
+      return;
+    }
+
+    String name = ((Identifier) node.getObject()).getName() + "." + node.getField().getName();
+    if (deprecatedMethods.containsKey(name)) {
+      issues.add(
+          Issue.create(
+              DEPRECATED_API,
+              "This method is deprecated: " + deprecatedMethods.get(name),
+              node.getLocation()));
+    }
+  }
+}
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 3334dcae27..d537281f30 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
@@ -39,13 +39,14 @@ public class Linter {
   private static final ImmutableMap<String, Check> nameToCheck =
       ImmutableMap.<String, Check>builder()
           .put("bad-operation", BadOperationChecker::check)
+          .put("bad-recursive-glob", NativeRecursiveGlobChecker::check)
           .put("control-flow", ControlFlowChecker::check)
+          .put("deprecated-api", DeprecatedApiChecker::check)
           .put("docstring", DocstringChecker::check)
           .put("load", LoadStatementChecker::check)
           .put("naming", NamingConventionsChecker::check)
           .put("no-effect", StatementWithoutEffectChecker::check)
           .put("usage", UsageChecker::check)
-          .put("bad-recursive-glob", NativeRecursiveGlobChecker::check)
           .build();
   /** Map of all multi-file checks and their names. */
   private static final ImmutableMap<String, MultiFileCheck> nameToMultiFileCheck =
diff --git a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DeprecatedApiCheckerTest.java b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DeprecatedApiCheckerTest.java
new file mode 100644
index 0000000000..20af00dd12
--- /dev/null
+++ b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DeprecatedApiCheckerTest.java
@@ -0,0 +1,48 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// 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.devtools.skylark.skylint;
+
+import com.google.common.truth.Truth;
+import com.google.devtools.build.lib.syntax.BuildFileAST;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class DeprecatedApiCheckerTest {
+  private static List<Issue> findIssues(String... lines) {
+    String content = String.join("\n", lines);
+    BuildFileAST ast =
+        BuildFileAST.parseString(
+            event -> {
+              throw new IllegalArgumentException(event.getMessage());
+            },
+            content);
+    return DeprecatedApiChecker.check(ast);
+  }
+
+  @Test
+  public void deprecatedCtxMethods() {
+    Truth.assertThat(findIssues("ctx.action()").toString())
+        .contains("1:1-1:10: This method is deprecated");
+    Truth.assertThat(findIssues("ctx.empty_action").toString())
+        .contains("1:1-1:16: This method is deprecated");
+    Truth.assertThat(findIssues("ctx.default_provider()").toString())
+        .contains("1:1-1:20: This method is deprecated");
+
+    Truth.assertThat(findIssues("ctx.actions()")).isEmpty();
+  }
+}