Create a basic annotation processor for validating SkylarkCallable uses at compile time.

RELNOTES: None.
PiperOrigin-RevId: 185432867

12 files changed, 427 insertions, 20 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 196e2a3d96..51d8726c24 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -55,6 +55,7 @@ filegroup(
         "//src/main/java/com/google/devtools/build/lib/skyframe/packages:srcs",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:srcs",
         "//src/main/java/com/google/devtools/build/lib/skylarkdebug/proto:srcs",
+        "//src/main/java/com/google/devtools/build/lib/skylarkinterface/processor:srcs",
         "//src/main/java/com/google/devtools/build/lib/ssd:srcs",
         "//src/main/java/com/google/devtools/build/lib/standalone:srcs",
         "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs:srcs",
@@ -286,9 +287,16 @@ java_library(
 
 java_library(
     name = "skylarkinterface",
+    exported_plugins = ["//src/main/java/com/google/devtools/build/lib/skylarkinterface/processor:annotation_preprocessor"],
+    exports = [":skylarkinterface_internal"],
+)
+
+java_library(
+    name = "skylarkinterface_internal",
     srcs = glob([
         "skylarkinterface/*.java",
     ]),
+    visibility = ["//src/main/java/com/google/devtools/build/lib/skylarkinterface/processor:__pkg__"],
     deps = [
         "//third_party:jsr305",
     ],
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/BUILD b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/BUILD
new file mode 100644
index 0000000000..c5a14392fa
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/BUILD
@@ -0,0 +1,25 @@
+# Description:
+#   A preprocessor for skylarkinterface annotations.
+package(default_visibility = [
+    "//src/main/java/com/google/devtools/build/lib:__pkg__",
+    "//src/test/java/com/google/devtools/build/lib/skylarkinterface/processor:__pkg__",
+])
+
+licenses(["notice"])  # Apache 2.0
+
+filegroup(
+    name = "srcs",
+    srcs = glob(
+        ["**"],
+    ),
+)
+
+java_plugin(
+    name = "annotation_preprocessor",
+    srcs = glob(["*.java"]),
+    processor_class = "com.google.devtools.build.lib.skylarkinterface.processor.SkylarkCallableProcessor",
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib:skylarkinterface_internal",
+        "//third_party:guava",
+    ],
+)
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java
new file mode 100644
index 0000000000..cc98597932
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java
@@ -0,0 +1,95 @@
+// Copyright 2018 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.build.lib.skylarkinterface.processor;
+
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+import java.util.Set;
+import javax.annotation.processing.AbstractProcessor;
+import javax.annotation.processing.Messager;
+import javax.annotation.processing.ProcessingEnvironment;
+import javax.annotation.processing.RoundEnvironment;
+import javax.annotation.processing.SupportedAnnotationTypes;
+import javax.annotation.processing.SupportedSourceVersion;
+import javax.lang.model.SourceVersion;
+import javax.lang.model.element.Element;
+import javax.lang.model.element.ExecutableElement;
+import javax.lang.model.element.Modifier;
+import javax.lang.model.element.TypeElement;
+import javax.tools.Diagnostic;
+
+/**
+ * Annotation processor for {@link SkylarkCallable}.
+ *
+ * <p>Checks the following invariants about {@link SkylarkCallable}-annotated methods:
+ * <ul>
+ * <li>The method must be public.</li>
+ * <li>The number of method parameters much match the number of annotation-declared parameters.</li>
+ * <li>If structField=true, there must be zero arguments.</li>
+ * </ul>
+ *
+ * <p>These properties can be relied upon at runtime without additional checks.
+ */
+@SupportedAnnotationTypes({"com.google.devtools.build.lib.skylarkinterface.SkylarkCallable"})
+@SupportedSourceVersion(SourceVersion.RELEASE_8)
+public final class SkylarkCallableProcessor extends AbstractProcessor {
+
+  private Messager messager;
+
+  @Override
+  public synchronized void init(ProcessingEnvironment processingEnv) {
+    super.init(processingEnv);
+    messager = processingEnv.getMessager();
+  }
+
+  @Override
+  public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
+    for (Element element : roundEnv.getElementsAnnotatedWith(SkylarkCallable.class)) {
+      // Only methods are annotated with SkylarkCallable. This is verified by the
+      // @Target(ElementType.METHOD) annotation.
+      ExecutableElement methodElement = (ExecutableElement) element;
+      SkylarkCallable annotation = methodElement.getAnnotation(SkylarkCallable.class);
+
+      if (!methodElement.getModifiers().contains(Modifier.PUBLIC)) {
+        error(methodElement, "@SkylarkCallable annotated methods must be public.");
+      }
+      if (annotation.parameters().length > 0 || annotation.mandatoryPositionals() >= 0) {
+         int numDeclaredArgs = annotation.parameters().length
+             + Math.max(0, annotation.mandatoryPositionals());
+        if (methodElement.getParameters().size() != numDeclaredArgs) {
+          error(methodElement, String.format(
+              "@SkylarkCallable annotated method has %d parameters, but annotation declared %d.",
+              methodElement.getParameters().size(), numDeclaredArgs));
+        }
+      }
+      if (annotation.structField()) {
+        if (!methodElement.getParameters().isEmpty()) {
+          error(methodElement,
+              "@SkylarkCallable annotated methods with structField=true must have zero arguments.");
+        }
+      }
+    }
+    return true;
+  }
+
+  /**
+   * Prints an error message & fails the compilation.
+   *
+   * @param e The element which has caused the error. Can be null
+   * @param msg The error message
+   */
+  public void error(Element e, String msg) {
+    messager.printMessage(Diagnostic.Kind.ERROR, msg, e);
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 241789d268..5584dfdea0 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -101,26 +101,6 @@ public final class FuncallExpression extends Expression {
                     if (callable == null) {
                       continue;
                     }
-                    Preconditions.checkArgument(
-                        callable.parameters().length == 0 || !callable.structField(),
-                        "Method "
-                            + method
-                            + " was annotated with both structField and parameters.");
-                    if (callable.parameters().length > 0 || callable.mandatoryPositionals() >= 0) {
-                      int nbArgs =
-                          callable.parameters().length
-                              + Math.max(0, callable.mandatoryPositionals());
-                      Preconditions.checkArgument(
-                          nbArgs == method.getParameterTypes().length,
-                          "Method "
-                              + method
-                              + " was annotated for "
-                              + nbArgs
-                              + " arguments "
-                              + "but accept only "
-                              + method.getParameterTypes().length
-                              + " arguments.");
-                    }
                     String name = callable.name();
                     if (name.isEmpty()) {
                       name = StringUtilities.toPythonStyleFunctionName(method.getName());
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 538cb8fd2c..cf66e2fe07 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -58,6 +58,7 @@ filegroup(
         "//src/test/java/com/google/devtools/build/lib/skyframe/serialization:srcs",
         "//src/test/java/com/google/devtools/build/lib/skyframe:srcs",
         "//src/test/java/com/google/devtools/build/lib/skylark:srcs",
+        "//src/test/java/com/google/devtools/build/lib/skylarkinterface/processor:srcs",
     ],
     visibility = ["//src:__pkg__"],
 )
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/BUILD b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/BUILD
new file mode 100644
index 0000000000..d26c2d4c41
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/BUILD
@@ -0,0 +1,29 @@
+licenses(["notice"])  # Apache 2.0
+
+filegroup(
+    name = "srcs",
+    srcs = glob(
+        ["**"],
+    ),
+    visibility = [
+        "//src/test/java/com/google/devtools/build/lib:__pkg__",
+    ],
+)
+
+java_test(
+    name = "SkylarkCallableProcessorTest",
+    srcs = ["SkylarkCallableProcessorTest.java"],
+    resources = [":ProcessorTestFiles"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/skylarkinterface/processor:annotation_preprocessor",
+        "//third_party:compile_testing",
+        "//third_party:guava",
+        "//third_party:junit4",
+        "//third_party:truth",
+    ],
+)
+
+filegroup(
+    name = "ProcessorTestFiles",
+    srcs = glob(["testsources/*.java"]),
+)
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java
new file mode 100644
index 0000000000..3b2ce81bc3
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java
@@ -0,0 +1,85 @@
+// Copyright 2018 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.build.lib.skylarkinterface.processor;
+
+import static com.google.common.truth.Truth.assertAbout;
+import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource;
+
+import com.google.common.io.Resources;
+import com.google.testing.compile.JavaFileObjects;
+import javax.tools.JavaFileObject;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Unit tests for SkylarkCallableProcessor.
+ */
+@RunWith(JUnit4.class)
+public final class SkylarkCallableProcessorTest {
+
+  private static JavaFileObject getFile(String pathToFile) {
+    return JavaFileObjects.forResource(Resources.getResource(
+        SkylarkCallableProcessorTest.class, "testsources/" + pathToFile));
+  }
+
+  @Test
+  public void testGoldenCase() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("GoldenCase.java"))
+        .processedWith(new SkylarkCallableProcessor())
+        .compilesWithoutError();
+  }
+
+  @Test
+  public void testPrivateMethod() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("PrivateMethod.java"))
+        .processedWith(new SkylarkCallableProcessor())
+        .failsToCompile()
+        .withErrorContaining("@SkylarkCallable annotated methods must be public.");
+  }
+
+  @Test
+  public void testStructFieldWithArguments() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("StructFieldWithArguments.java"))
+        .processedWith(new SkylarkCallableProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            "@SkylarkCallable annotated methods with structField=true must have zero arguments.");
+  }
+
+  @Test
+  public void testArgumentMissing() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("ArgumentMissing.java"))
+        .processedWith(new SkylarkCallableProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            "@SkylarkCallable annotated method has 0 parameters, but annotation declared 1.");
+  }
+
+  @Test
+  public void testTooManyArguments() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("TooManyArguments.java"))
+        .processedWith(new SkylarkCallableProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            "@SkylarkCallable annotated method has 2 parameters, but annotation declared 1.");
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ArgumentMissing.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ArgumentMissing.java
new file mode 100644
index 0000000000..f9face4b0b
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ArgumentMissing.java
@@ -0,0 +1,35 @@
+// Copyright 2018 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.build.lib.analysis.skylarkinterface.processor.testsources;
+
+import com.google.devtools.build.lib.skylarkinterface.Param;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+
+/**
+ * Test case for a SkylarkCallable method which has no arguments when the annotation indicates it
+ * should.
+ */
+public class ArgumentMissing {
+
+  @SkylarkCallable(
+    name = "method_with_params",
+    doc = "",
+    parameters = {
+      @Param(name = "a_parameter", type = String.class, named = true),
+    })
+  public String methodWithParams() {
+    return "bunny";
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java
new file mode 100644
index 0000000000..7682e57ce6
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java
@@ -0,0 +1,58 @@
+// Copyright 2018 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.build.lib.analysis.skylarkinterface.processor.testsources;
+
+import com.google.devtools.build.lib.skylarkinterface.Param;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+
+/**
+ * Test source file verifying various proper uses of SkylarkCallable.
+ */
+public class GoldenCase {
+
+  @SkylarkCallable(
+    name = "struct_field_method",
+    doc = "",
+    structField = true)
+  public String structFieldMethod() {
+    return "foo";
+  }
+
+  @SkylarkCallable(
+    name = "zero_arg_method",
+    doc = "")
+  public Integer zeroArgMethod() {
+    return 0;
+  }
+
+  @SkylarkCallable(
+    name = "three_arg_method",
+    doc = "")
+  public String threeArgMethod(String one, Integer two, String three) {
+    return "bar";
+  }
+
+  @SkylarkCallable(
+    name = "three_arg_method_with_params",
+    doc = "",
+    parameters = {
+      @Param(name = "one", type = String.class, named = true),
+      @Param(name = "two", type = Integer.class, named = true),
+      @Param(name = "three", type = String.class, named = true),
+    })
+  public String threeArgMethodWithParams(String one, Integer two, String three) {
+    return "baz";
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/PrivateMethod.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/PrivateMethod.java
new file mode 100644
index 0000000000..b0ae76a524
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/PrivateMethod.java
@@ -0,0 +1,28 @@
+// Copyright 2018 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.build.lib.analysis.skylarkinterface.processor.testsources;
+
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+
+/**
+ * Test case which verifies a method annotated with SkylarkCallable cannot be private.
+ */
+public class PrivateMethod {
+
+  @SkylarkCallable(name = "private_method", doc = "A private method")
+  private String privateMethod() {
+    return "kitten";
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithArguments.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithArguments.java
new file mode 100644
index 0000000000..7f9a674ae8
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithArguments.java
@@ -0,0 +1,28 @@
+// Copyright 2018 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.build.lib.analysis.skylarkinterface.processor.testsources;
+
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+
+/**
+ * Test case which verifies a struct field method cannot have arguments.
+ */
+public class StructFieldWithArguments {
+
+  @SkylarkCallable(name = "struct_field_method", structField = true, doc = "A private method")
+  public String structFieldMethod(String foo) {
+    return "puppy";
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/TooManyArguments.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/TooManyArguments.java
new file mode 100644
index 0000000000..476099c353
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/TooManyArguments.java
@@ -0,0 +1,35 @@
+// Copyright 2018 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.build.lib.analysis.skylarkinterface.processor.testsources;
+
+import com.google.devtools.build.lib.skylarkinterface.Param;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+
+/**
+ * Test case for a SkylarkCallable method which has more arguments than are declared by the
+ * annotation.
+ */
+public class TooManyArguments {
+
+  @SkylarkCallable(
+    name = "method_with_too_many_arguments",
+    doc = "",
+    parameters = {
+      @Param(name = "parameter_one", type = String.class, named = true),
+    })
+  public String methodWithTooManyArguments(String parameterOne, String parameterTwo) {
+    return "dolphin";
+  }
+}