From 05f6c3d5c8b7fe51fcc4ad9475d3f71a4f2099ee Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Mon, 30 Sep 2019 13:49:56 -0700 Subject: [PATCH 01/24] avoidoverwritingbuiltincmdlets first draft --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 307 ++++++++++++++++++++++++ Rules/Strings.Designer.cs | 44 ++++ Rules/Strings.resx | 12 + 3 files changed, 363 insertions(+) create mode 100644 Rules/AvoidOverwritingBuiltInCmdlets.cs diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs new file mode 100644 index 000000000..43dd64af1 --- /dev/null +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -0,0 +1,307 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +#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 +{ + /// + /// AvoidLongLines: 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 + { + /// + /// Construct an object of AvoidOverwritingBuiltInCmdlets type. + /// + public AvoidOverwritingBuiltInCmdlets() + { + initialized = false; + } + + + [ConfigurableRuleProperty(defaultValue: "core-6.1.0-windows")] + public object PowerShellVersion { get; set; } + + private Dictionary> cmdletMap; + private bool initialized; + + + /// + /// 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 functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true); + if (functionDefinitions.Count() < 1) + { + // There are no function definitions in this AST and so it's not worth checking the rest of this rule + return null; + } + + else + { + var diagnosticRecords = new List(); + if (!initialized) + { + Initialize(); + } + + foreach (var functionDef in functionDefinitions) + { + FunctionDefinitionAst funcDef = functionDef as FunctionDefinitionAst; + if (funcDef == null) + { + continue; + } + + string functionName = funcDef.Name; + foreach (var map in cmdletMap) + { + if (map.Value.Contains(functionName)) + { + diagnosticRecords.Add(CreateDiagnosticRecord(functionName, map.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 void Initialize() + { + var psVerObjectArray = PowerShellVersion as object[]; + var psVerList = new List(); + + if (psVerObjectArray == null) + { + psVerList = PowerShellVersion as List; + if (psVerList == null) + { + return; + } + } + else + { + foreach (var psVer in psVerObjectArray) + { + var psVerString = psVer as string; + if (psVerString == null) + { + // Ignore non-string invalid entries + continue; + } + psVerList.Add(psVerString); + } + } + + string settingsPath = Settings.GetShippedSettingsDirectory(); + + if (settingsPath == null || !ContainsReferenceFile(settingsPath)) + { + return; + } + + ProcessDirectory(settingsPath, psVerList); + + if (cmdletMap.Keys.Count != psVerList.Count()) + { + return; + } + + initialized = true; + } + + + private bool ContainsReferenceFile(string directory) + { + return File.Exists(Path.Combine(directory, PowerShellVersion + ".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; + } + + cmdletMap[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.ToObject(); + } + cmdlets.Add(name); + } + } + + return cmdlets; + } + + + private bool IsValidPlatformString(string fileNameWithoutExt) + { + string psedition, psversion, os; + return GetVersionInfoFromPlatformString( + fileNameWithoutExt, + out psedition, + out psversion, + out os); + } + + + private bool GetVersionInfoFromPlatformString( + string fileName, + out string psedition, + out string psversion, + out string os) + { + psedition = null; + psversion = null; + os = null; + const string pattern = @"^(?core|desktop)-(?[\S]+)-(?windows|linux|macos)$"; + var match = Regex.Match(fileName, pattern, RegexOptions.IgnoreCase); + if (match == Match.Empty) + { + return false; + } + psedition = match.Groups["psedition"].Value; + psversion = match.Groups["psversion"].Value; + os = match.Groups["os"].Value; + return true; + } + + + /// + /// 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 8c4f6fc5f..159cd5e17 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -2048,6 +2048,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 4f6a29df3..e772bf8c1 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 cmdlets that are included with PowerShell + + + '{0}' is a cmdlet that is included with PowerShell (version {1}) whose definition should not be overridden + UseCompatibleCommands From 8aa60a9a140631a6a0a5281946c945507977f94c Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Mon, 30 Sep 2019 15:44:59 -0700 Subject: [PATCH 02/24] rough draft avoidoverwritingcmdlets working --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 37 +++++++------------------ 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index 43dd64af1..a4ea05a41 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -31,14 +31,16 @@ public class AvoidOverwritingBuiltInCmdlets : ConfigurableRule /// /// Construct an object of AvoidOverwritingBuiltInCmdlets type. /// - public AvoidOverwritingBuiltInCmdlets() + public AvoidOverwritingBuiltInCmdlets() : base() { initialized = false; + cmdletMap = new Dictionary>(); + Enable = true; // Enable rule by default } [ConfigurableRuleProperty(defaultValue: "core-6.1.0-windows")] - public object PowerShellVersion { get; set; } + public string PowerShellVersion { get; set; } private Dictionary> cmdletMap; private bool initialized; @@ -70,6 +72,10 @@ public override IEnumerable AnalyzeScript(Ast ast, string file if (!initialized) { Initialize(); + if (!initialized) + { + throw new Exception("Failed to initialize rule " + GetName()); + } } foreach (var functionDef in functionDefinitions) @@ -112,30 +118,7 @@ private DiagnosticRecord CreateDiagnosticRecord(string FunctionName, string PSVe private void Initialize() { - var psVerObjectArray = PowerShellVersion as object[]; - var psVerList = new List(); - - if (psVerObjectArray == null) - { - psVerList = PowerShellVersion as List; - if (psVerList == null) - { - return; - } - } - else - { - foreach (var psVer in psVerObjectArray) - { - var psVerString = psVer as string; - if (psVerString == null) - { - // Ignore non-string invalid entries - continue; - } - psVerList.Add(psVerString); - } - } + var psVerList = PowerShellVersion.Split(',').ToList(); string settingsPath = Settings.GetShippedSettingsDirectory(); @@ -179,7 +162,7 @@ private void ProcessDirectory(string path, IEnumerable acceptablePlatfor continue; } - cmdletMap[fileNameWithoutExt] = GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath))); + cmdletMap.Add(fileNameWithoutExt, GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath)))); } } From 651ed5d301f03fd514a7fd7d26cc522826dd99be Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Tue, 1 Oct 2019 16:01:24 -0700 Subject: [PATCH 03/24] Added tests, fixed typos, changed default PowerShellVersion behavior --- .../AvoidOverwritingBuiltInCmdlets.md | 42 +++++++++ RuleDocumentation/README.md | 1 + Rules/AvoidOverwritingBuiltInCmdlets.cs | 90 +++++++++---------- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- .../AvoidOverwritingBuiltInCmdlets.tests.ps1 | 86 ++++++++++++++++++ 5 files changed, 172 insertions(+), 49 deletions(-) create mode 100644 RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md create mode 100644 Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 diff --git a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md new file mode 100644 index 000000000..c934f2a53 --- /dev/null +++ b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md @@ -0,0 +1,42 @@ +# 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. They can be found at `/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. + +## 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 that contain any of the following + +- desktop-2.0-windows +- desktop-3.0-windows +- desktop-4.0-windows (taken from Windows Server 2012R2) +- desktop-5.1.14393.206-windows +- core-6.1.0-windows (taken from Windows 10 - 1803) +- core-6.1.0-linux (taken from Ubuntu 18.04) +- core-6.1.0-linux-arm (taken from Raspbian) +- core-6.1.0-macos + +**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 02f56a394..4f81c6b76 100644 --- a/RuleDocumentation/README.md +++ b/RuleDocumentation/README.md @@ -12,6 +12,7 @@ |[AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | | |[AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | | |[AvoidInvokingEmptyMembers](./AvoidInvokingEmptyMembers.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 index a4ea05a41..e714f4a1b 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; #if !CORECLR using System.ComponentModel.Composition; #endif @@ -18,7 +19,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// AvoidLongLines: Checks if a script overwrites a cmdlet that comes with PowerShell + /// AvoidOverwritingBuiltInCmdlets: Checks if a script overwrites a cmdlet that comes with PowerShell /// #if !CORECLR [Export(typeof(IScriptRule))] @@ -28,6 +29,12 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// public class AvoidOverwritingBuiltInCmdlets : ConfigurableRule { + [ConfigurableRuleProperty(defaultValue: "")] + public string[] PowerShellVersion { get; set; } + private Dictionary> cmdletMap; + private bool initialized; + + /// /// Construct an object of AvoidOverwritingBuiltInCmdlets type. /// @@ -36,14 +43,31 @@ public AvoidOverwritingBuiltInCmdlets() : base() initialized = false; cmdletMap = new Dictionary>(); Enable = true; // Enable rule by default - } - - - [ConfigurableRuleProperty(defaultValue: "core-6.1.0-windows")] - public string PowerShellVersion { get; set; } + + string versionTest = string.Join("", PowerShellVersion); - private Dictionary> cmdletMap; - private bool initialized; + if (versionTest != "core-6.1.0-windows" && versionTest != "desktop-5.1.14393.206-windows") + { + // 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. + try + { + var testProcess = new Process(); + testProcess.StartInfo.FileName = "pwsh"; + testProcess.StartInfo.Arguments = "-v"; + testProcess.StartInfo.CreateNoWindow = true; + testProcess.StartInfo.UseShellExecute = false; + testProcess.Start(); + PowerShellVersion = new[] {"core-6.1.0-windows"}; + } + catch + { + PowerShellVersion = new[] {"desktop-5.1.14393.206-windows"}; + } + } + } /// @@ -118,29 +142,32 @@ private DiagnosticRecord CreateDiagnosticRecord(string FunctionName, string PSVe private void Initialize() { - var psVerList = PowerShellVersion.Split(',').ToList(); + var psVerList = PowerShellVersion; string settingsPath = Settings.GetShippedSettingsDirectory(); - if (settingsPath == null || !ContainsReferenceFile(settingsPath)) + foreach (string reference in psVerList) { - return; + if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) + { + return; + } } - + ProcessDirectory(settingsPath, psVerList); if (cmdletMap.Keys.Count != psVerList.Count()) { return; } - + initialized = true; } - private bool ContainsReferenceFile(string directory) + private bool ContainsReferenceFile(string directory, string reference) { - return File.Exists(Path.Combine(directory, PowerShellVersion + ".json")); + return File.Exists(Path.Combine(directory, reference + ".json")); } @@ -193,39 +220,6 @@ private HashSet GetCmdletsFromData(dynamic deserializedObject) } - private bool IsValidPlatformString(string fileNameWithoutExt) - { - string psedition, psversion, os; - return GetVersionInfoFromPlatformString( - fileNameWithoutExt, - out psedition, - out psversion, - out os); - } - - - private bool GetVersionInfoFromPlatformString( - string fileName, - out string psedition, - out string psversion, - out string os) - { - psedition = null; - psversion = null; - os = null; - const string pattern = @"^(?core|desktop)-(?[\S]+)-(?windows|linux|macos)$"; - var match = Regex.Match(fileName, pattern, RegexOptions.IgnoreCase); - if (match == Match.Empty) - { - return false; - } - psedition = match.Groups["psedition"].Value; - psversion = match.Groups["psversion"].Value; - os = match.Groups["os"].Value; - return true; - } - - /// /// Retrieves the common name of this rule. /// diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 668590672..8b68c63f3 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -59,7 +59,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 59 + $expectedNumRules = 60 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..beb265ec1 --- /dev/null +++ b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 @@ -0,0 +1,86 @@ +$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' { + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 1 + $violations.Extent.StartLineNumber | Should -Be 5 + } + } + + 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 + + } + } + +} From aeea36c5be3a258810fccc419d86bb43f5b90df4 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 16 Oct 2019 13:50:43 -0700 Subject: [PATCH 04/24] updates a/p rjmholt --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 125 +++++++++++------------- 1 file changed, 57 insertions(+), 68 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index e714f4a1b..6b7f669c1 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -29,44 +29,26 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// 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 Dictionary> cmdletMap; - private bool initialized; + private readonly Dictionary> _cmdletMap; /// /// Construct an object of AvoidOverwritingBuiltInCmdlets type. /// - public AvoidOverwritingBuiltInCmdlets() : base() + public AvoidOverwritingBuiltInCmdlets() { - initialized = false; - cmdletMap = new Dictionary>(); + _cmdletMap = new Dictionary>(); Enable = true; // Enable rule by default - - string versionTest = string.Join("", PowerShellVersion); - - if (versionTest != "core-6.1.0-windows" && versionTest != "desktop-5.1.14393.206-windows") - { - // 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. - try - { - var testProcess = new Process(); - testProcess.StartInfo.FileName = "pwsh"; - testProcess.StartInfo.Arguments = "-v"; - testProcess.StartInfo.CreateNoWindow = true; - testProcess.StartInfo.UseShellExecute = false; - testProcess.Start(); - PowerShellVersion = new[] {"core-6.1.0-windows"}; - } - catch - { - PowerShellVersion = new[] {"desktop-5.1.14393.206-windows"}; - } - } } @@ -83,7 +65,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file throw new ArgumentNullException(nameof(ast)); } - var functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true); + IEnumerable functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true).OfType(); if (functionDefinitions.Count() < 1) { // There are no function definitions in this AST and so it's not worth checking the rest of this rule @@ -93,29 +75,61 @@ public override IEnumerable AnalyzeScript(Ast ast, string file else { var diagnosticRecords = new List(); - if (!initialized) + string versionTest = string.Join("", PowerShellVersion); + + if (string.IsNullOrEmpty(versionTest)) { - Initialize(); - if (!initialized) + // 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. + var testProcess = new Process(); + testProcess.StartInfo.FileName = "pwsh"; + testProcess.StartInfo.Arguments = "-v"; + testProcess.StartInfo.CreateNoWindow = true; + testProcess.StartInfo.UseShellExecute = false; + + try { - throw new Exception("Failed to initialize rule " + GetName()); + testProcess.Start(); + PowerShellVersion = new[] { "core-6.1.0-windows" }; + } + catch + { + PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; + } + finally + { + testProcess.Dispose(); } } - foreach (var functionDef in functionDefinitions) + var psVerList = PowerShellVersion; + string settingsPath = Settings.GetShippedSettingsDirectory(); + + foreach (string reference in psVerList) { - FunctionDefinitionAst funcDef = functionDef as FunctionDefinitionAst; - if (funcDef == null) + if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) { - continue; + throw new ArgumentException(nameof(PowerShellVersion)); } + } + + ProcessDirectory(settingsPath, psVerList); + + if (_cmdletMap.Keys.Count != psVerList.Count()) + { + throw new ArgumentException(nameof(PowerShellVersion)); + } - string functionName = funcDef.Name; - foreach (var map in cmdletMap) + foreach (FunctionDefinitionAst functionDef in functionDefinitions) + { + string functionName = functionDef.Name; + foreach (KeyValuePair> cmdletSet in _cmdletMap) { - if (map.Value.Contains(functionName)) + if (cmdletSet.Value.Contains(functionName)) { - diagnosticRecords.Add(CreateDiagnosticRecord(functionName, map.Key, functionDef.Extent)); + diagnosticRecords.Add(CreateDiagnosticRecord(functionName, cmdletSet.Key, functionDef.Extent)); } } } @@ -140,31 +154,6 @@ private DiagnosticRecord CreateDiagnosticRecord(string FunctionName, string PSVe } - private void Initialize() - { - var psVerList = PowerShellVersion; - - string settingsPath = Settings.GetShippedSettingsDirectory(); - - foreach (string reference in psVerList) - { - if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) - { - return; - } - } - - ProcessDirectory(settingsPath, psVerList); - - if (cmdletMap.Keys.Count != psVerList.Count()) - { - return; - } - - initialized = true; - } - - private bool ContainsReferenceFile(string directory, string reference) { return File.Exists(Path.Combine(directory, reference + ".json")); @@ -189,7 +178,7 @@ private void ProcessDirectory(string path, IEnumerable acceptablePlatfor continue; } - cmdletMap.Add(fileNameWithoutExt, GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath)))); + _cmdletMap.Add(fileNameWithoutExt, GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath)))); } } From 07825ce9bbcca8dfb0d5df65b8f1cd0d34d93d8c Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 16 Oct 2019 14:05:25 -0700 Subject: [PATCH 05/24] remove unneeded else --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 94 ++++++++++++------------- 1 file changed, 46 insertions(+), 48 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index 6b7f669c1..f131f3176 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -72,70 +72,68 @@ public override IEnumerable AnalyzeScript(Ast ast, string file return null; } - else - { - var diagnosticRecords = new List(); - string versionTest = string.Join("", PowerShellVersion); - if (string.IsNullOrEmpty(versionTest)) + var diagnosticRecords = new List(); + string versionTest = string.Join("", PowerShellVersion); + + if (string.IsNullOrEmpty(versionTest)) + { + // 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. + var testProcess = new Process(); + testProcess.StartInfo.FileName = "pwsh"; + testProcess.StartInfo.Arguments = "-v"; + testProcess.StartInfo.CreateNoWindow = true; + testProcess.StartInfo.UseShellExecute = false; + + try { - // 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. - var testProcess = new Process(); - testProcess.StartInfo.FileName = "pwsh"; - testProcess.StartInfo.Arguments = "-v"; - testProcess.StartInfo.CreateNoWindow = true; - testProcess.StartInfo.UseShellExecute = false; - - try - { - testProcess.Start(); - PowerShellVersion = new[] { "core-6.1.0-windows" }; - } - catch - { - PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; - } - finally - { - testProcess.Dispose(); - } + testProcess.Start(); + PowerShellVersion = new[] { "core-6.1.0-windows" }; } - - var psVerList = PowerShellVersion; - string settingsPath = Settings.GetShippedSettingsDirectory(); - - foreach (string reference in psVerList) + catch { - if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) - { - throw new ArgumentException(nameof(PowerShellVersion)); - } + PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; } + finally + { + testProcess.Dispose(); + } + } - ProcessDirectory(settingsPath, psVerList); + var psVerList = PowerShellVersion; + string settingsPath = Settings.GetShippedSettingsDirectory(); - if (_cmdletMap.Keys.Count != psVerList.Count()) + foreach (string reference in psVerList) + { + if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) { throw new ArgumentException(nameof(PowerShellVersion)); } + } + + ProcessDirectory(settingsPath, psVerList); - foreach (FunctionDefinitionAst functionDef in functionDefinitions) + 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) { - string functionName = functionDef.Name; - foreach (KeyValuePair> cmdletSet in _cmdletMap) + if (cmdletSet.Value.Contains(functionName)) { - if (cmdletSet.Value.Contains(functionName)) - { - diagnosticRecords.Add(CreateDiagnosticRecord(functionName, cmdletSet.Key, functionDef.Extent)); - } + diagnosticRecords.Add(CreateDiagnosticRecord(functionName, cmdletSet.Key, functionDef.Extent)); } } - - return diagnosticRecords; } + + return diagnosticRecords; } From 4583924c9dc824815be6f9ef6ce1465792d1532d Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Mon, 30 Sep 2019 13:49:56 -0700 Subject: [PATCH 06/24] avoidoverwritingbuiltincmdlets first draft --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 307 ++++++++++++++++++++++++ Rules/Strings.Designer.cs | 44 ++++ Rules/Strings.resx | 12 + 3 files changed, 363 insertions(+) create mode 100644 Rules/AvoidOverwritingBuiltInCmdlets.cs diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs new file mode 100644 index 000000000..43dd64af1 --- /dev/null +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -0,0 +1,307 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +#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 +{ + /// + /// AvoidLongLines: 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 + { + /// + /// Construct an object of AvoidOverwritingBuiltInCmdlets type. + /// + public AvoidOverwritingBuiltInCmdlets() + { + initialized = false; + } + + + [ConfigurableRuleProperty(defaultValue: "core-6.1.0-windows")] + public object PowerShellVersion { get; set; } + + private Dictionary> cmdletMap; + private bool initialized; + + + /// + /// 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 functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true); + if (functionDefinitions.Count() < 1) + { + // There are no function definitions in this AST and so it's not worth checking the rest of this rule + return null; + } + + else + { + var diagnosticRecords = new List(); + if (!initialized) + { + Initialize(); + } + + foreach (var functionDef in functionDefinitions) + { + FunctionDefinitionAst funcDef = functionDef as FunctionDefinitionAst; + if (funcDef == null) + { + continue; + } + + string functionName = funcDef.Name; + foreach (var map in cmdletMap) + { + if (map.Value.Contains(functionName)) + { + diagnosticRecords.Add(CreateDiagnosticRecord(functionName, map.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 void Initialize() + { + var psVerObjectArray = PowerShellVersion as object[]; + var psVerList = new List(); + + if (psVerObjectArray == null) + { + psVerList = PowerShellVersion as List; + if (psVerList == null) + { + return; + } + } + else + { + foreach (var psVer in psVerObjectArray) + { + var psVerString = psVer as string; + if (psVerString == null) + { + // Ignore non-string invalid entries + continue; + } + psVerList.Add(psVerString); + } + } + + string settingsPath = Settings.GetShippedSettingsDirectory(); + + if (settingsPath == null || !ContainsReferenceFile(settingsPath)) + { + return; + } + + ProcessDirectory(settingsPath, psVerList); + + if (cmdletMap.Keys.Count != psVerList.Count()) + { + return; + } + + initialized = true; + } + + + private bool ContainsReferenceFile(string directory) + { + return File.Exists(Path.Combine(directory, PowerShellVersion + ".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; + } + + cmdletMap[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.ToObject(); + } + cmdlets.Add(name); + } + } + + return cmdlets; + } + + + private bool IsValidPlatformString(string fileNameWithoutExt) + { + string psedition, psversion, os; + return GetVersionInfoFromPlatformString( + fileNameWithoutExt, + out psedition, + out psversion, + out os); + } + + + private bool GetVersionInfoFromPlatformString( + string fileName, + out string psedition, + out string psversion, + out string os) + { + psedition = null; + psversion = null; + os = null; + const string pattern = @"^(?core|desktop)-(?[\S]+)-(?windows|linux|macos)$"; + var match = Regex.Match(fileName, pattern, RegexOptions.IgnoreCase); + if (match == Match.Empty) + { + return false; + } + psedition = match.Groups["psedition"].Value; + psversion = match.Groups["psversion"].Value; + os = match.Groups["os"].Value; + return true; + } + + + /// + /// 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 42d5a846e..519c361fb 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -2103,6 +2103,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 dd5825e53..790e0ce7d 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 cmdlets that are included with PowerShell + + + '{0}' is a cmdlet that is included with PowerShell (version {1}) whose definition should not be overridden + UseCompatibleCommands From 9c90ccddcc024e346140ebf182abafceb2592b66 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Mon, 30 Sep 2019 15:44:59 -0700 Subject: [PATCH 07/24] rough draft avoidoverwritingcmdlets working --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 37 +++++++------------------ 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index 43dd64af1..a4ea05a41 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -31,14 +31,16 @@ public class AvoidOverwritingBuiltInCmdlets : ConfigurableRule /// /// Construct an object of AvoidOverwritingBuiltInCmdlets type. /// - public AvoidOverwritingBuiltInCmdlets() + public AvoidOverwritingBuiltInCmdlets() : base() { initialized = false; + cmdletMap = new Dictionary>(); + Enable = true; // Enable rule by default } [ConfigurableRuleProperty(defaultValue: "core-6.1.0-windows")] - public object PowerShellVersion { get; set; } + public string PowerShellVersion { get; set; } private Dictionary> cmdletMap; private bool initialized; @@ -70,6 +72,10 @@ public override IEnumerable AnalyzeScript(Ast ast, string file if (!initialized) { Initialize(); + if (!initialized) + { + throw new Exception("Failed to initialize rule " + GetName()); + } } foreach (var functionDef in functionDefinitions) @@ -112,30 +118,7 @@ private DiagnosticRecord CreateDiagnosticRecord(string FunctionName, string PSVe private void Initialize() { - var psVerObjectArray = PowerShellVersion as object[]; - var psVerList = new List(); - - if (psVerObjectArray == null) - { - psVerList = PowerShellVersion as List; - if (psVerList == null) - { - return; - } - } - else - { - foreach (var psVer in psVerObjectArray) - { - var psVerString = psVer as string; - if (psVerString == null) - { - // Ignore non-string invalid entries - continue; - } - psVerList.Add(psVerString); - } - } + var psVerList = PowerShellVersion.Split(',').ToList(); string settingsPath = Settings.GetShippedSettingsDirectory(); @@ -179,7 +162,7 @@ private void ProcessDirectory(string path, IEnumerable acceptablePlatfor continue; } - cmdletMap[fileNameWithoutExt] = GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath))); + cmdletMap.Add(fileNameWithoutExt, GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath)))); } } From f5c6a9d2863876d12d4e450f6d49340082758540 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Tue, 1 Oct 2019 16:01:24 -0700 Subject: [PATCH 08/24] Added tests, fixed typos, changed default PowerShellVersion behavior --- .../AvoidOverwritingBuiltInCmdlets.md | 42 +++++++++ RuleDocumentation/README.md | 1 + Rules/AvoidOverwritingBuiltInCmdlets.cs | 90 +++++++++---------- .../AvoidOverwritingBuiltInCmdlets.tests.ps1 | 86 ++++++++++++++++++ 4 files changed, 171 insertions(+), 48 deletions(-) create mode 100644 RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md create mode 100644 Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 diff --git a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md new file mode 100644 index 000000000..c934f2a53 --- /dev/null +++ b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md @@ -0,0 +1,42 @@ +# 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. They can be found at `/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. + +## 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 that contain any of the following + +- desktop-2.0-windows +- desktop-3.0-windows +- desktop-4.0-windows (taken from Windows Server 2012R2) +- desktop-5.1.14393.206-windows +- core-6.1.0-windows (taken from Windows 10 - 1803) +- core-6.1.0-linux (taken from Ubuntu 18.04) +- core-6.1.0-linux-arm (taken from Raspbian) +- core-6.1.0-macos + +**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 e9d3f66ce..ecd2bb44f 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 index a4ea05a41..e714f4a1b 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; #if !CORECLR using System.ComponentModel.Composition; #endif @@ -18,7 +19,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// AvoidLongLines: Checks if a script overwrites a cmdlet that comes with PowerShell + /// AvoidOverwritingBuiltInCmdlets: Checks if a script overwrites a cmdlet that comes with PowerShell /// #if !CORECLR [Export(typeof(IScriptRule))] @@ -28,6 +29,12 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// public class AvoidOverwritingBuiltInCmdlets : ConfigurableRule { + [ConfigurableRuleProperty(defaultValue: "")] + public string[] PowerShellVersion { get; set; } + private Dictionary> cmdletMap; + private bool initialized; + + /// /// Construct an object of AvoidOverwritingBuiltInCmdlets type. /// @@ -36,14 +43,31 @@ public AvoidOverwritingBuiltInCmdlets() : base() initialized = false; cmdletMap = new Dictionary>(); Enable = true; // Enable rule by default - } - - - [ConfigurableRuleProperty(defaultValue: "core-6.1.0-windows")] - public string PowerShellVersion { get; set; } + + string versionTest = string.Join("", PowerShellVersion); - private Dictionary> cmdletMap; - private bool initialized; + if (versionTest != "core-6.1.0-windows" && versionTest != "desktop-5.1.14393.206-windows") + { + // 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. + try + { + var testProcess = new Process(); + testProcess.StartInfo.FileName = "pwsh"; + testProcess.StartInfo.Arguments = "-v"; + testProcess.StartInfo.CreateNoWindow = true; + testProcess.StartInfo.UseShellExecute = false; + testProcess.Start(); + PowerShellVersion = new[] {"core-6.1.0-windows"}; + } + catch + { + PowerShellVersion = new[] {"desktop-5.1.14393.206-windows"}; + } + } + } /// @@ -118,29 +142,32 @@ private DiagnosticRecord CreateDiagnosticRecord(string FunctionName, string PSVe private void Initialize() { - var psVerList = PowerShellVersion.Split(',').ToList(); + var psVerList = PowerShellVersion; string settingsPath = Settings.GetShippedSettingsDirectory(); - if (settingsPath == null || !ContainsReferenceFile(settingsPath)) + foreach (string reference in psVerList) { - return; + if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) + { + return; + } } - + ProcessDirectory(settingsPath, psVerList); if (cmdletMap.Keys.Count != psVerList.Count()) { return; } - + initialized = true; } - private bool ContainsReferenceFile(string directory) + private bool ContainsReferenceFile(string directory, string reference) { - return File.Exists(Path.Combine(directory, PowerShellVersion + ".json")); + return File.Exists(Path.Combine(directory, reference + ".json")); } @@ -193,39 +220,6 @@ private HashSet GetCmdletsFromData(dynamic deserializedObject) } - private bool IsValidPlatformString(string fileNameWithoutExt) - { - string psedition, psversion, os; - return GetVersionInfoFromPlatformString( - fileNameWithoutExt, - out psedition, - out psversion, - out os); - } - - - private bool GetVersionInfoFromPlatformString( - string fileName, - out string psedition, - out string psversion, - out string os) - { - psedition = null; - psversion = null; - os = null; - const string pattern = @"^(?core|desktop)-(?[\S]+)-(?windows|linux|macos)$"; - var match = Regex.Match(fileName, pattern, RegexOptions.IgnoreCase); - if (match == Match.Empty) - { - return false; - } - psedition = match.Groups["psedition"].Value; - psversion = match.Groups["psversion"].Value; - os = match.Groups["os"].Value; - return true; - } - - /// /// Retrieves the common name of this rule. /// diff --git a/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 new file mode 100644 index 000000000..beb265ec1 --- /dev/null +++ b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 @@ -0,0 +1,86 @@ +$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' { + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 1 + $violations.Extent.StartLineNumber | Should -Be 5 + } + } + + 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 + + } + } + +} From cedb19b1eac2dec4f81739174a0d5a729263a1f6 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 16 Oct 2019 13:50:43 -0700 Subject: [PATCH 09/24] updates a/p rjmholt --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 125 +++++++++++------------- 1 file changed, 57 insertions(+), 68 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index e714f4a1b..6b7f669c1 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -29,44 +29,26 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// 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 Dictionary> cmdletMap; - private bool initialized; + private readonly Dictionary> _cmdletMap; /// /// Construct an object of AvoidOverwritingBuiltInCmdlets type. /// - public AvoidOverwritingBuiltInCmdlets() : base() + public AvoidOverwritingBuiltInCmdlets() { - initialized = false; - cmdletMap = new Dictionary>(); + _cmdletMap = new Dictionary>(); Enable = true; // Enable rule by default - - string versionTest = string.Join("", PowerShellVersion); - - if (versionTest != "core-6.1.0-windows" && versionTest != "desktop-5.1.14393.206-windows") - { - // 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. - try - { - var testProcess = new Process(); - testProcess.StartInfo.FileName = "pwsh"; - testProcess.StartInfo.Arguments = "-v"; - testProcess.StartInfo.CreateNoWindow = true; - testProcess.StartInfo.UseShellExecute = false; - testProcess.Start(); - PowerShellVersion = new[] {"core-6.1.0-windows"}; - } - catch - { - PowerShellVersion = new[] {"desktop-5.1.14393.206-windows"}; - } - } } @@ -83,7 +65,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file throw new ArgumentNullException(nameof(ast)); } - var functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true); + IEnumerable functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true).OfType(); if (functionDefinitions.Count() < 1) { // There are no function definitions in this AST and so it's not worth checking the rest of this rule @@ -93,29 +75,61 @@ public override IEnumerable AnalyzeScript(Ast ast, string file else { var diagnosticRecords = new List(); - if (!initialized) + string versionTest = string.Join("", PowerShellVersion); + + if (string.IsNullOrEmpty(versionTest)) { - Initialize(); - if (!initialized) + // 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. + var testProcess = new Process(); + testProcess.StartInfo.FileName = "pwsh"; + testProcess.StartInfo.Arguments = "-v"; + testProcess.StartInfo.CreateNoWindow = true; + testProcess.StartInfo.UseShellExecute = false; + + try { - throw new Exception("Failed to initialize rule " + GetName()); + testProcess.Start(); + PowerShellVersion = new[] { "core-6.1.0-windows" }; + } + catch + { + PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; + } + finally + { + testProcess.Dispose(); } } - foreach (var functionDef in functionDefinitions) + var psVerList = PowerShellVersion; + string settingsPath = Settings.GetShippedSettingsDirectory(); + + foreach (string reference in psVerList) { - FunctionDefinitionAst funcDef = functionDef as FunctionDefinitionAst; - if (funcDef == null) + if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) { - continue; + throw new ArgumentException(nameof(PowerShellVersion)); } + } + + ProcessDirectory(settingsPath, psVerList); + + if (_cmdletMap.Keys.Count != psVerList.Count()) + { + throw new ArgumentException(nameof(PowerShellVersion)); + } - string functionName = funcDef.Name; - foreach (var map in cmdletMap) + foreach (FunctionDefinitionAst functionDef in functionDefinitions) + { + string functionName = functionDef.Name; + foreach (KeyValuePair> cmdletSet in _cmdletMap) { - if (map.Value.Contains(functionName)) + if (cmdletSet.Value.Contains(functionName)) { - diagnosticRecords.Add(CreateDiagnosticRecord(functionName, map.Key, functionDef.Extent)); + diagnosticRecords.Add(CreateDiagnosticRecord(functionName, cmdletSet.Key, functionDef.Extent)); } } } @@ -140,31 +154,6 @@ private DiagnosticRecord CreateDiagnosticRecord(string FunctionName, string PSVe } - private void Initialize() - { - var psVerList = PowerShellVersion; - - string settingsPath = Settings.GetShippedSettingsDirectory(); - - foreach (string reference in psVerList) - { - if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) - { - return; - } - } - - ProcessDirectory(settingsPath, psVerList); - - if (cmdletMap.Keys.Count != psVerList.Count()) - { - return; - } - - initialized = true; - } - - private bool ContainsReferenceFile(string directory, string reference) { return File.Exists(Path.Combine(directory, reference + ".json")); @@ -189,7 +178,7 @@ private void ProcessDirectory(string path, IEnumerable acceptablePlatfor continue; } - cmdletMap.Add(fileNameWithoutExt, GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath)))); + _cmdletMap.Add(fileNameWithoutExt, GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath)))); } } From 6fb31b6afa6a8b9ddd49a6ac9730681139dad4cd Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 16 Oct 2019 14:05:25 -0700 Subject: [PATCH 10/24] remove unneeded else --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 94 ++++++++++++------------- 1 file changed, 46 insertions(+), 48 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index 6b7f669c1..f131f3176 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -72,70 +72,68 @@ public override IEnumerable AnalyzeScript(Ast ast, string file return null; } - else - { - var diagnosticRecords = new List(); - string versionTest = string.Join("", PowerShellVersion); - if (string.IsNullOrEmpty(versionTest)) + var diagnosticRecords = new List(); + string versionTest = string.Join("", PowerShellVersion); + + if (string.IsNullOrEmpty(versionTest)) + { + // 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. + var testProcess = new Process(); + testProcess.StartInfo.FileName = "pwsh"; + testProcess.StartInfo.Arguments = "-v"; + testProcess.StartInfo.CreateNoWindow = true; + testProcess.StartInfo.UseShellExecute = false; + + try { - // 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. - var testProcess = new Process(); - testProcess.StartInfo.FileName = "pwsh"; - testProcess.StartInfo.Arguments = "-v"; - testProcess.StartInfo.CreateNoWindow = true; - testProcess.StartInfo.UseShellExecute = false; - - try - { - testProcess.Start(); - PowerShellVersion = new[] { "core-6.1.0-windows" }; - } - catch - { - PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; - } - finally - { - testProcess.Dispose(); - } + testProcess.Start(); + PowerShellVersion = new[] { "core-6.1.0-windows" }; } - - var psVerList = PowerShellVersion; - string settingsPath = Settings.GetShippedSettingsDirectory(); - - foreach (string reference in psVerList) + catch { - if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) - { - throw new ArgumentException(nameof(PowerShellVersion)); - } + PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; } + finally + { + testProcess.Dispose(); + } + } - ProcessDirectory(settingsPath, psVerList); + var psVerList = PowerShellVersion; + string settingsPath = Settings.GetShippedSettingsDirectory(); - if (_cmdletMap.Keys.Count != psVerList.Count()) + foreach (string reference in psVerList) + { + if (settingsPath == null || !ContainsReferenceFile(settingsPath, reference)) { throw new ArgumentException(nameof(PowerShellVersion)); } + } + + ProcessDirectory(settingsPath, psVerList); - foreach (FunctionDefinitionAst functionDef in functionDefinitions) + 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) { - string functionName = functionDef.Name; - foreach (KeyValuePair> cmdletSet in _cmdletMap) + if (cmdletSet.Value.Contains(functionName)) { - if (cmdletSet.Value.Contains(functionName)) - { - diagnosticRecords.Add(CreateDiagnosticRecord(functionName, cmdletSet.Key, functionDef.Extent)); - } + diagnosticRecords.Add(CreateDiagnosticRecord(functionName, cmdletSet.Key, functionDef.Extent)); } } - - return diagnosticRecords; } + + return diagnosticRecords; } From c55369cad460484264d023b17b592093f2895ec1 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 23 Oct 2019 08:51:33 -0700 Subject: [PATCH 11/24] updated readme - want tests to run in CI again --- RuleDocumentation/README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/RuleDocumentation/README.md b/RuleDocumentation/README.md index dcd24be2c..ecd2bb44f 100644 --- a/RuleDocumentation/README.md +++ b/RuleDocumentation/README.md @@ -12,10 +12,7 @@ |[AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | | |[AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | | |[AvoidInvokingEmptyMembers](./AvoidInvokingEmptyMembers.md) | Warning | | -<<<<<<< HEAD |[AvoidLongLines](./AvoidLongLines.md) | Warning | | -======= ->>>>>>> 07825ce9bbcca8dfb0d5df65b8f1cd0d34d93d8c |[AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | | |[AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | | |[AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | | From a67d3e940415ed28e9f32a301df5b2781a89a341 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 23 Oct 2019 09:35:59 -0700 Subject: [PATCH 12/24] prevent adding duplicate keys --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index f131f3176..ba2fe931d 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -176,6 +176,11 @@ private void ProcessDirectory(string path, IEnumerable acceptablePlatfor continue; } + if (_cmdletMap.Keys.Contains(fileNameWithoutExt)) + { + continue; + } + _cmdletMap.Add(fileNameWithoutExt, GetCmdletsFromData(JObject.Parse(File.ReadAllText(filePath)))); } } From bb6ae0ac01365471cf7268b798bea8e6d2d98347 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 23 Oct 2019 10:00:46 -0700 Subject: [PATCH 13/24] return an empty list instead of null --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index ba2fe931d..1a5ac7c7b 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -65,15 +65,16 @@ public override IEnumerable AnalyzeScript(Ast ast, string file throw new ArgumentNullException(nameof(ast)); } + var diagnosticRecords = new List(); + IEnumerable functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true).OfType(); if (functionDefinitions.Count() < 1) { // There are no function definitions in this AST and so it's not worth checking the rest of this rule - return null; + return diagnosticRecords; } - var diagnosticRecords = new List(); string versionTest = string.Join("", PowerShellVersion); if (string.IsNullOrEmpty(versionTest)) From debee2e551f5f8dbbe12384848bc12c16abb95bb Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 23 Oct 2019 10:12:09 -0700 Subject: [PATCH 14/24] update rule count --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 8b68c63f3..25d2bcc69 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -59,7 +59,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 60 + $expectedNumRules = 61 if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4)) { # for PSv3 PSAvoidGlobalAliases is not shipped because From ee3bb2e636e8c96a33ae3c49e78853e85da64b70 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 23 Oct 2019 10:41:00 -0700 Subject: [PATCH 15/24] fixing pwsh not present issue in test --- .../AvoidOverwritingBuiltInCmdlets.tests.ps1 | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 index beb265ec1..de1764faf 100644 --- a/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 +++ b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 @@ -27,10 +27,20 @@ function Get-Clipboard { describe 'AvoidOverwritingBuiltInCmdlets' { context 'No settings specified' { - it 'should default to core-6.1.0-windows' { + it 'should default to core-6.1.0-windows if pwsh is present and desktop-5.1.14393.206-windows if it is not' { $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings - $violations.Count | Should -Be 1 - $violations.Extent.StartLineNumber | Should -Be 5 + + # Detecting the presence of pwsh this way because this is how the rule does it + try { + Start-Process -FilePath 'pwsh' -ArgumentList '-v' -WindowStyle Hidden + $violations.Count | Should -Be 1 + $violations.Extent.StartLineNumber | Should -Be 5 + } + + catch { + $violations.Count | Should -Be 2 + $violations[1].Extent.StartLineNumber | Should -Be 9 + } } } From e1688afe6832ab5d3bb88b3d4f21779204f3e4cd Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 23 Oct 2019 10:50:10 -0700 Subject: [PATCH 16/24] fixing a ps 4 test broke a linux test --- Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 index de1764faf..6c1f28509 100644 --- a/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 +++ b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 @@ -32,7 +32,9 @@ describe 'AvoidOverwritingBuiltInCmdlets' { # Detecting the presence of pwsh this way because this is how the rule does it try { - Start-Process -FilePath 'pwsh' -ArgumentList '-v' -WindowStyle Hidden + if (-not $IsLinux) { + Start-Process -FilePath 'pwsh' -ArgumentList '-v' -WindowStyle Hidden + } $violations.Count | Should -Be 1 $violations.Extent.StartLineNumber | Should -Be 5 } From c9bb6b6acd4216b427cbff4dd94872a088ac0336 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Wed, 23 Oct 2019 13:34:12 -0700 Subject: [PATCH 17/24] better PS core detection --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 23 ++++--------------- .../AvoidOverwritingBuiltInCmdlets.tests.ps1 | 16 +++++-------- 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index 1a5ac7c7b..e4f70a39e 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -83,25 +83,12 @@ public override IEnumerable AnalyzeScript(Ast ast, string file // 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. - var testProcess = new Process(); - testProcess.StartInfo.FileName = "pwsh"; - testProcess.StartInfo.Arguments = "-v"; - testProcess.StartInfo.CreateNoWindow = true; - testProcess.StartInfo.UseShellExecute = false; - try - { - testProcess.Start(); - PowerShellVersion = new[] { "core-6.1.0-windows" }; - } - catch - { - PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; - } - finally - { - testProcess.Dispose(); - } + PowerShellVersion = new[] { "desktop-5.1.14393.206-windows" }; +#if CORECLR + PowerShellVersion = new[] { "core-6.1.0-windows" }; +#endif + } var psVerList = PowerShellVersion; diff --git a/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 index 6c1f28509..eedebbd28 100644 --- a/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 +++ b/Tests/Rules/AvoidOverwritingBuiltInCmdlets.tests.ps1 @@ -1,8 +1,8 @@ $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')}} +$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) @@ -27,19 +27,15 @@ function Get-Clipboard { describe 'AvoidOverwritingBuiltInCmdlets' { context 'No settings specified' { - it 'should default to core-6.1.0-windows if pwsh is present and desktop-5.1.14393.206-windows if it is not' { + 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 - # Detecting the presence of pwsh this way because this is how the rule does it - try { - if (-not $IsLinux) { - Start-Process -FilePath 'pwsh' -ArgumentList '-v' -WindowStyle Hidden - } + if ($PSVersionTable.PSVersion.Major -gt 5) { $violations.Count | Should -Be 1 $violations.Extent.StartLineNumber | Should -Be 5 } - catch { + else { $violations.Count | Should -Be 2 $violations[1].Extent.StartLineNumber | Should -Be 9 } From 59e11d8865083c8a7564f16c4a0583f199a9a4bc Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Mon, 28 Oct 2019 13:59:27 -0700 Subject: [PATCH 18/24] Add reference to UseCompatibleCmdlets doc --- RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md index c934f2a53..4e7489ef1 100644 --- a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md +++ b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md @@ -4,7 +4,7 @@ ## 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. They can be found at `/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. +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 rule](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseCompatibleCmdlets.md). ## Configuration From 111495d8317923335a231e7e446ec358b4ce49b3 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Mon, 28 Oct 2019 14:16:47 -0700 Subject: [PATCH 19/24] changes a/p Chris --- Rules/AvoidOverwritingBuiltInCmdlets.cs | 6 ++---- Rules/Strings.resx | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index e4f70a39e..6158b6ace 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -75,9 +75,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file } - string versionTest = string.Join("", PowerShellVersion); - - if (string.IsNullOrEmpty(versionTest)) + 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 @@ -190,7 +188,7 @@ private HashSet GetCmdletsFromData(dynamic deserializedObject) var name = cmdlet.Name as string; if (name == null) { - name = cmdlet.Name.ToObject(); + name = cmdlet.Name.ToString(); } cmdlets.Add(name); } diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 790e0ce7d..2b40234c2 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -817,7 +817,7 @@ Avoid overwriting built in cmdlets - Do not overwrite the definition of cmdlets that are included with PowerShell + 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 From da05f30731feafd8d8757701e6a455f54f361960 Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Tue, 29 Oct 2019 08:13:33 -0700 Subject: [PATCH 20/24] Update RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md Co-Authored-By: Christoph Bergmeister [MVP] --- RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md index 4e7489ef1..cd987b2ce 100644 --- a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md +++ b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md @@ -4,7 +4,7 @@ ## 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 rule](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseCompatibleCmdlets.md). +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 From fbd5ba04a02a49041424201f55ce5c4e40c2602f Mon Sep 17 00:00:00 2001 From: Thomas Rayner Date: Tue, 29 Oct 2019 08:33:54 -0700 Subject: [PATCH 21/24] trimmed doc and changed functiondefinitions detection to be more performant --- RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md | 13 ++----------- Rules/AvoidOverwritingBuiltInCmdlets.cs | 2 +- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md index cd987b2ce..03d6d1ca7 100644 --- a/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md +++ b/RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md @@ -4,7 +4,7 @@ ## 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. +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 @@ -25,16 +25,7 @@ To enable the rule to check if your script is compatible on PowerShell Core on W #### PowerShellVersion -The parameter `PowerShellVersion` is a list that contain any of the following - -- desktop-2.0-windows -- desktop-3.0-windows -- desktop-4.0-windows (taken from Windows Server 2012R2) -- desktop-5.1.14393.206-windows -- core-6.1.0-windows (taken from Windows 10 - 1803) -- core-6.1.0-linux (taken from Ubuntu 18.04) -- core-6.1.0-linux-arm (taken from Raspbian) -- core-6.1.0-macos +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. diff --git a/Rules/AvoidOverwritingBuiltInCmdlets.cs b/Rules/AvoidOverwritingBuiltInCmdlets.cs index 6158b6ace..a390178d5 100644 --- a/Rules/AvoidOverwritingBuiltInCmdlets.cs +++ b/Rules/AvoidOverwritingBuiltInCmdlets.cs @@ -68,7 +68,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file var diagnosticRecords = new List(); IEnumerable functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true).OfType(); - if (functionDefinitions.Count() < 1) + 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; From f2df044b28f3fa6b5d9bf14c02b472a12eb28d7b Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 19 Nov 2019 19:29:27 +0000 Subject: [PATCH 22/24] retrigger-ci after fix was made in master From c2961eb14430c5572e2619f52f89d7a3bcba938c Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 20 Nov 2019 09:06:39 +0000 Subject: [PATCH 23/24] retrigger-ci due to sporadic test failure From 5a8e109406a893066bdc080dd2edc0361fe17c11 Mon Sep 17 00:00:00 2001 From: "Christoph Bergmeister [MVP]" Date: Mon, 9 Dec 2019 22:03:31 +0000 Subject: [PATCH 24/24] Update number of expected rules due to recent merge of PR #1373 --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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