Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System.ComponentModel.DataAnnotations;
using ModularPipelines.OptionsGenerator.Models;

namespace ModularPipelines.OptionsGenerator.Tests.Generators;

public partial class RequiredConstructorValidationTests
{
[Test]
[Arguments("Enabled", null, false, true)]
[Arguments("Enabled", false, false, true)]
[Arguments("Enabled", true, false, false)]
[Arguments("Enabled", true, true, true)]
[Arguments("Enabled", null, true, true)]
[Arguments("Clone", false, false, true)]
[Arguments("Clone", true, false, false)]
[Arguments("Clone", true, true, true)]
public async Task Conditional_Requirement_Activates_Only_For_A_Present_Flag(string triggerName, bool? enabled, bool supplied, bool valid)
{
var generated = await Generate(
[
new() { SwitchName = "--enabled", PropertyName = triggerName, CSharpType = "bool?", IsFlag = true },
new() { SwitchName = "--value", PropertyName = "Value", CSharpType = "string?" },
], alternativeGroups:
[
new CliRequiredAlternativeGroup
{
RequiredWhen = new() { PropertyName = triggerName, OptionSwitch = "--enabled" },
IsChoice = false,
Members = [new() { PropertyName = "Value", OptionSwitch = "--value", IsRequired = true }],
},
]);
var type = Compile(generated).GetType("ModularPipelines.Tool.Options.ToolRunOptions")!;
var instance = Activator.CreateInstance(type)!;
type.GetProperties().Single(property => property.PropertyType == typeof(bool?)).SetValue(instance, enabled);
type.GetProperty("Value")!.SetValue(instance, supplied ? "value" : null);
await Assert.That(Validator.TryValidateObject(instance, new(instance), [], true)).IsEqualTo(valid);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,60 @@ namespace ModularPipelines.OptionsGenerator.Tests.Generators;

public partial class RequiredConstructorValidationTests
{
[Test]
public async Task Gcloud_Composer_Updates_Preserve_Optional_Resource_Settings()
{
var command = await GcloudCapturedSemanticsTests.Scrape("composer environments update");
var group = command.RequiredAlternativeGroups.Single(group => group.PropertyNames.Contains("MaxWorkers"));
await ValidateCapturedGroup(command, group,
[
("", false),
("MaxWorkers", true),
("SchedulerCount", true),
("MaxWorkers,SchedulerCount", true),
("NodeCount", true),
("NodeCount,MaxWorkers", false),
("MaintenanceWindowStart,MaintenanceWindowEnd,MaintenanceWindowRecurrence", true),
("MaintenanceWindowStart", false),
]);
}

[Test]
public async Task Gcloud_Container_Autoprovisioning_Preserves_Optional_Settings()
{
var command = await GcloudCapturedSemanticsTests.Scrape("container clusters update");
await ValidateCapturedGroups(command, command.RequiredAlternativeGroups,
[
("EnableAutoprovisioning", false),
("AutoprovisioningConfigFile", true),
("EnableAutoprovisioning,AutoprovisioningConfigFile", true),
("MaxCpu", true),
("MaxMemory", true),
("EnableAutoprovisioning,MaxCpu", false),
("EnableAutoprovisioning,MaxMemory", false),
("EnableAutoprovisioning,MaxCpu,MaxMemory", true),
("AutoprovisioningConfigFile,AutoprovisioningMinCpuPlatform", false),
]);
}

[Test]
public async Task Gcloud_Cluster_Director_Updates_Do_Not_Require_Unrelated_Granular_Flags()
{
var command = await GcloudCapturedSemanticsTests.Scrape("cluster-director clusters update");
var group = command.RequiredAlternativeGroups.Single(group => group.PropertyNames.Contains("Config"));
await ValidateCapturedGroup(command, group,
[
("", false),
("Description", true),
("RemoveLabels", true),
("Description,RemoveLabels", true),
("Config,UpdateMask", true),
("Config", false),
("UpdateMask", false),
("Config,UpdateMask,Description", false),
]);
}

[Test]
public async Task Gcloud_Build_Trigger_Updates_Preserve_Documented_Nested_Choices()
{
Expand Down Expand Up @@ -75,7 +129,7 @@ await ValidateCapturedGroup(command, group,
}

[Test]
public async Task Gcloud_Agent_Identity_Oauth_Requires_Every_Selected_Branch_Member()
public async Task Gcloud_Agent_Identity_Oauth_Preserves_Optional_Members_Within_Exclusive_Branches()
{
var command = await GcloudCapturedSemanticsTests.Scrape("agent-identity auth-providers create");
var group = command.RequiredAlternativeGroups.Single(group => group.PropertyNames.Contains("ApiKey"));
Expand All @@ -92,7 +146,8 @@ public async Task Gcloud_Agent_Identity_Oauth_Requires_Every_Selected_Branch_Mem
foreach (var branch in new[] { threeLegged, twoLegged })
{
var members = branch.Split(',');
cases.AddRange(members.Select(missing => (string.Join(',', members.Where(member => member != missing)), false)));
cases.AddRange(members.Select(missing => (string.Join(',', members.Where(member => member != missing)), true)));
cases.AddRange(members.Select(member => (member, true)));
}

await ValidateCapturedGroup(command, group, [.. cases]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.ComponentModel.DataAnnotations;
using ModularPipelines.OptionsGenerator.Generators;
using ModularPipelines.OptionsGenerator.Models;
using ModularPipelines.OptionsGenerator.Tests.Scrapers.Cli;

Expand Down Expand Up @@ -125,13 +126,26 @@ await ValidateCapturedGroup(command, group,
private static IEnumerable<CliRequiredAlternativeGroup> Descendants(IEnumerable<CliRequiredAlternativeGroup> groups) =>
groups.SelectMany(group => new[] { group }.Concat(Descendants(group.Groups)));

private static async Task ValidateCapturedGroup(CliCommandDefinition command, CliRequiredAlternativeGroup group,
private static Task ValidateCapturedGroup(CliCommandDefinition command, CliRequiredAlternativeGroup group,
(string Properties, bool Valid)[] cases) => ValidateCapturedGroups(command, [group], cases);

private static async Task ValidateCapturedGroups(CliCommandDefinition command, IReadOnlyList<CliRequiredAlternativeGroup> groups,
(string Properties, bool Valid)[] cases)
{
var options = command.Options.Where(option => group.PropertyNames.Contains(option.PropertyName)).ToList();
var generated = await Generate(options, alternativeGroups: [group]);
var names = groups.SelectMany(group => group.PropertyNames)
.Concat(groups.SelectMany(EnumerateGroups).Select(group => group.RequiredWhen?.PropertyName).OfType<string>()).ToHashSet(StringComparer.Ordinal);
var options = command.Options.Where(option => names.Contains(option.PropertyName)).ToList();
var generated = await Generate(options, alternativeGroups: groups);
var enums = await new EnumGenerator().GenerateAsync(new CliToolDefinition
{
ToolName = "tool",
NamespacePrefix = "Tool",
TargetNamespace = "ModularPipelines.Tool",
OutputDirectory = "src/ModularPipelines.Tool",
Commands = [command],
});
const string secretAttribute = "namespace ModularPipelines.Secrets { public sealed class SecretValueAttribute : System.Attribute; }";
var optionsType = Compile(generated, secretAttribute).GetType("ModularPipelines.Tool.Options.ToolRunOptions")!;
var optionsType = Compile([generated, secretAttribute, .. enums.Select(file => file.Content)]).GetType("ModularPipelines.Tool.Options.ToolRunOptions")!;
foreach (var (properties, valid) in cases)
{
var instance = Activator.CreateInstance(optionsType)!;
Expand Down Expand Up @@ -162,5 +176,8 @@ private static async Task ValidateCapturedGroup(CliCommandDefinition command, Cl
await Assert.That(Validator.TryValidateObject(instance, new(instance), errors, true))
.IsEqualTo(valid).Because($"Selected {properties}: {string.Join("; ", errors)}");
}

static IEnumerable<CliRequiredAlternativeGroup> EnumerateGroups(CliRequiredAlternativeGroup group) =>
new[] { group }.Concat(group.Groups.SelectMany(EnumerateGroups));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ private static CliCommandDefinition ResolveCommand(
.ToArray();
CliRequiredAlternativeGroup ResolveGroup(CliRequiredAlternativeGroup group) => group with
{
RequiredWhen = group.RequiredWhen is { } trigger ? trigger with
{
PropertyName = ResolveAlternativeMemberName(command, options, positionalArguments, trigger),
} : null,
Members = [.. group.Members.Select(member => member with
{
PropertyName = ResolveAlternativeMemberName(command, options, positionalArguments, member),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,8 @@ private static void GenerateGroupValidation(
}

string Presence(string propertyName) => GetPresenceExpression(command, positionalArguments, propertyName);
(required, activation) = ResolveConditionalActivation(group, required, activation, Presence);

if (group.IsUsageFormChoice)
{
// Usage forms describe sufficient combinations. A complete form remains valid
Expand Down Expand Up @@ -532,6 +534,18 @@ string GroupPresence(CliRequiredAlternativeGroup nested) =>
}
}

private static (bool Required, string? Activation) ResolveConditionalActivation(CliRequiredAlternativeGroup group,
bool required, string? activation, Func<string, string> presence)
{
if (group.RequiredWhen is not { } trigger)
{
return (required, activation);
}

var triggerPresence = presence(trigger.PropertyName);
return (true, activation is null ? triggerPresence : $"({activation}) && ({triggerPresence})");
}

private static string GetCompleteUsageExpression(CliRequiredAlternativeGroup group, Func<string, string> presence)
{
var expressions = group.Members.Select(member => presence(member.PropertyName))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ namespace ModularPipelines.OptionsGenerator.Models;
/// </summary>
public sealed record CliRequiredAlternativeGroup
{
/// <summary>
/// An option whose presence activates this constraint; otherwise the constraint is inactive.
/// </summary>
public CliRequiredAlternativeMember? RequiredWhen { get; init; }

/// <summary>
/// Whether the group must be present when its containing bundle is selected.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ private static IReadOnlyList<CliRequiredAlternativeGroup> ResolveRequiredAlterna
// A richer required help constraint already enforces presence over these members.
// Optional help constraints cannot replace a synopsis requirement.
if (inferred.IsUsageFormChoice
|| !groups.Any(group => group.IsRequired && identities.SetEquals(GetAlternativeGroupIdentities(group))))
|| !groups.Any(group => group.IsRequired && group.RequiredWhen is null && identities.SetEquals(GetAlternativeGroupIdentities(group))))
{
groups.Add(inferred);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
using System.Text.RegularExpressions;
using ModularPipelines.OptionsGenerator.Models;

namespace ModularPipelines.OptionsGenerator.Scrapers.Cli;

public partial class GcloudCliScraper
{
private static void ApplyNamedConditionalRequirements(IReadOnlyList<CliOptionDefinition> options,
List<CliRequiredAlternativeGroup> constraints)
{
var dependencies = options.SelectMany(option => NamedRequirementPattern().Matches(option.Description ?? "")
.Select(match => (Option: option, Trigger: match.Groups["switch"].Value)))
.GroupBy(dependency => dependency.Trigger, StringComparer.Ordinal);
foreach (var dependency in dependencies)
{
var trigger = options.FirstOrDefault(option => option.SwitchName == dependency.Key);
if (trigger is null)
{
continue;
}

var required = dependency.Select(item => item.Option.PropertyName).ToHashSet(StringComparer.Ordinal);
if (required.Contains(trigger.PropertyName))
{
continue;
}

// A configuration file can replace the inline settings containing the
// named requirements. Retain that documented exclusive alternative.
var alternative = constraints.SelectMany(EnumerateConstraints)
.Where(group => group.RequiredWhen is null && group.IsChoice && group.IsMutuallyExclusive
&& !group.PropertyNames.Contains(trigger.PropertyName)
&& group.Members.Count + group.Groups.Count > 1
&& group.Groups.Any(branch => required.IsSubsetOf(branch.PropertyNames)))
.OrderBy(group => group.PropertyNames.Count).FirstOrDefault();
var conditional = alternative is null ? new CliRequiredAlternativeGroup
{
IsChoice = false,
Members = [.. dependency.DistinctBy(item => item.Option.PropertyName).Select(item => new CliRequiredAlternativeMember
{
PropertyName = item.Option.PropertyName,
OptionSwitch = item.Option.SwitchName,
IsRequired = true,
})],
} : MarkNamedRequiredMembers(alternative, required);
constraints.Add(conditional with
{
IsRequired = true,
RequiredWhen = new() { PropertyName = trigger.PropertyName, OptionSwitch = trigger.SwitchName },
});
}
}

private static CliRequiredAlternativeGroup MarkNamedRequiredMembers(CliRequiredAlternativeGroup group,
IReadOnlySet<string> required) => group with
{
Members = [.. group.Members.Select(member => member with
{
IsRequired = member.IsRequired || required.Contains(member.PropertyName),
})],
Groups = [.. group.Groups.Select(child => MarkNamedRequiredMembers(child, required))],
};

[GeneratedRegex(@"\bRequired\s+to\s+be\s+set\s+when\s+(?<switch>--[\w-]+)\s+is\s+used\b", RegexOptions.IgnoreCase)]
private static partial Regex NamedRequirementPattern();
}
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ private static List<string> ExtractFromSection(string helpText, string sectionNa
}

ReconcileRequiredSynopsisChoices(usage.ArgumentGroupSynopsis ?? usage.Synopsis, options, requiredAlternativeGroups);
ApplyNamedConditionalRequirements(options, requiredAlternativeGroups);

return (options, argumentGroups, requiredAlternativeGroups, positionalArguments, usage);
}
Expand Down Expand Up @@ -421,6 +422,15 @@ private static CliRequiredAlternativeGroup PreserveDocumentedChoices(CliRequired
&& group.PropertyNames.ToHashSet(StringComparer.Ordinal).SetEquals(candidate.PropertyNames));
return choice ?? group with
{
// Adjacent flags inside a synopsis branch can all be optional. Only the
// documented FLAGS constraints establish mandatory companion members.
Members = [.. group.Members.Select(member => member with
{
IsRequired = member.IsRequired && documented.SelectMany(candidate => candidate.Members)
.Any(candidate => candidate.PropertyName == member.PropertyName && candidate.IsRequired),
})],
IsRequired = group.IsChoice ? documented.FirstOrDefault(candidate => candidate.IsChoice
&& group.PropertyNames.ToHashSet(StringComparer.Ordinal).SetEquals(candidate.PropertyNames))?.IsRequired ?? group.IsRequired : group.IsRequired,
Comment thread
thomhurst marked this conversation as resolved.
Groups = [.. group.Groups.Select(child => PreserveDocumentedChoices(child, documented))],
};
}
Expand Down
Loading