RFC0001-Mutual-Exclusion-Parameters-and-Properties comments

[SteveL-MSFT]

Use this to track comments for https://github.com/PowerShell/PowerShell-Language-RFC/blob/master/1-Draft/RFC0001-Mutual-Exclusion-Parameters-and-Properties.md

[danlyons-home]

It looks like the link above should instead be the following: https://github.com/PowerShell/PowerShell-Language-RFC/blob/master/1-Draft/RFC0001-Mutual-Exclusion-Parameters-and-Properties.md

The original link is missing the 1-Draft folder between master and the file.

[SteveL-MSFT]

Fixed, thanks! That link was before we thought it was a good idea to have the folders.

[alexneumann12]

Is it feasible to define both methods and allow the cmdlet author to choose whichever style they prefer? If not then I'd personally pursue the [MutuallyExclusive()] attribute since it's more concrete as to what is being excluded against another.

[WouterDeKort]

My preference is the MutuallyExclusive syntax. When you have a lot of properties it's easier to see them all grouped together then have the ExclusionSet spread over all properties.

I do wonder however how hard it is to set these attributes for a large number of parameters. But that's an issue that we already have today and that's not easily solved with syntax (I think). Having run time validation to make sure that you don't exclude a property from all combinations would be nice.

[MikeShepard]

I feel like both the existing ParameterSet syntax and the MutuallyExclusive syntax hide the intent. Since we're specifying "use cases", could we list the use cases with the distinguishing parameters?

For instance: [UseCase('Pipeline Input',DistinguishingParameters='InputObject')] [UseCase('Standard Path',DistinguishingParameters='Path')] [UseCase('Literal Path',DistinguisingParameters='LiteralPath')]

This is more readable in my mind, and you don't have to do a logic puzzle to figure out what's going on.

[DerpMcDerp]

Often times when I use ParameterSets, the internals of the cmdlet are so different that it would be more convenient if PowerShell could use multimethods on cmdlets instead rather than having me clutter the internals of the begin, process, and end blocks with if tests to do my own dispatching.

I'd rather have multiple dispatch on cmdlets rather than this feature which just slightly tweaks ParameterSets.

[mattmcnabb]

I'd like to clarify - This RFC is only for Powershell classes, right? Not for Advanced Functions or Cmdlets?

If so, I tend toward the proposed usage of [MutuallyExclusive()], although I'd like to see the attribute in the class definition rather than outside of it.

[MikeShepard]

The "motivation" section mentions parameters for advanced functions as well.

[replicaJunction]

While I could see the [MutuallyExclusive()] attribute making sense in a class, when writing an advanced function, this would separate part of the definition of each parameter from the parameter itself. This doesn't make sense from an organizational standpoint, and it also makes the code more difficult to read.

Here's an example of an advanced function, with the MutuallyExclusive attribute added before the start of the function. When going over parameter definitions, it's tough to see which parameters are mutually exclusive, since that attribute is placed in a completely different place than the parameter attributes.

# Prop2 and Prop3 are mutually exclusive
[MutuallyExclusive("Prop2", "Prop3")]
function Invoke-Stuff {
    [CmdletBinding(DefaultParameterSetName = 'ParameterSet1',
                   SupportsShouldProcess = $true)]
    param(
        [Parameter(ParameterSetName = 'ParameterSet1',
                    ValueFromPipeline = $true,
                    ValueFromPipelineByPropertyName = $true)]
        [Alias('p1')]
        [String] $Prop1,

        [Parameter(ParameterSetName = 'ParameterSet1',
                    Mandatory = $true,
                    Position = 0)]
        [Alias('p2')]
        [ValidateScript({Test-Path $_})]
        [string] $Prop2,

        [Parameter(Mandatory = $false)]
        [Alias('p3')]
        [ValidateNotNullOrEmpty()]
        [string] $Prop3,

        [Parameter(ParameterSetName = 'ParameterSet2')]
        [Alias('p4')]
        [string] $Prop4
    )

    # ...
}

I like the ExclusionSet suggestion much better, since this becomes a parameter attribute. This can be logically placed with all the other attributes of each parameter.

[smokedlinq]

Alternate suggestion, use ParameterSet attribute that defaults like adding a [Parameter(ParameterSetName='Name')] attribute, also include the ability to write a negative parameter set, e.g. [ParameterSet(Exclude='Name')] would mean all parameter sets except Name, or alternatively accept multiple parameter set names [ParameterSet('Set1','Set2')] for explicit set definitions.

class Fu
{
    [ParameterSet('ByName')]
    [string] $Name

