diff options
author | 2015-04-10 18:56:21 +0000 | |
---|---|---|
committer | 2015-04-13 11:46:41 +0000 | |
commit | 4a8da557b5878db1bcf66da32f48a0f97ca37e7a (patch) | |
tree | b3131bec195c9962ad96ddfa80b772b6946e96b4 /src/main/java/com/google/devtools/build | |
parent | e6a46b82e8e4e402fc70848f761d5486d2316d8f (diff) |
Skylark builtin function cleanups
Clean up related to Skylark builtin functions.
Replace "hidden" field of some annotations with a "documented" field
(with reversed semantics).
--
MOS_MIGRATED_REVID=90827020
Diffstat (limited to 'src/main/java/com/google/devtools/build')
11 files changed, 49 insertions, 39 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 d93411d313..4c9743c7c7 100644 --- a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java +++ b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java @@ -114,7 +114,7 @@ public class SkylarkDocumentationProcessor { SkylarkModuleDoc topLevelModule = modules.remove(getTopLevelModule().name()); generateModuleDoc(topLevelModule, sb); for (SkylarkModuleDoc module : modules.values()) { - if (!module.getAnnotation().hidden()) { + if (module.getAnnotation().documented()) { sb.append("<hr>"); generateModuleDoc(module, sb); } @@ -162,7 +162,7 @@ public class SkylarkDocumentationProcessor { private void generateBuiltinItemDoc( String moduleId, SkylarkBuiltinMethod method, StringBuilder sb) { SkylarkBuiltin annotation = method.annotation; - if (annotation.hidden()) { + if (!annotation.documented()) { return; } sb.append(String.format("<li><h3 id=\"modules.%s.%s\">%s</h3>\n", @@ -194,9 +194,13 @@ public class SkylarkDocumentationProcessor { private void generateDirectJavaMethodDoc(String objectName, String methodName, Method method, SkylarkCallable annotation, StringBuilder sb) { - if (annotation.hidden()) { + if (!annotation.documented()) { return; } + if (annotation.doc().isEmpty()) { + throw new RuntimeException(String.format( + "empty SkylarkCallable.doc() for object %s, method %s", objectName, methodName)); + } sb.append(String.format("<li><h3 id=\"modules.%s.%s\">%s</h3>\n%s\n", objectName, @@ -325,7 +329,7 @@ public class SkylarkDocumentationProcessor { for (SkylarkModuleDoc doc : collectBuiltinModules().values()) { if (doc.getAnnotation() == getTopLevelModule()) { for (Map.Entry<String, SkylarkBuiltinMethod> entry : doc.getBuiltinMethods().entrySet()) { - if (!entry.getValue().annotation.hidden()) { + if (entry.getValue().annotation.documented()) { modules.put(entry.getKey(), DocgenConsts.toCommandLineFormat(entry.getValue().annotation.doc())); } 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 fe5c4534fe..4bafd3f2f9 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 @@ -300,6 +300,7 @@ public final class BuildConfiguration implements Serializable { } } + /** TODO(bazel-team): document this */ public static class PluginOptionConverter implements Converter<Map.Entry<String, String>> { @Override public Map.Entry<String, String> convert(String input) throws OptionsParsingException { @@ -318,6 +319,7 @@ public final class BuildConfiguration implements Serializable { } } + /** TODO(bazel-team): document this */ public static class RunsPerTestConverter extends PerLabelOptions.PerLabelOptionsConverter { @Override public PerLabelOptions convert(String input) throws OptionsParsingException { @@ -1640,7 +1642,7 @@ public final class BuildConfiguration implements Serializable { /** * Returns a configuration fragment instances of the given class. */ - @SkylarkCallable(name = "fragment", hidden = true, + @SkylarkCallable(name = "fragment", documented = false, doc = "Returns a configuration fragment using the key.") public <T extends Fragment> T getFragment(Class<T> clazz) { return clazz.cast(fragments.get(clazz)); 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 23e54a1507..57d986596b 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 @@ -416,7 +416,7 @@ public class MethodLibrary { }; // slice operator - @SkylarkBuiltin(name = "$slice", hidden = true, + @SkylarkBuiltin(name = "$slice", documented = false, doc = "x[<code>start</code>:<code>end</code>] returns a slice or a list slice.") private static Function slice = new MixedModeFunction("$slice", ImmutableList.of("this", "start", "end"), 3, false) { @@ -446,7 +446,7 @@ public class MethodLibrary { }; // supported list methods - @SkylarkBuiltin(name = "append", hidden = true, + @SkylarkBuiltin(name = "append", documented = false, doc = "Adds an item to the end of the list.") private static Function append = new MixedModeFunction("append", ImmutableList.of("this", "x"), 2, false) { @@ -459,7 +459,7 @@ public class MethodLibrary { } }; - @SkylarkBuiltin(name = "extend", hidden = true, + @SkylarkBuiltin(name = "extend", documented = false, doc = "Adds all items to the end of the list.") private static Function extend = new MixedModeFunction("extend", ImmutableList.of("this", "x"), 2, false) { @@ -474,7 +474,7 @@ public class MethodLibrary { }; // dictionary access operator - @SkylarkBuiltin(name = "$index", hidden = true, + @SkylarkBuiltin(name = "$index", documented = false, doc = "Returns the nth element of a list or string, " + "or looks up a value in a dictionary.") private static Function indexOperator = new MixedModeFunction("$index", @@ -608,7 +608,7 @@ public class MethodLibrary { } // unary minus - @SkylarkBuiltin(name = "-", hidden = true, doc = "Unary minus operator.") + @SkylarkBuiltin(name = "-", documented = false, doc = "Unary minus operator.") private static Function minus = new MixedModeFunction("-", ImmutableList.of("this"), 1, false) { @Override public Object call(Object[] args, FuncallExpression ast) throws ConversionException { diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java index ec00b301ef..28eaf79401 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java @@ -407,7 +407,7 @@ public final class SkylarkRuleContext { return ruleContext.getLabel().toString(); } - @SkylarkCallable(doc = "Splits a shell command to a list of tokens.", hidden = true) + @SkylarkCallable(doc = "Splits a shell command to a list of tokens.", documented = false) public List<String> tokenize(String optionString) throws FuncallException { List<String> options = new ArrayList<>(); try { @@ -421,7 +421,7 @@ public final class SkylarkRuleContext { @SkylarkCallable(doc = "Expands all references to labels embedded within a string for all files using a mapping " + "from definition labels (i.e. the label in the output type attribute) to files. Deprecated.", - hidden = true) + documented = false) public String expand(@Nullable String expression, List<Artifact> artifacts, Label labelResolver) throws FuncallException { try { @@ -446,7 +446,7 @@ public final class SkylarkRuleContext { } // Kept for compatibility with old code. - @SkylarkCallable(hidden = true, doc = "") + @SkylarkCallable(documented = false) public Artifact newFile(Root root, String filename) { PathFragment fragment = ruleContext.getLabel().getPackageFragment(); for (String pathFragmentString : filename.split("/")) { @@ -465,19 +465,19 @@ public final class SkylarkRuleContext { } // Kept for compatibility with old code. - @SkylarkCallable(hidden = true, doc = "") + @SkylarkCallable(documented = false) public Artifact newFile(Root root, Artifact baseArtifact, String suffix) { PathFragment original = baseArtifact.getRootRelativePath(); PathFragment fragment = original.replaceName(original.getBaseName() + suffix); return ruleContext.getAnalysisEnvironment().getDerivedArtifact(fragment, root); } - @SkylarkCallable(doc = "", hidden = true) + @SkylarkCallable(documented = false) public NestedSet<Artifact> middleMan(String attribute) { return AnalysisUtils.getMiddlemanFor(ruleContext, attribute); } - @SkylarkCallable(doc = "", hidden = true) + @SkylarkCallable(documented = false) public boolean checkPlaceholders(String template, List<String> allowedPlaceholders) { List<String> actualPlaceHolders = new LinkedList<>(); Set<String> allowedPlaceholderSet = ImmutableSet.copyOf(allowedPlaceholders); @@ -527,14 +527,14 @@ public final class SkylarkRuleContext { return executableRunfilesMap.get(executable); } - @SkylarkCallable(name = "info_file", structField = true, hidden = true, + @SkylarkCallable(name = "info_file", structField = true, documented = false, doc = "Returns the file that is used to hold the non-volatile workspace status for the " + "current build request.") public Artifact getStableWorkspaceStatus() { return ruleContext.getAnalysisEnvironment().getStableWorkspaceStatusArtifact(); } - @SkylarkCallable(name = "version_file", structField = true, hidden = true, + @SkylarkCallable(name = "version_file", structField = true, documented = false, doc = "Returns the file that is used to hold the volatile workspace status for the " + "current build request.") public Artifact getVolatileWorkspaceStatus() { 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 c9f49ee390..dce075ad4a 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 @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.syntax; import com.google.common.base.Preconditions; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.Type.ConversionException; +import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -231,7 +232,9 @@ public class BuiltinFunction extends BaseFunction { int arguments = signature.getSignature().getShape().getArguments(); innerArgumentCount = arguments + (extraArgs == null ? 0 : extraArgs.length); Class<?>[] parameterTypes = invokeMethod.getParameterTypes(); - Preconditions.checkArgument(innerArgumentCount == parameterTypes.length, getName()); + Preconditions.checkArgument(innerArgumentCount == parameterTypes.length, + "bad argument count for %s: method has %s arguments, type list has %s", + getName(), innerArgumentCount, parameterTypes.length); // TODO(bazel-team): also grab the returnType from the annotations, // and check it against method return type @@ -240,10 +243,12 @@ public class BuiltinFunction extends BaseFunction { SkylarkType enforcedType = enforcedArgumentTypes.get(i); if (enforcedType != null) { Class<?> parameterType = parameterTypes[i]; - String msg = String.format("fun %s, param %s, enforcedType: %s (%s); parameterType: %s", - getName(), signature.getSignature().getNames().get(i), + 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); - if (enforcedType instanceof SkylarkType.Simple) { + if (enforcedType instanceof SkylarkType.Simple + || enforcedType instanceof SkylarkFunctionType) { Preconditions.checkArgument( enforcedType == SkylarkType.of(parameterType), msg); // No need to enforce Simple types on the Skylark side, the JVM will do it for us. diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java index 91b7304cc7..8a8ff9e3ef 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.util.LoggingUtil; import java.util.logging.Level; - /** * Exceptions thrown during evaluation of BUILD ASTs or Skylark extensions. * @@ -72,7 +71,7 @@ public class EvalException extends Exception { message = ""; } if (cause != null) { - message = message + (message.isEmpty() ? "" : "\n") + cause.getMessage(); + message = message + (message.isEmpty() ? "" : ": ") + cause.getMessage(); } if (message.isEmpty()) { LoggingUtil.logToRemote(Level.SEVERE, "Invalid EvalException", cause); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkBuiltin.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkBuiltin.java index a2f0d1b117..894a77a9ea 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkBuiltin.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkBuiltin.java @@ -34,7 +34,7 @@ public @interface SkylarkBuiltin { Param[] optionalParams() default {}; - boolean hidden() default false; + boolean documented() default true; Class<?> objectType() default Object.class; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallable.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallable.java index ae6987ff32..89d36f1a68 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallable.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallable.java @@ -26,9 +26,9 @@ import java.lang.annotation.Target; public @interface SkylarkCallable { String name() default ""; - String doc(); + String doc() default ""; // only allowed to be empty if documented() is false. - boolean hidden() default false; + boolean documented() default true; boolean structField() default false; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java index 96421b2d95..a9c0f9b1f4 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java @@ -30,7 +30,7 @@ public @interface SkylarkModule { String doc(); - boolean hidden() default false; + boolean documented() default true; boolean namespace() default false; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java index 0772c42cdd..8329a096af 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java @@ -46,7 +46,7 @@ public @interface SkylarkSignature { Param[] extraKeywords() default {}; - boolean undocumented() default false; + boolean documented() default true; Class<?> objectType() default Object.class; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java index 6d7b1dbb1f..bacb78a7dc 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java @@ -54,8 +54,8 @@ public class SkylarkSignatureProcessor { ? null : new HashMap<String, SkylarkType>(); HashMap<String, String> doc = new HashMap<>(); - boolean undocumented = annotation.undocumented(); - if (annotation.doc().isEmpty() && !undocumented) { + boolean documented = annotation.documented(); + if (annotation.doc().isEmpty() && documented) { throw new RuntimeException(String.format("function %s is undocumented", name)); } @@ -63,11 +63,11 @@ public class SkylarkSignatureProcessor { ? null : defaultValues.iterator(); try { for (Param param : annotation.mandatoryPositionals()) { - paramList.add(getParameter(name, param, enforcedTypes, doc, undocumented, + paramList.add(getParameter(name, param, enforcedTypes, doc, documented, /*mandatory=*/true, /*star=*/false, /*starStar=*/false, /*defaultValue=*/null)); } for (Param param : annotation.optionalPositionals()) { - paramList.add(getParameter(name, param, enforcedTypes, doc, undocumented, + paramList.add(getParameter(name, param, enforcedTypes, doc, documented, /*mandatory=*/false, /*star=*/false, /*starStar=*/false, /*defaultValue=*/getDefaultValue(param, defaultValuesIterator))); } @@ -79,22 +79,22 @@ public class SkylarkSignatureProcessor { Preconditions.checkArgument(annotation.extraPositionals().length == 1); starParam = annotation.extraPositionals()[0]; } - paramList.add(getParameter(name, starParam, enforcedTypes, doc, undocumented, + paramList.add(getParameter(name, starParam, enforcedTypes, doc, documented, /*mandatory=*/false, /*star=*/true, /*starStar=*/false, /*defaultValue=*/null)); } for (Param param : annotation.mandatoryNamedOnly()) { - paramList.add(getParameter(name, param, enforcedTypes, doc, undocumented, + paramList.add(getParameter(name, param, enforcedTypes, doc, documented, /*mandatory=*/true, /*star=*/false, /*starStar=*/false, /*defaultValue=*/null)); } for (Param param : annotation.optionalNamedOnly()) { - paramList.add(getParameter(name, param, enforcedTypes, doc, undocumented, + paramList.add(getParameter(name, param, enforcedTypes, doc, documented, /*mandatory=*/false, /*star=*/false, /*starStar=*/false, /*defaultValue=*/getDefaultValue(param, defaultValuesIterator))); } if (annotation.extraKeywords().length > 0) { Preconditions.checkArgument(annotation.extraKeywords().length == 1); paramList.add( - getParameter(name, annotation.extraKeywords()[0], enforcedTypes, doc, undocumented, + getParameter(name, annotation.extraKeywords()[0], enforcedTypes, doc, documented, /*mandatory=*/false, /*star=*/false, /*starStar=*/true, /*defaultValue=*/null)); } FunctionSignature.WithValues<Object, SkylarkType> signature = @@ -124,7 +124,7 @@ public class SkylarkSignatureProcessor { // Then the only per-parameter information needed is a documentation string. private static Parameter<Object, SkylarkType> getParameter( String name, Param param, Map<String, SkylarkType> enforcedTypes, - Map<String, String> paramDoc, boolean undocumented, + Map<String, String> paramDoc, boolean documented, boolean mandatory, boolean star, boolean starStar, @Nullable Object defaultValue) throws FunctionSignature.SignatureException { @@ -166,7 +166,7 @@ public class SkylarkSignatureProcessor { if (enforcedTypes != null) { enforcedTypes.put(param.name(), enforcedType); } - if (param.doc().isEmpty() && !undocumented) { + if (param.doc().isEmpty() && documented) { throw new RuntimeException(String.format("parameter %s is undocumented", name)); } if (paramDoc != null) { |