From 8919af706da291a16a0c645b977d863c554d82a9 Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Mon, 4 May 2015 13:29:17 -0700 Subject: [PATCH 01/20] Fix exceptions in importing CustomizedRule --- .../Commands/InvokeScriptAnalyzerCommand.cs | 3 - Engine/Generic/SourceType.cs | 6 -- Engine/ScriptAnalyzer.cs | 66 +++++++++++-------- Engine/ScriptAnalyzerEngine.csproj | 4 +- Engine/Strings.Designer.cs | 27 +++++--- Engine/Strings.resx | 3 + Rules/Strings.Designer.cs | 2 +- Rules/Strings.resx | 3 + Rules/UseCmdletCorrectly.cs | 3 - Tests/Disabled Rules/AvoidOneChar.tests.ps1 | 2 +- .../Disabled Rules/CommandNotFound.tests.ps1 | 2 +- 11 files changed, 66 insertions(+), 55 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index cdfd7bd4e..c66c53622 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -14,14 +14,11 @@ using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; using System; using System.Collections.Generic; -using System.ComponentModel.Composition; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Management.Automation; using System.Management.Automation.Language; -using System.Resources; -using System.Threading; using System.Reflection; using System.IO; using System.Text; diff --git a/Engine/Generic/SourceType.cs b/Engine/Generic/SourceType.cs index bab7cd0e3..9863a913f 100644 --- a/Engine/Generic/SourceType.cs +++ b/Engine/Generic/SourceType.cs @@ -10,12 +10,6 @@ // THE SOFTWARE. // -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - namespace Microsoft.Windows.Powershell.ScriptAnalyzer.Generic { /// diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 036007ce4..f86b9b8c0 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -142,7 +142,7 @@ public void Initilaize(Dictionary> result) paths = result.ContainsKey("ValidDllPaths") ? result["ValidDllPaths"] : result["ValidPaths"]; foreach (string path in paths) { - if (Path.GetExtension(path).ToLower(CultureInfo.CurrentCulture) == ".dll") + if (String.Equals(Path.GetExtension(path),".dll", StringComparison.OrdinalIgnoreCase)) { catalog.Catalogs.Add(new AssemblyCatalog(path)); } @@ -241,8 +241,8 @@ public List GetExternalRule(string[] moduleNames) FunctionInfo funcInfo = (FunctionInfo)psobject.ImmediateBaseObject; ParameterMetadata param = funcInfo.Parameters.Values - .First(item => item.Name.ToLower(CultureInfo.CurrentCulture).EndsWith("ast", StringComparison.CurrentCulture) || - item.Name.ToLower(CultureInfo.CurrentCulture).EndsWith("token", StringComparison.CurrentCulture)); + .First(item => item.Name.EndsWith("ast", StringComparison.OrdinalIgnoreCase) || + item.Name.EndsWith("token", StringComparison.OrdinalIgnoreCase)); //Only add functions that are defined as rules. if (param != null) @@ -251,7 +251,7 @@ public List GetExternalRule(string[] moduleNames) string desc =posh.AddScript(script).Invoke()[0].ImmediateBaseObject.ToString() .Replace("\r\n", " ").Trim(); - rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.Name, + rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.ParameterType.Name, funcInfo.ModuleName, funcInfo.Module.Path)); } } @@ -337,7 +337,7 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E { // Find all AstTypes that appeared in rule groups. IEnumerable childAsts = ast.FindAll(new Func((testAst) => - (testAst.GetType().Name.ToLower(CultureInfo.CurrentCulture) == astRuleGroup.Key.ToLower(CultureInfo.CurrentCulture))), false); + Strings.Equals(testAst.GetType().Name, astRuleGroup.Key)), false); foreach (Ast childAst in childAsts) { @@ -381,30 +381,34 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E string message = string.Empty; string ruleName = string.Empty; - // Because error stream is merged to output stream, - // we need to handle the error records. - if (psobject.ImmediateBaseObject is ErrorRecord) + //Make sure returned DiagnosticRecord is not null + if (psobject!= null && psobject.ImmediateBaseObject != null) { - ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject; - command.WriteError(record); - continue; - } - - // DiagnosticRecord may not be correctly returned from external rule. - try - { - Enum.TryParse(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity); - message = psobject.Properties["Message"].Value.ToString(); - extent = (IScriptExtent)psobject.Properties["Extent"].Value; - ruleName = psobject.Properties["RuleName"].Value.ToString(); - } - catch (Exception ex) - { - command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); - continue; + // Because error stream is merged to output stream, + // we need to handle the error records. + if (psobject.ImmediateBaseObject is ErrorRecord) + { + ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject; + command.WriteError(record); + continue; + } + + // DiagnosticRecord may not be correctly returned from external rule. + try + { + Enum.TryParse(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity); + message = psobject.Properties["Message"].Value.ToString(); + extent = (IScriptExtent)psobject.Properties["Extent"].Value; + ruleName = psobject.Properties["RuleName"].Value.ToString(); + } + catch (Exception ex) + { + command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); + continue; + } + + if (!string.IsNullOrEmpty(message)) yield return new DiagnosticRecord(message, extent, ruleName, severity, null); } - - if (!string.IsNullOrEmpty(message)) yield return new DiagnosticRecord(message, extent, ruleName, severity, null); } } @@ -453,7 +457,11 @@ public Dictionary> CheckRuleExtension(string[] path, PSCmdl // Adds original path, otherwise path.Except(validModPaths) will fail. // It's possible that user can provide something like this: // "..\..\..\ScriptAnalyzer.UnitTest\modules\CommunityAnalyzerRules\CommunityAnalyzerRules.psd1" - if (moduleInfo.ExportedFunctions.Count > 0) validModPaths.Add(childPath); + if (moduleInfo.ExportedFunctions.Count > 0) + { + validModPaths.Add(childPath); + cmdlet.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.ImportCustomizedRuleSuccess, childPath)); + } } } catch @@ -478,7 +486,7 @@ public Dictionary> CheckRuleExtension(string[] path, PSCmdl cmdlet.WriteDebug(string.Format(CultureInfo.CurrentCulture, Strings.CheckAssemblyFile, resolvedPath)); - if (Path.GetExtension(resolvedPath).ToLower(CultureInfo.CurrentCulture) == ".dll") + if (String.Equals(Path.GetExtension(resolvedPath),".dll",StringComparison.OrdinalIgnoreCase)) { if (!File.Exists(resolvedPath)) { diff --git a/Engine/ScriptAnalyzerEngine.csproj b/Engine/ScriptAnalyzerEngine.csproj index c259d0ea9..c3a285fd2 100644 --- a/Engine/ScriptAnalyzerEngine.csproj +++ b/Engine/ScriptAnalyzerEngine.csproj @@ -82,7 +82,9 @@ - + + Designer + diff --git a/Engine/Strings.Designer.cs b/Engine/Strings.Designer.cs index 41b4079d7..d262eafd7 100644 --- a/Engine/Strings.Designer.cs +++ b/Engine/Strings.Designer.cs @@ -1,14 +1,12 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by a tool. +// Runtime Version:4.0.30319.35317 // -// Copyright (c) Microsoft Corporation. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. -// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ namespace Microsoft.Windows.Powershell.ScriptAnalyzer { using System; @@ -116,6 +114,15 @@ internal static string FileNotFound { } } + /// + /// Looks up a localized string similar to Customized rules found. Imported to PSScriptAnalyzer rule collections successfully.... + /// + internal static string ImportCustomizedRuleSuccess { + get { + return ResourceManager.GetString("ImportCustomizedRuleSuccess", resourceCulture); + } + } + /// /// Looks up a localized string similar to Cannot find the path '{0}'.. /// diff --git a/Engine/Strings.resx b/Engine/Strings.resx index 369025dca..e4cd9d723 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -135,6 +135,9 @@ Cannot find file '{0}'. + + Customized rules found. Imported to PSScriptAnalyzer rule collections successfully... + Cannot find the path '{0}'. diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 1ec732308..d5ba6269c 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.34014 +// Runtime Version:4.0.30319.35317 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. diff --git a/Rules/Strings.resx b/Rules/Strings.resx index f7501a45c..7a9b73465 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -678,4 +678,7 @@ AvoidUsingWMIObjectCmdlet + + Customized rules found. Imported to PSScriptAnalyzer engine successfully... + \ No newline at end of file diff --git a/Rules/UseCmdletCorrectly.cs b/Rules/UseCmdletCorrectly.cs index a5e854b0b..e0e19d728 100644 --- a/Rules/UseCmdletCorrectly.cs +++ b/Rules/UseCmdletCorrectly.cs @@ -45,9 +45,6 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { CommandAst cmdAst = (CommandAst)foundAst; - // Handles the exception caused by commands like, {& $PLINK $args 2> $TempErrorFile}. - // You can also review the remark section in following document, - // MSDN: CommandAst.GetCommandName Method if (cmdAst.GetCommandName() == null) continue; // Checks mandatory parameters. diff --git a/Tests/Disabled Rules/AvoidOneChar.tests.ps1 b/Tests/Disabled Rules/AvoidOneChar.tests.ps1 index 6c9820fe1..f29d3aacd 100644 --- a/Tests/Disabled Rules/AvoidOneChar.tests.ps1 +++ b/Tests/Disabled Rules/AvoidOneChar.tests.ps1 @@ -3,7 +3,7 @@ $oneCharMessage = "The cmdlet name O only has one character." $oneCharName = "PSOneChar" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $invoke = Invoke-ScriptAnalyzer $directory\AvoidUsingReservedCharOneCharNames.ps1 | Where-Object {$_.RuleName -eq $oneCharName} -$noViolations = Invoke-ScriptAnalyzer $directory\GoodCmdlet.ps1 | Where-Object {$_.RuleName -eq $oneCharName} +$noViolations = Invoke-ScriptAnalyzer $directory\..\Rules\GoodCmdlet.ps1 | Where-Object {$_.RuleName -eq $oneCharName} Describe "Avoid Using One Char" { Context "When there are violations" { diff --git a/Tests/Disabled Rules/CommandNotFound.tests.ps1 b/Tests/Disabled Rules/CommandNotFound.tests.ps1 index 968c31cef..714ca07ec 100644 --- a/Tests/Disabled Rules/CommandNotFound.tests.ps1 +++ b/Tests/Disabled Rules/CommandNotFound.tests.ps1 @@ -1,4 +1,4 @@ -Import-Module -Verbose ScriptAnalyzer +Import-Module -Verbose PSScriptAnalyzer $violationMessage = "Command Get-WrongCommand Is Not Found" $violationName = "PSCommandNotFound" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path From ff75584eadd3eed6e071149655edb5b75f1ceb0a Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Mon, 4 May 2015 13:39:39 -0700 Subject: [PATCH 02/20] Revert "Fix exceptions in importing CustomizedRule" This reverts commit 8919af706da291a16a0c645b977d863c554d82a9. --- .../Commands/InvokeScriptAnalyzerCommand.cs | 3 + Engine/Generic/SourceType.cs | 6 ++ Engine/ScriptAnalyzer.cs | 66 ++++++++----------- Engine/ScriptAnalyzerEngine.csproj | 4 +- Engine/Strings.Designer.cs | 27 +++----- Engine/Strings.resx | 3 - Rules/Strings.Designer.cs | 2 +- Rules/Strings.resx | 3 - Rules/UseCmdletCorrectly.cs | 3 + Tests/Disabled Rules/AvoidOneChar.tests.ps1 | 2 +- .../Disabled Rules/CommandNotFound.tests.ps1 | 2 +- 11 files changed, 55 insertions(+), 66 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index c66c53622..cdfd7bd4e 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -14,11 +14,14 @@ using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; using System; using System.Collections.Generic; +using System.ComponentModel.Composition; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Management.Automation; using System.Management.Automation.Language; +using System.Resources; +using System.Threading; using System.Reflection; using System.IO; using System.Text; diff --git a/Engine/Generic/SourceType.cs b/Engine/Generic/SourceType.cs index 9863a913f..bab7cd0e3 100644 --- a/Engine/Generic/SourceType.cs +++ b/Engine/Generic/SourceType.cs @@ -10,6 +10,12 @@ // THE SOFTWARE. // +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + namespace Microsoft.Windows.Powershell.ScriptAnalyzer.Generic { /// diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index f86b9b8c0..036007ce4 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -142,7 +142,7 @@ public void Initilaize(Dictionary> result) paths = result.ContainsKey("ValidDllPaths") ? result["ValidDllPaths"] : result["ValidPaths"]; foreach (string path in paths) { - if (String.Equals(Path.GetExtension(path),".dll", StringComparison.OrdinalIgnoreCase)) + if (Path.GetExtension(path).ToLower(CultureInfo.CurrentCulture) == ".dll") { catalog.Catalogs.Add(new AssemblyCatalog(path)); } @@ -241,8 +241,8 @@ public List GetExternalRule(string[] moduleNames) FunctionInfo funcInfo = (FunctionInfo)psobject.ImmediateBaseObject; ParameterMetadata param = funcInfo.Parameters.Values - .First(item => item.Name.EndsWith("ast", StringComparison.OrdinalIgnoreCase) || - item.Name.EndsWith("token", StringComparison.OrdinalIgnoreCase)); + .First(item => item.Name.ToLower(CultureInfo.CurrentCulture).EndsWith("ast", StringComparison.CurrentCulture) || + item.Name.ToLower(CultureInfo.CurrentCulture).EndsWith("token", StringComparison.CurrentCulture)); //Only add functions that are defined as rules. if (param != null) @@ -251,7 +251,7 @@ public List GetExternalRule(string[] moduleNames) string desc =posh.AddScript(script).Invoke()[0].ImmediateBaseObject.ToString() .Replace("\r\n", " ").Trim(); - rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.ParameterType.Name, + rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.Name, funcInfo.ModuleName, funcInfo.Module.Path)); } } @@ -337,7 +337,7 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E { // Find all AstTypes that appeared in rule groups. IEnumerable childAsts = ast.FindAll(new Func((testAst) => - Strings.Equals(testAst.GetType().Name, astRuleGroup.Key)), false); + (testAst.GetType().Name.ToLower(CultureInfo.CurrentCulture) == astRuleGroup.Key.ToLower(CultureInfo.CurrentCulture))), false); foreach (Ast childAst in childAsts) { @@ -381,34 +381,30 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E string message = string.Empty; string ruleName = string.Empty; - //Make sure returned DiagnosticRecord is not null - if (psobject!= null && psobject.ImmediateBaseObject != null) + // Because error stream is merged to output stream, + // we need to handle the error records. + if (psobject.ImmediateBaseObject is ErrorRecord) { - // Because error stream is merged to output stream, - // we need to handle the error records. - if (psobject.ImmediateBaseObject is ErrorRecord) - { - ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject; - command.WriteError(record); - continue; - } - - // DiagnosticRecord may not be correctly returned from external rule. - try - { - Enum.TryParse(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity); - message = psobject.Properties["Message"].Value.ToString(); - extent = (IScriptExtent)psobject.Properties["Extent"].Value; - ruleName = psobject.Properties["RuleName"].Value.ToString(); - } - catch (Exception ex) - { - command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); - continue; - } - - if (!string.IsNullOrEmpty(message)) yield return new DiagnosticRecord(message, extent, ruleName, severity, null); + ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject; + command.WriteError(record); + continue; + } + + // DiagnosticRecord may not be correctly returned from external rule. + try + { + Enum.TryParse(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity); + message = psobject.Properties["Message"].Value.ToString(); + extent = (IScriptExtent)psobject.Properties["Extent"].Value; + ruleName = psobject.Properties["RuleName"].Value.ToString(); + } + catch (Exception ex) + { + command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); + continue; } + + if (!string.IsNullOrEmpty(message)) yield return new DiagnosticRecord(message, extent, ruleName, severity, null); } } @@ -457,11 +453,7 @@ public Dictionary> CheckRuleExtension(string[] path, PSCmdl // Adds original path, otherwise path.Except(validModPaths) will fail. // It's possible that user can provide something like this: // "..\..\..\ScriptAnalyzer.UnitTest\modules\CommunityAnalyzerRules\CommunityAnalyzerRules.psd1" - if (moduleInfo.ExportedFunctions.Count > 0) - { - validModPaths.Add(childPath); - cmdlet.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.ImportCustomizedRuleSuccess, childPath)); - } + if (moduleInfo.ExportedFunctions.Count > 0) validModPaths.Add(childPath); } } catch @@ -486,7 +478,7 @@ public Dictionary> CheckRuleExtension(string[] path, PSCmdl cmdlet.WriteDebug(string.Format(CultureInfo.CurrentCulture, Strings.CheckAssemblyFile, resolvedPath)); - if (String.Equals(Path.GetExtension(resolvedPath),".dll",StringComparison.OrdinalIgnoreCase)) + if (Path.GetExtension(resolvedPath).ToLower(CultureInfo.CurrentCulture) == ".dll") { if (!File.Exists(resolvedPath)) { diff --git a/Engine/ScriptAnalyzerEngine.csproj b/Engine/ScriptAnalyzerEngine.csproj index c3a285fd2..c259d0ea9 100644 --- a/Engine/ScriptAnalyzerEngine.csproj +++ b/Engine/ScriptAnalyzerEngine.csproj @@ -82,9 +82,7 @@ - - Designer - + diff --git a/Engine/Strings.Designer.cs b/Engine/Strings.Designer.cs index d262eafd7..41b4079d7 100644 --- a/Engine/Strings.Designer.cs +++ b/Engine/Strings.Designer.cs @@ -1,12 +1,14 @@ -//------------------------------------------------------------------------------ -// -// This code was generated by a tool. -// Runtime Version:4.0.30319.35317 // -// Changes to this file may cause incorrect behavior and will be lost if -// the code is regenerated. -// -//------------------------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// namespace Microsoft.Windows.Powershell.ScriptAnalyzer { using System; @@ -114,15 +116,6 @@ internal static string FileNotFound { } } - /// - /// Looks up a localized string similar to Customized rules found. Imported to PSScriptAnalyzer rule collections successfully.... - /// - internal static string ImportCustomizedRuleSuccess { - get { - return ResourceManager.GetString("ImportCustomizedRuleSuccess", resourceCulture); - } - } - /// /// Looks up a localized string similar to Cannot find the path '{0}'.. /// diff --git a/Engine/Strings.resx b/Engine/Strings.resx index e4cd9d723..369025dca 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -135,9 +135,6 @@ Cannot find file '{0}'. - - Customized rules found. Imported to PSScriptAnalyzer rule collections successfully... - Cannot find the path '{0}'. diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index d5ba6269c..1ec732308 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.35317 +// Runtime Version:4.0.30319.34014 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 7a9b73465..f7501a45c 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -678,7 +678,4 @@ AvoidUsingWMIObjectCmdlet - - Customized rules found. Imported to PSScriptAnalyzer engine successfully... - \ No newline at end of file diff --git a/Rules/UseCmdletCorrectly.cs b/Rules/UseCmdletCorrectly.cs index e0e19d728..a5e854b0b 100644 --- a/Rules/UseCmdletCorrectly.cs +++ b/Rules/UseCmdletCorrectly.cs @@ -45,6 +45,9 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { CommandAst cmdAst = (CommandAst)foundAst; + // Handles the exception caused by commands like, {& $PLINK $args 2> $TempErrorFile}. + // You can also review the remark section in following document, + // MSDN: CommandAst.GetCommandName Method if (cmdAst.GetCommandName() == null) continue; // Checks mandatory parameters. diff --git a/Tests/Disabled Rules/AvoidOneChar.tests.ps1 b/Tests/Disabled Rules/AvoidOneChar.tests.ps1 index f29d3aacd..6c9820fe1 100644 --- a/Tests/Disabled Rules/AvoidOneChar.tests.ps1 +++ b/Tests/Disabled Rules/AvoidOneChar.tests.ps1 @@ -3,7 +3,7 @@ $oneCharMessage = "The cmdlet name O only has one character." $oneCharName = "PSOneChar" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $invoke = Invoke-ScriptAnalyzer $directory\AvoidUsingReservedCharOneCharNames.ps1 | Where-Object {$_.RuleName -eq $oneCharName} -$noViolations = Invoke-ScriptAnalyzer $directory\..\Rules\GoodCmdlet.ps1 | Where-Object {$_.RuleName -eq $oneCharName} +$noViolations = Invoke-ScriptAnalyzer $directory\GoodCmdlet.ps1 | Where-Object {$_.RuleName -eq $oneCharName} Describe "Avoid Using One Char" { Context "When there are violations" { diff --git a/Tests/Disabled Rules/CommandNotFound.tests.ps1 b/Tests/Disabled Rules/CommandNotFound.tests.ps1 index 714ca07ec..968c31cef 100644 --- a/Tests/Disabled Rules/CommandNotFound.tests.ps1 +++ b/Tests/Disabled Rules/CommandNotFound.tests.ps1 @@ -1,4 +1,4 @@ -Import-Module -Verbose PSScriptAnalyzer +Import-Module -Verbose ScriptAnalyzer $violationMessage = "Command Get-WrongCommand Is Not Found" $violationName = "PSCommandNotFound" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path From 53ca175a46868d36471cfcce0feb54bc916ba053 Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Mon, 4 May 2015 14:03:39 -0700 Subject: [PATCH 03/20] Fix bugs in importing external rule When importing customized rules, we take Ast type instead of name to apply rules. Modify string comparsion Add check for null diagnosticRecord returned. --- Engine/ScriptAnalyzer.cs | 57 +++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 036007ce4..0626f02ed 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -142,7 +142,7 @@ public void Initilaize(Dictionary> result) paths = result.ContainsKey("ValidDllPaths") ? result["ValidDllPaths"] : result["ValidPaths"]; foreach (string path in paths) { - if (Path.GetExtension(path).ToLower(CultureInfo.CurrentCulture) == ".dll") + if (String.Equals(Path.GetExtension(path),".dll",StringComparison.OrdinalIgnoreCase)) { catalog.Catalogs.Add(new AssemblyCatalog(path)); } @@ -241,8 +241,8 @@ public List GetExternalRule(string[] moduleNames) FunctionInfo funcInfo = (FunctionInfo)psobject.ImmediateBaseObject; ParameterMetadata param = funcInfo.Parameters.Values - .First(item => item.Name.ToLower(CultureInfo.CurrentCulture).EndsWith("ast", StringComparison.CurrentCulture) || - item.Name.ToLower(CultureInfo.CurrentCulture).EndsWith("token", StringComparison.CurrentCulture)); + .First(item => item.Name.EndsWith("ast", StringComparison.OrdinalIgnoreCase) || + item.Name.EndsWith("token", StringComparison.CurrentCulture)); //Only add functions that are defined as rules. if (param != null) @@ -251,7 +251,7 @@ public List GetExternalRule(string[] moduleNames) string desc =posh.AddScript(script).Invoke()[0].ImmediateBaseObject.ToString() .Replace("\r\n", " ").Trim(); - rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.Name, + rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.ParameterType.Name, funcInfo.ModuleName, funcInfo.Module.Path)); } } @@ -381,30 +381,33 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E string message = string.Empty; string ruleName = string.Empty; - // Because error stream is merged to output stream, - // we need to handle the error records. - if (psobject.ImmediateBaseObject is ErrorRecord) + if (psobject != null && psobject.ImmediateBaseObject != null) { - ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject; - command.WriteError(record); - continue; - } - - // DiagnosticRecord may not be correctly returned from external rule. - try - { - Enum.TryParse(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity); - message = psobject.Properties["Message"].Value.ToString(); - extent = (IScriptExtent)psobject.Properties["Extent"].Value; - ruleName = psobject.Properties["RuleName"].Value.ToString(); + // Because error stream is merged to output stream, + // we need to handle the error records. + if (psobject.ImmediateBaseObject is ErrorRecord) + { + ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject; + command.WriteError(record); + continue; + } + + // DiagnosticRecord may not be correctly returned from external rule. + try + { + Enum.TryParse(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity); + message = psobject.Properties["Message"].Value.ToString(); + extent = (IScriptExtent)psobject.Properties["Extent"].Value; + ruleName = psobject.Properties["RuleName"].Value.ToString(); + } + catch (Exception ex) + { + command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); + continue; + } + + if (!string.IsNullOrEmpty(message)) yield return new DiagnosticRecord(message, extent, ruleName, severity, null); } - catch (Exception ex) - { - command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); - continue; - } - - if (!string.IsNullOrEmpty(message)) yield return new DiagnosticRecord(message, extent, ruleName, severity, null); } } @@ -478,7 +481,7 @@ public Dictionary> CheckRuleExtension(string[] path, PSCmdl cmdlet.WriteDebug(string.Format(CultureInfo.CurrentCulture, Strings.CheckAssemblyFile, resolvedPath)); - if (Path.GetExtension(resolvedPath).ToLower(CultureInfo.CurrentCulture) == ".dll") + if (String.Equals(Path.GetExtension(resolvedPath),".dll")) { if (!File.Exists(resolvedPath)) { From c5f1ecce11760356e5be194075a6067fd09dcaf1 Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Mon, 4 May 2015 14:11:45 -0700 Subject: [PATCH 04/20] Updated string comparison --- Engine/ScriptAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 0626f02ed..f099b318b 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -242,7 +242,7 @@ public List GetExternalRule(string[] moduleNames) FunctionInfo funcInfo = (FunctionInfo)psobject.ImmediateBaseObject; ParameterMetadata param = funcInfo.Parameters.Values .First(item => item.Name.EndsWith("ast", StringComparison.OrdinalIgnoreCase) || - item.Name.EndsWith("token", StringComparison.CurrentCulture)); + item.Name.EndsWith("token", StringComparison.OrdinalIgnoreCase)); //Only add functions that are defined as rules. if (param != null) From 89b4e2393dcd76d83c8eef94477fb73db5245a56 Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Mon, 4 May 2015 14:16:13 -0700 Subject: [PATCH 05/20] Modified string comparison options --- Engine/ScriptAnalyzer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index f099b318b..708181a34 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -251,7 +251,7 @@ public List GetExternalRule(string[] moduleNames) string desc =posh.AddScript(script).Invoke()[0].ImmediateBaseObject.ToString() .Replace("\r\n", " ").Trim(); - rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.ParameterType.Name, + rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.ParameterType.FullName, funcInfo.ModuleName, funcInfo.Module.Path)); } } @@ -481,7 +481,7 @@ public Dictionary> CheckRuleExtension(string[] path, PSCmdl cmdlet.WriteDebug(string.Format(CultureInfo.CurrentCulture, Strings.CheckAssemblyFile, resolvedPath)); - if (String.Equals(Path.GetExtension(resolvedPath),".dll")) + if (String.Equals(Path.GetExtension(resolvedPath),".dll", StringComparison.OrdinalIgnoreCase)) { if (!File.Exists(resolvedPath)) { From be781aa19a30cc5eadc2e95d46623f57e3f8252f Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Mon, 4 May 2015 16:18:06 -0700 Subject: [PATCH 06/20] Added unit test Also added try/catch to capture errors in customized rule modules and throw non-terminating errors --- Engine/ScriptAnalyzer.cs | 87 ++++++++++++++----------- Tests/Engine/samplerule/samplerule.psm1 | 45 +++++++++++++ 2 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 Tests/Engine/samplerule/samplerule.psm1 diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 708181a34..be3069fb0 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -251,7 +251,7 @@ public List GetExternalRule(string[] moduleNames) string desc =posh.AddScript(script).Invoke()[0].ImmediateBaseObject.ToString() .Replace("\r\n", " ").Trim(); - rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.ParameterType.FullName, + rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.ParameterType.Name, funcInfo.ModuleName, funcInfo.Module.Path)); } } @@ -289,12 +289,12 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E // Groups rules by AstType and Tokens. Dictionary> astRuleGroups = rules - .Where(item => item.GetParameter().EndsWith("ast", true, CultureInfo.CurrentCulture)) + .Where(item => item.GetParameter().EndsWith("ast", StringComparison.OrdinalIgnoreCase)) .GroupBy(item => item.GetParameter()) .ToDictionary(item => item.Key, item => item.ToList()); Dictionary> tokenRuleGroups = rules - .Where(item => item.GetParameter().EndsWith("token", true, CultureInfo.CurrentCulture)) + .Where(item => item.GetParameter().EndsWith("token", StringComparison.OrdinalIgnoreCase)) .GroupBy(item => item.GetParameter()) .ToDictionary(item => item.Key, item => item.ToList()); @@ -337,7 +337,7 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E { // Find all AstTypes that appeared in rule groups. IEnumerable childAsts = ast.FindAll(new Func((testAst) => - (testAst.GetType().Name.ToLower(CultureInfo.CurrentCulture) == astRuleGroup.Key.ToLower(CultureInfo.CurrentCulture))), false); + (String.Equals(testAst.GetType().Name, astRuleGroup.Key, StringComparison.OrdinalIgnoreCase))), false); foreach (Ast childAst in childAsts) { @@ -365,52 +365,63 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E } #endregion - #region Collects the results from commands. - - for (int i = 0; i < powerShellCommands.Count; i++) + List diagnostics = new List(); + try { - // EndInvoke will wait for each command to finish, so we will be getting the commands - // in the same order that they have been invoked withy BeginInvoke. - PSDataCollection psobjects = powerShellCommands[i].EndInvoke(powerShellCommandResults[i]); - - foreach (var psobject in psobjects) + for (int i = 0; i < powerShellCommands.Count; i++) { - DiagnosticSeverity severity; - IScriptExtent extent; - string message = string.Empty; - string ruleName = string.Empty; + // EndInvoke will wait for each command to finish, so we will be getting the commands + // in the same order that they have been invoked withy BeginInvoke. + PSDataCollection psobjects = powerShellCommands[i].EndInvoke(powerShellCommandResults[i]); - if (psobject != null && psobject.ImmediateBaseObject != null) + foreach (var psobject in psobjects) { - // Because error stream is merged to output stream, - // we need to handle the error records. - if (psobject.ImmediateBaseObject is ErrorRecord) - { - ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject; - command.WriteError(record); - continue; - } + DiagnosticSeverity severity; + IScriptExtent extent; + string message = string.Empty; + string ruleName = string.Empty; - // DiagnosticRecord may not be correctly returned from external rule. - try + if (psobject != null && psobject.ImmediateBaseObject != null) { - Enum.TryParse(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity); - message = psobject.Properties["Message"].Value.ToString(); - extent = (IScriptExtent)psobject.Properties["Extent"].Value; - ruleName = psobject.Properties["RuleName"].Value.ToString(); + // Because error stream is merged to output stream, + // we need to handle the error records. + if (psobject.ImmediateBaseObject is ErrorRecord) + { + ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject; + command.WriteError(record); + continue; + } + + // DiagnosticRecord may not be correctly returned from external rule. + try + { + Enum.TryParse(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity); + message = psobject.Properties["Message"].Value.ToString(); + extent = (IScriptExtent)psobject.Properties["Extent"].Value; + ruleName = psobject.Properties["RuleName"].Value.ToString(); + } + catch (Exception ex) + { + command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); + continue; + } + + if (!string.IsNullOrEmpty(message)) + { + diagnostics.Add(new DiagnosticRecord(message, extent, ruleName, severity, null)); + } } - catch (Exception ex) - { - command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); - continue; - } - - if (!string.IsNullOrEmpty(message)) yield return new DiagnosticRecord(message, extent, ruleName, severity, null); } } } + //Catch exception where customized defined rules have exceptins when doing invoke + catch(Exception ex) + { + command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); + } + return diagnostics; #endregion } } diff --git a/Tests/Engine/samplerule/samplerule.psm1 b/Tests/Engine/samplerule/samplerule.psm1 new file mode 100644 index 000000000..18f34d3c7 --- /dev/null +++ b/Tests/Engine/samplerule/samplerule.psm1 @@ -0,0 +1,45 @@ +#Requires -Version 3.0 + +<# +.SYNOPSIS + Uses #Requires -RunAsAdministrator instead of your own methods. +.DESCRIPTION + The #Requires statement prevents a script from running unless the Windows PowerShell version, modules, snap-ins, and module and snap-in version prerequisites are met. + From Windows PowerShell 4.0, the #Requires statement let script developers require that sessions be run with elevated user rights (run as Administrator). + Script developers does not need to write their own methods any more. + To fix a violation of this rule, please consider to use #Requires -RunAsAdministrator instead of your own methods. +.EXAMPLE + Measure-RequiresRunAsAdministrator -ScriptBlockAst $ScriptBlockAst +.INPUTS + [System.Management.Automation.Language.ScriptBlockAst] +.OUTPUTS + [OutputType([PSCustomObject[])] +.NOTES + None +#> +function Measure-RequiresRunAsAdministrator +{ + [CmdletBinding()] + [OutputType([Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]])] + Param + ( + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [System.Management.Automation.Language.ScriptBlockAst] + $Ast + ) + + + $results = @() + + $result = [Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]]@{"Message" = "this is help"; + "Extent" = $FunctionDefinitionAst.Extent; + "RuleName" = $PSCmdlet.MyInvocation.InvocationName; + "Severity" = "Warning"} + + $results += $result + return $results + + +} +Export-ModuleMember -Function Measure* \ No newline at end of file From b73aa4d20fbc197b2506ef77cb61e34f9c0a1171 Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Mon, 4 May 2015 17:55:10 -0700 Subject: [PATCH 07/20] Added unit tests Also added parameterType to external rule to separate parameter type and parameter name --- Engine/ScriptAnalyzer.cs | 8 ++-- Tests/Engine/CustomizedRule.tests.ps1 | 40 +++++++++++++++++++ .../samplerule/SampleRulesWithErrors.psm1 | 40 +++++++++++++++++++ Tests/Engine/samplerule/samplerule.psm1 | 10 +++-- 4 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 Tests/Engine/CustomizedRule.tests.ps1 create mode 100644 Tests/Engine/samplerule/SampleRulesWithErrors.psm1 diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index be3069fb0..de3e0c23e 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -251,7 +251,7 @@ public List GetExternalRule(string[] moduleNames) string desc =posh.AddScript(script).Invoke()[0].ImmediateBaseObject.ToString() .Replace("\r\n", " ").Trim(); - rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.ParameterType.Name, + rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.Name,param.ParameterType.FullName, funcInfo.ModuleName, funcInfo.Module.Path)); } } @@ -290,12 +290,12 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E // Groups rules by AstType and Tokens. Dictionary> astRuleGroups = rules .Where(item => item.GetParameter().EndsWith("ast", StringComparison.OrdinalIgnoreCase)) - .GroupBy(item => item.GetParameter()) + .GroupBy(item => item.GetParameterType()) .ToDictionary(item => item.Key, item => item.ToList()); Dictionary> tokenRuleGroups = rules .Where(item => item.GetParameter().EndsWith("token", StringComparison.OrdinalIgnoreCase)) - .GroupBy(item => item.GetParameter()) + .GroupBy(item => item.GetParameterType()) .ToDictionary(item => item.Key, item => item.ToList()); using (rsp) @@ -337,7 +337,7 @@ public IEnumerable GetExternalRecord(Ast ast, Token[] token, E { // Find all AstTypes that appeared in rule groups. IEnumerable childAsts = ast.FindAll(new Func((testAst) => - (String.Equals(testAst.GetType().Name, astRuleGroup.Key, StringComparison.OrdinalIgnoreCase))), false); + (astRuleGroup.Key.IndexOf(testAst.GetType().FullName,StringComparison.OrdinalIgnoreCase) != -1)), false); foreach (Ast childAst in childAsts) { diff --git a/Tests/Engine/CustomizedRule.tests.ps1 b/Tests/Engine/CustomizedRule.tests.ps1 new file mode 100644 index 000000000..2c5777bfd --- /dev/null +++ b/Tests/Engine/CustomizedRule.tests.ps1 @@ -0,0 +1,40 @@ +Import-Module PSScriptAnalyzer +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$message = "this is help" +$measure = "Measure-RequiresRunAsAdministrator" + +Describe "Test importing customized rules with null return results" { + Context "Test Get-ScriptAnalyzer with customized rules" { + It "will not terminate the engine" { + $customizedRulePath = Get-ScriptAnalyzerRule -CustomizedRulePath $directory\samplerule\SampleRulesWithErrors.psm1 | Where-Object {$_.RuleName -eq $measure} + $customizedRulePath.Count | Should Be 1 + } + + } + + Context "Test Invoke-ScriptAnalyzer with customized rules" { + It "will not terminate the engine" { + $customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -CustomizedRulePath $directory\samplerule\SampleRulesWithErrors.psm1 | Where-Object {$_.RuleName -eq $measure} + $customizedRulePath.Count | Should Be 0 + } + } + +} + +Describe "Test importing correct customized rules" { + Context "Test Get-ScriptAnalyzer with customized rules" { + It "will show the customized rule" { + $customizedRulePath = Get-ScriptAnalyzerRule -CustomizedRulePath $directory\samplerule\samplerule.psm1 | Where-Object {$_.RuleName -eq $measure} + $customizedRulePath.Count | Should Be 1 + } + + } + + Context "Test Invoke-ScriptAnalyzer with customized rules" { + It "will show the customized rule in the results" { + $customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -CustomizedRulePath $directory\samplerule\samplerule.psm1 | Where-Object {$_.Message -eq $message} + $customizedRulePath.Count | Should Be 1 + } + } + +} \ No newline at end of file diff --git a/Tests/Engine/samplerule/SampleRulesWithErrors.psm1 b/Tests/Engine/samplerule/SampleRulesWithErrors.psm1 new file mode 100644 index 000000000..3fe13d9b8 --- /dev/null +++ b/Tests/Engine/samplerule/SampleRulesWithErrors.psm1 @@ -0,0 +1,40 @@ +#Requires -Version 3.0 + +<# +.SYNOPSIS + Uses #Requires -RunAsAdministrator instead of your own methods. +.DESCRIPTION + The #Requires statement prevents a script from running unless the Windows PowerShell version, modules, snap-ins, and module and snap-in version prerequisites are met. + From Windows PowerShell 4.0, the #Requires statement let script developers require that sessions be run with elevated user rights (run as Administrator). + Script developers does not need to write their own methods any more. + To fix a violation of this rule, please consider to use #Requires -RunAsAdministrator instead of your own methods. +.EXAMPLE + Measure-RequiresRunAsAdministrator -ScriptBlockAst $ScriptBlockAst +.INPUTS + [System.Management.Automation.Language.ScriptBlockAst] +.OUTPUTS + [OutputType([PSCustomObject[])] +.NOTES + None +#> +function Measure-RequiresRunAsAdministrator +{ + [CmdletBinding()] + [OutputType([Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]])] + Param + ( + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [System.Management.Automation.Language.ScriptBlockAst] + $ScriptBlockAst + ) + + + $results = @() + + $results += $null + return $results + + +} +Export-ModuleMember -Function Measure* \ No newline at end of file diff --git a/Tests/Engine/samplerule/samplerule.psm1 b/Tests/Engine/samplerule/samplerule.psm1 index 18f34d3c7..cc94c6b1b 100644 --- a/Tests/Engine/samplerule/samplerule.psm1 +++ b/Tests/Engine/samplerule/samplerule.psm1 @@ -26,18 +26,20 @@ function Measure-RequiresRunAsAdministrator [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] [System.Management.Automation.Language.ScriptBlockAst] - $Ast + $testAst ) $results = @() - + $result = [Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]]@{"Message" = "this is help"; - "Extent" = $FunctionDefinitionAst.Extent; + "Extent" = $ast.Extent; "RuleName" = $PSCmdlet.MyInvocation.InvocationName; "Severity" = "Warning"} - $results += $result + $results += $result + + return $results From cdcc4e54368896535816100986debfa5f294928d Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Mon, 4 May 2015 17:56:41 -0700 Subject: [PATCH 08/20] Added changes in ExternalRule --- Engine/Generic/ExternalRule.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Engine/Generic/ExternalRule.cs b/Engine/Generic/ExternalRule.cs index 934bbc831..c73792e28 100644 --- a/Engine/Generic/ExternalRule.cs +++ b/Engine/Generic/ExternalRule.cs @@ -28,7 +28,7 @@ internal class ExternalRule : IExternalRule string param = string.Empty; string srcName = string.Empty; string modPath = string.Empty; - + string paramType = string.Empty; public string GetName() { @@ -55,6 +55,11 @@ public SourceType GetSourceType() return SourceType.Module; } + public string GetParameterType() + { + return this.paramType; + } + //Set the community rule level as warning as the current implementation does not require user to specify rule severity when defining their functions in PS scripts public RuleSeverity GetSeverity() { @@ -80,7 +85,7 @@ public ExternalRule() } - public ExternalRule(string name, string commonName, string desc, string param, string srcName, string modPath) + public ExternalRule(string name, string commonName, string desc, string param, string paramType, string srcName, string modPath) { this.name = name; this.commonName = commonName; @@ -88,6 +93,7 @@ public ExternalRule(string name, string commonName, string desc, string param, s this.param = param; this.srcName = srcName; this.modPath = modPath; + this.paramType = paramType; } #endregion From 49c0e1e2441faba24efe05860386890d0a3491b7 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Tue, 5 May 2015 11:01:51 -0700 Subject: [PATCH 09/20] Suppress write host for function that starts with show --- Rules/AvoidUsingWriteHost.cs | 91 ++++++++++++++----- Tests/Rules/AvoidUsingWriteHost.ps1 | 7 +- Tests/Rules/AvoidUsingWriteHost.tests.ps1 | 4 +- .../Rules/AvoidUsingWriteHostNoViolations.ps1 | 8 +- 4 files changed, 84 insertions(+), 26 deletions(-) diff --git a/Rules/AvoidUsingWriteHost.cs b/Rules/AvoidUsingWriteHost.cs index 597d7cf4a..fcc06a38b 100644 --- a/Rules/AvoidUsingWriteHost.cs +++ b/Rules/AvoidUsingWriteHost.cs @@ -23,8 +23,11 @@ namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules /// AvoidUsingWriteHost: Check that Write-Host or Console.Write are not used /// [Export(typeof(IScriptRule))] - public class AvoidUsingWriteHost : IScriptRule + public class AvoidUsingWriteHost : AstVisitor, IScriptRule { + List records; + string fileName; + /// /// AnalyzeScript: check that Write-Host or Console.Write are not used. /// @@ -32,34 +35,78 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - // Finds all CommandAsts. - IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); + records = new List(); + this.fileName = fileName; + + ast.Visit(this); + + return records; + } + + + /// + /// Visit function and skips any function that starts with show + /// + /// + /// + public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) + { + if (funcAst == null || funcAst.Name == null) + { + return AstVisitAction.SkipChildren; + } + + if (funcAst.Name.StartsWith("show", StringComparison.OrdinalIgnoreCase)) + { + return AstVisitAction.SkipChildren; + } + + return AstVisitAction.Continue; + } + + /// + /// Checks that write-host command is not used + /// + /// + /// + public override AstVisitAction VisitCommand(CommandAst cmdAst) + { + if (cmdAst == null) + { + return AstVisitAction.SkipChildren; + } - // Iterrates all CommandAsts and check the command name. - foreach (CommandAst cmdAst in commandAsts) + if (cmdAst.GetCommandName() != null && String.Equals(cmdAst.GetCommandName(), "write-host", StringComparison.OrdinalIgnoreCase)) { - if (cmdAst.GetCommandName() != null && String.Equals(cmdAst.GetCommandName(), "write-host", StringComparison.OrdinalIgnoreCase)) - { - yield return new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWriteHostError, System.IO.Path.GetFileName(fileName)), - cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); - } + records.Add(new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWriteHostError, System.IO.Path.GetFileName(fileName)), + cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName)); } - // Finds all InvokeMemberExpressionAst - IEnumerable invokeAsts = ast.FindAll(testAst => testAst is InvokeMemberExpressionAst, true); + return AstVisitAction.Continue; + } - foreach (InvokeMemberExpressionAst invokeAst in invokeAsts) + public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressionAst imeAst) + { + if (imeAst == null) { - TypeExpressionAst typeAst = invokeAst.Expression as TypeExpressionAst; - if (typeAst == null || typeAst.TypeName == null || typeAst.TypeName.FullName == null) continue; - - if (typeAst.TypeName.FullName.EndsWith("console", StringComparison.OrdinalIgnoreCase) - && !String.IsNullOrWhiteSpace(invokeAst.Member.Extent.Text) && invokeAst.Member.Extent.Text.StartsWith("Write", StringComparison.OrdinalIgnoreCase)) - { - yield return new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingConsoleWriteError, System.IO.Path.GetFileName(fileName), invokeAst.Member.Extent.Text), - invokeAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); - } + return AstVisitAction.SkipChildren; } + + TypeExpressionAst typeAst = imeAst.Expression as TypeExpressionAst; + + if (typeAst == null || typeAst.TypeName == null || typeAst.TypeName.FullName == null) + { + return AstVisitAction.SkipChildren; + } + + if (typeAst.TypeName.FullName.EndsWith("console", StringComparison.OrdinalIgnoreCase) + && !String.IsNullOrWhiteSpace(imeAst.Member.Extent.Text) && imeAst.Member.Extent.Text.StartsWith("Write", StringComparison.OrdinalIgnoreCase)) + { + records.Add(new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingConsoleWriteError, System.IO.Path.GetFileName(fileName), imeAst.Member.Extent.Text), + imeAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName)); + } + + return AstVisitAction.Continue; } /// diff --git a/Tests/Rules/AvoidUsingWriteHost.ps1 b/Tests/Rules/AvoidUsingWriteHost.ps1 index b9ea56344..d8f73259c 100644 --- a/Tests/Rules/AvoidUsingWriteHost.ps1 +++ b/Tests/Rules/AvoidUsingWriteHost.ps1 @@ -3,4 +3,9 @@ cls Write-Host "aaa" clear [System.Console]::Write("abcdefg"); -[System.Console]::WriteLine("No console.writeline plz!"); \ No newline at end of file +[System.Console]::WriteLine("No console.writeline plz!"); + +function Test +{ + Write-Host "aaaa" +} \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingWriteHost.tests.ps1 b/Tests/Rules/AvoidUsingWriteHost.tests.ps1 index f9cd926a2..2e28f5c37 100644 --- a/Tests/Rules/AvoidUsingWriteHost.tests.ps1 +++ b/Tests/Rules/AvoidUsingWriteHost.tests.ps1 @@ -8,8 +8,8 @@ $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingWriteHostNoViolations Describe "AvoidUsingWriteHost" { Context "When there are violations" { - It "has 3 Write-Host violations" { - $violations.Count | Should Be 3 + It "has 4 Write-Host violations" { + $violations.Count | Should Be 4 } It "has the correct description message for Write-Host" { diff --git a/Tests/Rules/AvoidUsingWriteHostNoViolations.ps1 b/Tests/Rules/AvoidUsingWriteHostNoViolations.ps1 index 4dadd1faa..28b18eeae 100644 --- a/Tests/Rules/AvoidUsingWriteHostNoViolations.ps1 +++ b/Tests/Rules/AvoidUsingWriteHostNoViolations.ps1 @@ -1 +1,7 @@ -Write-Output "This is the correct way to write output" \ No newline at end of file +Write-Output "This is the correct way to write output" + +# Even if write-host is used, error should not be raised in this function +function Show-Something +{ + Write-Host "show something on screen"; +} \ No newline at end of file From 6cb8120eaefff825623314f25db7c91ab68c5377 Mon Sep 17 00:00:00 2001 From: Raghu Shantha Date: Tue, 5 May 2015 15:22:10 -0700 Subject: [PATCH 10/20] Updated existing rule for verifiying the presence of WMI cmdlets in a script - new WMI cmdlets covered --- Rules/AvoidUsingWMICmdlet.cs | 144 ++++++++++++++++++++++++ Rules/ScriptAnalyzerBuiltinRules.csproj | 2 +- Rules/Strings.resx | 16 +-- 3 files changed, 153 insertions(+), 9 deletions(-) create mode 100644 Rules/AvoidUsingWMICmdlet.cs diff --git a/Rules/AvoidUsingWMICmdlet.cs b/Rules/AvoidUsingWMICmdlet.cs new file mode 100644 index 000000000..079f0ec14 --- /dev/null +++ b/Rules/AvoidUsingWMICmdlet.cs @@ -0,0 +1,144 @@ +// +// Copyright (c) Microsoft Corporation. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// + +using System; +using System.Collections.ObjectModel; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using System.Management.Automation; +using System.Management.Automation.Language; +using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; +using System.ComponentModel.Composition; +using System.Resources; +using System.Globalization; +using System.Threading; +using System.Reflection; + +namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidUsingWMICmdlet: Avoid Using Get-WMIObject, Remove-WMIObject, Invoke-WmiMethod, Register-WmiEvent, Set-WmiInstance + /// + [Export(typeof(IScriptRule))] + public class AvoidUsingWMICmdlet : IScriptRule + { + /// + /// AnalyzeScript: Avoid Using Get-WMIObject, Remove-WMIObject, Invoke-WmiMethod, Register-WmiEvent, Set-WmiInstance + /// + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + // Rule is applicable only when PowerShell Version is < 3.0, since CIM cmdlet was introduced in 3.0 + int majorPSVersion = GetPSMajorVersion(ast); + if (!(3 > majorPSVersion && 0 < majorPSVersion)) + { + // Finds all CommandAsts. + IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); + + // Iterate all CommandAsts and check the command name + foreach (CommandAst cmdAst in commandAsts) + { + if (cmdAst.GetCommandName() != null && + (String.Equals(cmdAst.GetCommandName(), "get-wmiobject", StringComparison.OrdinalIgnoreCase) + || String.Equals(cmdAst.GetCommandName(), "remove-wmiobject", StringComparison.OrdinalIgnoreCase) + || String.Equals(cmdAst.GetCommandName(), "invoke-wmimethod", StringComparison.OrdinalIgnoreCase) + || String.Equals(cmdAst.GetCommandName(), "register-wmievent", StringComparison.OrdinalIgnoreCase) + || String.Equals(cmdAst.GetCommandName(), "set-wmiinstance", StringComparison.OrdinalIgnoreCase)) + ) + { + yield return new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWMICmdletError, System.IO.Path.GetFileName(fileName)), + cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + } + } + } + } + + /// + /// GetPSMajorVersion: Retrieves Major PowerShell Version when supplied using #requires keyword in the script + /// + /// The name of this rule + private int GetPSMajorVersion(Ast ast) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + IEnumerable scriptBlockAsts = ast.FindAll(testAst => testAst is ScriptBlockAst, true); + + foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts) + { + if (null != scriptBlockAst.ScriptRequirements && null != scriptBlockAst.ScriptRequirements.RequiredPSVersion) + { + return scriptBlockAst.ScriptRequirements.RequiredPSVersion.Major; + } + } + + // return a non valid Major version if #requires -Version is not supplied in the Script + return -1; + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingWMICmdletName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWMICmdletCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWMICmdletDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + + /// + /// GetSeverity:Retrieves the severity of the rule: error, warning of information. + /// + /// + public RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + + /// + /// GetSourceName: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index f84a2072d..7cc1c0446 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -66,7 +66,7 @@ - + diff --git a/Rules/Strings.resx b/Rules/Strings.resx index f7501a45c..607777527 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -666,16 +666,16 @@ UseShouldProcessForStateChangingFunctions - - Avoid Using Get-WMIObject, Remove-WMIObject + + Avoid Using Get-WMIObject, Remove-WMIObject, Invoke-WmiMethod, Register-WmiEvent, Set-WmiInstance - - Depricated. Starting in Windows PowerShell 3.0, these cmdlets have been superseded by CimInstance cmdlets. + + Depricated. Starting in Windows PowerShell 3.0, these cmdlets have been superseded by CIM cmdlets. - - File '{0}' uses WMIObject cmdlet. For PowerShell 3.0 and above, this is not recommended because the cmdlet is based on a non-standard DCOM protocol. Use CIMInstance cmdlet instead. This is CIM and WS-Man standards compliant and works in a heterogeneous environment. + + File '{0}' uses WMI cmdlet. For PowerShell 3.0 and above, use CIM cmdlet which perform the same tasks as the WMI cmdlets. The CIM cmdlets comply with WS-Management (WSMan) standards and with the Common Information Model (CIM) standard, which enables the cmdlets to use the same techniques to manage Windows computers and those running other operating systems. - - AvoidUsingWMIObjectCmdlet + + AvoidUsingWMICmdlet \ No newline at end of file From 5e2a145363efcd451b135a21f2887fb873f8b2eb Mon Sep 17 00:00:00 2001 From: Raghu Shantha Date: Tue, 5 May 2015 15:23:57 -0700 Subject: [PATCH 11/20] Updated existing rule for verifiying the presence of WMI cmdlets in a script - new WMI cmdlets covered --- Rules/AvoidUsingWMIObjectCmdlet.cs | 138 ----------------------------- 1 file changed, 138 deletions(-) delete mode 100644 Rules/AvoidUsingWMIObjectCmdlet.cs diff --git a/Rules/AvoidUsingWMIObjectCmdlet.cs b/Rules/AvoidUsingWMIObjectCmdlet.cs deleted file mode 100644 index 6c71f1646..000000000 --- a/Rules/AvoidUsingWMIObjectCmdlet.cs +++ /dev/null @@ -1,138 +0,0 @@ -// -// Copyright (c) Microsoft Corporation. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. -// - -using System; -using System.Collections.ObjectModel; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using System.Management.Automation; -using System.Management.Automation.Language; -using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; -using System.ComponentModel.Composition; -using System.Resources; -using System.Globalization; -using System.Threading; -using System.Reflection; - -namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules -{ - /// - /// AvoidUsingWMIObjectCmdlet: Verify that Get-WMIObject, Remove-WMIObject are not used - /// - [Export(typeof(IScriptRule))] - public class AvoidUsingWMIObjectCmdlet : IScriptRule - { - /// - /// AnalyzeScript: Verify that Get-WMIObject, Remove-WMIObject are not used - /// - public IEnumerable AnalyzeScript(Ast ast, string fileName) - { - if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - - // Rule is applicable only when PowerShell Version is < 3.0, since Get-CIMInstance was introduced in 3.0 - int majorPSVersion = GetPSMajorVersion(ast); - if (!(3 > majorPSVersion && 0 < majorPSVersion)) - { - // Finds all CommandAsts. - IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); - - // Iterate all CommandAsts and check the command name - foreach (CommandAst cmdAst in commandAsts) - { - if (cmdAst.GetCommandName() != null && (String.Equals(cmdAst.GetCommandName(), "get-wmiobject", StringComparison.OrdinalIgnoreCase) || String.Equals(cmdAst.GetCommandName(), "remove-wmiobject", StringComparison.OrdinalIgnoreCase))) - { - yield return new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWMIObjectCmdletError, System.IO.Path.GetFileName(fileName)), - cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); - } - } - } - } - - /// - /// GetPSMajorVersion: Retrieves Major PowerShell Version when supplied using #requires keyword in the script - /// - /// The name of this rule - private int GetPSMajorVersion(Ast ast) - { - if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - - IEnumerable scriptBlockAsts = ast.FindAll(testAst => testAst is ScriptBlockAst, true); - - foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts) - { - if (null != scriptBlockAst.ScriptRequirements && null != scriptBlockAst.ScriptRequirements.RequiredPSVersion) - { - return scriptBlockAst.ScriptRequirements.RequiredPSVersion.Major; - } - } - - // return a non valid Major version if #requires -Version is not supplied in the Script - return -1; - } - - /// - /// GetName: Retrieves the name of this rule. - /// - /// The name of this rule - public string GetName() - { - return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingWMIObjectCmdletName); - } - - /// - /// GetCommonName: Retrieves the common name of this rule. - /// - /// The common name of this rule - public string GetCommonName() - { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWMIObjectCmdletCommonName); - } - - /// - /// GetDescription: Retrieves the description of this rule. - /// - /// The description of this rule - public string GetDescription() - { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWMIObjectCmdletDescription); - } - - /// - /// GetSourceType: Retrieves the type of the rule: builtin, managed or module. - /// - public SourceType GetSourceType() - { - return SourceType.Builtin; - } - - - /// - /// GetSeverity:Retrieves the severity of the rule: error, warning of information. - /// - /// - public RuleSeverity GetSeverity() - { - return RuleSeverity.Warning; - } - - - /// - /// GetSourceName: Retrieves the module/assembly name the rule is from. - /// - public string GetSourceName() - { - return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); - } - } -} From dcb5d6e6bd13aa46c1c5034670931c2a8e546649 Mon Sep 17 00:00:00 2001 From: Raghu Shantha Date: Tue, 5 May 2015 16:38:21 -0700 Subject: [PATCH 12/20] Tests for [AvoidUsingWMICmdlet Rule --- Tests/Rules/AvoidUsingWMICmdlet.ps1 | 18 ++++++++++++++ Tests/Rules/AvoidUsingWMICmdlet.tests.ps1 | 24 +++++++++++++++++++ .../Rules/AvoidUsingWMICmdletNoViolations.ps1 | 19 +++++++++++++++ Tests/Rules/AvoidUsingWMIObjectCmdlet.ps1 | 13 ---------- .../Rules/AvoidUsingWMIObjectCmdlet.tests.ps1 | 24 ------------------- .../AvoidUsingWMIObjectCmdletNoViolations.ps1 | 14 ----------- 6 files changed, 61 insertions(+), 51 deletions(-) create mode 100644 Tests/Rules/AvoidUsingWMICmdlet.ps1 create mode 100644 Tests/Rules/AvoidUsingWMICmdlet.tests.ps1 create mode 100644 Tests/Rules/AvoidUsingWMICmdletNoViolations.ps1 delete mode 100644 Tests/Rules/AvoidUsingWMIObjectCmdlet.ps1 delete mode 100644 Tests/Rules/AvoidUsingWMIObjectCmdlet.tests.ps1 delete mode 100644 Tests/Rules/AvoidUsingWMIObjectCmdletNoViolations.ps1 diff --git a/Tests/Rules/AvoidUsingWMICmdlet.ps1 b/Tests/Rules/AvoidUsingWMICmdlet.ps1 new file mode 100644 index 000000000..81c03f64d --- /dev/null +++ b/Tests/Rules/AvoidUsingWMICmdlet.ps1 @@ -0,0 +1,18 @@ +#Script violates the rule because Get-CIMInstance is available on PS 3.0 and needs to use that + +#requires -version 3.0 + +function TestFunction +{ + Get-WmiObject -Class Win32_ComputerSystem + + Invoke-WMIMethod -Path Win32_Process -Name Create -ArgumentList notepad.exe + + Register-WMIEvent -Class Win32_ProcessStartTrace -SourceIdentifier "ProcessStarted" + + Set-WMIInstance -Class Win32_Environment -Argument @{Name='MyEnvVar';VariableValue='VarValue';UserName=''} +} + +TestFunction + +Remove-WmiObject -Class Win32_OperatingSystem -Verbose \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingWMICmdlet.tests.ps1 b/Tests/Rules/AvoidUsingWMICmdlet.tests.ps1 new file mode 100644 index 000000000..b8c7a6e44 --- /dev/null +++ b/Tests/Rules/AvoidUsingWMICmdlet.tests.ps1 @@ -0,0 +1,24 @@ +Import-Module PSScriptAnalyzer +$WMIRuleName = "PSAvoidUsingWMICmdlet" +$violationMessage = "File 'AvoidUsingWMICmdlet.ps1' uses WMI cmdlet. For PowerShell 3.0 and above, use CIM cmdlet which perform the same tasks as the WMI cmdlets. The CIM cmdlets comply with WS-Management (WSMan) standards and with the Common Information Model (CIM) standard, which enables the cmdlets to use the same techniques to manage Windows computers and those running other operating systems." +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$violations = Invoke-ScriptAnalyzer $directory\AvoidUsingWMICmdlet.ps1 -IncludeRule $WMIRuleName +$noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingWMICmdletNoViolations.ps1 -IncludeRule $WMIRuleName + +Describe "AvoidUsingWMICmdlet" { + Context "Script contains references to WMI cmdlets - Violation" { + It "Have 5 WMI cmdlet Violations" { + $violations.Count | Should Be 5 + } + + It "has the correct description message for WMI rule violation" { + $violations[0].Message | Should Be $violationMessage + } + } + + Context "Script contains no calls to WMI cmdlet - No violation" { + It "results in no rule violations" { + $noViolations.Count | Should Be 0 + } + } +} \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingWMICmdletNoViolations.ps1 b/Tests/Rules/AvoidUsingWMICmdletNoViolations.ps1 new file mode 100644 index 000000000..51809547a --- /dev/null +++ b/Tests/Rules/AvoidUsingWMICmdletNoViolations.ps1 @@ -0,0 +1,19 @@ +# No Rule violations since this script requires PS 2.0 and Get-CIMInstance is not available for this version +# So using Get-WMIObject is OK + +#requires -Version 2.0 + +Invoke-WMIMethod -Path Win32_Process -Name Create -ArgumentList notepad.exe + +function TestFunction +{ + Get-WmiObject -Class Win32_ComputerSystem + + Register-WMIEvent -Class Win32_ProcessStartTrace -SourceIdentifier "ProcessStarted" + + Set-WMIInstance -Class Win32_Environment -Argument @{Name='MyEnvVar';VariableValue='VarValue';UserName=''} +} + +TestFunction + +Remove-WmiObject -Class Win32_OperatingSystem -Verbose \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingWMIObjectCmdlet.ps1 b/Tests/Rules/AvoidUsingWMIObjectCmdlet.ps1 deleted file mode 100644 index 13f0412c9..000000000 --- a/Tests/Rules/AvoidUsingWMIObjectCmdlet.ps1 +++ /dev/null @@ -1,13 +0,0 @@ -#Script violates the rule because Get-CIMInstance is available on PS 3.0 and needs to use that - -#requires -version 3.0 - -function TestFunction -{ - Get-WmiObject -Class Win32_ComputerSystem - -} - -TestFunction - -Remove-WmiObject -Class Win32_OperatingSystem -Verbose \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingWMIObjectCmdlet.tests.ps1 b/Tests/Rules/AvoidUsingWMIObjectCmdlet.tests.ps1 deleted file mode 100644 index 0a96ad16e..000000000 --- a/Tests/Rules/AvoidUsingWMIObjectCmdlet.tests.ps1 +++ /dev/null @@ -1,24 +0,0 @@ -Import-Module PSScriptAnalyzer -$wmiObjectRuleName = "PSAvoidUsingWMIObjectCmdlet" -$violationMessage = "File 'AvoidUsingWMIObjectCmdlet.ps1' uses WMIObject cmdlet. For PowerShell 3.0 and above, this is not recommended because the cmdlet is based on a non-standard DCOM protocol. Use CIMInstance cmdlet instead. This is CIM and WS-Man standards compliant and works in a heterogeneous environment." -$directory = Split-Path -Parent $MyInvocation.MyCommand.Path -$violations = Invoke-ScriptAnalyzer $directory\AvoidUsingWMIObjectCmdlet.ps1 | Where-Object {$_.RuleName -eq $wmiObjectRuleName} -$noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingWMIObjectCmdletNoViolations.ps1 | Where-Object {$_.RuleName -eq $wmiObjectRuleName} - -Describe "AvoidUsingWMIObjectCmdlet" { - Context "Script contains references to WMIObject cmdlets - Violation" { - It "Have 2 WMIObject cmdlet Violations" { - $violations.Count | Should Be 2 - } - - It "has the correct description message for WMIObject rule violation" { - $violations[0].Message | Should Match $violationMessage - } - } - - Context "Script contains no calls to WMIObject cmdlet - No violation" { - It "results in no rule violations" { - $noViolations.Count | Should Be 0 - } - } -} \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingWMIObjectCmdletNoViolations.ps1 b/Tests/Rules/AvoidUsingWMIObjectCmdletNoViolations.ps1 deleted file mode 100644 index b5d2e6f14..000000000 --- a/Tests/Rules/AvoidUsingWMIObjectCmdletNoViolations.ps1 +++ /dev/null @@ -1,14 +0,0 @@ -# No Rule violations since this script requires PS 2.0 and Get-CIMInstance is not available for this version -# So using Get-WMIObject is OK - -#requires -Version 2.0 - -function TestFunction -{ - Remove-WmiObject -Class Win32_ComputerSystem - -} - -TestFunction - -Get-WmiObject -Class Win32_OperatingSystem -Verbose \ No newline at end of file From 4238f90be93df2798ab4b59d24f09510ae090935 Mon Sep 17 00:00:00 2001 From: GoodOlClint Date: Tue, 5 May 2015 19:34:48 -0500 Subject: [PATCH 13/20] Do not require Write-Verbose in scripts or functions without the CmdletBinding attribute specified. --- Rules/ProvideVerboseMessage.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Rules/ProvideVerboseMessage.cs b/Rules/ProvideVerboseMessage.cs index a69447c0e..af1c6fa10 100644 --- a/Rules/ProvideVerboseMessage.cs +++ b/Rules/ProvideVerboseMessage.cs @@ -12,10 +12,12 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Management.Automation.Language; using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; using System.ComponentModel.Composition; using System.Globalization; +using System.Management.Automation; namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules { @@ -33,11 +35,11 @@ public class ProvideVerboseMessage : SkipNamedBlock, IScriptRule public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - + ClearList(); this.AddNames(new List() { "Configuration", "Workflow" }); DiagnosticRecords.Clear(); - + this.fileName = fileName; //We only check that advanced functions should have Write-Verbose ast.Visit(this); @@ -57,6 +59,17 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun return AstVisitAction.SkipChildren; } + //Write-Verbose is not required for non-advanced functions + if (funcAst.Body != null && funcAst.Body.ParamBlock != null + && funcAst.Body.ParamBlock.Attributes != null && + funcAst.Body.ParamBlock.Parameters != null) + { + if (!funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute))) + { + return AstVisitAction.Continue; + } + } + var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false); bool hasVerbose = false; From 4f8e7a61865dcef1890f035c7ff18bcf022fd0a0 Mon Sep 17 00:00:00 2001 From: "Yuting Chen[MSFT]" Date: Wed, 6 May 2015 09:47:43 -0700 Subject: [PATCH 14/20] Revert "Only Trigger PSProvideVerboseMessage in Advanced Scripts or Functions" --- Rules/ProvideVerboseMessage.cs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/Rules/ProvideVerboseMessage.cs b/Rules/ProvideVerboseMessage.cs index af1c6fa10..a69447c0e 100644 --- a/Rules/ProvideVerboseMessage.cs +++ b/Rules/ProvideVerboseMessage.cs @@ -12,12 +12,10 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Management.Automation.Language; using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; using System.ComponentModel.Composition; using System.Globalization; -using System.Management.Automation; namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules { @@ -35,11 +33,11 @@ public class ProvideVerboseMessage : SkipNamedBlock, IScriptRule public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - + ClearList(); this.AddNames(new List() { "Configuration", "Workflow" }); DiagnosticRecords.Clear(); - + this.fileName = fileName; //We only check that advanced functions should have Write-Verbose ast.Visit(this); @@ -59,17 +57,6 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun return AstVisitAction.SkipChildren; } - //Write-Verbose is not required for non-advanced functions - if (funcAst.Body != null && funcAst.Body.ParamBlock != null - && funcAst.Body.ParamBlock.Attributes != null && - funcAst.Body.ParamBlock.Parameters != null) - { - if (!funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute))) - { - return AstVisitAction.Continue; - } - } - var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false); bool hasVerbose = false; From 3aa8ee84b9dfc80e23a64128050fa29010a75b66 Mon Sep 17 00:00:00 2001 From: "Yuting Chen[MSFT]" Date: Wed, 6 May 2015 09:53:43 -0700 Subject: [PATCH 15/20] =?UTF-8?q?Revert=20"Revert=20"Only=20Trigger=20PSPr?= =?UTF-8?q?ovideVerboseMessage=20in=20Advanced=20Scripts=20or=20F=E2=80=A6?= =?UTF-8?q?"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Rules/ProvideVerboseMessage.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Rules/ProvideVerboseMessage.cs b/Rules/ProvideVerboseMessage.cs index a69447c0e..af1c6fa10 100644 --- a/Rules/ProvideVerboseMessage.cs +++ b/Rules/ProvideVerboseMessage.cs @@ -12,10 +12,12 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Management.Automation.Language; using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; using System.ComponentModel.Composition; using System.Globalization; +using System.Management.Automation; namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules { @@ -33,11 +35,11 @@ public class ProvideVerboseMessage : SkipNamedBlock, IScriptRule public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - + ClearList(); this.AddNames(new List() { "Configuration", "Workflow" }); DiagnosticRecords.Clear(); - + this.fileName = fileName; //We only check that advanced functions should have Write-Verbose ast.Visit(this); @@ -57,6 +59,17 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun return AstVisitAction.SkipChildren; } + //Write-Verbose is not required for non-advanced functions + if (funcAst.Body != null && funcAst.Body.ParamBlock != null + && funcAst.Body.ParamBlock.Attributes != null && + funcAst.Body.ParamBlock.Parameters != null) + { + if (!funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute))) + { + return AstVisitAction.Continue; + } + } + var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false); bool hasVerbose = false; From f3b55e60c26f6b1144584c94f4ff3a6c13c1f89f Mon Sep 17 00:00:00 2001 From: Raghu Shantha Date: Wed, 6 May 2015 10:22:01 -0700 Subject: [PATCH 16/20] Rule Documentation for WMI Cmdlet --- RuleDocumentation/AvoidUsingWMICmdlet.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 RuleDocumentation/AvoidUsingWMICmdlet.md diff --git a/RuleDocumentation/AvoidUsingWMICmdlet.md b/RuleDocumentation/AvoidUsingWMICmdlet.md new file mode 100644 index 000000000..b5b308076 --- /dev/null +++ b/RuleDocumentation/AvoidUsingWMICmdlet.md @@ -0,0 +1,17 @@ +#AvoidAlias +**Severity Level: Warning** + + +##Description + +An alias is an alternate name or nickname for a cmdlet or for a command element, such as a function, script, file, or executable file. But when writing scripts that will potentially need to be maintained over time, either by the original author or another Windows PowerShell scripter, please consider using full cmdlet name instead of alias. Aliases can introduce these problems, readability, understandability and availability. + +##How to Fix + +Please consider using full cmdlet name instead of alias. + +##Example + +Wrong： gps | Where-Object {$_.WorkingSet -gt 20000000} + +Correct: Get-Process | Where-Object {$_.WorkingSet -gt 20000000} From 72858d2f63a134e95cd3a571489856dddc39b1b8 Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Wed, 6 May 2015 10:32:01 -0700 Subject: [PATCH 17/20] Update AvoidUsingWMICmdlet.md --- RuleDocumentation/AvoidUsingWMICmdlet.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/RuleDocumentation/AvoidUsingWMICmdlet.md b/RuleDocumentation/AvoidUsingWMICmdlet.md index b5b308076..6065d1083 100644 --- a/RuleDocumentation/AvoidUsingWMICmdlet.md +++ b/RuleDocumentation/AvoidUsingWMICmdlet.md @@ -1,17 +1,18 @@ -#AvoidAlias +#AvoidUsingWMICmdlet **Severity Level: Warning** ##Description -An alias is an alternate name or nickname for a cmdlet or for a command element, such as a function, script, file, or executable file. But when writing scripts that will potentially need to be maintained over time, either by the original author or another Windows PowerShell scripter, please consider using full cmdlet name instead of alias. Aliases can introduce these problems, readability, understandability and availability. +Avoid Using Get-WMIObject, Remove-WMIObject, Invoke-WmiMethod, Register-WmiEvent, Set-WmiInstance + +For PowerShell 3.0 and above, use CIM cmdlet which perform the same tasks as the WMI cmdlets. The CIM cmdlets comply with WS-Management (WSMan) standards and with the Common Information Model (CIM) standard, which enables the cmdlets to use the same techniques to manage Windows computers and those running other operating systems. ##How to Fix -Please consider using full cmdlet name instead of alias. +Use corresponding CIM cmdlets such as Get-CIMInstance, Remove-CIMInstance, Invoke-CIMMethod, Register-CimIndicationEvent, Set-CimInstance ##Example -Wrong： gps | Where-Object {$_.WorkingSet -gt 20000000} - -Correct: Get-Process | Where-Object {$_.WorkingSet -gt 20000000} +Get-CimInstance -Query 'Select * from Win32_Process where name LIKE "myprocess%"' | Remove-CIMInstance +Invoke-CimMethod –ClassName Win32_Process –MethodName "Create" –Arguments @{ CommandLine = "notepad.exe" } From 6f2a57b7bd46442d52bc2f99e9dcd86aa786b8a2 Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Wed, 6 May 2015 11:18:18 -0700 Subject: [PATCH 18/20] Update AvoidUsingWMICmdlet.md --- RuleDocumentation/AvoidUsingWMICmdlet.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RuleDocumentation/AvoidUsingWMICmdlet.md b/RuleDocumentation/AvoidUsingWMICmdlet.md index 6065d1083..2171c4a62 100644 --- a/RuleDocumentation/AvoidUsingWMICmdlet.md +++ b/RuleDocumentation/AvoidUsingWMICmdlet.md @@ -14,5 +14,10 @@ Use corresponding CIM cmdlets such as Get-CIMInstance, Remove-CIMInstance, Invok ##Example +Wrong: +Get-WmiObject -Query 'Select * from Win32_Process where name LIKE "myprocess%"' | Remove-WmiObject +Invoke-WmiMethod –Class Win32_Process –Name "Create" –ArgumentList @{ CommandLine = "notepad.exe" } + +Correct: Get-CimInstance -Query 'Select * from Win32_Process where name LIKE "myprocess%"' | Remove-CIMInstance Invoke-CimMethod –ClassName Win32_Process –MethodName "Create" –Arguments @{ CommandLine = "notepad.exe" } From 5e7d2f61a9294f30775e3a3591d074f026851860 Mon Sep 17 00:00:00 2001 From: "Yuting Chen[MSFT]" Date: Wed, 6 May 2015 11:21:20 -0700 Subject: [PATCH 19/20] Update AvoidUsingWMICmdlet.md Added quotes for code --- RuleDocumentation/AvoidUsingWMICmdlet.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/RuleDocumentation/AvoidUsingWMICmdlet.md b/RuleDocumentation/AvoidUsingWMICmdlet.md index 2171c4a62..01d6cca01 100644 --- a/RuleDocumentation/AvoidUsingWMICmdlet.md +++ b/RuleDocumentation/AvoidUsingWMICmdlet.md @@ -15,9 +15,12 @@ Use corresponding CIM cmdlets such as Get-CIMInstance, Remove-CIMInstance, Invok ##Example Wrong: +``` Get-WmiObject -Query 'Select * from Win32_Process where name LIKE "myprocess%"' | Remove-WmiObject Invoke-WmiMethod –Class Win32_Process –Name "Create" –ArgumentList @{ CommandLine = "notepad.exe" } - +``` Correct: +``` Get-CimInstance -Query 'Select * from Win32_Process where name LIKE "myprocess%"' | Remove-CIMInstance Invoke-CimMethod –ClassName Win32_Process –MethodName "Create" –Arguments @{ CommandLine = "notepad.exe" } +``` From a29203d5c8e038b534817ef83012b92697c5e90b Mon Sep 17 00:00:00 2001 From: "Yuting Chen[MSFT]" Date: Wed, 6 May 2015 11:51:19 -0700 Subject: [PATCH 20/20] =?UTF-8?q?Revert=20"Revert=20"Revert=20"Only=20Trig?= =?UTF-8?q?ger=20PSProvideVerboseMessage=20in=20Advanced=20Scri=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Rules/ProvideVerboseMessage.cs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/Rules/ProvideVerboseMessage.cs b/Rules/ProvideVerboseMessage.cs index af1c6fa10..a69447c0e 100644 --- a/Rules/ProvideVerboseMessage.cs +++ b/Rules/ProvideVerboseMessage.cs @@ -12,12 +12,10 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Management.Automation.Language; using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; using System.ComponentModel.Composition; using System.Globalization; -using System.Management.Automation; namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules { @@ -35,11 +33,11 @@ public class ProvideVerboseMessage : SkipNamedBlock, IScriptRule public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - + ClearList(); this.AddNames(new List() { "Configuration", "Workflow" }); DiagnosticRecords.Clear(); - + this.fileName = fileName; //We only check that advanced functions should have Write-Verbose ast.Visit(this); @@ -59,17 +57,6 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun return AstVisitAction.SkipChildren; } - //Write-Verbose is not required for non-advanced functions - if (funcAst.Body != null && funcAst.Body.ParamBlock != null - && funcAst.Body.ParamBlock.Attributes != null && - funcAst.Body.ParamBlock.Parameters != null) - { - if (!funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute))) - { - return AstVisitAction.Continue; - } - } - var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false); bool hasVerbose = false;