cake-build / cake

:cake: Cake (C# Make) is a cross platform build automation system.
https://cakebuild.net
MIT License
3.89k stars 725 forks source link

Tools parameters settings from attributes #1672

Open Meir017 opened 7 years ago

Meir017 commented 7 years ago

Suggestion:

Right now for tool the settings are converted to the command line arguments manually which seems to me a bit awkward.

I had an idea to use attributes on the settings classes properties that will contain metadata on how each property should be handled - similar to how Powershell function work.

not sure if it would be possible to use the powershell attributes directly

for example:

[AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
public abstract class CakeToolParameterAttribute : Attribute
{
    public CakeToolParameterAttribute(string template)
    {
        Template = template ?? throw new ArgumentNullException(nameof(template));
    }

    public string Template { get; }

    public abstract string Render(object property, PropertyInfo info);
}

For properties that are represented as -X=Y or similar the template property will be something like -X={0}.

for switch parameters:

public sealed class SwitchParameterAttribute : CakeToolParameterAttribute
{
    public SwitchParameterAttribute(string template) : base(template)
    {
    }

    public override string Render(object property, PropertyInfo info)
    {
        if (info.GetValue(property) is bool flag && flag)
        {
            return Template;
        }

        return string.Empty;
    }
}

for collection parameters:

public sealed class CollectionParameterAttribute : CakeToolParameterAttribute
{
    public CollectionParameterAttribute(string template) : base(template)
    {
    }

    public int? Min { get; set; }
    public int? Max { get; set; }

    public override string Render(object property, PropertyInfo info)
    {
        var items = (info.GetValue(property) as IEnumerable)?.Cast<object>()?.ToArray();
        if (items == null)
        {
            throw new ArgumentException("property is invalid, must be a collection.");
        }
        if (Min.HasValue && items.Length < Min)
        {
            throw new ArgumentOutOfRangeException($"must have at least {Min} items. found {items.Length}");
        }
        if (Max.HasValue && items.Length > Max)
        {
            throw new ArgumentOutOfRangeException($"must have at most {Max} items. found {items.Length}");
        }

        if (!Template.Contains("{0}"))
        {
            throw new FormatException("The template must contain a placeholder {0}");
        }

        return string.Join(" ", items.Select(item => string.Format(Template, item.ToString().Quote())));
    }
}

This will improve the generated docs by listing next to each property it's template

Tested a small POC - https://github.com/Meir017/Playground/blob/master/cake-tool-settings-attributes.cs

any thoughts?

patriksvensson commented 7 years ago

@Meir017 Good initiative!

I think however that the current way of building tool arguments gives more flexibility of how and when to include parameters, which have proven useful in the past. Especially in cases where you want to exclude or include other arguments depending what settings have been set and similar. Sometimes it's not a 1:1 mapping between what settings have been set and what arguments that get rendered.

This could absolutely be added in an optional addin though!

Meir017 commented 7 years ago

What you are describing sounds a bit like the ParameterSetName in the attribute ParameterAttribute in powershell. (source code)

  1. Would it be possible to somehow reused the attributes from powershell? If something like this would be possible it will also mean that we will be able to generate from these settings files powershell autocomplete for tools.

  2. Can you give an example of the case you described where an argument depends on another argument?

gep13 commented 7 years ago

@Meir017 as @patriksvensson said, I don't think that this is something that we would want to roll out across the board, however, there is nothing to stop you doing something like this in your own addins.

There is already examples of doing this in this addin:

https://github.com/AgileArchitect/Cake.Sonar