diff options
Diffstat (limited to 'src')
8 files changed, 115 insertions, 48 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index ab616e5c90..c02e014353 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -1408,8 +1408,12 @@ public class BuildConfiguration implements BuildConfigurationApi { repositoryName, outputDirName, directories, mainRepositoryName); } - /** Returns the bin directory for this build configuration. */ @Override + public ArtifactRoot getBinDir() { + return getBinDirectory(RepositoryName.MAIN); + } + + /** Returns the bin directory for this build configuration. */ public ArtifactRoot getBinDirectory() { return getBinDirectory(RepositoryName.MAIN); } @@ -1442,8 +1446,12 @@ public class BuildConfiguration implements BuildConfigurationApi { repositoryName, outputDirName, directories, mainRepositoryName); } - /** Returns the genfiles directory for this build configuration. */ @Override + public ArtifactRoot getGenfilesDir() { + return getGenfilesDirectory(RepositoryName.MAIN); + } + + /** Returns the genfiles directory for this build configuration. */ public ArtifactRoot getGenfilesDirectory() { return getGenfilesDirectory(RepositoryName.MAIN); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java index df2b386c4f..6d440191dd 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java @@ -690,7 +690,7 @@ public final class SkylarkRuleContext implements SkylarkRuleContextApi { } @Override - public Artifact newFile(String filename) throws EvalException { + public Artifact newFileFromFilename(String filename) throws EvalException { checkDeprecated("ctx.actions.declare_file", "ctx.new_file", null, skylarkSemantics); checkMutable("new_file"); return actionFactory.declareFile(filename, Runtime.NONE); @@ -698,13 +698,14 @@ public final class SkylarkRuleContext implements SkylarkRuleContextApi { // Kept for compatibility with old code. @Override - public Artifact newFile(FileRootApi root, String filename) throws EvalException { + public Artifact newFileFromRoot(FileRootApi root, String filename) throws EvalException { checkMutable("new_file"); return ruleContext.getPackageRelativeArtifact(filename, (ArtifactRoot) root); } @Override - public Artifact newFile(FileApi baseArtifact, String newBaseName) throws EvalException { + public Artifact newFileFromBaseFile(FileApi baseArtifact, String newBaseName) + throws EvalException { checkDeprecated("ctx.actions.declare_file", "ctx.new_file", null, skylarkSemantics); checkMutable("new_file"); return actionFactory.declareFile(newBaseName, baseArtifact); @@ -712,7 +713,7 @@ public final class SkylarkRuleContext implements SkylarkRuleContextApi { // Kept for compatibility with old code. @Override - public Artifact newFile(FileRootApi root, FileApi baseArtifact, String suffix) + public Artifact newFileFromRootAndBase(FileRootApi root, FileApi baseArtifact, String suffix) throws EvalException { checkMutable("new_file"); PathFragment original = ((Artifact) baseArtifact).getRootRelativePath(); diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/BuildConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/BuildConfigurationApi.java index ad73c80e88..c445eb9e13 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/BuildConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/BuildConfigurationApi.java @@ -31,11 +31,11 @@ public interface BuildConfigurationApi { @SkylarkCallable(name = "bin_dir", structField = true, documented = false) @Deprecated - public FileRootApi getBinDirectory(); + public FileRootApi getBinDir(); @SkylarkCallable(name = "genfiles_dir", structField = true, documented = false) @Deprecated - public FileRootApi getGenfilesDirectory(); + public FileRootApi getGenfilesDir(); @SkylarkCallable(name = "host_path_separator", structField = true, doc = "Returns the separator for PATH environment variable, which is ':' on Unix.") diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleContextApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleContextApi.java index 64b50ebdb5..ca23f57fc3 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleContextApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleContextApi.java @@ -315,10 +315,10 @@ public interface SkylarkRuleContextApi extends SkylarkValue { ) } ) - public FileApi newFile(String filename) throws EvalException; + public FileApi newFileFromFilename(String filename) throws EvalException; - @SkylarkCallable(documented = false) - public FileApi newFile(FileRootApi root, String filename) throws EvalException; + @SkylarkCallable(name = "new_file", documented = false) + public FileApi newFileFromRoot(FileRootApi root, String filename) throws EvalException; @SkylarkCallable( name = "new_file", @@ -338,10 +338,10 @@ public interface SkylarkRuleContextApi extends SkylarkValue { ) } ) - public FileApi newFile(FileApi baseArtifact, String newBaseName) throws EvalException; + public FileApi newFileFromBaseFile(FileApi baseArtifact, String newBaseName) throws EvalException; - @SkylarkCallable(documented = false) - public FileApi newFile(FileRootApi root, FileApi baseArtifact, String suffix) + @SkylarkCallable(name = "new_file", documented = false) + public FileApi newFileFromRootAndBase(FileRootApi root, FileApi baseArtifact, String suffix) throws EvalException; @SkylarkCallable( diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java index 7788b478d8..81cd6aec06 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java @@ -93,24 +93,18 @@ public class SkylarkInterfaceUtils { /** * Returns the {@link SkylarkCallable} annotation for the given method, if it exists, and - * null otherwise. The first annotation of an overridden version of the method that is found - * will be returned, starting with {@code classObj} and following its base classes and - * interfaces recursively. This skips any method annotated inside a class that is not - * marked {@link SkylarkModule} or is not a subclass of a class or interface marked - * {@link SkylarkModule}. + * null otherwise. + * + * <p>Note that the annotation may be defined on a supermethod, rather than directly on the given + * method. + * + * <p>{@code classObj} is the class on which the given method is defined. */ @Nullable public static SkylarkCallable getSkylarkCallable(Class<?> classObj, Method method) { - try { - Method superMethod = classObj.getMethod(method.getName(), method.getParameterTypes()); - boolean classAnnotatedForCallables = getParentWithSkylarkModule(classObj) != null - || hasSkylarkGlobalLibrary(classObj); - if (classAnnotatedForCallables - && superMethod.isAnnotationPresent(SkylarkCallable.class)) { - return superMethod.getAnnotation(SkylarkCallable.class); - } - } catch (NoSuchMethodException e) { - // The class might not have the specified method, so an exception is OK. + SkylarkCallable callable = getCallableOnClassMatchingSignature(classObj, method); + if (callable != null) { + return callable; } if (classObj.getSuperclass() != null) { SkylarkCallable annotation = getSkylarkCallable(classObj.getSuperclass(), method); @@ -135,4 +129,56 @@ public class SkylarkInterfaceUtils { public static SkylarkCallable getSkylarkCallable(Method method) { return getSkylarkCallable(method.getDeclaringClass(), method); } + + /** + * Returns the {@code SkylarkCallable} annotation corresponding to the given method of the given + * class, or null if there is no such annotation. + * + * <p>This method checks assignability instead of exact matches for purposes of generics. If + * Clazz has parameters BarT (extends BarInterface) and BazT (extends BazInterface), then + * foo(BarT, BazT) should match if the given method signature is foo(BarImpl, BazImpl). The + * signatures are in inexact match, but an "assignable" match. + */ + @Nullable + private static SkylarkCallable getCallableOnClassMatchingSignature( + Class<?> classObj, Method signatureToMatch) { + // TODO(b/79877079): This method validates several invariants of @SkylarkCallable. These + // invariants should be verified in annotation processor or in test, and left out of this + // method. + Method[] methods = classObj.getDeclaredMethods(); + Class<?>[] paramsToMatch = signatureToMatch.getParameterTypes(); + + SkylarkCallable callable = null; + + for (Method method : methods) { + if (signatureToMatch.getName().equals(method.getName()) + && method.isAnnotationPresent(SkylarkCallable.class)) { + Class<?>[] paramTypes = method.getParameterTypes(); + + if (paramTypes.length == paramsToMatch.length) { + for (int i = 0; i < paramTypes.length; i++) { + // This verifies assignability of the method signature to ensure this is not a + // coincidental overload. We verify assignability instead of matching exact parameter + // classes in order to match generic methods. + if (!paramTypes[i].isAssignableFrom(paramsToMatch[i])) { + throw new IllegalStateException( + String.format( + "Class %s has an incompatible overload of annotated method %s declared by %s", + classObj, signatureToMatch.getName(), signatureToMatch.getDeclaringClass())); + } + } + } + if (callable == null) { + callable = method.getAnnotation(SkylarkCallable.class); + } else { + throw new IllegalStateException( + String.format( + "Class %s has multiple overloaded methods named '%s' annotated " + + "with @SkylarkCallable", + classObj, signatureToMatch.getName())); + } + } + } + return callable; + } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java index 6f5c2ef751..9157de4b3c 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java @@ -377,7 +377,7 @@ public final class SkylarkDict<K, V> extends MutableMap<K, V> useLocation = true, useEnvironment = true ) - public Runtime.NoneType clear( + public Runtime.NoneType clearDict( Location loc, Environment env) throws EvalException { clear(loc, env.mutability()); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java index 396e06457d..670c4d2c21 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java @@ -441,7 +441,7 @@ public abstract class SkylarkList<E> extends BaseMutableList<E> useLocation = true, useEnvironment = true ) - public Runtime.NoneType remove(Object x, Location loc, Environment env) + public Runtime.NoneType removeObject(Object x, Location loc, Environment env) throws EvalException { for (int i = 0; i < size(); i++) { if (get(i).equals(x)) { 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 422ac033a5..adb8810b12 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 @@ -510,14 +510,35 @@ public class SkylarkEvaluationTest extends EvaluationTest { } } - @SkylarkModule(name = "MockMultipleMethodClass", doc = "") - static final class MockMultipleMethodClass { - @SuppressWarnings("unused") - @SkylarkCallable(documented = false) - public void method(Object o) {} - @SuppressWarnings("unused") - @SkylarkCallable(documented = false) - public void method(String i) {} + @SkylarkModule(name = "ParamterizedMock", doc = "") + static interface ParameterizedApi<ObjectT> { + @SkylarkCallable( + name = "method", + documented = false, + parameters = { + @Param(name = "foo", named = true, positional = true, type = Object.class), + } + ) + public ObjectT method(ObjectT o); + } + + static final class ParameterizedMock implements ParameterizedApi<String> { + @Override + public String method(String o) { + return o; + } + } + + // Verifies that a method implementation overriding a parameterized annotated interface method + // is still treated as skylark-callable. Concretely, method() below should be treated as + // callable even though its method signature isn't an *exact* match of the annotated method + // declaration, due to the interface's method declaration being generic. + @Test + public void testParameterizedMock() throws Exception { + new SkylarkTest() + .update("mock", new ParameterizedMock()) + .setUp("result = mock.method('bar')") + .testLookup("result", "bar"); } @Test @@ -1000,15 +1021,6 @@ public class SkylarkEvaluationTest extends EvaluationTest { } @Test - public void testJavaCallsMultipleMethod() throws Exception { - new SkylarkTest() - .update("mock", new MockMultipleMethodClass()) - .testIfExactError( - "type 'MockMultipleMethodClass' has multiple matches for function method(string)", - "s = mock.method('string')"); - } - - @Test public void testJavaCallWithKwargs() throws Exception { new SkylarkTest() .update("mock", new Mock()) |