Refactor SyntaxTests to not rely on actions/analysis packages arbitrarily

In the process, clean up SkylarkType-related tests.

  - Factor test of EvalUtils.getSkylarkType logic from SkylarkEvaluationTest and ValidationTest to EvalUtilsTest.

  - Move test of EvalUtils.getSkylarkType's behavior on lists to SkylarkListTest.

  - Move other SkylarkType tests from ValidationTest to new SkylarkTypeTest and split them up a little.

  - Throughout, don't use Bazel types like Artifact as test subjects.

RELNOTES: None
PiperOrigin-RevId: 193303463

5 files changed, 126 insertions, 98 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
index 6430d0e037..a34eab24ba 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
@@ -22,6 +23,8 @@ import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.NativeProvider;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.syntax.EvalUtils.ComparisonException;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
@@ -152,4 +155,39 @@ public class EvalUtilsTest extends EvaluationTestCase {
       // expected
     }
   }
+
+  @SkylarkModule(
+      name = "ParentType",
+      doc = "A parent class annotated with @SkylarkModule."
+  )
+  private static class ParentClassWithSkylarkModule {}
+
+  private static class ChildClass extends ParentClassWithSkylarkModule {}
+
+  private static class SkylarkValueSubclass implements SkylarkValue {
+    @Override
+    public void repr(SkylarkPrinter printer) {
+      printer.append("SkylarkValueSubclass");
+    }
+  }
+
+  private static class NonSkylarkValueSubclass {}
+
+  @Test
+  public void testGetSkylarkType() {
+    assertThat(EvalUtils.getSkylarkType(ParentClassWithSkylarkModule.class))
+        .isEqualTo(ParentClassWithSkylarkModule.class);
+    assertThat(EvalUtils.getSkylarkType(ChildClass.class))
+        .isEqualTo(ParentClassWithSkylarkModule.class);
+    assertThat(EvalUtils.getSkylarkType(SkylarkValueSubclass.class))
+        .isEqualTo(SkylarkValueSubclass.class);
+
+    IllegalArgumentException expected =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> EvalUtils.getSkylarkType(NonSkylarkValueSubclass.class));
+    assertThat(expected).hasMessageThat().contains(
+        "class com.google.devtools.build.lib.syntax.EvalUtilsTest$NonSkylarkValueSubclass "
+            + "is not allowed as a Skylark value");
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 7657ced3bb..f302535288 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -19,11 +19,6 @@ import static java.util.stream.Collectors.joining;
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
-import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
-import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
@@ -1865,15 +1860,6 @@ public class SkylarkEvaluationTest extends EvaluationTest {
         "print(end='x', other='y')");
   }
 
-  @Test
-  public void testSkylarkTypes() {
-    assertThat(EvalUtils.getSkylarkType(FileConfiguredTarget.class))
-        .isEqualTo(TransitiveInfoCollection.class);
-    assertThat(EvalUtils.getSkylarkType(RuleConfiguredTarget.class))
-        .isEqualTo(TransitiveInfoCollection.class);
-    assertThat(EvalUtils.getSkylarkType(SpecialArtifact.class)).isEqualTo(Artifact.class);
-  }
-
   // Override tests in EvaluationTest incompatible with Skylark
 
   @SuppressWarnings("unchecked")
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
index 016c30f85a..7d427e66d0 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
@@ -316,4 +316,15 @@ public class SkylarkListTest extends EvaluationTestCase {
     assertThat(wrapped).containsExactly("hi", "added1", "added2").inOrder();
     assertThat(mutableList).containsExactly("hi", "added1", "added2").inOrder();
   }
