# Azure PowerShell: mostly harmless

Hello, everyone\. Today we have another Microsoft project on the check\. By the title of this article, you can guess that this time developers didn't "please" us with a large number of errors\. We hope the project's authors won't be offended by the title\. After all, a small number of errors is great, isn't it? However, we still managed to find something intriguing in the Azure PowerShell code\. We suggest getting to know the features of this project and checking out errors, found using the PVS\-Studio C\# analyzer\.

![0678_azure_powershell/image1.png](https://import.viva64.com/docx/blog/0678_azure_powershell/image1.png)

## About the project

Azure PowerShell is a set of commandlets \([cmdlet](https://docs.microsoft.com/en-us/powershell/azure/?view=azps-4.8.0)\) that lets you manage [Azure](https://azure.microsoft.com/en-us/) resources right from the [PowerShell](https://docs.microsoft.com/en-us/powershell/) command line\. The main purpose of this set is to simplify the process of studying and quickly start the development for Azure\. Azure PowerShell provides administrators and developers with compelling features to create, deploy, and manage Microsoft Azure applications\.

Azure PowerShell is developed in the \.NET Standard environment, is supported by PowerShell 5\.1 for Windows and PowerShell 6\.x and later for all platforms\. The  Azure PowerShell [source code](https://github.com/Azure/azure-powershell) is available on GitHub\.

Recently I've been often getting Microsoft projects for a check\. In my opinion, the quality of these projects is usually top\-notch\. Although, of course, not without exceptions, as described in the article "[WinForms: errors, Holmes](https://pvs-studio.com/en/blog/posts/csharp/0653/)"\. But this time everything is fine\. The project is large: 6845 \.cs source code files contain approximately 700,000 lines, excluding blank ones \(I didn't take into account tests and warnings of the third level of certainty\)\. Very few errors were found for such amount of code: no more than a hundred\.  There were quite many similar cases, so I chose the most interesting ones for the article\. As usually, I sorted errors by the PVS\-Studio warnings' numbers\.

Also I stumbled upon some code fragments that looked like errors, but couldn't be recognized as definitely erroneous, as I'm not familiar enough with PowerShell development peculiarities\. I hope that among readers there will be specialists in this issue who will help me\. I'll describe it in detail below\.

Before the feedback part, I'd like to mention the specific structure of the project\. The Azure PowerShell source code consists of more than seventy Visual Studio solutions\. Some solutions include projects from other ones\. This structure slowed down the analysis a little \(not much\)\. Still the check didn't cause any other difficulties\. For convenience, in the error message \(in brackets\) I will specify the name of the solution where the error was found\.

## Analysis results

[V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'strTimespan\.Contains\("M"\)' to the left and to the right of the '\|\|' operator\. AzureServiceBusCmdletBase\.cs 187 \(EventGrid\)

```cpp
public static TimeSpan ParseTimespan(string strTimespan)
{
  ....
  if (strTimespan.Contains("P") 
    || strTimespan.Contains("D") 
    || strTimespan.Contains("T") 
    || strTimespan.Contains("H") 
    || strTimespan.Contains("M") 
    || strTimespan.Contains("M"))
  ....
}
```

An example of a fairly obvious error that only a developer can fix\. In this case, it's absolutely unclear whether we deal with code duplication that affects nothing or something else has to take place instead of _"M"_ in one of two last checks\.

[V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'this\.AggregationType \!\= null' to the left and to the right of the '&&' operator\. GetAzureRmMetricCommand\.cs 156 \(Monitor\)

```cpp
public AggregationType? AggregationType { get; set; }
....
protected override void ProcessRecordInternal()
{
  ....
  string aggregation = (this.AggregationType != null &&
    this.AggregationType.HasValue) ?
    this.AggregationType.Value.ToString() : null;
  ....
}
```

There's probably no error here\. This is an example of redundant code\. Sometimes such code might indicate lack of developer's knowledge\. The point is that the checks _this\.AggregationType \!\= null_ and_ this\.AggregationType\.HasValue_ are identical\.  It is enough to use only one of them \(any one\)\. Personally, I prefer the option with _HasValue:_

```cpp
string aggregation = this.AggregationType.HasValue ?
  this.AggregationType.Value.ToString() : 
  null;
```

[V3003](https://pvs-studio.com/en/docs/warnings/v3003/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 152, 163\. GetAzureRmRecoveryServicesBackupProtectionPolicy\.cs 152 \(RecoveryServices\)

```cpp
public override void ExecuteCmdlet()
{
  ....
  if( WorkloadType == Models.WorkloadType.AzureVM )
  {
    ....
  }
  ....
  else if( WorkloadType == Models.WorkloadType.AzureFiles )
  {
    if( BackupManagementType != Models.BackupManagementType.AzureStorage )
    {
      throw new ArgumentException(
        Resources.AzureFileUnsupportedBackupManagementTypeException );
    }
    serviceClientProviderType = ServiceClientHelpers.
      GetServiceClientProviderType( Models.WorkloadType.AzureFiles );
  }
  else if( WorkloadType == Models.WorkloadType.AzureFiles )
  {
    if( BackupManagementType != Models.BackupManagementType.AzureStorage )
    {
      throw new ArgumentException(
        Resources.AzureFileUnsupportedBackupManagementTypeException );
    }
    serviceClientProviderType = ServiceClientHelpers.
      GetServiceClientProviderType( Models.WorkloadType.AzureFiles );
  }
  ....
}
```

Two _else if_ blocks are absolutely identical, including both the condition and the body of the block\.  Such errors are usually made when using the copy\-paste method\. The issue here is again the error's criticality\. If it's not simple code duplication, it may be the absent needed check and appropriate set of actions\. The author definitely has to edit the code\.

[V3005](https://pvs-studio.com/en/docs/warnings/v3005/) The 'this\.VM\.OSProfile\.WindowsConfiguration\.ProvisionVMAgent' variable is assigned to itself\. SetAzureVMOperatingSystemCommand\.cs 298 \(Compute\)

```cpp
public override void ExecuteCmdlet()
{
  ....
  // OS Profile
  this.VM.OSProfile.WindowsConfiguration.ProvisionVMAgent =
    this.VM.OSProfile.WindowsConfiguration.ProvisionVMAgent;
  ....
}
```

The value of the property is self\-assigned\. Take a look at its declaration:

```cpp
[JsonProperty(PropertyName = "provisionVMAgent")]
public bool? ProvisionVMAgent { get; set; }
```

The _JsonProperty _description states: "Instructs the Newtonsoft\.Json\.JsonSerializer to always serialize the member with the specified name"\. It seems that everything is guileless and the obvious error has been made\. Explicit usage of _this_ to access the property is also quite confusing\. Perhaps, another variable wasn't mistakenly specified instead of _this\._ But let's not jump to conclusions\. The fact of the matter is that I came across quite a lot of such assignments \(a property is self\-assigned\)\. Here is an example of an assignment, very similar to an error: 

[V3005](https://pvs-studio.com/en/docs/warnings/v3005/) The 'this\.LastHeartbeat' variable is assigned to itself\. PSFabricDetails\.cs 804 \(RecoveryServices\)

```cpp
public ASRInMageAzureV2SpecificRPIDetails(
  InMageAzureV2ReplicationDetails details)
{
  this.LastHeartbeat = this.LastHeartbeat;  // <=
  this.RecoveryAvailabilitySetId = details.RecoveryAvailabilitySetId;
  this.AgentVersion = details.AgentVersion;
  this.DiscoveryType = details.DiscoveryType;
  ....
}
```

Let's have a closer look at the second and subsequent assignments\. In the right part of the expression, _details _takes place instead of _this\._ Now look at the declaration of the _this\.LastHeartbeat_ property:

```cpp
public DateTime? LastHeartbeat { get; set; }
```

At last, let's find the property with the same name in the _InMageAzureV2ReplicationDetails_ class\. Such property is declared there:

```cpp
public class InMageAzureV2ReplicationDetails :
  ReplicationProviderSpecificSettings
{
  ....
  [JsonProperty(PropertyName = "lastHeartbeat")]
  public DateTime? LastHeartbeat { get; set; }
  ....  
}
```

Well, in this case, I'm willing to admit it's a real error\. But what shall we do with the next warnings? Unlike two previous code fragments, there are multiple self\-assigned properties\. Well, this looks less like an error:

* [V3005](https://pvs-studio.com/en/docs/warnings/v3005/) The 'this\.ResourceGroupName' variable is assigned to itself\. RemoveAzureRmExpressRouteConnectionCommand\.cs 84 \(CognitiveServices\)
* [V3005](https://pvs-studio.com/en/docs/warnings/v3005/) The 'this\.ExpressRouteGatewayName' variable is assigned to itself\. RemoveAzureRmExpressRouteConnectionCommand\.cs 85  \(CognitiveServices\)
* [V3005](https://pvs-studio.com/en/docs/warnings/v3005/) The 'this\.Name' variable is assigned to itself\. RemoveAzureRmExpressRouteConnectionCommand\.cs 86  \(CognitiveServices\)

```cpp
[Cmdlet(VerbsCommon.Remove,
  ResourceManager.Common.AzureRMConstants.AzureRMPrefix +
    "ExpressRouteConnection",
  DefaultParameterSetName =
    CortexParameterSetNames.ByExpressRouteConnectionName,
  SupportsShouldProcess = true),
  OutputType(typeof(bool))]
public class RemoveExpressRouteConnectionCommand :
  ExpressRouteConnectionBaseCmdlet
{
  [Parameter(
    Mandatory = true,
    ParameterSetName = CortexParameterSetNames.ByExpressRouteConnectionName,
    HelpMessage = "The resource group name.")]
  [ResourceGroupCompleter]
  [ValidateNotNullOrEmpty]
  public string ResourceGroupName { get; set; }
  ....
  public override void Execute()
  {
    if (....)
    {
      this.ResourceGroupName = this.ResourceGroupName;
      this.ExpressRouteGatewayName = this.ExpressRouteGatewayName;
      this.Name = this.Name;
    }
    ....
  }    
  ....
}
```

The _Execute _method contains three properties' self\-assignments in a row\. Just in case, I cited full declaration of the class _RemoveExpressRouteConnectionCommand _and all its attributes, as well as the _ResourceGroupName_ property declaration \(other two properties are declared in a similar way\)\. It was these warnings that made me think about the question: "Is it an error?" I suspect that some internal magic of PowerShell development may be happening here\. I hope that among readers there will be experts who are informed about this issue\. I'm not ready to draw any conclusions in this case\.

[V3006](https://pvs-studio.com/en/docs/warnings/v3006/) The object was created but it is not being used\. The 'throw' keyword could be missing: throw new ArgumentException\(FOO\)\. StartAzureRmRecoveryServicesAsrTestFailoverJob\.cs 259 \(RecoveryServices\)

```cpp
private void StartRPITestFailover()
{
  ....
  if (....)
  {
    ....
  }
  else
  {
    new ArgumentException(
      Resources
        .UnsupportedDirectionForTFO); // Throw Unsupported Direction
                                      // Exception
  }
  ....
}
```

The _throw _keyword is omitted\. And the comment says that the exception just has to be thrown\. I encountered several more similar errors in the _RecoveryServices_ solution:

* V3006 The object was created but it is not being used\. The 'throw' keyword could be missing: throw new ArgumentException\(FOO\)\. StartAzureRmRecoveryServicesAsrTestFailoverJob\.cs 305 \(RecoveryServices\)
* V3006 The object was created but it is not being used\. The 'throw' keyword could be missing: throw new ArgumentException\(FOO\)\. StartAzureRmRecoveryServicesAsrUnPlannedFailover\.cs 278 \(RecoveryServices\)
* V3006 The object was created but it is not being used\. The 'throw' keyword could be missing: throw new ArgumentException\(FOO\)\. StartAzureRmRecoveryServicesAsrUnPlannedFailover\.cs 322 \(RecoveryServices\)
* V3006 The object was created but it is not being used\. The 'throw' keyword could be missing: throw new ArgumentException\(FOO\)\. UpdateAzureRmRecoveryServicesAsrProtectionDirection\.cs 421 \(RecoveryServices\)
* V3006 The object was created but it is not being used\. The 'throw' keyword could be missing: throw new ArgumentException\(FOO\)\. UpdateAzureRmRecoveryServicesAsrProtectionDirection\.cs 452 \(RecoveryServices\)

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'apiType\.HasValue' is always false\. ApiManagementClient\.cs 1134 \(ApiManagement\)

```cpp
private string GetApiTypeForImport(...., PsApiManagementApiType? apiType)
{
  ....
  if (apiType.HasValue)
  {
    switch(apiType.Value)
    {
      case PsApiManagementApiType.Http: return SoapApiType.SoapToRest;
      case PsApiManagementApiType.Soap: return SoapApiType.SoapPassThrough;
      default: return SoapApiType.SoapPassThrough;
    }
  }

  return apiType.HasValue ?        // <=
    apiType.Value.ToString("g") : 
    PsApiManagementApiType.Http.ToString("g");
}
```

The logic of the work has been broken\. If _apiType_ contains a value, the control won't reach the _return _expression at the end of the method \(all _switch _branches contain _return_\)\. Otherwise, the method will always return _PsApiManagementApiType\.Http\.ToString\("g"\)_, whereas the _apiType\.Value\.ToString\("g"\)_ value will never be returned\.

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'automationJob \!\= null && automationJob \=\= null' is always false\. NodeConfigurationDeployment\.cs 199 \(Automation\)

```cpp
public NodeConfigurationDeployment(
  ....,
  Management.Automation.Models.Job automationJob = null, 
  ....)
{
  ....
  if (automationJob != null && automationJob == null) return;
  ....
}
```

Counter\-intuitive code\. Two checks that contradict each other\. Probably the second check for _null_ contains the wrong variable\. 

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression is always false\. DataFactoryClient\.Encrypt\.cs 37 \(DataFactory\)

```cpp
public virtual string OnPremisesEncryptString(....)
{
  ....
  if ( linkedServiceType == LinkedServiceType.OnPremisesSqlLinkedService 
    && linkedServiceType == LinkedServiceType.OnPremisesOracleLinkedService
    && linkedServiceType == LinkedServiceType.OnPremisesFileSystemLinkedService
    && (value == null || value.Length == 0))
  {
    throw new ArgumentNullException("value");
  }
  ....
}
```

The check is pointless and the exception will never be thrown\. The condition requires simultaneous _linkedServiceType_ variable's equality to three different values\. The operators && and \|\| are likely to be confused\.  Fixed code:

```cpp
if (( linkedServiceType == LinkedServiceType.OnPremisesSqlLinkedService 
  || linkedServiceType == LinkedServiceType.OnPremisesOracleLinkedService
  || linkedServiceType == LinkedServiceType.OnPremisesFileSystemLinkedService)
  && (value == null || value.Length == 0))
....
```

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'Ekus \=\= null' is always false\. PSKeyVaultCertificatePolicy\.cs 129 \(KeyVault\)

```cpp
internal CertificatePolicy ToCertificatePolicy()
{
  ....
  if (Ekus != null)
  {
    x509CertificateProperties.Ekus = Ekus == null ? 
      null : new List<string>(Ekus);
  }                
  ....
}
```

Redundant check of the _Ekus _variable for _null_\.  It's probably fine, but the code doesn't look nice\.

[V3023](https://pvs-studio.com/en/docs/warnings/v3023/) Consider inspecting this expression\. The expression is excessive or contains a misprint\. PolicyRetentionObjects\.cs 207 \(RecoveryServices\)

```cpp
public virtual void Validate()
{
  if (RetentionTimes == null 
    || RetentionTimes.Count == 0 
    || RetentionTimes.Count != 1)
  {
    throw new ArgumentException(
      Resources.InvalidRetentionTimesInPolicyException);
  }
}
```

Here's an excessive check or an excessive condition\. The check _RetentionTimes\.Count \=\= 0_ is pointless, as after that, the check _RetentionTimes\.Count \!\= 1_ follows\.  

[V3025](https://pvs-studio.com/en/docs/warnings/v3025/) Incorrect format\. A different number of format items is expected while calling 'Format' function\. Arguments not used: this\.ResourceGroupName\. NewScheduledQueryRuleCommand\.cs 117 \(Monitor\)

```cpp
protected override void ProcessRecordInternal()
{
  ....
  if (this.ShouldProcess(this.Name,
    string.Format("Creating Log Alert Rule '{0}' in resource group {0}",
      this.Name, this.ResourceGroupName)))
  {
    ....
  }
  ....
}
```

An error in the formatting line\. The specifier _\{0\} _is used twice, and the _Format_ method is passed two arguments\. Here is the correct version:

```cpp
if (this.ShouldProcess(this.Name,
  string.Format("Creating Log Alert Rule '{0}' in resource group {1}",
    this.Name, this.ResourceGroupName)))
....
```

Another similar error:

* V3025 Incorrect format\. A different number of format items is expected while calling 'Format' function\. Arguments not used: this\.ResourceGroupName\. RemoveScheduledQueryRuleCommand\.cs 88 \(Monitor\)

[V3042](https://pvs-studio.com/en/docs/warnings/v3042/) Possible NullReferenceException\. The '?\.' and '\.' operators are used for accessing members of the 'imageAndOsType' object VirtualMachineScaleSetStrategy\.cs 81 \(Compute\)

```cpp
internal static ResourceConfig<VirtualMachineScaleSet> 
CreateVirtualMachineScaleSetConfig(...., ImageAndOsType imageAndOsType, ....)
{
  ....
  VirtualMachineProfile = new VirtualMachineScaleSetVMProfile
  {
    OsProfile = new VirtualMachineScaleSetOSProfile
    {
        ....,
        WindowsConfiguration = 
          imageAndOsType.CreateWindowsConfiguration(),  // <=
        ....,
    },
    StorageProfile = new VirtualMachineScaleSetStorageProfile
    {
        ImageReference = imageAndOsType?.Image,  // <=
        DataDisks = DataDiskStrategy.CreateVmssDataDisks(
          imageAndOsType?.DataDiskLuns, dataDisks)  // <=
    },  
  },
  ....
}
```

When creating the _VirtualMachineScaleSetVMProfile _object, the _imageAndOsType _variable is checked for _null _without any preliminary check\.  However, further when creating _VirtualMachineScaleSetStorageProfile_, this variable is already checked using the conditional access operator even twice\. The code doesn't look safe\.

[V3042](https://pvs-studio.com/en/docs/warnings/v3042/) Possible NullReferenceException\. The '?\.' and '\.' operators are used for accessing members of the 'existingContacts' object RemoveAzureKeyVaultCertificateContact\.cs 123 \(KeyVault\)

```cpp
public override void ExecuteCmdlet()
{
  ....
  List<PSKeyVaultCertificateContact> existingContacts;
  
  try
  {
    existingContacts = this.DataServiceClient.
                       GetCertificateContacts(VaultName)?.ToList();
  }
  catch (KeyVaultErrorException exception)
  {
    ....
    existingContacts = null;
  }
  
  foreach (var email in EmailAddress)
  {
    existingContacts.RemoveAll(....);  // <=
  }
  ....
}
```

In both normal execution and as a result of handling an exception, the variable _existingContacts_ can get the _null_ value, after which the execution will continue\. Further in the code, this variable is used without any specific reason\.

[V3066](https://pvs-studio.com/en/docs/warnings/v3066/) Possible incorrect order of arguments passed to 'PersistSyncServerRegistration' method: 'storageSyncServiceUid' and 'discoveryUri'\. EcsManagementInteropClient\.cs 364 \(StorageSync\)

```cpp
public class EcsManagementInteropClient : IEcsManagement
{
  ....
  public int PersistSyncServerRegistration(....)
  {
    return m_managementObject.PersistSyncServerRegistration(
      serviceUri,
      subscriptionId,
      storageSyncServiceName,
      resourceGroupName,
      clusterId,
      clusterName,
      storageSyncServiceUid,  // <=
      discoveryUri,           // <=
      serviceLocation,
      resourceLocation);
  }
  ....
}
```

The analyzer suspected that the order of arguments of the _PersistSyncServerRegistration_ method is confused\. Method's declaration:

```cpp
public interface IEcsManagement : IDisposable
{
  ....
  int PersistSyncServerRegistration(
    [In, MarshalAs(UnmanagedType.BStr)]
    string serviceUri,
    [In, MarshalAs(UnmanagedType.BStr)]
    string subscriptionId,
    [In, MarshalAs(UnmanagedType.BStr)]
    string storageSyncServiceName,
    [In, MarshalAs(UnmanagedType.BStr)]
    string resourceGroupName,
    [In, MarshalAs(UnmanagedType.BStr)]
    string clusterId,
    [In, MarshalAs(UnmanagedType.BStr)]
    string clusterName,
    [In, MarshalAs(UnmanagedType.BStr)]
    string discoveryUri,                              // <=
    [In, MarshalAs(UnmanagedType.BStr)]
    string storageSyncServiceUid,                     // <=
    [In, MarshalAs(UnmanagedType.BStr)]
    string serviceLocation,
    [In, MarshalAs(UnmanagedType.BStr)]
    string resourceLocation);
  ....
}
```

Indeed, something's wrong here with the arguments number seven and eight\. The author has to check the code\.

[V3077](https://pvs-studio.com/en/docs/warnings/v3077/) The setter of 'GetGuid' property does not utilize its 'value' parameter\. RecoveryServicesBackupCmdletBase\.cs 54 \(RecoveryServices\)

```cpp
public abstract class RecoveryServicesBackupCmdletBase : AzureRMCmdlet
{
  ....
  static string _guid;
  
  protected static string GetGuid
  {
    get { return _guid; }
    set { _guid = Guid.NewGuid().ToString(); }
  }
  ....
}
```

The setter doesn't use the passed parameter\. Instead, it creates a new GUID and assigns it to the _\_guid_ field\. I think most readers would agree that such code looks at least ugly\. This construction isn't very convenient to use: when \(re\)initializing the _GetGuid_ property, one has to assign it a fake value, which is not very obvious\.  But most of all I was amused by the way authors used this pattern\. There's only one place, where _GetGuid_ is handled\. Check it out:

```cpp
public override void ExecuteCmdlet()
{
  ....
  var itemResponse = ServiceClientAdapter.CreateOrUpdateProtectionIntent(
    GetGuid ?? Guid.NewGuid().ToString(),
    ....);  
  ....
}
```

Brilliant\!

[V3091](https://pvs-studio.com/en/docs/warnings/v3091/) Empirical analysis\. It is possible that a typo is present inside the string literal: "Management Group Id"\. The 'Id' word is suspicious\. Constants\.cs 36 \(Resources\)

```cpp
public class HelpMessages
{
  public const string SubscriptionId = "Subscription Id of the subscription
                                        associated with the management";
  public const string GroupId = "Management Group Id";  // <=
  public const string Recurse = "Recursively list the children of the
                                 management group";
  public const string ParentId = "Parent Id of the management group";
  public const string GroupName = "Management Group Id";  // <=
  public const string DisplayName = "Display Name of the management group";
  public const string Expand = "Expand the output to list the children of the
                                management group";
  public const string Force = "Force the action and skip confirmations";
  public const string InputObject = "Input Object from the Get call";
  public const string ParentObject = "Parent Object";
}
```

The analyzer pointed to a possible error in the assigned string for the _GroupName_ constant\. The conclusion is based on empirical analysis of other assignments, taking into account the names of variables\. I think that in this case the analyzer is right, and the value of the _GroupName_ constant_ _should be a kind of "Management Group name"\. Probably the error took place due to the fact that the value for the _GroupId_ constant was copied, but not changed\.

Another similar error:

* V3091 Empirical analysis\. It is possible that a typo is present inside the string literal\. The 'Name' word is suspicious\. ParamHelpMsgs\.cs 153 \(RecoveryServices\)

[V3093](https://pvs-studio.com/en/docs/warnings/v3093/) The '\|' operator evaluates both operands\. Perhaps a short\-circuit '\|\|' operator should be used instead\. PSKeyVaultCertificatePolicy\.cs 114 \(KeyVault\)

```cpp
internal CertificatePolicy ToCertificatePolicy()
{
  ....
  if (!string.IsNullOrWhiteSpace(SubjectName) ||
    DnsNames != null ||
    Ekus != null ||
    KeyUsage != null |        // <=
    ValidityInMonths.HasValue)
  {
    ....
  }
  ....
}
```

In this fragment an error might take place and in the _if_ block between two last conditions the \|\| operator might have been used\. But as it often happens, only the developer can give the right answer\.

[V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'certificate' object was used before it was verified against null\. Check lines: 41, 43\. CertificateInfo\.cs 41 \(Automation\)

```cpp
public CertificateInfo(
  ...., 
  Azure.Management.Automation.Models.Certificate certificate)
{
  ....
  this.Name = certificate.Name;
  
  if (certificate == null) return;
  ....
}
```

Classic\. First the object is used and only after that the reference is checked for _null_\. We come across such errors [very often](https://pvs-studio.com/en/blog/examples/v3095/)\. Let's consider another similar error\.

[V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'clusterCred' object was used before it was verified against null\. Check lines: 115, 118\. InvokeHiveCommand\.cs 115 \(HDInsight\)

```cpp
public override void ExecuteCmdlet()
{
  ....
  _credential = new BasicAuthenticationCloudCredentials
  {
    Username = clusterCred.UserName,
    Password = clusterCred.Password.ConvertToString()
  };
  
  if (clusterConnection == null || clusterCred == null)
  ....
}
```

Here's a couple of similar errors:

* V3095 The '\_profile' object was used before it was verified against null\. Check lines: 47, 49\. RMProfileClient\.cs 47 \(Accounts\)
* V3095 The 'this\.LoadBalancer\.BackendAddressPools' object was used before it was verified against null\. Check lines: 56, 63\. AddAzureRmLoadBalancerBackendAddressPoolConfigCommand\.cs 56 \(CognitiveServices\)
* Generally speaking, I saw many [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) errors in the Azure PowerShell code\. But all of them are quite similar, so I won't dwell on this issue\.

[V3125](https://pvs-studio.com/en/docs/warnings/v3125/) The 'startTime' object was used after it was verified against null\. Check lines: 1752, 1738\. AutomationPSClientDSC\.cs 1752 \(Automation\)

```cpp
private string GetNodeReportListFilterString(
  ....,
  DateTimeOffset? startTime,
  ....,
  DateTimeOffset? lastModifiedTime)
{
  ....
  if (startTime.HasValue)
  {
    odataFilter.Add("properties/startTime ge " +
      this.FormatDateTime(startTime.Value));      // <=
  }
  ....
  if (lastModifiedTime.HasValue)
  {
    odataFilter.Add("properties/lastModifiedTime ge " +
      this.FormatDateTime(startTime.Value));      // <=
  }
  ....
}
```

It is also quite a wide\-spread type of errors\. The _startTime _variable is checked for a value presence when first used\. But it is not done in the subsequent usage\. Well, the situation can be even worse\. Look at the second _if_ block\.  I think the _startTime _variable mustn't be here at all\.  Firstly, there is no check for a value presence before its usage\. Secondly, the string formed to be passed to the _Add _method also confirms my suggestion\. Another variable _\(lastModifiedTime_\) is mentioned in the first part of this string\. 

[V3125](https://pvs-studio.com/en/docs/warnings/v3125/) The 'firstPage' object was used after it was verified against null\. Check lines: 113, 108\. IntegrationAccountAgreementOperations\.cs 113 \(LogicApp\)

```cpp
public IList<IntegrationAccountAgreement> 
ListIntegrationAccountAgreements(....)
{
  var compositeList = new List<IntegrationAccountAgreement>();
  var firstPage = this.LogicManagementClient.
                  IntegrationAccountAgreements.List(....);

  if (firstPage != null)
  {
    compositeList.AddRange(firstPage);
  }

  if (!string.IsNullOrEmpty(firstPage.NextPageLink))  // <=
  {
    ....
  }
  ....
}
```

Another obvious error\.  The _firstPage _variable is unsafely used despite the fact that earlier in the code this variable is already used being preliminary checked for _null_\.

I found even more of [V3125](https://pvs-studio.com/en/docs/warnings/v3125/) warnings in the Azure PowerShell code than [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) ones described above\.  All of them are also of the same type\. I think, two of them that we've considered are enough\.

[V3137](https://pvs-studio.com/en/docs/warnings/v3137/) The 'apiVersionSetId' variable is assigned but is not used by the end of the function\. GetAzureApiManagementApiVersionSet\.cs 69 \(ApiManagement\)

```cpp
public String ApiVersionSetId { get; set; }
....
public override void ExecuteApiManagementCmdlet()
{
  ....
  string apiVersionSetId;

  if (ParameterSetName.Equals(ContextParameterSet))
  {
    ....
    apiVersionSetId = ApiVersionSetId;
  }
  else
  {
    apiVersionSetId = ....;
  }

  if (string.IsNullOrEmpty(ApiVersionSetId))  // <=
  {
    WriteObject(....);
  }
  else
  {
    WriteObject(Client.GetApiVersionSet(...., ApiVersionSetId))  // <=
  }
}
```

The analyzer reports that the _apiVersionSetId_ local variable was initialized, but not used in any way\.  Often this pattern indicates an error\. I think, in this case it's most likely an error, especially taking into account the fact that the name of the _apiVersionSetId _local variable and the name of the _ApiVersionSetId_ property differ only by the case of the first letter\. Take a look at the code\. After initializing _apiVersionSetId _\(by one way or another\), only the _ApiVersionSetId_ property is used further in code\. It looks extremely suspicious\.

[V3137](https://pvs-studio.com/en/docs/warnings/v3137/) The 'cacheId' variable is assigned but is not used by the end of the function\. RemoveAzureApiManagementCache\.cs 94 \(ApiManagement\)

```cpp
public String CacheId { get; set; }
....
public override void ExecuteApiManagementCmdlet()
{
  ....
  string cacheId;

  if (....)
  {
    ....
    cacheId = InputObject.CacheId;
  }
  else if (....)
  {
    ....
    cacheId = cache.CacheId;
  }
  else
  {
    ....
    cacheId = CacheId;
  }
  var actionDescription = string.Format(...., CacheId);   // <=
  var actionWarning = string.Format(...., CacheId);       // <=
  ....
  Client.CacheRemove(resourceGroupName, serviceName, CacheId);  // <=
  ....  
}
```

This is the case which is almost the same as the one described earlier\. The _cacheId_ local variable isn't used after initialization in any way\. Instead, another property with a very similar name _CacheId_ is used\. I don't know for sure, may be that's just a programming pattern of Azure PowerShell developers\. Anyway, it looks like an error\.

[V3143](https://pvs-studio.com/en/docs/warnings/v3143/) The 'value' parameter is rewritten inside a property setter, and is not used after that\. NewAzureIntegrationAccountPartnerCommand\.cs 67 \(LogicApp\)

```cpp
[Parameter(Mandatory = false, 
  HelpMessage = "The integration account partner type.",
  ValueFromPipelineByPropertyName = false)]
[ValidateSet("B2B", IgnoreCase = false)]
[ValidateNotNullOrEmpty]
public string PartnerType
{
  get { return this.partnerType; }
  set { value = this.partnerType; }  // <=
}
```

The _partnerType_ field is declared in the following way: 

```cpp
/// <summary>
/// Default partner type.
/// </summary>
private string partnerType = "B2B";
```

Despite the name of the solution \(LogicApp\) where an error was detected, I don't find logic in it\. Modifying _value_ in the setter isn't a rare occurrence, but in this case it deals with a loss of the original value\. It looks weird\. In the code property is read only once\. Perhaps, we need to ask for experts' advice again\. Maybe I just don't get it\. The point is that I came across several same patterns:

* V3143 The 'value' parameter is rewritten inside a property setter, and is not used after that\. NewAzureIntegrationAccountSchemaCommand\.cs 79 \(LogicApp\)
* V3143 The 'value' parameter is rewritten inside a property setter, and is not used after that\. NewAzureIntegrationAccountSchemaCommand\.cs 87 \(LogicApp\)
* V3143 The 'value' parameter is rewritten inside a property setter, and is not used after that\. UpdateAzureIntegrationAccountPartnerCommand\.cs 67 \(LogicApp\)
* V3143 The 'value' parameter is rewritten inside a property setter, and is not used after that\. UpdateAzureIntegrationAccountSchemaCommand\.cs 80 \(LogicApp\)
* V3143 The 'value' parameter is rewritten inside a property setter, and is not used after that\. UpdateAzureIntegrationAccountSchemaCommand\.cs 88 \(LogicApp\)

## Conclusion

These are all interesting bugs that were found in the Azure PowerShell code\. Enthusiasts and those who are interested are welcome to review errors themselves in this \(or any other\) project\. I could probably miss something offbeat\. To do the review, you only need to download and install [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/download/)\.

Thank you for reading up to the end\. And, of course, bugless code to everyone\!