From 4cf3a3042bb5842acdbf35d7c775f78b24e84de1 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 16 Sep 2024 09:01:39 -0700 Subject: [PATCH] [7.5.0] Backport support for all attribute types in Starlark documentation extraction Previously, we only supported attribute types which could be defined for Starlark rules, with the result that a cherry-pick in the 7.x branch exposing a formerly native-only attribute type to Starlark broke Stardoc: https://github.com/bazelbuild/stardoc/issues/277 Instead of picking and choosing, let's support all attribute types, same as we already do in Bazel 8 and newer. Cherry-pick of a subset of changes in commit https://github.com/bazelbuild/bazel/commit/1ce4ab15fab18063d8291132ea84e71fa02d27c4 PiperOrigin-RevId: 675169119 Change-Id: I6541a5eb99d6257339032850d360d2da4bd5aeb4 --- .../build/lib/packages/Attribute.java | 15 +++++++-- .../ModuleInfoExtractor.java | 31 +++++++++++++++---- .../ModuleInfoExtractorTest.java | 6 ++++ 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index fefdf568595788..2c0b5d8cbf4df2 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -61,6 +61,7 @@ import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Starlark; +import net.starlark.java.eval.StarlarkList; import net.starlark.java.eval.StarlarkValue; import net.starlark.java.eval.Structure; @@ -1439,7 +1440,7 @@ private Object computeValue(EventHandler eventHandler, AttributeMap rule) if (!Starlark.isNullOrNone(value)) { // Some attribute values are not valid Starlark values: // visibility is an ImmutableList, for example. - attrValues.put(attr.getName(), Starlark.fromJava(value, /*mutability=*/ null)); + attrValues.put(attr.getName(), Starlark.fromJava(value, /* mutability= */ null)); } } } @@ -1732,7 +1733,6 @@ LabelLateBoundDefault fromTargetConfigurationWithRuleBasedDefault( + "configuration."); return new LabelLateBoundDefault<>(fragmentClass, defaultValueEvaluator, resolver); } - } /** A {@link LateBoundDefault} for a {@link List} of {@link Label} objects. */ @@ -2364,8 +2364,8 @@ public Attribute.Builder cloneBuilder() { * */ public static Object valueToStarlark(Object x) { - // Is x a non-empty string_list_dict? if (x instanceof Map) { + // Is x a non-empty string_list_dict? Map map = (Map) x; if (!map.isEmpty() && map.values().iterator().next() instanceof List) { // Recursively convert subelements. @@ -2375,6 +2375,15 @@ public static Object valueToStarlark(Object x) { } return dict.buildImmutable(); } + } else if (x instanceof Set) { + // Until Starlark gains a set data type, shallow-convert Java sets (e.g. DISTRIBUTION values) + // to Starlark lists. + Set set = (Set) x; + return StarlarkList.immutableCopyOf(set); + } else if (x instanceof TriState) { + // Convert TriState to integer (same as in query output and native.existing_rules()) + TriState triState = (TriState) x; + return triState.toInt(); } // For all other attribute values, shallow conversion is safe. diff --git a/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java b/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java index f52ae7afafa628..5c0ca0fb81169b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java +++ b/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java @@ -55,6 +55,7 @@ import java.util.function.Predicate; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Module; +import net.starlark.java.eval.Starlark.InvalidStarlarkValueException; import net.starlark.java.eval.StarlarkFunction; import net.starlark.java.eval.Structure; @@ -511,9 +512,11 @@ private static AttributeType getAttributeType(Attribute attribute, String where) Type type = attribute.getType(); if (type.equals(Type.INTEGER)) { return AttributeType.INT; - } else if (type.equals(BuildType.LABEL)) { + } else if (type.equals(BuildType.LABEL) + || type.equals(BuildType.NODEP_LABEL) + || type.equals(BuildType.GENQUERY_SCOPE_TYPE)) { return AttributeType.LABEL; - } else if (type.equals(Type.STRING)) { + } else if (type.equals(Type.STRING) || type.equals(Type.STRING_NO_INTERN)) { if (attribute.getPublicName().equals("name")) { return AttributeType.NAME; } else { @@ -523,12 +526,16 @@ private static AttributeType getAttributeType(Attribute attribute, String where) return AttributeType.STRING_LIST; } else if (type.equals(Type.INTEGER_LIST)) { return AttributeType.INT_LIST; - } else if (type.equals(BuildType.LABEL_LIST)) { + } else if (type.equals(BuildType.LABEL_LIST) + || type.equals(BuildType.NODEP_LABEL_LIST) + || type.equals(BuildType.GENQUERY_SCOPE_TYPE_LIST)) { return AttributeType.LABEL_LIST; } else if (type.equals(Type.BOOLEAN)) { return AttributeType.BOOLEAN; } else if (type.equals(BuildType.LABEL_KEYED_STRING_DICT)) { return AttributeType.LABEL_STRING_DICT; + } else if (type.equals(BuildType.LABEL_DICT_UNARY)) { + return AttributeType.LABEL_DICT_UNARY; } else if (type.equals(Type.STRING_DICT)) { return AttributeType.STRING_DICT; } else if (type.equals(Type.STRING_LIST_DICT)) { @@ -537,12 +544,16 @@ private static AttributeType getAttributeType(Attribute attribute, String where) return AttributeType.OUTPUT; } else if (type.equals(BuildType.OUTPUT_LIST)) { return AttributeType.OUTPUT_LIST; - } else if (type.equals(BuildType.LICENSE)) { + } else if (type.equals(BuildType.LICENSE) || type.equals(BuildType.DISTRIBUTIONS)) { // TODO(https://github.com/bazelbuild/bazel/issues/6420): deprecated, disabled in Bazel by // default, broken and with almost no remaining users, so we don't have an AttributeType for // it. Until this type is removed, following the example of legacy Stardoc, pretend it's a // list of strings. return AttributeType.STRING_LIST; + } else if (type.equals(BuildType.TRISTATE)) { + // Given that the native TRISTATE type is not exposed to Starlark attr API, let's treat it + // as an integer. + return AttributeType.INT; } throw new ExtractionException( @@ -569,8 +580,16 @@ private AttributeInfo buildAttributeInfo(Attribute attribute, String where) } if (!attribute.isMandatory()) { - Object defaultValue = Attribute.valueToStarlark(attribute.getDefaultValueUnchecked()); - builder.setDefaultValue(labelRenderer.reprWithoutLabelConstructor(defaultValue)); + try { + Object defaultValue = Attribute.valueToStarlark(attribute.getDefaultValueUnchecked()); + builder.setDefaultValue(labelRenderer.reprWithoutLabelConstructor(defaultValue)); + } catch (InvalidStarlarkValueException e) { + throw new ExtractionException( + String.format( + "in %s attribute %s: failed to convert default value to Starlark: %s", + where, attribute.getPublicName(), e.getMessage()), + e); + } } return builder.build(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java b/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java index 40deba3f9cbe26..c7547f5ce5184c 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java @@ -664,6 +664,7 @@ public void attributeTypes() throws Exception { " 'j': attr.string_list_dict(),", " 'k': attr.output(),", " 'l': attr.output_list(),", + " 'm': attr.string_keyed_label_dict(),", " }", ")"); ModuleInfo moduleInfo = getExtractor().extractFrom(module); @@ -731,6 +732,11 @@ public void attributeTypes() throws Exception { .setType(AttributeType.OUTPUT_LIST) .setDefaultValue("[]") .setNonconfigurable(true) + .build(), + AttributeInfo.newBuilder() + .setName("m") + .setType(AttributeType.LABEL_DICT_UNARY) + .setDefaultValue("{}") .build()); }