Adding IArgumentCompleterFactory for parameterized ArgumentCompleters

[powercode]

PR Summary

Adding support for derived, parameterized, ArgumentCompleterAttributes. This makes it possible to create more generic completers, either in libraries or shipping with the product.

This allows for parameterized ArgumentCompleters.
Deriving from ArgumentCompleterAttribute makes it possible to create generic completers, like

[DirectoryCompleter(ContainingFile="pswh.exe", Depth=2)]

[DateCompleter(WeekDay='Monday', From="LastYear")]

[GitCommits(Branch='release')]

The derived attributes implement the IArgumentCompleterFactory interface and use property values to create a specialized completer.

PR Context

It is difficult (if even possible) to write generic completers. This PR would allow for the following:

   /// <summary>
    /// Creates new number completions
    /// </summary>
    public class NumberCompletionsAttribute : ArgumentCompleterAttribute, IArgumentCompleterFactory
    {
        private readonly int _from;
        private readonly int _to;
        private readonly int _step;

        /// <summary>
        /// Creates a new <see cref="NumberCompletionsAttribute"/>.
        /// </summary>
        /// <param name="from"></param>
        /// <param name="to"></param>
        /// <param name="step"></param>
        public NumberCompletionsAttribute(int from, int to, int step = 1) 
        {
            _from = @from;
            _to = to;
            _step = step;
        }

        /// <inheritdoc />
        public IArgumentCompleter Create() => new NumberCompleter(_from, _to, _step);
    }

    /// <summary>
    /// Completes numbers
    /// </summary>
    public class NumberCompleter : IArgumentCompleter
    {
        private readonly int _from;
        private readonly int _to;
        private readonly int _step;

        /// <summary>
        /// Creates a new instance of a NumberCompleter
        /// </summary>
        /// <param name="from"></param>
        /// <param name="to"></param>
        /// <param name="step"></param>
        public NumberCompleter(int from, int to, int step)
        {
            _from = @from;
            _to = to;
            _step = step;
        }

        /// <inheritdoc />
        public IEnumerable<CompletionResult> CompleteArgument(string commandName, string parameterName, string wordToComplete, CommandAst commandAst, IDictionary fakeBoundParameters)
        {
            var res = new List<CompletionResult>((_to - _from) / _step);

            for (int i = _from; i <= _to; i += _step)
            {
                string completionText = i.ToString();
                if (completionText.StartsWith(wordToComplete, StringComparison.Ordinal))
                    res.Add(new CompletionResult(completionText));
            }

            return res;
        }
    }

or in PowerShell:

using namespace System.Management.Automation
using namespace System.Management.Automation.Language
using namespace System.Collections
using namespace System.Collections.Generic


class NumberCompleter : IArgumentCompleter {

    [int] $From
    [int] $To
    [int] $Step
    
    NumberCompleter([int] $from, [int] $to, [int] $step) {
        if ($from -gt $to) {
            throw [ArgumentOutOfRangeException]::new("from")
        }
        $this.From = $from
        $this.To = $to
        $this.Step = $step -lt 1 ? 1 : $step
    }

    [IEnumerable[CompletionResult]] CompleteArgument(
        [string] $CommandName,
        [string] $parameterName,
        [string] $wordToComplete,
        [CommandAst] $commandAst,
        [IDictionary] $fakeBoundParameters) {
        
        $resultList = [List[CompletionResult]]::new()
        $local:to = $this.To
        $local:step = $this.Step
        for ($i = $this.From; $i -lt $to; $i += $step) {
            $resultList.Add([CompletionResult]::new($i.ToString()))
        }
        
        return $resultList
    }
}

class NumberCompletionAttribute : ArgumentCompleterAttribute, IArgumentCompleterFactory {
    [int] $From
    [int] $To
    [int] $Step
    
    NumberCompletionAttribute([int] $from, [int] $to, [int] $step) {
        $this.From = $from
        $this.To = $to
        $this.Step = $step
    }

    [IArgumentCompleter] Create() { return [NumberCompleter]::new($this.From, $this.To, $this.Step) }
}

Usage from PowerShell would then be:

function Add{
    param(
       [NumberCompletions(0, 100, 5)]
       [int] $X
      ,
       [NumberCompletions(0, 100, 5)]
       [int] $Y
    )
    $X + $Y
}

