diff --git a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md new file mode 100644 index 000000000..03d6d1ca7 --- /dev/null +++ b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md @@ -0,0 +1,33 @@ +# AvoidOverwritingBuiltInCmdlets + +**Severity Level: Warning** + +## Description + +This rule flags cmdlets that are available in a given edition/version of PowerShell on a given operating system which are overwritten by a function declaration. It works by comparing function declarations against a set of whitelists which ship with PSScriptAnalyzer. These whitelist files are used by other PSScriptAnalyzer rules. More information can be found in the documentation for the [UseCompatibleCmdlets](./UseCompatibleCmdlets.md) rule. + +## Configuration + +To enable the rule to check if your script is compatible on PowerShell Core on Windows, put the following your settings file. + + +```PowerShell +@{ + 'Rules' = @{ + 'PSAvoidOverwritingBuiltInCmdlets' = @{ + 'PowerShellVersion' = @("core-6.1.0-windows") + } + } +} +``` + +### Parameters + +#### PowerShellVersion + +The parameter `PowerShellVersion` is a list of whitelists that ship with PSScriptAnalyzer. + +**Note**: The default value for `PowerShellVersion` is `"core-6.1.0-windows"` if PowerShell 6 or later is installed, and `"desktop-5.1.14393.206-windows"` if it is not. + +Usually, patched versions of PowerShell have the same cmdlet data, therefore only settings of major and minor versions of PowerShell are supplied. One can also create a custom settings file as well with the [New-CommandDataFile.ps1](https://github.com/PowerShell/PSScriptAnalyzer/blob/development/Utils/New-CommandDataFile.ps1) script and use it by placing the created `JSON` into the `Settings` folder of the `PSScriptAnalyzer` module installation folder, then the `PowerShellVersion` parameter is just its file name (that can also be changed if desired). +Note that the `core-6.0.2-*` files were removed in PSScriptAnalyzer 1.18 since PowerShell 6.0 reached it's end of life. diff --git a/RuleDocumentation/README.md b/RuleDocumentation/README.md index b13e29c50..3372326a2 100644 --- a/RuleDocumentation/README.md +++ b/RuleDocumentation/README.md @@ -13,6 +13,7 @@ |[AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | | |[AvoidInvokingEmptyMembers](./AvoidInvokingEmptyMembers.md) | Warning | | |[AvoidLongLines](./AvoidLongLines.md) | Warning | | +|[AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | | |[AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | | |[AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | | |[AvoidUsingCmdletAliases](./AvoidUsingCmdletAliases.md) | Warning | Yes | diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs new file mode 100644 index 000000000..a390178d5 --- /dev/null +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -0,0 +1,262 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +#if !CORECLR +using System.ComponentModel.Composition; +#endif +using System.Globalization; +using System.IO; +using System.Linq; +using System.Management.Automation.Language; +using System.Text.RegularExpressions; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; + +using Newtonsoft.Json.Linq; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidOverwritingBuiltInCmdlets: Checks if a script overwrites a cmdlet that comes with PowerShell + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + /// + /// A class to check if a script overwrites a cmdlet that comes with PowerShell + /// + public class AvoidOverwritingBuiltInCmdlets : ConfigurableRule + { + /// + /// Specify the version of PowerShell to compare against since different versions of PowerShell + /// ship with different sets of built in cmdlets. The default value for PowerShellVersion is + /// "core-6.1.0-windows" if PowerShell 6 or later is installed, and "desktop-5.1.14393.206-windows" + /// if it is not. The version specified aligns with a JSON file in `/path/to/PSScriptAnalyzerModule/Settings`. + /// These files are of the form, `PSEDITION-PSVERSION-OS.json` where `PSEDITION` can be either `Core` or + /// `Desktop`, `OS` can be either `Windows`, `Linux` or `MacOS`, and `Version` is the PowerShell version. + /// + [ConfigurableRuleProperty(defaultValue: "")] + public string[] PowerShellVersion { get; set; } + private readonly Dictionary> _cmdletMap; + + + /// + /// Construct an object of AvoidOverwritingBuiltInCmdlets type. + /// + public AvoidOverwritingBuiltInCmdlets() + { + _cmdletMap = new Dictionary>(); + Enable = true; // Enable rule by default + } + + + /// + /// Analyzes the given ast to find the [violation] + /// + /// AST to be analyzed. This should be non-null + /// Name of file that corresponds to the input AST. + /// A an enumerable type containing the violations + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) + { + throw new ArgumentNullException(nameof(ast)); + } + + var diagnosticRecords = new List(); + + IEnumerable functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true).OfType(); + if (!functionDefinitions.Any()) + { + // There are no function definitions in this AST and so it's not worth checking the rest of this rule + return diagnosticRecords; + } + + + if (PowerShellVersion.Length == 0 || string.IsNullOrEmpty(PowerShellVersion[0])) + { + // PowerShellVersion is not already set to one of the acceptable defaults + // Try launching `pwsh -v` to see if PowerShell 6+ is installed, and use those cmdlets + // as a default. If 6+ is not installed this will throw an error, which when caught will + // allow us to use the PowerShell 5 cmdlets as a default. + + PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; +#if CORECLR + PowerShellVersion = new[] { "core-6.1.0-windows" }; +#endif + + } + + var psVerList = PowerShellVersion; + string settingsPath = Settings.GetShippedSettingsDirectory(); + + foreach (string reference in psVerList) + { + if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) + { + throw new ArgumentException(nameof(PowerShellVersion)); + } + } + + ProcessDirectory(settingsPath, psVerList); + + if (_cmdletMap.Keys.Count != psVerList.Count()) + { + throw new ArgumentException(nameof(PowerShellVersion)); + } + + foreach (FunctionDefinitionAst functionDef in functionDefinitions) + { + string functionName = functionDef.Name; + foreach (KeyValuePair> cmdletSet in _cmdletMap) + { + if (cmdletSet.Value.Contains(functionName)) + { + diagnosticRecords.Add(CreateDiagnosticRecord(functionName, cmdletSet.Key, functionDef.Extent)); + } + } + } + + return diagnosticRecords; + } + + + private DiagnosticRecord CreateDiagnosticRecord(string FunctionName, string PSVer, IScriptExtent ViolationExtent) + { + var record = new DiagnosticRecord( + string.Format(CultureInfo.CurrentCulture, + string.Format(Strings.AvoidOverwritingBuiltInCmdletsError, FunctionName, PSVer)), + ViolationExtent, + GetName(), + GetDiagnosticSeverity(), + ViolationExtent.File, + null + ); + return record; + } + + + private bool ContainsReferenceFile(string directory, string reference) + { + return File.Exists(Path.Combine(directory, reference + ".json")); + } + + + private void ProcessDirectory(string path, IEnumerable acceptablePlatformSpecs) + { + foreach (var filePath in Directory.EnumerateFiles(path)) + { + var extension = Path.GetExtension(filePath); + if (String.IsNullOrWhiteSpace(extension) + || !extension.Equals(".json", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + var fileNameWithoutExt = Path.GetFileNameWithoutExtension(filePath); + if (acceptablePlatformSpecs != null + && !acceptablePlatformSpecs.Contains(fileNameWithoutExt, StringComparer.OrdinalIgnoreCase)) + { + continue; + } + + if (_cmdletMap.Keys.Contains(fileNameWithoutExt)) + { + continue; + } + + _cmdletMap.Add(fileNameWithoutExt, GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath)))); + } + } + + + private HashSet GetCmdletsFromData(dynamic deserializedObject) + { + var cmdlets = new HashSet(StringComparer.OrdinalIgnoreCase); + dynamic modules = deserializedObject.Modules; + foreach (dynamic module in modules) + { + if (module.ExportedCommands == null) + { + continue; + } + + foreach (dynamic cmdlet in module.ExportedCommands) + { + var name = cmdlet.Name as string; + if (name == null) + { + name = cmdlet.Name.ToString(); + } + cmdlets.Add(name); + } + } + + return cmdlets; + } + + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidOverwritingBuiltInCmdletsCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidOverwritingBuiltInCmdletsDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidOverwritingBuiltInCmdletsName); + } + + /// + /// Retrieves the severity of the rule: error, warning or information. + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// Gets the severity of the returned diagnostic record: error, warning, or information. + /// + /// + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + + /// + /// Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + } +} diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index ad071a5a1..54cba38eb 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -2084,6 +2084,50 @@ internal static string UseCompatibleCmdletsName { return ResourceManager.GetString("UseCompatibleCmdletsName", resourceCulture); } } + + /// + /// Looks up a localized string similar to Avoid overwriting built in cmdlets. + /// + internal static string AvoidOverwritingBuiltInCmdletsCommonName + { + get + { + return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid overwriting built in cmdlets. + /// + internal static string AvoidOverwritingBuiltInCmdletsDescription + { + get + { + return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to '{0}' is a cmdlet that is included with PowerShell whose definition should not be overridden. + /// + internal static string AvoidOverwritingBuiltInCmdletsError + { + get + { + return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to AvoidOverwritingBuiltInCmdlets. + /// + internal static string AvoidOverwritingBuiltInCmdletsName + { + get + { + return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsName", resourceCulture); + } + } /// /// Looks up a localized string similar to The command '{0}' is not available by default in PowerShell version '{1}' on platform '{2}'. diff --git a/Rules/Strings.resx b/Rules/Strings.resx index a11cf0922..50362080f 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -810,6 +810,18 @@ '{0}' is not compatible with PowerShell edition '{1}', version '{2}' and OS '{3}' + + AvoidOverwritingBuiltInCmdlets + + + Avoid overwriting built in cmdlets + + + Do not overwrite the definition of a cmdlet that is included with PowerShell + + + '{0}' is a cmdlet that is included with PowerShell (version {1}) whose definition should not be overridden + UseCompatibleCommands diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 25d2bcc69..be42ced25 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -58,8 +58,8 @@ Describe "Test Name parameters" { } It "get Rules with no parameters supplied" { - $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 61 + $defaultRules = Get-ScriptAnalyzerRule + $expectedNumRules = 62 if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4)) { # for PSv3 PSAvoidGlobalAliases is not shipped because diff --git a/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 new file mode 100644 index 000000000..eedebbd28 --- /dev/null +++ b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 @@ -0,0 +1,94 @@ +$ruleName = "PSAvoidOverwritingBuiltInCmdlets" + +$ruleSettingsWindows = @{$ruleName = @{PowerShellVersion = @('desktop-5.1.14393.206-windows') } } +$ruleSettingsCore = @{$ruleName = @{PowerShellVersion = @('core-6.1.0-windows') } } +$ruleSettingsBoth = @{$ruleName = @{PowerShellVersion = @('core-6.1.0-windows', 'desktop-5.1.14393.206-windows') } } + +$settings = @{ + IncludeRules = @($ruleName) +} + +# Get-Something is not a built in cmdlet on any platform and should never be flagged +# Get-ChildItem is available on all versions of PowerShell and should always be flagged +# Get-Clipboard is available on PowerShell 5 but not 6 and should be flagged conditionally +$scriptDefinition = @" +function Get-Something { + Write-Output "Get-Something" +} + +function Get-ChildItem { + Write-Output "Get-ChildItem" +} + +function Get-Clipboard { + Write-Output "Get-Clipboard" +} +"@ + +describe 'AvoidOverwritingBuiltInCmdlets' { + context 'No settings specified' { + it 'should default to core-6.1.0-windows if running PS 6+ and desktop-5.1.14393.206-windows if it is not' { + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + + if ($PSVersionTable.PSVersion.Major -gt 5) { + $violations.Count | Should -Be 1 + $violations.Extent.StartLineNumber | Should -Be 5 + } + + else { + $violations.Count | Should -Be 2 + $violations[1].Extent.StartLineNumber | Should -Be 9 + } + } + } + + context 'PowerShellVersion explicitly set to Windows PowerShell' { + $settings['Rules'] = $ruleSettingsWindows + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + + it 'should find two violations' { + $violations.Count | Should -Be 2 + } + it 'should find the violations on the correct line' { + $violations[0].Extent.StartLineNumber | Should -Be 5 + $violations[0].Extent.EndLineNumber | Should -Be 7 + + $violations[1].Extent.StartLineNumber | Should -Be 9 + $violations[1].Extent.EndLineNumber | Should -Be 11 + } + } + + context 'PowerShellVersion explicitly set to PowerShell 6' { + $settings['Rules'] = $ruleSettingsCore + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + + it 'should find one violation' { + $violations.Count | Should -Be 1 + } + it 'should find the correct violating function' { + $violations.Extent.StartLineNumber | Should -Be 5 + $violations.Extent.EndLineNumber | Should -Be 7 + } + } + + context 'PowerShellVersion explicitly set to both Windows PowerShell and PowerShell 6' { + $settings['Rules'] = $ruleSettingsBoth + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + + it 'should find three violations' { + $violations.Count | Should -Be 3 + } + it 'should find the correct violating functions' { + $violations[0].Extent.StartLineNumber | Should -Be 5 + $violations[0].Extent.EndLineNumber | Should -Be 7 + + $violations[1].Extent.StartLineNumber | Should -Be 5 + $violations[1].Extent.EndLineNumber | Should -Be 7 + + $violations[2].Extent.StartLineNumber | Should -Be 9 + $violations[2].Extent.EndLineNumber | Should -Be 11 + + } + } + +}