Update UseIdenticalMandatoryParametersForDSC logic

This update the logic in check for properties that have the `required` attribute. Also updates the error string to report if the missing parameters is `key` or `required`.

Author: Kapil Borle

Rules/Strings.resx

@@ -619,7 +619,7 @@
     <value>The Get/Test/Set TargetResource functions of DSC resource must have the same mandatory parameters.</value>
   </data>
   <data name="UseIdenticalMandatoryParametersDSCError" xml:space="preserve">
-    <value>The mandatory parameter '{0}' is not present in '{1}' DSC resource function(s).</value>
+    <value>The '{0}' parameter '{1}' is not present in '{2}' DSC resource function(s).</value>
   </data>
   <data name="UseIdenticalMandatoryParametersDSCName" xml:space="preserve">
     <value>UseIdenticalMandatoryParametersForDSC</value>

Rules/UseIdenticalMandatoryParametersDSC.cs

@@ -56,30 +56,27 @@ public IEnumerable<DiagnosticRecord> AnalyzeDSCResource(Ast ast, string fileName
             }
 
             // Get the keys in the corresponding mof file
-            var keys = GetKeys(fileName);
+            var propAttrDict = GetKeys(fileName);
 
             // Loop through Set/Test/Get TargetResource DSC cmdlets
             foreach (FunctionDefinitionAst functionDefinitionAst in Helper.Instance.DscResourceFunctions(ast))
             {
-                var manParams = new HashSet<string>(
-                    GetMandatoryParameters(functionDefinitionAst).Select(p => p.Name.VariablePath.UserPath),
-                    StringComparer.OrdinalIgnoreCase);
+                var manParams = GetMandatoryParameters(functionDefinitionAst)
+                                    .Select(p => p.Name.VariablePath.UserPath);
 
-                foreach (var key in keys)
+                foreach (var key in propAttrDict.Keys.Except(manParams))
                 {
-                    if (!manParams.Contains(key))
-                    {
-                        yield return new DiagnosticRecord(
-                         string.Format(
-                             CultureInfo.InvariantCulture,
-                             Strings.UseIdenticalMandatoryParametersDSCError,
-                             key,
-                             functionDefinitionAst.Name),
-                         Helper.Instance.GetScriptExtentForFunctionName(functionDefinitionAst),
-                         GetName(),
-                         DiagnosticSeverity.Error,
-                         fileName);
-                    }
+                    yield return new DiagnosticRecord(
+                     string.Format(
+                         CultureInfo.InvariantCulture,
+                         Strings.UseIdenticalMandatoryParametersDSCError,
+                         propAttrDict[key],
+                         key,
+                         functionDefinitionAst.Name),
+                     Helper.Instance.GetScriptExtentForFunctionName(functionDefinitionAst),
+                     GetName(),
+                     DiagnosticSeverity.Error,
+                     fileName);
                 }
             }
         }
@@ -175,19 +172,19 @@ private bool IsNamedAttributeArgumentMandatory(NamedAttributeArgumentAst namedAt
                     namedAttrArgAst.GetValue();
         }
 
-        private IEnumerable<string> GetKeys(string fileName)
+        private IDictionary<string, string> GetKeys(string fileName)
         {
             var moduleInfo = GetModuleInfo(fileName);
-            var emptyArray = new string[0];
+            var emptyDictionary = new Dictionary<string, string>();
             if (moduleInfo == null)
             {
-                return emptyArray;
+                return emptyDictionary;
             }
 
             var mofFilepath = GetMofFilepath(fileName);
             if (mofFilepath == null)
             {
-                return emptyArray;
+                return emptyDictionary;
             }
 
             var errors = new System.Collections.ObjectModel.Collection<Exception>();
@@ -208,13 +205,22 @@ private IEnumerable<string> GetKeys(string fileName)
                 // todo log the error
             }
 
-            return cimClasses?
-                    .FirstOrDefault()?
+            var cimClass = cimClasses?.FirstOrDefault();
+            var cimSuperClassProperties = new HashSet<string>(
+                cimClass?.CimSuperClass.CimClassProperties.Select(p => p.Name) ??
+                Enumerable.Empty<string>());
+
+            return cimClass?
                     .CimClassProperties?
-                    .Where(p => p.Flags.HasFlag(CimFlags.Key))
-                    .Select(p => p.Name)
-                    .ToArray() ??
-                    emptyArray;
+                    .Where(p => (p.Flags.HasFlag(CimFlags.Key) ||
+                            p.Flags.HasFlag(CimFlags.Required)) &&
+                            !cimSuperClassProperties.Contains(p.Name))
+                    .ToDictionary(
+                        p => p.Name,
+                        p => p.Flags.HasFlag(CimFlags.Key) ?
+                                CimFlags.Key.ToString() :
+                                CimFlags.Required.ToString()) ??
+                    emptyDictionary;
         }
 
         private string GetMofFilepath(string filePath)

Tests/Rules/DSCResourceModule/DSCResources/MSFT_WaitForAnyNoIdenticalMandatoryParameter/MSFT_WaitForAnyNoIdenticalMandatoryParameter.psm1

