Skip to content

Commit

Permalink
Disallow rule implementation functions to return a struct
Browse files Browse the repository at this point in the history
Such functions should return a list of providers instead.
This change represents a deprecation of the "old" syntax.

This is an incompatible change attached to new flag --incompatible_disallow_struct_provider_syntax.

See https://docs.google.com/document/d/14A9HK8Jn2jErMayLEE3nrNJIxNfZWN_slFbhgtS6-aM for details.

Migration tracker: #7347
Progress toward #6241

RELNOTES: New incompatible flag --incompatible_disallow_struct_provider_syntax removes the ability for rule implementation functions to return struct. Such functions should return a list of providers instead. Migration tracking: #7347
PiperOrigin-RevId: 232547460
  • Loading branch information
c-parsons authored and Copybara-Service committed Feb 5, 2019
1 parent fc7941b commit 5d53171
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,16 @@ private static void addProviders(
// Use the creation location of this struct as a better reference in error messages
loc = info.getCreationLoc();
if (info.getProvider().getKey().equals(StructProvider.STRUCT.getKey())) {

if (context.getSkylarkSemantics().incompatibleDisallowStructProviderSyntax()) {
throw new EvalException(
loc,
"Returning a struct from a rule implementation function is deprecated and will "
+ "be removed soon. It may be temporarily re-enabled by setting "
+ "--incompatible_disallow_struct_provider_syntax=false . See "
+ "https://github.com/bazelbuild/bazel/issues/7347 for details.");
}

// Old-style struct, but it may contain declared providers
StructImpl struct = (StructImpl) target;
oldStyleProviders = struct;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,20 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
)
public boolean incompatibleDisallowLoadLabelsToCrossPackageBoundaries;

@Option(
name = "incompatible_disallow_struct_provider_syntax",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If set to true, rule implementation functions may not return a struct. They must "
+ "instead return a list of provider instances.")
public boolean incompatibleDisallowStructProviderSyntax;

@Option(
name = "incompatible_generate_javacommon_source_jar",
defaultValue = "true",
Expand Down Expand Up @@ -555,6 +569,7 @@ public SkylarkSemantics toSkylarkSemantics() {
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(
incompatibleDisallowLoadLabelsToCrossPackageBoundaries)
.incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
.incompatibleDisallowStructProviderSyntax(incompatibleDisallowStructProviderSyntax)
.incompatibleExpandDirectories(incompatibleExpandDirectories)
.incompatibleGenerateJavaCommonSourceJar(incompatibleGenerateJavaCommonSourceJar)
.incompatibleNewActionsApi(incompatibleNewActionsApi)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {

public abstract boolean incompatibleDisallowOldStyleArgsAdd();

public abstract boolean incompatibleDisallowStructProviderSyntax();

public abstract boolean incompatibleExpandDirectories();

public abstract boolean incompatibleGenerateJavaCommonSourceJar();
Expand Down Expand Up @@ -225,6 +227,7 @@ public static Builder builderWithDefaults() {
.incompatibleDisallowLegacyJavaInfo(false)
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(false)
.incompatibleDisallowOldStyleArgsAdd(false)
.incompatibleDisallowStructProviderSyntax(false)
.incompatibleExpandDirectories(true)
.incompatibleGenerateJavaCommonSourceJar(true)
.incompatibleNewActionsApi(false)
Expand Down Expand Up @@ -290,6 +293,8 @@ public abstract static class Builder {

public abstract Builder incompatibleDisallowOldStyleArgsAdd(boolean value);

public abstract Builder incompatibleDisallowStructProviderSyntax(boolean value);

public abstract Builder incompatibleExpandDirectories(boolean value);

public abstract Builder incompatibleGenerateJavaCommonSourceJar(boolean value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
"--incompatible_disallow_legacy_java_provider=" + rand.nextBoolean(),
"--incompatible_disallow_load_labels_to_cross_package_boundaries=" + rand.nextBoolean(),
"--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
"--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_expand_directories=" + rand.nextBoolean(),
"--incompatible_generate_javacommon_source_jar=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
Expand Down Expand Up @@ -192,6 +193,7 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
.incompatibleDisallowLegacyJavaProvider(rand.nextBoolean())
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(rand.nextBoolean())
.incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
.incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
.incompatibleExpandDirectories(rand.nextBoolean())
.incompatibleGenerateJavaCommonSourceJar(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2749,6 +2749,29 @@ public void testLicenseType() throws Exception {
assertContainsEvent("<license object>");
}

@Test
public void testDisallowStructProviderSyntax() throws Exception {
setSkylarkSemanticsOptions("--incompatible_disallow_struct_provider_syntax=true");
scratch.file(
"test/skylark/extension.bzl",
"def custom_rule_impl(ctx):",
" return struct()",
"",
"custom_rule = rule(implementation = custom_rule_impl)");
scratch.file(
"test/skylark/BUILD",
"load('//test/skylark:extension.bzl', 'custom_rule')",
"",
"custom_rule(name = 'cr')");

reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test/skylark:cr");
assertContainsEvent(
"Returning a struct from a rule implementation function is deprecated and will be "
+ "removed soon. It may be temporarily re-enabled by setting "
+ "--incompatible_disallow_struct_provider_syntax=false");
}

/**
* Skylark integration test that forces inlining.
*/
Expand Down

0 comments on commit 5d53171

Please # to comment.