diff options
author | Francois-Rene Rideau <tunes@google.com> | 2015-04-17 15:31:59 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2015-04-17 15:43:19 +0000 |
commit | 76023b9180d7a18defc526126a943fab684971dc (patch) | |
tree | 0b65fe1803ebfb3eeff439a2d354987889044bd7 /src/main | |
parent | a8628bfffa72994759e4ac8bb45f317c4f33c05d (diff) |
More skylark function cleanups
--
MOS_MIGRATED_REVID=91407816
Diffstat (limited to 'src/main')
6 files changed, 103 insertions, 98 deletions
diff --git a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java index 4c9743c7c7..d27e71da40 100644 --- a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java +++ b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java @@ -19,6 +19,7 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.io.Files; import com.google.devtools.build.docgen.SkylarkJavaInterfaceExplorer.SkylarkBuiltinMethod; import com.google.devtools.build.docgen.SkylarkJavaInterfaceExplorer.SkylarkJavaMethod; import com.google.devtools.build.docgen.SkylarkJavaInterfaceExplorer.SkylarkModuleDoc; @@ -36,10 +37,10 @@ import com.google.devtools.build.lib.syntax.SkylarkModule; import java.io.BufferedWriter; import java.io.File; -import java.io.FileWriter; import java.io.IOException; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -70,7 +71,8 @@ public class SkylarkDocumentationProcessor { public void generateDocumentation(String outputPath) throws IOException, BuildEncyclopediaDocException { File skylarkDocPath = new File(outputPath); - try (BufferedWriter bw = new BufferedWriter(new FileWriter(skylarkDocPath))) { + try (BufferedWriter bw = new BufferedWriter( + Files.newWriter(skylarkDocPath, StandardCharsets.UTF_8))) { if (USE_TEMPLATE) { bw.write(SourceFileReader.readTemplateContents(DocgenConsts.SKYLARK_BODY_TEMPLATE, ImmutableMap.<String, String>of( @@ -130,7 +132,7 @@ public class SkylarkDocumentationProcessor { .append(annotation.doc()) .append("\n"); sb.append("<ul>"); - // Sort Java and SkylarkBuiltin methods together. The map key is only used for sorting. + // Sort Java and Skylark builtin methods together. The map key is only used for sorting. TreeMap<String, Object> methodMap = new TreeMap<>(); for (SkylarkJavaMethod method : module.getJavaMethods()) { methodMap.put(method.name + method.method.getParameterTypes().length, method); @@ -291,13 +293,13 @@ public class SkylarkDocumentationProcessor { ? " (" + getTypeAnchor(param.type()) + ")" : " (" + getTypeAnchor(param.type(), param.generic1()) + ")"); sb.append(String.format("\t<li id=\"modules.%s.%s.%s\"><code>%s%s</code>: ", - moduleId, - methodName, - param.name(), - param.name(), - paramType)) - .append(param.doc()) - .append("\n\t</li>\n"); + moduleId, + methodName, + param.name(), + param.name(), + paramType)) + .append(param.doc()) + .append("\n\t</li>\n"); } sb.append("</ul>\n"); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java index 7104ead7c2..84775f4caa 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java @@ -68,7 +68,7 @@ public class MethodLibrary { // Convert string index in the same way Python does. // If index is negative, starts from the end. // If index is outside bounds, it is restricted to the valid range. - private static int getClampedIndex(int index, int length) { + private static int clampIndex(int index, int length) { if (index < 0) { index += length; } @@ -77,14 +77,22 @@ public class MethodLibrary { // Emulate Python substring function // It converts out of range indices, and never fails - private static String getPythonSubstring(String str, int start, int end) { - start = getClampedIndex(start, str.length()); - end = getClampedIndex(end, str.length()); - if (start > end) { - return ""; + private static String pythonSubstring(String str, int start, Object end, String msg) + throws ConversionException { + if (start == 0 && EvalUtils.isNullOrNone(end)) { + return str; + } + start = clampIndex(start, str.length()); + int stop; + if (EvalUtils.isNullOrNone(end)) { + stop = str.length(); } else { - return str.substring(start, end); + stop = clampIndex(Type.INTEGER.convert(end, msg), str.length()); } + if (start >= stop) { + return ""; + } + return str.substring(start, stop); } public static int getListIndex(Object key, int listSize, FuncallExpression ast) @@ -193,13 +201,25 @@ public class MethodLibrary { int maxsplit = args[2] != null ? Type.INTEGER.convert(args[2], "'split' argument") + 1 // last is remainder : -1; - String[] ss = Pattern.compile(sep, Pattern.LITERAL).split(thiz, - maxsplit); + String[] ss = Pattern.compile(sep, Pattern.LITERAL).split(thiz, maxsplit); List<String> result = java.util.Arrays.asList(ss); return env.isSkylarkEnabled() ? SkylarkList.list(result, String.class) : result; } }; + /** + * Common implementation for find, rfind, index, rindex. + * @param forward true if we want to return the last matching index. + */ + private static int stringFind(boolean forward, + String self, String sub, int start, Object end, String msg) + throws ConversionException { + String substr = pythonSubstring(self, start, end, msg); + int subpos = forward ? substr.indexOf(sub) : substr.lastIndexOf(sub); + start = clampIndex(start, self.length()); + return subpos < 0 ? subpos : subpos + start; + } + // Common implementation for find, rfind, index, rindex. // forward is true iff we want to return the last matching index. private static int stringFind(String functionName, boolean forward, Object[] args) @@ -207,17 +227,10 @@ public class MethodLibrary { String thiz = Type.STRING.convert(args[0], functionName + " operand"); String sub = Type.STRING.convert(args[1], functionName + " argument"); int start = 0; - if (args[2] != null) { + if (!EvalUtils.isNullOrNone(args[2])) { start = Type.INTEGER.convert(args[2], functionName + " argument"); } - int end = thiz.length(); - if (args[3] != null) { - end = Type.INTEGER.convert(args[3], functionName + " argument"); - } - String substr = getPythonSubstring(thiz, start, end); - int subpos = forward ? substr.indexOf(sub) : substr.lastIndexOf(sub); - start = getClampedIndex(start, thiz.length()); - return subpos < 0 ? subpos : subpos + start; + return stringFind(forward, thiz, sub, start, args[3], functionName + " argument"); } @SkylarkBuiltin(name = "rfind", objectType = StringModule.class, returnType = Integer.class, @@ -275,7 +288,8 @@ public class MethodLibrary { int res = stringFind("rindex", false, args); if (res < 0) { throw new EvalException(ast.getLocation(), - "substring '" + args[1] + "' not found in '" + args[0] + "'"); + "substring " + EvalUtils.prettyPrintValue(args[1]) + + " not found in " + EvalUtils.prettyPrintValue(args[0])); } return res; } @@ -299,7 +313,8 @@ public class MethodLibrary { int res = stringFind("index", true, args); if (res < 0) { throw new EvalException(ast.getLocation(), - "substring '" + args[1] + "' not found in '" + args[0] + "'"); + "substring " + EvalUtils.prettyPrintValue(args[1]) + + " not found in " + EvalUtils.prettyPrintValue(args[0])); } return res; } @@ -325,11 +340,7 @@ public class MethodLibrary { if (args[2] != null) { start = Type.INTEGER.convert(args[2], "'count' argument"); } - int end = thiz.length(); - if (args[3] != null) { - end = Type.INTEGER.convert(args[3], "'count' argument"); - } - String str = getPythonSubstring(thiz, start, end); + String str = pythonSubstring(thiz, start, args[3], "'end' argument to 'count'"); if (sub.isEmpty()) { return str.length() + 1; } @@ -363,12 +374,8 @@ public class MethodLibrary { if (args[2] != null) { start = Type.INTEGER.convert(args[2], "'endswith' argument"); } - int end = thiz.length(); - if (args[3] != null) { - end = Type.INTEGER.convert(args[3], ""); - } - - return getPythonSubstring(thiz, start, end).endsWith(sub); + return pythonSubstring(thiz, start, args[3], "'end' argument to 'endswith'") + .endsWith(sub); } }; @@ -391,11 +398,8 @@ public class MethodLibrary { if (args[2] != null) { start = Type.INTEGER.convert(args[2], "'startswith' argument"); } - int end = thiz.length(); - if (args[3] != null) { - end = Type.INTEGER.convert(args[3], "'startswith' argument"); - } - return getPythonSubstring(thiz, start, end).startsWith(sub); + return pythonSubstring(thiz, start, args[3], "'end' argument to 'startswith'") + .startsWith(sub); } }; @@ -427,13 +431,13 @@ public class MethodLibrary { // Substring if (args[0] instanceof String) { String thiz = Type.STRING.convert(args[0], "substring operand"); - return getPythonSubstring(thiz, left, right); + return pythonSubstring(thiz, left, right, ""); } // List slice List<Object> list = Type.OBJECT_LIST.convert(args[0], "list operand"); - left = getClampedIndex(left, list.size()); - right = getClampedIndex(right, list.size()); + left = clampIndex(left, list.size()); + right = clampIndex(right, list.size()); List<Object> result = Lists.newArrayList(); for (int i = left; i < right; i++) { @@ -502,7 +506,8 @@ public class MethodLibrary { if (collectionCandidate instanceof Map<?, ?>) { Map<?, ?> dictionary = (Map<?, ?>) collectionCandidate; if (!dictionary.containsKey(key)) { - throw new EvalException(ast.getLocation(), "Key '" + key + "' not found in dictionary"); + throw new EvalException(ast.getLocation(), + "Key " + EvalUtils.prettyPrintValue(key) + " not found in dictionary"); } return dictionary.get(key); } else if (collectionCandidate instanceof List<?>) { @@ -578,6 +583,8 @@ public class MethodLibrary { + "</pre>\n") private static Function keys = new NoArgFunction("keys") { @Override + @SuppressWarnings("unchecked") // Skylark will only call this on a dict; and + // allowed keys are all Comparable... if not mutually, it's OK to get a runtime exception. public Object call(Object self, FuncallExpression ast, Environment env) throws EvalException, InterruptedException { Map<Comparable<?>, Object> dict = (Map<Comparable<?>, Object>) self; @@ -700,7 +707,8 @@ public class MethodLibrary { try { return Integer.parseInt(str); } catch (NumberFormatException e) { - throw new EvalException(ast.getLocation(), "invalid literal for int(): " + str); + throw new EvalException(ast.getLocation(), + "invalid literal for int(): " + EvalUtils.prettyPrintValue(str)); } } }; diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java index e75ebb25e2..7ab0cee813 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java @@ -47,10 +47,13 @@ public class SkylarkModules { * modules. They are also registered with the {@link ValidationEnvironment} and the * {@link SkylarkEnvironment}. Note that only {@link SkylarkFunction}s are handled properly. */ + // TODO(bazel-team): find a more general, more automated way of registering classes and building + // initial environments. And don't give syntax.Environment and packages.MethodLibrary a special + // treatment, have them use the same registration mechanism as other classes currently below. public static final ImmutableList<Class<?>> MODULES = ImmutableList.of( - SkylarkNativeModule.class, SkylarkAttr.class, SkylarkCommandLine.class, + SkylarkNativeModule.class, SkylarkRuleClassFunctions.class, SkylarkRuleImplementationFunctions.class); @@ -140,7 +143,7 @@ public class SkylarkModules { } /** - * Collects the SkylarkFunctions from the fields of the class of the object parameter + * Collects the Functions from the fields of the class of the object parameter * and adds them into the builder. */ private static void collectSkylarkFunctionsAndObjectsFromFields(Class<?> type, @@ -148,18 +151,19 @@ public class SkylarkModules { try { for (Field field : type.getDeclaredFields()) { if (field.isAnnotationPresent(SkylarkBuiltin.class)) { - // Fields in Skylark modules are sometimes private. Nevertheless they have to - // be annotated with SkylarkBuiltin. + // Fields in Skylark modules are sometimes private. + // Nevertheless they have to be annotated with SkylarkBuiltin. field.setAccessible(true); SkylarkBuiltin annotation = field.getAnnotation(SkylarkBuiltin.class); + Object value = field.get(null); if (SkylarkFunction.class.isAssignableFrom(field.getType())) { - SkylarkFunction function = (SkylarkFunction) field.get(null); + SkylarkFunction function = (SkylarkFunction) value; if (!function.isConfigured()) { function.configure(annotation); } functions.add(function); } else { - objects.put(annotation.name(), field.get(null)); + objects.put(annotation.name(), value); } } } @@ -170,7 +174,7 @@ public class SkylarkModules { } /** - * Collects the SkylarkFunctions from the fields of the class of the object parameter + * Collects the Functions from the fields of the class of the object parameter * and adds their class and their corresponding return value to the builder. */ private static void collectSkylarkTypesFromFields(Class<?> classObject, Set<String> builtIn) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java index 071185ccba..9f5b6be371 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java @@ -212,7 +212,8 @@ public class SkylarkRuleClassFunctions { private static final SkylarkFunction rule = new SkylarkFunction("rule") { @Override - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "unchecked"}) // castMap produces + // an Attribute.Builder instead of a Attribute.Builder<?> but it's OK. public Object call(Map<String, Object> arguments, FuncallExpression ast, Environment funcallEnv) throws EvalException, ConversionException { final Location loc = ast.getLocation(); @@ -282,6 +283,8 @@ public class SkylarkRuleClassFunctions { this.type = type; } + @SuppressWarnings("unchecked") // the magic hidden $pkg_context variable is guaranteed + // to be a PackageContext @Override public Object call(List<Object> args, Map<String, Object> kwargs, FuncallExpression ast, Environment env) throws EvalException, InterruptedException { @@ -363,9 +366,9 @@ public class SkylarkRuleClassFunctions { @Override public Object call(Map<String, Object> arguments, Location loc) throws EvalException, ConversionException { - ClassObject object = (ClassObject) arguments.get("self"); + ClassObject self = (ClassObject) arguments.get("self"); StringBuilder sb = new StringBuilder(); - printTextMessage(object, sb, 0, loc); + printTextMessage(self, sb, 0, loc); return sb.toString(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java index 034bf3b50a..c8448930b2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java @@ -16,10 +16,10 @@ package com.google.devtools.build.lib.skyframe; import static com.google.devtools.build.lib.syntax.Environment.NONE; -import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.LabelValidator; import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.ExternalPackage.Binding; import com.google.devtools.build.lib.packages.ExternalPackage.Builder; @@ -28,16 +28,15 @@ import com.google.devtools.build.lib.packages.Package.NameConflictException; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleFactory; -import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.packages.Type.ConversionException; -import com.google.devtools.build.lib.syntax.AbstractFunction; import com.google.devtools.build.lib.syntax.BuildFileAST; +import com.google.devtools.build.lib.syntax.BuiltinFunction; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.FuncallExpression; import com.google.devtools.build.lib.syntax.Function; +import com.google.devtools.build.lib.syntax.FunctionSignature; import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.lib.syntax.Label.SyntaxException; -import com.google.devtools.build.lib.syntax.MixedModeFunction; import com.google.devtools.build.lib.syntax.ParserInputSource; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -134,16 +133,16 @@ public class WorkspaceFileFunction implements SkyFunction { return null; } - private static Function newWorkspaceNameFunction(final Builder builder) { - List<String> params = ImmutableList.of("name"); - return new MixedModeFunction("workspace", params, 1, true) { - @Override - public Object call(Object[] namedArgs, FuncallExpression ast) throws EvalException, - ConversionException, InterruptedException { - String name = Type.STRING.convert(namedArgs[0], "'name' argument"); + // TODO(bazel-team): use @SkylarkSignature annotations on a BuiltinFunction.Factory + // for signature + documentation of this and other functions in this file. + private static BuiltinFunction newWorkspaceNameFunction(final Builder builder) { + return new BuiltinFunction("workspace", + FunctionSignature.namedOnly("name"), BuiltinFunction.USE_LOC) { + public Object invoke(String name, + Location loc) throws EvalException { String errorMessage = LabelValidator.validateTargetName(name); if (errorMessage != null) { - throw new EvalException(ast.getLocation(), errorMessage); + throw new EvalException(loc, errorMessage); } builder.setWorkspaceName(name); return NONE; @@ -151,24 +150,19 @@ public class WorkspaceFileFunction implements SkyFunction { }; } - private static Function newBindFunction(final Builder builder) { - List<String> params = ImmutableList.of("name", "actual"); - return new MixedModeFunction(BIND, params, 2, true) { - @Override - public Object call(Object[] namedArgs, FuncallExpression ast) - throws EvalException, ConversionException { - String name = Type.STRING.convert(namedArgs[0], "'name' argument"); - String actual = Type.STRING.convert(namedArgs[1], "'actual' argument"); - + private static BuiltinFunction newBindFunction(final Builder builder) { + return new BuiltinFunction(BIND, + FunctionSignature.namedOnly("name", "actual"), BuiltinFunction.USE_LOC) { + public Object invoke(String name, String actual, + Location loc) throws EvalException, ConversionException, InterruptedException { Label nameLabel = null; try { nameLabel = Label.parseAbsolute("//external:" + name); builder.addBinding( - nameLabel, new Binding(Label.parseRepositoryLabel(actual), ast.getLocation())); + nameLabel, new Binding(Label.parseRepositoryLabel(actual), loc)); } catch (SyntaxException e) { - throw new EvalException(ast.getLocation(), e.getMessage()); + throw new EvalException(loc, e.getMessage()); } - return NONE; } }; @@ -178,18 +172,12 @@ public class WorkspaceFileFunction implements SkyFunction { * Returns a function-value implementing the build rule "ruleClass" (e.g. cc_library) in the * specified package context. */ - private static Function newRuleFunction(final RuleFactory ruleFactory, + private static BuiltinFunction newRuleFunction(final RuleFactory ruleFactory, final Builder builder, final String ruleClassName) { - return new AbstractFunction(ruleClassName) { - @Override - public Object call(List<Object> args, Map<String, Object> kwargs, FuncallExpression ast, - com.google.devtools.build.lib.syntax.Environment env) - throws EvalException { - if (!args.isEmpty()) { - throw new EvalException(ast.getLocation(), - "build rules do not accept positional parameters"); - } - + return new BuiltinFunction(ruleClassName, + FunctionSignature.KWARGS, BuiltinFunction.USE_AST) { + public Object invoke(Map<String, Object> kwargs, + FuncallExpression ast) throws EvalException { try { RuleClass ruleClass = ruleFactory.getRuleClass(ruleClassName); builder.createAndAddRepositoryRule(ruleClass, kwargs, ast); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java index dce075ad4a..fb9e2fdabb 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java @@ -246,11 +246,11 @@ public class BuiltinFunction extends BaseFunction { String msg = String.format( "fun %s(%s), param %s, enforcedType: %s (%s); parameterType: %s", getName(), signature, signature.getSignature().getNames().get(i), - enforcedType, enforcedType.getClass(), parameterType); + enforcedType, enforcedType.getType(), parameterType); if (enforcedType instanceof SkylarkType.Simple || enforcedType instanceof SkylarkFunctionType) { Preconditions.checkArgument( - enforcedType == SkylarkType.of(parameterType), msg); + enforcedType.getType() == parameterType, msg); // No need to enforce Simple types on the Skylark side, the JVM will do it for us. enforcedArgumentTypes.set(i, null); } else if (enforcedType instanceof SkylarkType.Combination) { |