    [ParameterSet('ById')]
    [string] $Id

    [Ensure] $Ensure = ([Ensure]::Present)
    [string] $Description

    # Don't allow DisplayName when Name is used
    [ParameterSet(Exclude='ByName')]
    [string] $DisplayName
}

function Set-TargetResource
{
    [CmdletBinding()]
    param
    (
        [Parameter(Mandatory)]
        [ParameterSet('ByName')]
        [string] $Name,

        [Parameter(Mandatory)]
        [ParameterSet('ById')]
        [string] $Id,

        [Ensure] $Ensure = ([Ensure]::Present),
        [string] $Description,

        [ParameterSet(Exlcude='ByName')]
        [string] $DisplayName,
    )
}

[SteveL-MSFT]

@mattmcnabb intent is that this would be for both functions and classes. I realize now that the sample only shows for classes, but the attributes would be the same for functions

[mattmcnabb]

@SteveL-MSFT Quite all right, I see now that the header title mentions functions as well. Thanks!

[ckennedy666]

I strongly support @adweigert suggestion. This seems far better, clearer, and more in the spirit of attributes than that put forward in the RFC. Frankly both alternatives in the RFC just seem to complicate things more than necessary.

[MikeShepard]

I especially like @adweigert suggestion that we could include multiple parameter sets in the same attribute. Not being able to do so introduces a need for duplicating the rest of the parameter attributes.

[SteveL-MSFT]

@adweigert Good suggestion, however, it seems that the problem you are trying to solve is not exactly the same as the intent of this RFC. We'll update the RFC to clarify the intent as well as adding some real examples based on invoke-command and/or where-object. Hopefully we'll get this out before the holidays.

[smokedlinq]

If the feature is geared directly towards DSC resource designers in v5, then I agree what I proposed would not be needed as the ParameterSet feature is already part of function/cmdlet design with [Parameter(ParameterSetName=)].

If this is specifically for v5 DSC class resource designers, then simply using something along the lines of [DscPropertySet([[Name=]][, Exclude=])] similar to what I previously proposed would follow the current attribute/markup of DSC class resources, or adding an additional property to the [DscProperty()] attribute named ParameterSetName to bring it inline with the Parameter attribute usage.

Things could be tricky around the resource key properties and different parameter sets, maybe the handling of that could be explained more.

Example:

[DscResource()]
class MyResource
{
    [DscProperty(Key,ParameterSetName='ByName')]
    [string] $Name

    [DscProperty(Key,ParameterSetName='ById')]
    [Guid] $Id

    [bool] $Enabled

    # Required for ById property set
    [DscProperty(Mandatory, ParameterSetName='ById')]
    [string] $DisplayName

    /* implementation omitted */
}

[KirkMunro]

I meant to comment on this a long time ago. Then there were holidays. And busy work after the holidays. Nothing quite like cutting it down to the wire...

I love that you're using an RFC process for language enhancements. I think that is very, very important at this stage of PowerShell's development. Thank you for opening these ideas up for discussion.

For this particular issue, I don't think that the proposed design is thought out well enough. It seems very narrow in vision and isn't really looking at the big picture.

I think it makes sense to analyze this from the perspective of cmdlets and advanced functions first, since they have been around a lot longer than classes and there is much more data to sample. For these types of commands, there are several challenges that come to mind when I think about mutual exclusivity among parameters in PowerShell:

1. Having too many parameter sets can make it difficult for an end user to learn how to use a command. This difficulty is further compounded when many parameters apply to many, but not all, parameter sets. My favorite example of this is Invoke-Command in PowerShell 5. Have a look at the results of Get-Command Invoke-Command -Syntax, as well as the results of [System.Management.Automation.ProxyCommand]::GetParamBlock([System.Management.Automation.CommandMetadata]::new((Get-Command Invoke-Command))) in PowerShell 5, and you'll get a feel for why simply saying "ParameterSets" is not a solution to the mutually exclusive problem. The former clearly illustrates how overwhelming many parameter sets can be for the end user, and the latter clearly illustrates how challenging it can be to create commands with many parameter sets for the author. With this specific example, there are a number of design bugs in the Invoke-Command command that are very likely the result of the author not being able to identify them among the 15 parameter sets that are included in the command definition. Yet while many parameter sets complicate a command like this, providing mutual exclusivity support to allow authors to reduce the number of parameter sets is very likely not enough to isolate the parameters that matter so that users can clearly identify what is needed when using a command to get something done.
2. Mutual exclusivity is only one of several relationships between parameters/properties that are important. There are also parameters that require the use of other parameters in the same parameter set, where the command will fail if the required parameter is missing (this is different than mandatory, which is for parameters that are required all of the time). There are also parameters that automatically apply the use of other parameters in the same parameter set if those parameters are used. There may be other relationships as well. Each of these is in use in various commands in PowerShell today, and each of these relationships make it more difficult to learn how to use the commands that contain the relationships because there is no official mechanism for defining those relationships, so the relevant details are hidden inside parameter documentation and code.
3. The current syntax documentation that is shown in Get-Help output as well as Get-Command -Syntax output contributes to the problem because it overwhelms the end user for more complex commands with too much information to be helpful. With that syntax documentation, it is interesting to note that every parameter set on every command includes [<CommonParameters>], which represents the built-in, common parameters that apply to every command, including -ErrorAction, -PipelineVariable, and others. Imagine what the syntax would look like if every parameter set included these parameters in the syntax -- the result would be overwhelming. Having the ability to displace parameters that apply in multiple locations in the syntax is a helpful technique -- more on that later.