+
+  @Test
+  public void testGetSkylarkType_GivesExpectedClassesForListsAndTuples() throws Exception {
+    Class<?> emptyTupleClass = Tuple.empty().getClass();
+    Class<?> tupleClass = Tuple.of(1, "a", "b").getClass();
+    Class<?> mutableListClass = MutableList.copyOf(env, Tuple.of(1, 2, 3)).getClass();
+
+    assertThat(EvalUtils.getSkylarkType(mutableListClass)).isEqualTo(MutableList.class);
+    assertThat(EvalUtils.getSkylarkType(emptyTupleClass)).isEqualTo(Tuple.class);
+    assertThat(EvalUtils.getSkylarkType(tupleClass)).isEqualTo(Tuple.class);
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkTypeTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkTypeTest.java
new file mode 100644
index 0000000000..c16f347e63
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkTypeTest.java
@@ -0,0 +1,77 @@
+// 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.syntax;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for Skylark's type system. */
+@RunWith(JUnit4.class)
+public final class SkylarkTypeTest {
+
+  @SkylarkModule(
+      name = "ParentType",
+      doc = "A parent class annotated with @SkylarkModule."
+  )
+  private static class ParentClassWithSkylarkModule {}
+
+  private static class ChildClass extends ParentClassWithSkylarkModule {}
+
+  @Test
+  public void simpleTypeOfChild_UsesTypeOfAncestorHavingSkylarkModule() {
+    assertThat(SkylarkType.of(ChildClass.class).getType())
+        .isEqualTo(ParentClassWithSkylarkModule.class);
+  }
+
+  @Test
+  public void typeHierarchy_Basic() throws Exception {
+    assertThat(SkylarkType.INT.includes(SkylarkType.BOTTOM)).isTrue();
+    assertThat(SkylarkType.BOTTOM.includes(SkylarkType.INT)).isFalse();
+    assertThat(SkylarkType.TOP.includes(SkylarkType.INT)).isTrue();
+  }
+
+  @Test
+  public void typeHierarchy_Combination() throws Exception {
+    SkylarkType combo = SkylarkType.Combination.of(SkylarkType.LIST, SkylarkType.INT);
+    assertThat(SkylarkType.LIST.includes(combo)).isTrue();
+  }
+
+  @Test
+  public void typeHierarchy_Union() throws Exception {
+    SkylarkType combo = SkylarkType.Combination.of(SkylarkType.LIST, SkylarkType.INT);
+    SkylarkType union = SkylarkType.Union.of(SkylarkType.DICT, SkylarkType.LIST);
+    assertThat(union.includes(SkylarkType.DICT)).isTrue();
+    assertThat(union.includes(combo)).isTrue();
+    assertThat(union.includes(SkylarkType.STRING)).isFalse();
+  }
+
+  @Test
+  public void typeHierarchy_Intersection() throws Exception {
+    SkylarkType combo = SkylarkType.Combination.of(SkylarkType.LIST, SkylarkType.INT);
+    SkylarkType union1 = SkylarkType.Union.of(SkylarkType.DICT, SkylarkType.LIST);
+    SkylarkType union2 =
+        SkylarkType.Union.of(
+            SkylarkType.LIST, SkylarkType.DICT, SkylarkType.STRING, SkylarkType.INT);
+    SkylarkType inter = SkylarkType.intersection(union1, union2);
+    assertThat(inter.includes(SkylarkType.DICT)).isTrue();
+    assertThat(inter.includes(SkylarkType.LIST)).isTrue();
+    assertThat(inter.includes(combo)).isTrue();
+    assertThat(inter.includes(SkylarkType.INT)).isFalse();
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
index 6e352ac15b..518e871391 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
@@ -15,13 +15,7 @@ package com.google.devtools.build.lib.syntax;
 
 import static com.google.common.truth.Truth.assertThat;
 
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
 import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.packages.Info;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
-import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
-import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -226,84 +220,6 @@ public class ValidationTest extends EvaluationTestCase {
     }
   }
 
-  @Test
-  public void testGetSkylarkType() throws Exception {
-    Class<?> emptyTupleClass = Tuple.empty().getClass();
-    Class<?> tupleClass = Tuple.of(1, "a", "b").getClass();
-    Class<?> mutableListClass = MutableList.copyOf(env, Tuple.of(1, 2, 3)).getClass();
-
-    assertThat(EvalUtils.getSkylarkType(mutableListClass)).isEqualTo(MutableList.class);
-    assertThat(MutableList.class.isAnnotationPresent(SkylarkModule.class)).isTrue();
-    assertThat(EvalUtils.getSkylarkType(emptyTupleClass)).isEqualTo(Tuple.class);
-    assertThat(EvalUtils.getSkylarkType(tupleClass)).isEqualTo(Tuple.class);
-
-    assertThat(EvalUtils.getSkylarkType(Info.class)).isEqualTo(Info.class);
-    try {
-      EvalUtils.getSkylarkType(ClassObject.class);
-      throw new Exception("Should have raised IllegalArgumentException exception");
-    } catch (IllegalArgumentException e) {
-      assertThat(e)
-          .hasMessageThat()
-          .contains(
-              "interface com.google.devtools.build.lib.syntax.ClassObject is not allowed "
-                  + "as a Skylark value");
-    }
-  }
-
-  @Test
-  public void testSkylarkTypeEquivalence() throws Exception {
-    Class<?> emptyTupleClass = Tuple.empty().getClass();
-    Class<?> tupleClass = Tuple.of(1, "a", "b").getClass();
-    Class<?> mutableListClass = MutableList.copyOf(env, Tuple.of(1, 2, 3)).getClass();
-
-    assertThat(SkylarkType.of(mutableListClass)).isEqualTo(SkylarkType.LIST);
-    assertThat(SkylarkType.of(emptyTupleClass)).isEqualTo(SkylarkType.TUPLE);
-    assertThat(SkylarkType.of(tupleClass)).isEqualTo(SkylarkType.TUPLE);
-    assertThat(SkylarkType.TUPLE).isNotEqualTo(SkylarkType.LIST);
-
-    try {
-      SkylarkType.of(ClassObject.class);
-      throw new Exception("foo");
-    } catch (Exception e) {
-      assertThat(e)
-          .hasMessageThat()
-          .contains(
-              "interface com.google.devtools.build.lib.syntax.ClassObject "
-                  + "is not allowed as a Skylark value");
-    }
-
-    // Also test for these bazel classes, to avoid some regression.
-    // TODO(bazel-team): move to some other place to remove dependency of syntax tests on Artifact?
-    assertThat(SkylarkType.of(Artifact.SpecialArtifact.class))
-        .isEqualTo(SkylarkType.of(Artifact.class));
-    assertThat(SkylarkType.of(RuleConfiguredTarget.class)).isNotEqualTo(SkylarkType.of(Info.class));
-  }
-
-  @Test
-  public void testSkylarkTypeInclusion() throws Exception {
-    assertThat(SkylarkType.INT.includes(SkylarkType.BOTTOM)).isTrue();
-    assertThat(SkylarkType.BOTTOM.includes(SkylarkType.INT)).isFalse();
-    assertThat(SkylarkType.TOP.includes(SkylarkType.INT)).isTrue();
-
-    SkylarkType combo1 = SkylarkType.Combination.of(SkylarkType.LIST, SkylarkType.INT);
-    assertThat(SkylarkType.LIST.includes(combo1)).isTrue();
-
-    SkylarkType union1 =
-        SkylarkType.Union.of(SkylarkType.DICT, SkylarkType.LIST);
-    assertThat(union1.includes(SkylarkType.DICT)).isTrue();
-    assertThat(union1.includes(combo1)).isTrue();
-    assertThat(union1.includes(SkylarkType.STRING)).isFalse();
-
-    SkylarkType union2 =
-        SkylarkType.Union.of(
-            SkylarkType.LIST, SkylarkType.DICT, SkylarkType.STRING, SkylarkType.INT);
-    SkylarkType inter1 = SkylarkType.intersection(union1, union2);
-    assertThat(inter1.includes(SkylarkType.DICT)).isTrue();
-    assertThat(inter1.includes(SkylarkType.LIST)).isTrue();
-    assertThat(inter1.includes(combo1)).isTrue();
-    assertThat(inter1.includes(SkylarkType.INT)).isFalse();
-  }
-
   private void parse(String... lines) {
     parseFile(lines);
     assertNoWarningsOrErrors();