@@ -5,8 +5,9 @@
 #
 # The Get-TargetResource cmdlet.
 #
-function Get-TargetResource
-{
+function Get-TargetResource {
+
+    # This is missing `Key` properties `ResourceName` and `Dummy`
     param
     (
         [parameter(Mandatory)]
@@ -17,7 +18,7 @@ function Get-TargetResource
         [ValidateNotNullOrEmpty()]
         [PSCredential] $Credential,
 
-        [ValidateRange(1,[Uint64]::MaxValue)]
+        [ValidateRange(1, [Uint64]::MaxValue)]
         [Uint64] $RetryIntervalSec = 1,
 
         [Uint32] $RetryCount = 0,
@@ -31,16 +32,13 @@ function Get-TargetResource
 
     $b = @{"hash" = "table"}
 
-    if ($true)
-    {
+    if ($true) {
         return $b;
     }
-    elseif ($c)
-    {
-        return @{"hash2"="table2"}
+    elseif ($c) {
+        return @{"hash2" = "table2"}
     }
-    else
-    {
+    else {
         # can't determine type of c so error should not be raised as we're trying to be conservative
         return $c;
     }
@@ -49,8 +47,9 @@ function Get-TargetResource
 #
 # The Set-TargetResource cmdlet.
 #
-function Set-TargetResource
-{
+function Set-TargetResource {
+
+    # This is missing `required` property `credential` and `key` property `Dummy`
     param
     (
         [parameter(Mandatory)]
@@ -61,11 +60,7 @@ function Set-TargetResource
         [ValidateNotNullOrEmpty()]
         [string[]] $NodeName,
 
-        [parameter(Mandatory)]
-        [ValidateNotNullOrEmpty()]
-        [PSCredential] $Credential,
-
-        [ValidateRange(1,[Uint64]::MaxValue)]
+        [ValidateRange(1, [Uint64]::MaxValue)]
         [Uint64] $RetryIntervalSec = 1,
 
         [Uint32] $RetryCount = 0,
@@ -77,39 +72,38 @@ function Set-TargetResource
 
     Import-Module $PSScriptRoot\..\..\PSDSCxMachine.psm1
 
-    if ($PSBoundParameters["Verbose"])
-    {
+    if ($PSBoundParameters["Verbose"]) {
         Write-Verbose "Calling xMachine with Verbose parameter"
 
         PSDSCxMachine\Set-_InternalPSDscXMachineTR `
-               -RemoteResourceId $ResourceName `
-               -RemoteMachine $NodeName `
-               -RemoteCredential $Credential `
-               -MinimalNumberOfMachineInState 1 `
-               -RetryIntervalSec $RetryIntervalSec `
-               -RetryCount $RetryCount `
-               -ThrottleLimit $ThrottleLimit `
-               -Verbose
+            -RemoteResourceId $ResourceName `
+            -RemoteMachine $NodeName `
+            -RemoteCredential $Credential `
+            -MinimalNumberOfMachineInState 1 `
+            -RetryIntervalSec $RetryIntervalSec `
+            -RetryCount $RetryCount `
+            -ThrottleLimit $ThrottleLimit `
+            -Verbose
     }
-    else
-    {
+    else {
         PSDSCxMachine\Set-_InternalPSDscXMachineTR `
-               -RemoteResourceId $ResourceName `
-               -RemoteMachine $NodeName `
-               -RemoteCredential $Credential `
-               -MinimalNumberOfMachineInState 1 `
-               -RetryIntervalSec $RetryIntervalSec `
-               -RetryCount $RetryCount `
-               -ThrottleLimit $ThrottleLimit
+            -RemoteResourceId $ResourceName `
+            -RemoteMachine $NodeName `
+            -RemoteCredential $Credential `
+            -MinimalNumberOfMachineInState 1 `
+            -RetryIntervalSec $RetryIntervalSec `
+            -RetryCount $RetryCount `
+            -ThrottleLimit $ThrottleLimit
     }
 }
 
 #
 # Test-TargetResource
 #
 #
-function Test-TargetResource
-{
+function Test-TargetResource {
+
+    # This is missing `key` property `Dummy`
     param
     (
         [parameter(Mandatory)]
@@ -139,20 +133,16 @@ function Test-TargetResource
     $a = $true
     $a
 
-    if ($true)
-    {
+    if ($true) {
         $false;
     }
-    elseif ($b)
-    {
+    elseif ($b) {
         return $a -or $true
     }
-    elseif ($c)
-    {
+    elseif ($c) {
         return $false;
     }
-    else
-    {
+    else {
         return $true
     }
 }

Tests/Rules/UseIdenticalMandatoryParametersForDSC.tests.ps1

@@ -16,7 +16,7 @@ Describe "UseIdenticalMandatoryParametersForDSC" {
         }
 
         It "Should find a violations" {
-            $violations.Count | Should Be 4
+            $violations.Count | Should Be 5
         }
 
         It "Should mark only the function name" {