Whether or not you agree that these are challenges and whether or not you think they are in scope for this RFC, I would really like to see the RFC expanded upon to include impacts to syntax output in Get-Help/Get-Command -Syntax output, as well as in command authoring of a complex command like Invoke-Command. We should have a clear picture how both authors and end users will be helped by whatever solution is provided.

With these challenges in mind, since the problem seems to be larger than simply mutual exclusivity among parameters, I'd like to propose an alternate solution (draft, does not cover all use cases yet, but I need to share it before the RFC deadline closes):

[ValidateParameter()] attribute for parameters/properties and ParameterGroupName property added to Parameter attribute

Since there are multiple relationships between parameters/properties that are worth considering, I don't think either the MutuallyExclusive attribute or the ExclusionSet attribute set us up for solving many of the issues identified here. I would prefer to see an alternative ValidateParameter attribute as well as a ParameterGroupName property for the current Parameter attribute. Here's an example showing both of these attributes in an Invoke-Command function that tries to provide the same functionality that exists in the current Invoke-Command cmdlet, but with less complexity in the definition (and fixing some of the issues in the current definition as well):

function Invoke-Command {
    [CmdletBinding(DefaultParameterSetName='ScriptBlock')]
    [OutputType([System.Object])]
    param(
        [Parameter(ParameterSetName='ScriptBlock', Mandatory=$true, Position=0)]
        [ValidateNotNull()]
        [Alias('Command')]
        [scriptblock]
        $ScriptBlock,

        [Parameter(ParameterSetName='FilePath', Mandatory=$true, Position=0)]
        [ValidateNotNull()]
        [Alias('PSPath')]
        [string]
        $FilePath,

        [Alias('Args')]
        [System.Object[]]
        $ArgumentList,

        [Parameter(ValueFromPipeline=$true)]
        [psobject]
        $InputObject,

        [ValidateSet('LocalComputer','PsSession','RemoteComputer','Uri','VmById','VmByName','ContainerById','ContainerByName')]
        [string]
        $TargetType = 'LocalComputer',

        [ValidateParameter('TargetType', Eq='LocalComputer')]
        [switch]
        $NoNewScope,

        [Parameter(Mandatory=$true, Position=1)]
        [ValidateParameter('TargetType', Eq='PsSession')]
        [ValidateNotNullOrEmpty()]
        [System.Management.Automation.Runspaces.PSSession[]]
        $Session,

        [Parameter(Mandatory=$true, Position=1)]
        [ValidateParameter('TargetType', Eq='RemoteComputer')]
        [ValidateNotNullOrEmpty()]
        [Alias('Cn')]
        [string[]]
        $ComputerName,

        [ValidateParameter('TargetType', Eq='RemoteComputer')]
        [ValidateRange(1, 65535)]
        [int]
        $Port,

        [ValidateParameter('TargetType', Eq='RemoteComputer')]
        [switch]
        $UseSsl,

        [Parameter(Mandatory=$true, Position=1)]
        [ValidateParameter('TargetType', Eq='Uri')]
        [ValidateNotNullOrEmpty()]
        [Alias('URI','CU')]
        [uri[]]
        $ConnectionUri,

        [ValidateParameter('TargetType', Eq='Uri')]
        [switch]
        $AllowRedirection,

        [Parameter(ValueFromPipelineByPropertyName=$true)]
        [ValidateParameter('TargetType', In='RemoteComputer','Uri')]
        [ValidateParameter('TargetType', In='VmById','VmByName', Mandatory=$true)]
        [pscredential]
        [System.Management.Automation.CredentialAttribute()]
        $Credential,

        [ValidateParameter('TargetType', In='RemoteComputer','Uri')]
        [System.Management.Automation.Runspaces.AuthenticationMechanism]
        $Authentication,

        [ValidateParameter('TargetType', In='RemoteComputer','Uri')]
        [System.Management.Automation.Remoting.PSSessionOption]
        $SessionOption,

        [ValidateParameter('TargetType', In='RemoteComputer','Uri')]
        [switch]
        $EnableNetworkAccess,

        [Parameter(ValueFromPipelineByPropertyName=$true)]
        [ValidateParameter('TargetType', In='RemoteComputer','Uri')]
        [ValidateNotNullOrEmpty()]
        [string]
        $ConfigurationName,

        [Parameter(ValueFromPipelineByPropertyName=$true)]
        [ValidateParameter('TargetType', Eq='RemoteComputer')]
        [ValidateNotNullOrEmpty()]
        [string]
        $ApplicationName,

        [ValidateParameter('TargetType', In='ComputerName','Uri')]
        [ValidateNotNullOrEmpty()]
        [string]
        $CertificateThumbprint, 

        [ValidateParameter('TargetType', In='ComputerName','Uri')]
        [Alias('Disconnected')]
        [switch]
        $InDisconnectedSession,

        [ValidateParameter('TargetType', In='ComputerName','Uri')]
        [ValidateParameter('InDisconnectedSession', IsPresent=$true)]
        [ValidateNotNullOrEmpty()]
        [string[]]
        $SessionName,

        [Parameter(Mandatory=$true, Position=1, ValueFromPipelineByPropertyName=$true)]
        [ValidateParameter('TargetType', Eq='VmById')]
        [ValidateNotNullOrEmpty()]
        [Alias('VMGuid')]
        [guid[]]
        $VmId,

        [Parameter(Mandatory=$true, ValueFromPipelineByPropertyName=$true)]
        [ValidateParameter('TargetType', Eq='VmByName')]
        [ValidateNotNullOrEmpty()]
        [string[]]
        $VmName,

        [Parameter(Mandatory=$true, ValueFromPipelineByPropertyName=$true)]
        [ValidateParameter('TargetType', Eq='ContainerById')]
        [ValidateNotNullOrEmpty()]
        [string[]]
        $ContainerId,

        [Parameter(Mandatory=$true, ValueFromPipelineByPropertyName=$true)]
        [ValidateParameter('TargetType', Eq='ContainerByName')]
        [ValidateNotNullOrEmpty()]
        [string[]]
        ${ContainerName},

        [ValidateParameter('TargetType', In='ContainerById','ContainerByName')]
        [switch]
        ${RunAsAdministrator},

        [Parameter(ParameterGroupName='RemoteCommandOptionParameters')]
        [ValidateParameter('TargetType', Ne='LocalComputer')]
        [int]
        $ThrottleLimit,

        [Parameter(ParameterGroupName='RemoteCommandOptionParameters')]
        [ValidateParameter('TargetType', Ne='LocalComputer')]
        [switch]
        $AsJob,

        [Parameter(ParameterGroupName='RemoteCommandOptionParameters')]
        [ValidateParameter('TargetType', Ne='LocalComputer')]
        [Alias('HCN')]
        [switch]
        $HideComputerName,

        [Parameter(ParameterGroupName='RemoteCommandOptionParameters')]
        [ValidateParameter('TargetType', Ne='LocalComputer')]
        [string]
        $JobName
    )
    # Implementation goes here
}

