skydoc: pay interest on fakebuildapi technical debt commit 1f9b8ede2a59c3ef67d352f68278ca45fe6f115d caused symbols defined by load to no longer be recorded in Module.globals, as the spec requires, but this broke the fragile hacks used by the FakeStarlarkAttrModuleApi to guess the name of a provider. This change causes the fakery to more faithfully simulate Bazel's "export" mechanism (itself a hack) so that providers receive a name when a top-level "name = provider()" statement is executed. Also, all built-in providers that use the FakeProviderApi class (instead of a specialized subclass) now explicitly specify their names at construction, so that the heuristics get the correct name for nested fields such as cc_common.ToolchainInfo. Nested fields that define specialized built-in providers still yield the wrong value, but there are no tests that cover it and no complaints heard so far, and this CL is penance enough already. Fixes bazelbuild/stardoc#33 BUG=175703093 PiperOrigin-RevId: 351360957
diff --git a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java index b56391b..18f6b3c 100644 --- a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java +++ b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.starlarkbuildapi.java.JavaNativeLibraryInfoApi; import com.google.devtools.build.lib.starlarkbuildapi.javascript.JsModuleInfoApi; import com.google.devtools.build.skydoc.fakebuildapi.FakeApi; +import com.google.devtools.build.skydoc.fakebuildapi.FakeProviderApi; import com.google.devtools.build.skydoc.fakebuildapi.FakeStructApi; import com.google.devtools.build.skydoc.rendering.AspectInfoWrapper; import com.google.devtools.build.skydoc.rendering.DocstringParseException; @@ -421,6 +422,14 @@ StarlarkThread thread = new StarlarkThread(mu, semantics); // We use the default print handler, which writes to stderr. thread.setLoader(imports::get); + // Fake Bazel's "export" hack, by which provider symbols + // bound to global variables take on the name of the global variable. + thread.setPostAssignHook( + (name, value) -> { + if (value instanceof FakeProviderApi) { + ((FakeProviderApi) value).setName(name); + } + }); Starlark.execFileProgram(prog, module, thread); } catch (EvalException ex) {
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeProviderApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeProviderApi.java index 19a8de6..a2f029d 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeProviderApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeProviderApi.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.skydoc.fakebuildapi; import com.google.devtools.build.lib.starlarkbuildapi.core.ProviderApi; +import javax.annotation.Nullable; import net.starlark.java.eval.Dict; import net.starlark.java.eval.Printer; import net.starlark.java.eval.StarlarkCallable; @@ -24,12 +25,11 @@ /** Fake callable implementation of {@link ProviderApi}. */ public class FakeProviderApi implements StarlarkCallable, ProviderApi { - /** - * Each fake is constructed with a unique name, controlled by this counter being the name suffix. - */ - private static int idCounter = 0; + private String name; - private final String name = "ProviderIdentifier" + idCounter++; + public FakeProviderApi(@Nullable String name) { + this.name = name; + } @Override public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwargs) { @@ -38,7 +38,14 @@ @Override public String getName() { - return name; + return name != null ? name : "Unexported Provider"; + } + + /** Called when provider is "exported" by a top-level assignment {@code name = provider()}. */ + public void setName(String name) { + if (this.name == null) { + this.name = name; + } } @Override
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java index c11cce8..fac65d5 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java
@@ -259,29 +259,27 @@ return providerNameGroup; } - /** - * Returns the name of {@code provider}. - * - * <p>{@code thread} contains a {@code Map<String, Object>} where the values are built-in objects - * or objects defined in the file and the keys are the names of these objects. If a {@code - * provider} is in the map, the name of the provider is set as the key of this object in {@code - * bindings}. If it is not in the map, the provider may be part of a module in the map and the - * name will be set to "Unknown Provider". - */ + // Returns the name of the provider using fragile heuristics. private static String providerName(ProviderApi provider, StarlarkThread thread) { Module bzl = Module.ofInnermostEnclosingStarlarkFunction(thread); - // user-defined provider? - for (Map.Entry<String, Object> e : bzl.getGlobals().entrySet()) { - if (provider.equals(e.getValue())) { - return e.getKey(); - } + + // Generic fake provider? (e.g. Starlark-defined, or trivial fake) + // Return name set at construction, or by "export" operation, if any. + if (provider instanceof FakeProviderApi) { + return ((FakeProviderApi) provider).getName(); // may be "Unexported Provider" } - // predeclared provider? (e.g. DefaultInfo) + + // Specialized fake provider? (e.g. DefaultInfo) + // Return name under which FakeApi.addPredeclared added it to environment. + // (This only works for top-level names such as DefaultInfo, but not for + // nested ones such as cc_common.XyzInfo, but that has always been broken; + // it is not part of the regression that is b/175703093.) for (Map.Entry<String, Object> e : bzl.getPredeclaredBindings().entrySet()) { if (provider.equals(e.getValue())) { return e.getKey(); } } + return "Unknown Provider"; } }
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java index 746dd6c..daad2b7 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java
@@ -86,7 +86,7 @@ @Override public ProviderApi provider(String doc, Object fields, StarlarkThread thread) throws EvalException { - FakeProviderApi fakeProvider = new FakeProviderApi(); + FakeProviderApi fakeProvider = new FakeProviderApi(null); // Field documentation will be output preserving the order in which the fields are listed. ImmutableList.Builder<ProviderFieldInfo> providerFieldInfos = ImmutableList.builder(); if (fields instanceof Sequence) {
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/apple/FakeAppleCommon.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/apple/FakeAppleCommon.java index 0168bc8..2036348 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/apple/FakeAppleCommon.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/apple/FakeAppleCommon.java
@@ -81,32 +81,32 @@ @Override public ProviderApi getXcodeVersionPropertiesConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("XcodeProperties"); } @Override public ProviderApi getXcodeVersionConfigConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("XcodeVersionConfig"); } @Override public ProviderApi getObjcProviderConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("objc"); } @Override public ProviderApi getAppleDynamicFrameworkConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("AppleDynamicFramework"); } @Override public ProviderApi getAppleDylibBinaryConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("AppleDylibBinary"); } @Override public ProviderApi getAppleExecutableBinaryConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("AppleExecutableBinary"); } @Override @@ -116,12 +116,12 @@ @Override public ProviderApi getAppleDebugOutputsConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("AppleDebugOutputs"); } @Override public ProviderApi getAppleLoadableBundleBinaryConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("AppleLoadableBundleBinary"); } @Override
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigStarlarkCommon.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigStarlarkCommon.java index 3cb7c5e..5efe193 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigStarlarkCommon.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigStarlarkCommon.java
@@ -25,6 +25,6 @@ @Override public ProviderApi getConfigFeatureFlagProviderConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("FeatureFlagInfo"); } }
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java index 6985e7b..3ed9b39 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java
@@ -73,7 +73,7 @@ @Override public ProviderApi getCcToolchainProvider() { - return new FakeProviderApi(); + return new FakeProviderApi("CcToolchainInfo"); } @Override
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/java/FakeJavaCommon.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/java/FakeJavaCommon.java index 412adf3..28150c2 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/java/FakeJavaCommon.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/java/FakeJavaCommon.java
@@ -42,7 +42,7 @@ @Override public ProviderApi getJavaProvider() { - return new FakeProviderApi(); + return new FakeProviderApi("JavaInfo"); } @Override @@ -118,12 +118,12 @@ @Override public ProviderApi getJavaToolchainProvider() { - return new FakeProviderApi(); + return new FakeProviderApi("JavaToolchain"); } @Override public ProviderApi getJavaRuntimeProvider() { - return new FakeProviderApi(); + return new FakeProviderApi("JavaRuntime"); } @Override @@ -134,7 +134,7 @@ @Override public ProviderApi getMessageBundleInfo() { - return new FakeProviderApi(); + return new FakeProviderApi("JavaMessageBundle"); } @Override
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/platform/FakePlatformCommon.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/platform/FakePlatformCommon.java index a65a6da..e48b164 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/platform/FakePlatformCommon.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/platform/FakePlatformCommon.java
@@ -25,26 +25,26 @@ @Override public ProviderApi getMakeVariableProvider() { - return new FakeProviderApi(); + return new FakeProviderApi("TemplateVariableInfo"); } @Override public ProviderApi getToolchainInfoConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("ToolchainInfo"); } @Override public ProviderApi getPlatformInfoConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("PlatformInfo"); } @Override public ProviderApi getConstraintSettingInfoConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("ConstraintSettingInfo"); } @Override public ProviderApi getConstraintValueInfoConstructor() { - return new FakeProviderApi(); + return new FakeProviderApi("ConstraintValueInfo"); } }
diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/providers_for_attributes_test/golden.txt b/src/test/java/com/google/devtools/build/skydoc/testdata/providers_for_attributes_test/golden.txt index c4ab2d1..25eaa79 100644 --- a/src/test/java/com/google/devtools/build/skydoc/testdata/providers_for_attributes_test/golden.txt +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/providers_for_attributes_test/golden.txt
@@ -44,7 +44,7 @@ this is the first attribute. </p> <p> - The dependencies of this attribute must provide: MyProviderInfo, PyInfo, Unknown Provider + The dependencies of this attribute must provide: MyProviderInfo, PyInfo, CcToolchainInfo </p> </td> </tr> @@ -62,7 +62,7 @@ <td> <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a>; optional <p> - The dependencies of this attribute must provide: CcInfo; or OtherProviderInfo, Unknown Provider + The dependencies of this attribute must provide: CcInfo; or OtherProviderInfo, DepProviderInfo </p> </td> </tr>