From d5ef2b4956619c44c9d17ac097857508e4d53b40 Mon Sep 17 00:00:00 2001 From: Carmi Grushko Date: Mon, 1 Feb 2016 18:39:42 +0000 Subject: Rollback of commit f941d56acfad5f8c819c81b494f806ea74ea7fd8. *** Reason for rollback *** Breaks many targets, see [] *** Original change description *** Reinstate mutable SkylarkDict Add annotation to optionMap invocation in SkylarkAttr, to make JDK 1.7 happy. Give the visible name "aspect" to class SkylarkAspect. -- MOS_MIGRATED_REVID=113543873 --- .../lib/skylark/SkylarkRuleClassFunctionsTest.java | 2 +- .../build/lib/skylark/SkylarkRuleContextTest.java | 4 ++-- .../SkylarkRuleImplementationFunctionsTest.java | 8 ++------ .../devtools/build/lib/syntax/EvalUtilsTest.java | 8 +++++--- .../devtools/build/lib/syntax/EvaluationTest.java | 2 +- .../build/lib/syntax/MethodLibraryTest.java | 8 ++++---- .../build/lib/syntax/SkylarkEvaluationTest.java | 5 ++--- .../devtools/build/lib/syntax/ValidationTest.java | 22 ++++++++-------------- 8 files changed, 25 insertions(+), 34 deletions(-) (limited to 'src/test') diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java index 6fedceee06..717a132003 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java @@ -199,7 +199,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { @Test public void testLabelListWithAspectsError() throws Exception { checkErrorContains( - "Illegal argument: expected type aspect for 'aspects' element but got type int instead", + "Expected a list of aspects for 'aspects'", "def _impl(target, ctx):", " pass", "my_aspect = aspect(implementation = _impl)", diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java index 98cfe8dea5..e46c55fddf 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.FileConfiguredTarget; @@ -32,7 +33,6 @@ import com.google.devtools.build.lib.rules.SkylarkRuleContext; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; import com.google.devtools.build.lib.rules.python.PythonSourcesProvider; import com.google.devtools.build.lib.skylark.util.SkylarkTestCase; -import com.google.devtools.build.lib.syntax.SkylarkDict; import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.testutil.TestConstants; @@ -559,7 +559,7 @@ public class SkylarkRuleContextTest extends SkylarkTestCase { public void testSkylarkRuleContextGetDefaultShellEnv() throws Exception { SkylarkRuleContext ruleContext = createRuleContext("//foo:foo"); Object result = evalRuleContextCode(ruleContext, "ruleContext.configuration.default_shell_env"); - assertThat(result).isInstanceOf(SkylarkDict.class); + assertThat(result).isInstanceOf(ImmutableMap.class); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java index 10c7a6c685..961a2cb5ae 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.skylark.util.SkylarkTestCase; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature.Param; import com.google.devtools.build.lib.syntax.BuiltinFunction; -import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.EvalUtils; import com.google.devtools.build.lib.syntax.Printer; @@ -70,8 +69,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase { mandatoryPositionals = {@Param(name = "mandatory", doc = "")}, optionalPositionals = {@Param(name = "optional", doc = "")}, mandatoryNamedOnly = {@Param(name = "mandatory_key", doc = "")}, - optionalNamedOnly = {@Param(name = "optional_key", doc = "", defaultValue = "'x'")}, - useEnvironment = true + optionalNamedOnly = {@Param(name = "optional_key", doc = "", defaultValue = "'x'")} ) private BuiltinFunction mockFunc; @@ -124,10 +122,8 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase { new BuiltinFunction("mock") { @SuppressWarnings("unused") public Object invoke( - Object mandatory, Object optional, Object mandatoryKey, Object optionalKey, - Environment env) { + Object mandatory, Object optional, Object mandatoryKey, Object optionalKey) { return EvalUtils.optionMap( - env, "mandatory", mandatory, "optional", 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 cd0129dd70..4989cbd900 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 @@ -26,6 +26,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.TreeMap; /** @@ -35,8 +37,8 @@ import java.util.TreeMap; @RunWith(JUnit4.class) public class EvalUtilsTest { - private static SkylarkDict makeDict() { - return SkylarkDict.of(null); + private static Map makeDict() { + return new LinkedHashMap<>(); } @Test @@ -77,7 +79,7 @@ public class EvalUtilsTest { map.put("test", 7); map.put(-1, 2); map.put("4", 6); - map.put(true, 1); + map.put(2.0, 1); map.put(Runtime.NONE, 0); int expected = 0; diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java index 4396608f66..7a1033838e 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java @@ -259,7 +259,7 @@ public class EvaluationTest extends EvaluationTestCase { final Map kwargs, FuncallExpression ast, Environment env) { - return SkylarkDict.copyOf(env, kwargs); + return kwargs; } }; diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java index 7bbe8eafa7..a894ddfe64 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java @@ -418,8 +418,8 @@ public class MethodLibraryTest extends EvaluationTestCase { public void testBuiltinFunctionErrorMessage() throws Exception { new BothModesTest() .testIfErrorContains( - "Method set.union(new_elements: Iterable) is not applicable for arguments (string): " - + "'new_elements' is string, but should be Iterable", + "Method set.union(newElements: Iterable) is not applicable for arguments (string): " + + "'newElements' is string, but should be Iterable", "set([]).union('a')") .testIfErrorContains( "Method string.startswith(sub: string, start: int, end: int or NoneType) is not " @@ -1347,8 +1347,8 @@ public class MethodLibraryTest extends EvaluationTestCase { new BothModesTest() .testIfErrorContains("insufficient arguments received by union", "set(['a']).union()") .testIfErrorContains( - "Method set.union(new_elements: Iterable) is not applicable for arguments (string): " - + "'new_elements' is string, but should be Iterable", + "Method set.union(newElements: Iterable) is not applicable for arguments (string): " + + "'newElements' is string, but should be Iterable", "set(['a']).union('b')"); } 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 7ee189cd9e..6aae5c1b78 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 @@ -132,7 +132,6 @@ public class SkylarkEvaluationTest extends EvaluationTest { } } - @SkylarkModule(name = "MockClassObject", doc = "", documented = false) static final class MockClassObject implements ClassObject { @Override public Object getValue(String name) { @@ -836,11 +835,11 @@ public class SkylarkEvaluationTest extends EvaluationTest { } @Test - public void testDictAssignmentAsLValueSideEffects() throws Exception { + public void testDictAssignmentAsLValueNoSideEffects() throws Exception { new SkylarkTest().setUp("def func(d):", " d['b'] = 2", "d = {'a' : 1}", - "func(d)").testLookup("d", SkylarkDict.of(null, "a", 1, "b", 2)); + "func(d)").testLookup("d", ImmutableMap.of("a", 1)); } @Test 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 c44e5c918b..4c6eb6ac63 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 @@ -284,7 +284,7 @@ public class ValidationTest extends EvaluationTestCase { @Test public void testParentWithSkylarkModule() throws Exception { - Class emptyTupleClass = Tuple.empty().getClass(); + Class emptyTupleClass = Tuple.EMPTY.getClass(); Class tupleClass = Tuple.of(1, "a", "b").getClass(); Class mutableListClass = new MutableList(Tuple.of(1, 2, 3), env).getClass(); @@ -311,7 +311,7 @@ public class ValidationTest extends EvaluationTestCase { @Test public void testSkylarkTypeEquivalence() throws Exception { // All subclasses of SkylarkList are made equivalent - Class emptyTupleClass = Tuple.empty().getClass(); + Class emptyTupleClass = Tuple.EMPTY.getClass(); Class tupleClass = Tuple.of(1, "a", "b").getClass(); Class mutableListClass = new MutableList(Tuple.of(1, 2, 3), env).getClass(); @@ -322,14 +322,8 @@ public class ValidationTest extends EvaluationTestCase { // Also for ClassObject assertThat(SkylarkType.of(ClassObject.SkylarkClassObject.class)).isEqualTo(SkylarkType.STRUCT); - try { - SkylarkType.of(ClassObject.class); - throw new Exception("foo"); - } catch (Exception e) { - assertThat(e.getMessage()).contains( - "interface com.google.devtools.build.lib.syntax.ClassObject " - + "is not allowed as a Skylark value"); - } + // TODO(bazel-team): fix that? + assertThat(SkylarkType.of(ClassObject.class)).isNotEqualTo(SkylarkType.STRUCT); // 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? @@ -348,17 +342,17 @@ public class ValidationTest extends EvaluationTestCase { assertThat(SkylarkType.LIST.includes(combo1)).isTrue(); SkylarkType union1 = - SkylarkType.Union.of(SkylarkType.DICT, SkylarkType.LIST, SkylarkType.STRUCT); - assertThat(union1.includes(SkylarkType.DICT)).isTrue(); + SkylarkType.Union.of(SkylarkType.MAP, SkylarkType.LIST, SkylarkType.STRUCT); + assertThat(union1.includes(SkylarkType.MAP)).isTrue(); assertThat(union1.includes(SkylarkType.STRUCT)).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.LIST, SkylarkType.MAP, SkylarkType.STRING, SkylarkType.INT); SkylarkType inter1 = SkylarkType.intersection(union1, union2); - assertThat(inter1.includes(SkylarkType.DICT)).isTrue(); + assertThat(inter1.includes(SkylarkType.MAP)).isTrue(); assertThat(inter1.includes(SkylarkType.LIST)).isTrue(); assertThat(inter1.includes(combo1)).isTrue(); assertThat(inter1.includes(SkylarkType.INT)).isFalse(); -- cgit v1.2.3