Similarly, here is an alternate representation of the command syntax that could show up in Get-Help or Get-Command -Syntax output for that same command:

SYNTAX
    Invoke-Command [-ScriptBlock] <ScriptBlock> [-ArgumentList <Object[]>] [-InputObject <PSObject>] [-TargetType
    <String>] [<TargetTypeDependentParameters>] [<CommonParameters>]

    Invoke-Command [-FilePath] <String> [-ArgumentList <Object[]>] [-InputObject <PSObject>] [-TargetType <String>]
    [<TargetTypeDependentParameters>] [<CommonParameters>]

    [<TargetTypeDependentParameters>]
    LocalComputer   : [-NoNewScope]
    PsSession       : [-Session] <PSSession[]> [<RemoteCommandOptionParameters>]
    RemoteComputer  : [-ComputerName] <String[]> [-Port <Int32>] [-UseSsl] [-Credential <PSCredential>]
                      [-Authentication <AuthenticationMechanism>] [-SessionOption <PSSessionOption>]
                      [-EnableNetworkAccess] [-ConfigurationName <String>] [-ApplicationName <String>]
                      [-CertificateThumbprint <String>] [-InDisconnectedSession] [-SessionName <String>]
                      [<RemoteCommandOptionParameters>]
    Uri             : [-ConnectionUri] <Uri[]> [-AllowRedirection] [-Credential <PSCredential>] [-Authentication
                      <AuthenticationMechanism>] [-SessionOption <PSSessionOption>] [-EnableNetworkAccess]
                      [-ConfigurationName <String>] [-CertificateThumbprint <String>] [-InDisconnectedSession]
                      [-SessionName <String>] [<RemoteCommandOptionParameters>]
    VmById          : [-VmId] <Guid[]> [-Credential] <PSCredential> [<RemoteCommandOptionParameters>]
    VmByName        : -VmName <String[]> [-Credential] <PSCredential> [<RemoteCommandOptionParameters>]
    ContainerById   : -ContainerId <String[]> [-RunAsAdministrator] [<RemoteCommandOptionParameters>]
    ContainerByName : -ContainerName <String[]> [-RunAsAdministrator] [<RemoteCommandOptionParameters>]

    [<RemoteCommandOptionParameters>]
    [-ThrottleLimit <Int32>] [-AsJob] [-JobName <String>] [-HideComputerName]

