Prerequisites

• Write a descriptive title.
• Make sure you are able to repro it on the latest released version
• Search the existing issues.
• Refer to the FAQ.
• Refer to Differences between Windows PowerShell 5.1 and PowerShell.

Steps to reproduce

This issues found by static analyzer, so we don't have steps to reproduce.

Expected behavior

The described errors are not in the project source code

Actual behavior

The described errors are in the project source code

Error details

Environment data

No environment

Visuals

Hello! We released an article with checking this project with PVS-Studio static code analyzer and found some interesting bugs. Their description below.

1. src\Microsoft.Management.Infrastructure.CimCmdlets\CimGetInstance.cs, line 168

....
case CimBaseCommand.CimInstanceSessionSet:
	CimInstance instance = GetCimInstanceParameter(cmdlet);
	nameSpace = ConstValue.GetNamespace(instance.CimSystemProperties.Namespace);
	    foreach (CimSessionProxy proxy in proxys)
	    {
	        proxy.GetInstanceAsync(nameSpace, instance);
	    }
....

PVS-Studio Message:
V3080 Possible null dereference. Consider inspecting 'instance'. CimGetInstance.cs 168

instance is nullable because GetCimInstance method can return null:

protected static CimInstance GetCimInstanceParameter(CimBaseCommand cmdlet)
{
	if (cmdlet is GetCimInstanceCommand)
	{
		return (cmdlet as GetCimInstanceCommand).CimInstance;
	}
	
	else if (cmdlet is RemoveCimInstanceCommand)
	{
		return (cmdlet as RemoveCimInstanceCommand).CimInstance;
	}
	
	else if (cmdlet is SetCimInstanceCommand)
	{
		return (cmdlet as SetCimInstanceCommand).CimInstance;
	}
	return null;
}

2. src\Microsoft.Management.Infrastructure.CimCmdlets\Utils.cs, line 305

private static bool logInitialized = false;
....
if (!logInitialized)
{
	lock (logLock)
	{
		if (!logInitialized)
		{
			DebugHelper.GenerateLog = File.Exists(logFile);
			logInitialized = true;
		}
	}
}
....

PVS-Studio Message:
V3054 Potentially unsafe double-checked locking. Use volatile variable(s) or synchronization primitives to avoid this. Utils.cs 305

Here analyzer found unsafe double-checked locking because logInitialized field isn't declared as volatile.

3. src\Microsoft.PowerShell.Commands.Diagnostics\GetEventCommand.cs, line 1066

queriedLogsQueryMapSuppress.Add(logName.ToLowerInvariant(),
                                string.Format(CultureInfo.InvariantCulture, 
                                            suppressOpener, 
                                            queryId++, 
                                            logName)
                                            );

PVS-Studio Message:
V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: queryId++, logName. GetEventCommand.cs 1067

Here analyzer warns about string formatting because suppressOpener is "<Suppress>*" and doesn't have any placeholders. Perhaps there should be another variable here.

4. src\Microsoft.PowerShell.Commands.Utility\commands\utility\New-Object.cs, line 3002

....
switch (Context.LanguageMode)
{  
	case PSLanguageMode.ConstrainedLanguage:    
		....    
		break;  
	case PSLanguageMode.NoLanguage:  
	case PSLanguageMode.RestrictedLanguage:    
	....    
	break;
}
....

PVS-Studio Message:
V3002 The switch statement does not cover all values of the 'PSLanguageMode' enum: FullLanguage. New-Object.cs 190 undefined

This switch doesn't cover all vallues of PSLanguageMode enumeration and doesn't have deafult branch.

5. src\Microsoft.PowerShell.Commands.Utility\commands\utility\ShowCommand\ShowCommandCommandInfo.cs, line 81

...
this.Module = other.Members["Module"].Value as ShowCommandModuleInfo;
...
if (other.Members["Module"]?.Value is PSObject)

PVS-Studio Message:
V3095 The 'other.Members["Module"]' object was used before it was verified against null. Check lines: 81, 91. ShowCommandCommandInfo.cs 81

other.Members is being checked against null below in the code, but in this location it isn't.

6. src\Microsoft.PowerShell.ConsoleHost\host\msh\ConsoleHost.cs, line 781

internal LocalRunspace LocalRunspace
{
	get
	{
		if (_isRunspacePushed)
		{
			return RunspaceRef.OldRunspace as LocalRunspace;
		}

		if (RunspaceRef == null)
		{
			return null;
		}
		
		return RunspaceRef.Runspace as LocalRunspace;
	}
}

PVS-Studio Message:
V3095 The 'RunspaceRef' object was used before it was verified against null. Check lines: 781, 784. ConsoleHost.cs 781

I think this message doesn't need any explanation.

7. src\System.Management.Automation\FormatAndOutput\common\DisplayDatabase\typeDataQuery.cs, line 415

....
if (vd == null || mainControlType != vd.mainControl.GetType())
{
	ActiveTracer.WriteLine(
		"NOT MATCH {0}  NAME: {1}",
		ControlBase.GetControlShapeName(vd.mainControl), 
		(vd != null ? vd.name : string.Empty)
	);
    continue;
}
....

PVS-Studio Message:
V3095 The 'vd' object was used before it was verified against null. Check lines: 415, 415. typeDataQuery.cs 415

If vd will be null then ControlBase.GetControlShapeName(vd.mainControl) will lead to NullReferenceException.

8. src\System.Management.Automation\cimSupport\cmdletization\xml\CoreCLR\cmdlets-over-objects.xmlSerializer.autogen.cs, line 1471

if (Reader.NodeType == System.Xml.XmlNodeType.Element)
{
	UnknownNode((object)o, string.Empty);
}
else
{
	UnknownNode((object)o, string.Empty);
}

PVS-Studio Message:
V3004 The 'then' statement is equivalent to the 'else' statement. cmdlets-over-objects.xmlSerializer.autogen.cs 1471

Yes, just about refactoring.

9. src\System.Management.Automation\utils\StringUtil.cs, line 246

int capacity = length + prependStr?.Length ?? 0 + appendStr?.Length ?? 0;

PVS-Studio Message:
V3123 Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. StringUtil.cs 246

This expression may have unexpected result because ?? operator have lower priority than +.

10. src\System.Management.Automation\engine\hostifaces\Command.cs, line 779

[Flags]
public enum PipelineResultTypes
{
	None = 1,
    Output = 2,
    Error = 4,
    Warning = 8,
    Verbose = 16,
    Debug = 32,
    Information = 64,
    All = 128,
    Null = 0
}

PVS-Studio Message:
V3121 An enumeration 'PipelineResultTypes' was declared with 'Flags' attribute, but does not set any initializers to override default values. Command.cs 779

This enum is declared with Flags attribute but doesn't override default values so it may take an unexpected effect when combining elements. For example, result of this expression would be PipelineResultTypes.Warning:

_mergeUnclaimedPreviousCommandResults = PipelineResultTypes.Error |                                        PipelineResultTypes.Output;