ActiveDirectoryDsc: Using splatting when calling cmdlets where line exceeds 120 characters

[johlju]

There are places in the code that make calls to cmdlets and use a long list of parameters.

Example: https://github.com/PowerShell/xActiveDirectory/blob/07c878522df6015f5a74692e931ddeeeb5d9decf/DSCResources/MSFT_xADOrganizationalUnit/MSFT_xADOrganizationalUnit.psm1#L28

We should use splatting at these locations throughout the repo to make the code easier to read. See style guideline https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-parameter-usage-in-function-and-cmdlet-calls

[johlju]

I gathered the lines that could potentially be affected using the script Get-ScriptLinesWithMinimumLength.ps1.

Also the 120 character limit used is calculated including any starting whitespace. 120 characters is not a hard limit, just a hint what might look better in review tools.

Any of the below lines might not be better off splitting, splatting or refactoring. Use common sense. 🙂

A bug in the script outputted the same lines several times. Missed that at first. 😕 Below should be a correct output.

List updated 25/8/2020

Script                                  Line Length Text
------                                  ---- ------ ----
MSFT_ADComputer.psm1                     169    121         $computerObjectProperties = Convert-PropertyMapToObjectProperties -PropertyMap $script:computerObjectPro...
MSFT_ADDomain.psm1                        76    127         $sysvolPath = Get-ItemPropertyValue -Path 'HKLM:\SYSTEM\CurrentControlSet\Services\Netlogon\Parameters' ... 
MSFT_ADDomainDefaultPasswordPolicy.psm1   96    124         LockoutObservationWindow    = ConvertFrom-Timespan -Timespan $policy.LockoutObservationWindow -TimeSpanT... 
MSFT_ADDomainTrust.psm1                  101    138         Write-Verbose -Message ($script:localizedData.TrustPresentMessage -f $SourceDomainName, $TargetDomainNam...
MSFT_ADDomainTrust.psm1                  107    137         Write-Verbose -Message ($script:localizedData.TrustAbsentMessage -f $SourceDomainName, $TargetDomainName...
MSFT_ADForestProperties.psm1             231    125     if (-not ( Test-Members @assertMemberParameters -ExistingMembers ($targetResource.ServicePrincipalNameSuffix... 
MSFT_ADForestProperties.psm1             259    122     if (-not ( Test-Members @assertMemberParameters -ExistingMembers ($targetResource.UserPrincipalNameSuffix -s... 
MSFT_ADGroup.psm1                        440    136         Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f 'GroupScope', $GroupScope, $tar...
MSFT_ADGroup.psm1                        446    130         Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f 'Category', $Category, $targetR...
MSFT_ADGroup.psm1                        458    139         Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f 'Description', $Description, $t... 
MSFT_ADGroup.psm1                        464    139         Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f 'DisplayName', $DisplayName, $t... 
MSFT_ADGroup.psm1                        470    133         Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f 'ManagedBy', $ManagedBy, $targe...
MSFT_ADGroup.psm1                        476    121         Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f 'Notes', $Notes, $targetResourc... 
MSFT_ADGroup.psm1                        489    124         Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f 'Ensure', $Ensure, $targetResou... 
MSFT_ADGroup.psm1                        702    125                 Set-ADGroup -Identity $getTargetResourceResult.DistinguishedName -GroupScope 'Universal' -ErrorA... 
MSFT_ADGroup.psm1                        840    133                             Write-Verbose -Message ($script:localizedData.RemovingGroupMembers -f $adGroupMember...
MSFT_ADGroup.psm1                        866    129                         Write-Verbose -Message ($script:localizedData.AddingGroupMembers -f $MembersToInclude.Co... 
MSFT_ADGroup.psm1                        881    131                         Write-Verbose -Message ($script:localizedData.RemovingGroupMembers -f $MembersToExclude.... 
MSFT_ADGroup.psm1                        996    121                 Write-Verbose -Message ($script:localizedData.AddingGroupMembers -f $MembersToInclude.Count, $Gr...
MSFT_ADKDSKey.psm1                       489    124     Write-Verbose -Message ($script:localizedData.CheckingDomainAdminComputerRights -f $osInfo.CSName, $osInfo.P...
MSFT_ADManagedServiceAccount.psm1        329    122                     -IgnoreProperties 'DomainController', 'Credential' | Where-Object -Property InDesiredState -... 
MSFT_ADManagedServiceAccount.psm1        528    122                     -IgnoreProperties 'DomainController', 'Credential' | Where-Object -Property InDesiredState -...
MSFT_ADManagedServiceAccount.psm1        566    125                                 $AccountType, $ServiceAccountName, $property.ParameterName, ($property.Expected ... 
MSFT_ADOptionalFeature.psm1               54    146         $feature = Get-ADOptionalFeature -Identity $FeatureName -Server $forest.DomainNamingMaster -Credential $...
MSFT_ADOptionalFeature.psm1              140    146         $feature = Get-ADOptionalFeature -Identity $FeatureName -Server $forest.DomainNamingMaster -Credential $... 
MSFT_ADReplicationSite.psm1              107    135             $defaultFirstSiteName = Get-ADReplicationSite -Filter { Name -eq 'Default-First-Site-Name' } -ErrorA...
MSFT_ADReplicationSubnet.psm1            179    121             Set-ADReplicationSubnet -Identity $replicationSubnet.DistinguishedName -Location $nullableLocation -... 
MSFT_ADServicePrincipalName.psm1          50    134         Write-Verbose -Message ($script:localizedData.ServicePrincipalNamePresent -f $ServicePrincipalName, ($sp...
MSFT_ADServicePrincipalName.psm1          98    141     $spnAccounts = Get-ADObject -Filter { ServicePrincipalName -eq $ServicePrincipalName } -Properties 'SamAccou... 
MSFT_ADServicePrincipalName.psm1         106    122         if ([System.String]::IsNullOrEmpty($Account) -or ($null -eq (Get-ADObject -Filter { SamAccountName -eq $...
MSFT_ADServicePrincipalName.psm1         116    143                 Write-Verbose -Message ($script:localizedData.RemoveServicePrincipalName -f $ServicePrincipalNam... 
MSFT_ADServicePrincipalName.psm1         145    139             Write-Verbose -Message ($script:localizedData.RemoveServicePrincipalName -f $ServicePrincipalName, $...
ActiveDirectoryDsc.Common.psm1           369    125             Write-Verbose -Message ($script:localizedData.MembershipCountMismatch -f $Members.Count, $ExistingMe... 

[camusicjunkie]

A lot of the long lines are the ones where formatting is taking place. Would you be OK with something like this?

Before

Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f 'DisplayName', $DisplayName, $targetResource.DisplayName)
$targetResourceInCompliance = $false

After

Write-Verbose -Message (
    $script:localizedData.NotDesiredPropertyState -f 'ManagedBy', $ManagedBy, $targetResource.ManagedBy
)
$targetResourceInCompliance = $false

or

After

Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f
    'ManagedBy', $ManagedBy, $targetResource.ManagedBy
)
$targetResourceInCompliance = $false

or this last example based off the Style Guidelines

After

Write-Verbose -Message `
    ($script:localizedData.NotDesiredPropertyState -f 'ManagedBy', $ManagedBy, $targetResource.ManagedBy)
$targetResourceInCompliance = $false

[johlju]

I’m good with the first one:

Write-Verbose -Message (
    $script:localizedData.NotDesiredPropertyState -f 'ManagedBy', $ManagedBy, $targetResource.ManagedBy
)
$targetResourceInCompliance = $false

Or if there are one with more variables to format we could use an array after -f.

Write-Verbose -Message (
    $script:localizedData.NotDesiredPropertyState -f @(
        'ManagedBy', $ManagedBy, $targetResource.ManagedBy
    )
)
$targetResourceInCompliance = $false

Since this issue is a bit old now let’s ask for @X-Guardian opinion too. 🙂

[X-Guardian]

Powershell and PSScriptAnalyzer naturally splits and indents after the -f if you wrap the Message parameter in brackets, so:

Write-Verbose -Message ($script:localizedData.NotDesiredPropertyState -f
    'ManagedBy', $ManagedBy, $targetResource.ManagedBy)

This pattern is used throughout the newer and refactored resources (ADFineGrainedPasswordPolicy, ADManagedServiceAccount etc)

[johlju]

Sometime there are arguments that are longer so using a array makes it possible to do this

Write-Verbose -Message (
    $script:localizedData.NotDesiredPropertyState -f @(
        ('ManagedBy{0}{1}' -f property1, property2)
        $ManagedBy
        $targetResource.ManagedBy
    )
)

But then, that probably should be formatted on a row before the Write-Verbose 😊

So I’m good with @x-guardian’s suggestion.