Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New rule: AvoidOverwritingBuiltInCmdlets #1348

Merged
merged 26 commits into from
Dec 9, 2019
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
05f6c3d
avoidoverwritingbuiltincmdlets first draft
thomasrayner Sep 30, 2019
8aa60a9
rough draft avoidoverwritingcmdlets working
thomasrayner Sep 30, 2019
651ed5d
Added tests, fixed typos, changed default PowerShellVersion behavior
thomasrayner Oct 1, 2019
aeea36c
updates a/p rjmholt
thomasrayner Oct 16, 2019
07825ce
remove unneeded else
thomasrayner Oct 16, 2019
4583924
avoidoverwritingbuiltincmdlets first draft
thomasrayner Sep 30, 2019
9c90ccd
rough draft avoidoverwritingcmdlets working
thomasrayner Sep 30, 2019
f5c6a9d
Added tests, fixed typos, changed default PowerShellVersion behavior
thomasrayner Oct 1, 2019
cedb19b
updates a/p rjmholt
thomasrayner Oct 16, 2019
6fb31b6
remove unneeded else
thomasrayner Oct 16, 2019
e1bfadd
changes from upstream
thomasrayner Oct 16, 2019
c55369c
updated readme - want tests to run in CI again
thomasrayner Oct 23, 2019
a67d3e9
prevent adding duplicate keys
thomasrayner Oct 23, 2019
bb6ae0a
return an empty list instead of null
thomasrayner Oct 23, 2019
debee2e
update rule count
thomasrayner Oct 23, 2019
ee3bb2e
fixing pwsh not present issue in test
thomasrayner Oct 23, 2019
e1688af
fixing a ps 4 test broke a linux test
thomasrayner Oct 23, 2019
c9bb6b6
better PS core detection
thomasrayner Oct 23, 2019
59e11d8
Add reference to UseCompatibleCmdlets doc
thomasrayner Oct 28, 2019
111495d
changes a/p Chris
thomasrayner Oct 28, 2019
da05f30
Update RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md
thomasrayner Oct 29, 2019
fbd5ba0
trimmed doc and changed functiondefinitions detection to be more perf…
thomasrayner Oct 29, 2019
f2df044
retrigger-ci after fix was made in master
Nov 19, 2019
c2961eb
retrigger-ci due to sporadic test failure
Nov 20, 2019
5a8e109
Update number of expected rules due to recent merge of PR #1373
bergmeister Dec 9, 2019
bdd7dbe
Merge branch 'master' into avoidoverwritingcmdlets
bergmeister Dec 9, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions RuleDocumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
262 changes: 262 additions & 0 deletions Rules/AvoidOverwritingBuiltInCmdlets.cs
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// AvoidOverwritingBuiltInCmdlets: Checks if a script overwrites a cmdlet that comes with PowerShell
/// </summary>
#if !CORECLR
[Export(typeof(IScriptRule))]
#endif
/// <summary>
/// A class to check if a script overwrites a cmdlet that comes with PowerShell
/// </summary>
public class AvoidOverwritingBuiltInCmdlets : ConfigurableRule
{
/// <summary>
/// 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.
/// </summary>
[ConfigurableRuleProperty(defaultValue: "")]
public string[] PowerShellVersion { get; set; }
thomasrayner marked this conversation as resolved.
Show resolved Hide resolved
private readonly Dictionary<string, HashSet<string>> _cmdletMap;


/// <summary>
/// Construct an object of AvoidOverwritingBuiltInCmdlets type.
/// </summary>
public AvoidOverwritingBuiltInCmdlets()
{
_cmdletMap = new Dictionary<string, HashSet<string>>();
Enable = true; // Enable rule by default
thomasrayner marked this conversation as resolved.
Show resolved Hide resolved
}


/// <summary>
/// Analyzes the given ast to find the [violation]
/// </summary>
/// <param name="ast">AST to be analyzed. This should be non-null</param>
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
/// <returns>A an enumerable type containing the violations</returns>
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
if (ast == null)
{
throw new ArgumentNullException(nameof(ast));
}

var diagnosticRecords = new List<DiagnosticRecord>();

IEnumerable<FunctionDefinitionAst> functionDefinitions = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true).OfType<FunctionDefinitionAst>();
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<string, HashSet<string>> 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<string> 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<string> GetCmdletsFromData(dynamic deserializedObject)
thomasrayner marked this conversation as resolved.
Show resolved Hide resolved
{
var cmdlets = new HashSet<string>(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;
}


/// <summary>
/// Retrieves the common name of this rule.
/// </summary>
public override string GetCommonName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidOverwritingBuiltInCmdletsCommonName);
}

/// <summary>
/// Retrieves the description of this rule.
/// </summary>
public override string GetDescription()
{
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidOverwritingBuiltInCmdletsDescription);
}

/// <summary>
/// Retrieves the name of this rule.
/// </summary>
public override string GetName()
{
return string.Format(
CultureInfo.CurrentCulture,
Strings.NameSpaceFormat,
GetSourceName(),
Strings.AvoidOverwritingBuiltInCmdletsName);
}

/// <summary>
/// Retrieves the severity of the rule: error, warning or information.
/// </summary>
public override RuleSeverity GetSeverity()
{
return RuleSeverity.Warning;
}

/// <summary>
/// Gets the severity of the returned diagnostic record: error, warning, or information.
/// </summary>
/// <returns></returns>
public DiagnosticSeverity GetDiagnosticSeverity()
{
return DiagnosticSeverity.Warning;
}

/// <summary>
/// Retrieves the name of the module/assembly the rule is from.
/// </summary>
public override string GetSourceName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
}

/// <summary>
/// Retrieves the type of the rule, Builtin, Managed or Module.
/// </summary>
public override SourceType GetSourceType()
{
return SourceType.Builtin;
}
}
}
44 changes: 44 additions & 0 deletions Rules/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions Rules/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,18 @@
<data name="UseCompatibleCmdletsError" xml:space="preserve">
<value>'{0}' is not compatible with PowerShell edition '{1}', version '{2}' and OS '{3}'</value>
</data>
<data name="AvoidOverwritingBuiltInCmdletsName" xml:space="preserve">
<value>AvoidOverwritingBuiltInCmdlets</value>
</data>
<data name="AvoidOverwritingBuiltInCmdletsCommonName" xml:space="preserve">
<value>Avoid overwriting built in cmdlets</value>
</data>
<data name="AvoidOverwritingBuiltInCmdletsDescription" xml:space="preserve">
<value>Do not overwrite the definition of a cmdlet that is included with PowerShell</value>
</data>
<data name="AvoidOverwritingBuiltInCmdletsError" xml:space="preserve">
<value>'{0}' is a cmdlet that is included with PowerShell (version {1}) whose definition should not be overridden</value>
</data>
<data name="UseCompatibleCommandsName" xml:space="preserve">
<value>UseCompatibleCommands</value>
</data>
Expand Down
2 changes: 1 addition & 1 deletion Tests/Engine/GetScriptAnalyzerRule.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading