-
Notifications
You must be signed in to change notification settings - Fork 395
New rule: AvoidOverwritingBuiltInCmdlets #1348
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
Merged
bergmeister
merged 26 commits into
PowerShell:master
from
thomasrayner:avoidoverwritingcmdlets
Dec 9, 2019
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
05f6c3d
avoidoverwritingbuiltincmdlets first draft
thomasrayner 8aa60a9
rough draft avoidoverwritingcmdlets working
thomasrayner 651ed5d
Added tests, fixed typos, changed default PowerShellVersion behavior
thomasrayner aeea36c
updates a/p rjmholt
thomasrayner 07825ce
remove unneeded else
thomasrayner 4583924
avoidoverwritingbuiltincmdlets first draft
thomasrayner 9c90ccd
rough draft avoidoverwritingcmdlets working
thomasrayner f5c6a9d
Added tests, fixed typos, changed default PowerShellVersion behavior
thomasrayner cedb19b
updates a/p rjmholt
thomasrayner 6fb31b6
remove unneeded else
thomasrayner e1bfadd
changes from upstream
thomasrayner c55369c
updated readme - want tests to run in CI again
thomasrayner a67d3e9
prevent adding duplicate keys
thomasrayner bb6ae0a
return an empty list instead of null
thomasrayner debee2e
update rule count
thomasrayner ee3bb2e
fixing pwsh not present issue in test
thomasrayner e1688af
fixing a ps 4 test broke a linux test
thomasrayner c9bb6b6
better PS core detection
thomasrayner 59e11d8
Add reference to UseCompatibleCmdlets doc
thomasrayner 111495d
changes a/p Chris
thomasrayner da05f30
Update RuleDocumentation/AvoidOverwritingBuiltInCmdlets.md
thomasrayner fbd5ba0
trimmed doc and changed functiondefinitions detection to be more perf…
thomasrayner f2df044
retrigger-ci after fix was made in master
c2961eb
retrigger-ci due to sporadic test failure
5a8e109
Update number of expected rules due to recent merge of PR #1373
bergmeister bdd7dbe
Merge branch 'master' into avoidoverwritingcmdlets
bergmeister File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; } | ||
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; | ||
} | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.