Compare these to the results of the two PowerShell commands mentioned much earlier showing the poor authoring experience and help experience that are provided today for this particular command.

Pros:

1. The number of annotations required on parameters is greatly reduced, and code follows the DRY principle much more closely.
2. The ValidateParameter attribute supports a much broader range of comparisons than mutual exclusion.
3. The ParameterGroupName property dramatically reduces duplication of parameters in the syntax output for multiple parameter sets, allows authors to logically group like parameters together in named groupings that help identify the purpose of those parameters, and makes it easier for end users to logically put commands together in steps.

Cons:

1. The current design doesn't include validating an entire group of parameters against another parameter...I haven't yet figured out an elegant way to do this while keeping the code DRY.
2. The scope of this proposal will seem broader than what you asked for in this RFC (however, I truly believe that there is a lot more that should be looked at here than simply mutual exclusion).

Regardless of which way this proposal goes moving forward, I would like to see practical examples for existing, published commands that could take advantage of the additional parameter validation that is being added, both from the author's perspective and from the end users perspective, so that we're making authoring easier, making documentation smarter, and making command use easier. For what it's worth, I attempted to design this for help documentation first, and then once I found a syntax that was easier to consume and that fit my mental model better when using complex commands, then I tried to figure out what metadata I could add to parameters that would allow PowerShell to give me that documentation. Looking at this from a class-perspective first doesn't take documentation into account, and downplays some of the bigger issues that come from multiple parameter sets for authors/end users that are worth solving.

That's enough from me for now. I wrote this about 6 times before finally putting thoughts together here to share. Hopefully what I'm trying to illustrate here actually makes a little sense to you.

[KirkMunro]

I just remembered this morning one other area where the user is heavily impacted by the direction this goes: user experiences wrapped around specific PowerShell commands. It's not hard to imagine that such experiences could be built to wrap around PowerShell classes as well. For an example, consider this:

Show-Command -Name Invoke-Command

The UX with that dialog is not abyssmal, but it's much more challenging than it needs to be. If you were to build a UX on top of commands that had additional metadata such as the proposed ValidateParameterAttribute and ParameterGroupName property of the Parameter attribute, you could build something much more intuitive that breaks a command down into the way people think.

Other places where the UX built from command (or class) metadata matters is in Azure Automation and SMA. In these environments, runbook activities auto-generated from metadata surrounding PowerShell entities like commands (and maybe classes at some point) can be better tailored according to how the author of the command defines that command. You can imagine a toolset where while building a command, you see the syntax output for that command displayed inside another pane in the tool where you are creating the command, as well as the Show-Command UI output auto-updating according to the command definition.

How these experiences will be impacted (hopefully improved) by changes to parameter/property metadata should be considered by enhancements to metadata like this RFC is proposing.

[SteveL-MSFT]

Thanks for all the feedback. At this time, I'm going to withdraw this RFC. The feedback makes it clear that the current proposal doesn't completely solve the problem without introducing new problems in readability. Any RFC for this issue should also (as @KirkMunro pointed out) include addressing making help more readable as generating many parameter sets makes it much less readable for end users. The original spark that started this RFC was for DSC classes, but certainly any solution needs to work for general PowerShell classes as well as advanced functions. If anyone wants to continue to pursue this, please submit a new RFC.