Fix #12708

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• Breaking changes
  • None
  • OR
  • Experimental feature(s) needed
    • Experimental feature name(s):
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • N/A or can only be tested interactively
  • OR
  • Make sure you've added a new test if existing tests do not effectively test the code changed
• Tooling
  • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
  • OR
  • I have considered the user experience from a tooling perspective and enumerated concerns in the summary. This may include:
    • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
    • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
    • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
    • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

[powercode]

The CI errors seem unrelated to my commits

[powercode]

Yet again an unrelated CI failure

[iSazonov]

Add PowerShell Committee to approve new public API.

[iSazonov]

Do you think about IArgumentCompleter vs IArgumentCompleterFactory?

[joeyaiello]

@PowerShell/powershell-committee discussed this and we all think it's super interesting, and, at least in our initial look, a well thought out feature.

However, given it's scope, it'd be really nice to see an RFC (or even an issue) where the problem statement and any alternative solutions are discussed and other contributors have a chance to give feedback on the proposal.

@JamesWTruher also suggested that it might be useful to see a real-world example of how this could be used to refactor PowerShell built-in completers, or to make them more intelligent. E.g. Import-Module -Path <tab> might only show folders, *.psd1 files, and *.psm1 files, and that could be reused in other places where it's relevant.

[iSazonov]

Convert to Draft until RFC or issue discussion is finished.

[daxian-dbw]

I like the idea, very interesting, allowing people to have reusable argument completer attributes. About the implementation, why not have an abstract attribute class that defines an abstract method which returns an IArgumentCompleter instance? For example:

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
public abstract class ReusableArgumentCompletionAttribute : Attribute
{
    protected ReusableArgumentCompletionAttribute() {}
    public abstract IArgumentCompleter NewArgumentCompleter(/* EngineIntrinsics engineIntrinsics ?? */);
}

This seems more succinct than : ArgumentCompleterAttribute, IArgumentCompleterFactory

[SteveL-MSFT]

The @PowerShell/powershell-committee discussed this. In general, we all agree this capability would be useful so thanks for that! However, we're trying to update our RFC process to make it lighter-weight and would like to try it out with this feature if you're ok with this. Basically, we'd like to have an issue opened that describes the problem to be solved and/or value to the user and also some example code showing its use (which you have in this PR description already) and expected results. Based on the amount and quality of discussion, it will help inform if an RFC is needed.

[vexx32]

@SteveL-MSFT in case y'all missed it. 🙂
#12708

[SteveL-MSFT]

@vexx32 yes, we missed that. Thanks! I linked it to this PR.

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

[ghost]

Open PRs should not be assigned to milestone, so they are not assigned to the wrong milestone after they are merged. For backport consideration, use a backport label.

[powercode]

@SteveL-MSFT This PR is in a Change Requested state, but the requested change is on a commit that is no longer available due to a force push after a rebase, and the requested change is made. How can I get this into a green state?

[powercode]

@adityapatwardhan It's been 8 months since you last touched this. Can you act on it or maybe reassign it if you don't have the bandwidth to process it?

[powercode]

@iSazonov I'm suspecting that this is in limbo as there is 1 change requested, but it is requested on a commit that isn't available anymore, and hence cannot be fixed. The requested change is already in a different, force-pushed commit.

What do you think is the right way to move this forward? This is actually a very nice feature to have, that opens up new possibilities, both for the team and for regular users, and it's a shame that it's just stuck here.

[adityapatwardhan]

Sorry for the delay. I have reviewed this and looks good. Since it has been a while since we ran the CI tests for this PR. I am restarting all the tests. I will merge as soon as the tests pass.

[adityapatwardhan]

Closing and re-opening to re-trigger all CI tests

[adityapatwardhan]

@PoshChan please remind me in 1 hour

[PoshChan]

@adityapatwardhan, this is the reminder you requested 1 hour ago

[adityapatwardhan]

@powercode Thank you for your contribution and patience!

[iSazonov]

@powercode Please open new documentation issue.

/cc @sdwheeler

[sdwheeler]

@powercode @iSazonov The scripting example can be added to the docs. The C# usage needs to be added in ///-comments in the source code. Please open the appropriate documentation needed issue in this repo.

[ghost]

🎉v7.2.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes