From a406d8fff9852baad03d390c2095a5eefeaa4d43 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 17 Feb 2016 19:42:21 -0800 Subject: [PATCH 01/29] Fixes to UseToExportFieldsInManifest rule. --- Rules/UseToExportFieldsInManifest.cs | 41 +++++++++++++++++----------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/Rules/UseToExportFieldsInManifest.cs b/Rules/UseToExportFieldsInManifest.cs index f7623c085..2732fcda4 100644 --- a/Rules/UseToExportFieldsInManifest.cs +++ b/Rules/UseToExportFieldsInManifest.cs @@ -97,28 +97,37 @@ private bool HasAcceptableExportField(string key, HashtableAst hast, string scri extent = null; foreach (var pair in hast.KeyValuePairs) { - if (key.Equals(pair.Item1.Extent.Text.Trim(), StringComparison.OrdinalIgnoreCase)) + if (pair.Item1 is StringConstantExpressionAst + && key.Equals((pair.Item1 as StringConstantExpressionAst).Value, + StringComparison.OrdinalIgnoreCase)) { - // checks if the right hand side of the assignment is an array. - var arrayAst = pair.Item2.Find(x => x is ArrayLiteralAst || x is ArrayExpressionAst, true); - if (arrayAst == null) - { - extent = pair.Item2.Extent; - return false; - } - else + //checks for wildcard in the entry. + var elementWithWildcard = pair.Item2.Find(x => x is StringConstantExpressionAst + && WildcardPattern.ContainsWildcardCharacters((x as StringConstantExpressionAst).Value), false); + + if (elementWithWildcard == null) { - //checks if any entry within the array has a wildcard. - var elementWithWildcard = arrayAst.Find(x => x is StringConstantExpressionAst - && x.Extent.Text.Contains("*"), false); - if (elementWithWildcard != null) + //checks for $null in the entry. + var nullAst = pair.Item2.Find(x => x is VariableExpressionAst + && (x as VariableExpressionAst).ToString().Equals("$null", StringComparison.OrdinalIgnoreCase), false); + + if (nullAst == null) + { + return true; + } + else { - extent = elementWithWildcard.Extent; + extent = nullAst.Extent; return false; - } - return true; + } } + else + { + extent = elementWithWildcard.Extent; + return false; + } } + } return true; } From 118d8eed7e812e3ac22e95cb4c32133c9e2af523 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 18 Feb 2016 15:56:14 -0800 Subject: [PATCH 02/29] Fixes some bugs in UseToExportFieldsInManifest rule. --- Rules/UseToExportFieldsInManifest.cs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/Rules/UseToExportFieldsInManifest.cs b/Rules/UseToExportFieldsInManifest.cs index 2732fcda4..bace1a467 100644 --- a/Rules/UseToExportFieldsInManifest.cs +++ b/Rules/UseToExportFieldsInManifest.cs @@ -11,6 +11,7 @@ // using System; +using System.Linq; using System.Collections.Generic; using System.Management.Automation.Language; using System.Management.Automation; @@ -97,19 +98,26 @@ private bool HasAcceptableExportField(string key, HashtableAst hast, string scri extent = null; foreach (var pair in hast.KeyValuePairs) { - if (pair.Item1 is StringConstantExpressionAst - && key.Equals((pair.Item1 as StringConstantExpressionAst).Value, - StringComparison.OrdinalIgnoreCase)) - { - //checks for wildcard in the entry. - var elementWithWildcard = pair.Item2.Find(x => x is StringConstantExpressionAst - && WildcardPattern.ContainsWildcardCharacters((x as StringConstantExpressionAst).Value), false); + var keyStrConstAst = pair.Item1 is StringConstantExpressionAst ? pair.Item1 as StringConstantExpressionAst : null; + + if (keyStrConstAst != null && keyStrConstAst.Value.Equals(key, StringComparison.OrdinalIgnoreCase)) + { + // Checks for wildcard character in the entry. + var strConstAstElements = from element in pair.Item2.FindAll(x => x is StringConstantExpressionAst, false) + select element as StringConstantExpressionAst; + var elementWithWildcard = strConstAstElements.FirstOrDefault(x => x != null + && WildcardPattern.ContainsWildcardCharacters(x.Value)); + if (elementWithWildcard == null) { - //checks for $null in the entry. - var nullAst = pair.Item2.Find(x => x is VariableExpressionAst - && (x as VariableExpressionAst).ToString().Equals("$null", StringComparison.OrdinalIgnoreCase), false); + // Checks for $null in the entry. + var varAstElements = from element in pair.Item2.FindAll(x => x is VariableExpressionAst, false) + select element as VariableExpressionAst; + // VariablePath property is never null hence we don't need to check for it. + var nullAst = varAstElements.FirstOrDefault(x => x != null + && x.VariablePath.IsUnqualified + && x.VariablePath.UserPath.Equals("null", StringComparison.OrdinalIgnoreCase)); if (nullAst == null) { From f45004e852c087b7cc6fb7f66d8da15a645bab62 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 18 Feb 2016 23:50:06 -0800 Subject: [PATCH 03/29] Minor changes to FixUseToExportInManifestRule to improve performance. --- Rules/UseToExportFieldsInManifest.cs | 83 +++++++++++++++++++--------- 1 file changed, 57 insertions(+), 26 deletions(-) diff --git a/Rules/UseToExportFieldsInManifest.cs b/Rules/UseToExportFieldsInManifest.cs index bace1a467..41b840e61 100644 --- a/Rules/UseToExportFieldsInManifest.cs +++ b/Rules/UseToExportFieldsInManifest.cs @@ -84,6 +84,44 @@ private bool IsValidManifest(Ast ast, string fileName) return !missingManifestRule.AnalyzeScript(ast, fileName).GetEnumerator().MoveNext(); } + + /// + /// Checks if the ast contains wildcard character. + /// + /// + /// + private bool HasWildcardInExpression(Ast ast) + { + var strConstExprAst = ast as StringConstantExpressionAst; + if (strConstExprAst != null && WildcardPattern.ContainsWildcardCharacters(strConstExprAst.Value)) + { + return true; + } + else + { + return false; + } + } + + /// + /// Checks if the ast contains null expression. + /// + /// + /// + private bool HasNullInExpression(Ast ast) + { + var varExprAst = ast as VariableExpressionAst; + if (varExprAst != null + && varExprAst.VariablePath.IsUnqualified + && varExprAst.VariablePath.UserPath.Equals("null", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + else + { + return false; + } + } /// /// Checks if the *ToExport fields are explicitly set to arrays, eg. @(...), and the array entries do not contain any wildcard. @@ -98,48 +136,41 @@ private bool HasAcceptableExportField(string key, HashtableAst hast, string scri extent = null; foreach (var pair in hast.KeyValuePairs) { - - var keyStrConstAst = pair.Item1 is StringConstantExpressionAst ? pair.Item1 as StringConstantExpressionAst : null; - + var keyStrConstAst = pair.Item1 as StringConstantExpressionAst; if (keyStrConstAst != null && keyStrConstAst.Value.Equals(key, StringComparison.OrdinalIgnoreCase)) { // Checks for wildcard character in the entry. - var strConstAstElements = from element in pair.Item2.FindAll(x => x is StringConstantExpressionAst, false) - select element as StringConstantExpressionAst; - var elementWithWildcard = strConstAstElements.FirstOrDefault(x => x != null - && WildcardPattern.ContainsWildcardCharacters(x.Value)); - - if (elementWithWildcard == null) + var astWithWildcard = pair.Item2.Find(HasWildcardInExpression, false); + if (astWithWildcard != null) + { + extent = astWithWildcard.Extent; + return false; + } + else { // Checks for $null in the entry. - var varAstElements = from element in pair.Item2.FindAll(x => x is VariableExpressionAst, false) - select element as VariableExpressionAst; - // VariablePath property is never null hence we don't need to check for it. - var nullAst = varAstElements.FirstOrDefault(x => x != null - && x.VariablePath.IsUnqualified - && x.VariablePath.UserPath.Equals("null", StringComparison.OrdinalIgnoreCase)); - - if (nullAst == null) + var astWithNull = pair.Item2.Find(HasNullInExpression, false); + if (astWithNull != null) { - return true; + extent = astWithNull.Extent; + return false; } else { - extent = nullAst.Extent; - return false; + return true; } } - else - { - extent = elementWithWildcard.Extent; - return false; - } } - } return true; } + + /// + /// Gets the error string of the rule. + /// + /// + /// public string GetError(string field) { return string.Format(CultureInfo.CurrentCulture, Strings.UseToExportFieldsInManifestError, field); From 0f8235937f55462f2f35451cf922b6b0edd5b95d Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 22 Feb 2016 12:00:00 -0800 Subject: [PATCH 04/29] Minor changes to methods that implement finding wildcard or null in an ast in UseToExportFieldsInManifest. --- Rules/UseToExportFieldsInManifest.cs | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/Rules/UseToExportFieldsInManifest.cs b/Rules/UseToExportFieldsInManifest.cs index 41b840e61..324c60487 100644 --- a/Rules/UseToExportFieldsInManifest.cs +++ b/Rules/UseToExportFieldsInManifest.cs @@ -93,14 +93,7 @@ private bool IsValidManifest(Ast ast, string fileName) private bool HasWildcardInExpression(Ast ast) { var strConstExprAst = ast as StringConstantExpressionAst; - if (strConstExprAst != null && WildcardPattern.ContainsWildcardCharacters(strConstExprAst.Value)) - { - return true; - } - else - { - return false; - } + return strConstExprAst != null && WildcardPattern.ContainsWildcardCharacters(strConstExprAst.Value); } /// @@ -111,16 +104,9 @@ private bool HasWildcardInExpression(Ast ast) private bool HasNullInExpression(Ast ast) { var varExprAst = ast as VariableExpressionAst; - if (varExprAst != null - && varExprAst.VariablePath.IsUnqualified - && varExprAst.VariablePath.UserPath.Equals("null", StringComparison.OrdinalIgnoreCase)) - { - return true; - } - else - { - return false; - } + return varExprAst != null + && varExprAst.VariablePath.IsUnqualified + && varExprAst.VariablePath.UserPath.Equals("null", StringComparison.OrdinalIgnoreCase); } /// From 84d689611d32f99a4a6ffcac00fcfb9c2e699962 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 22 Feb 2016 19:54:07 -0800 Subject: [PATCH 05/29] Fixes to AvoidNullOrEmptyHelpMessageAttribute. Instead of using ast.extent property for comparing empty string or null argument we use the parsed values. --- Rules/AvoidNullOrEmptyHelpMessageAttribute.cs | 55 ++++++++++++++----- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs b/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs index 78311388b..07fd76e2d 100644 --- a/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs +++ b/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs @@ -38,43 +38,45 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) foreach (FunctionDefinitionAst funcAst in functionAsts) { - if (funcAst.Body == null || funcAst.Body.ParamBlock == null - || funcAst.Body.ParamBlock.Attributes == null || funcAst.Body.ParamBlock.Parameters == null) + if (funcAst.Body == null || funcAst.Body.ParamBlock == null || funcAst.Body.ParamBlock.Parameters == null) { continue; } - foreach (var paramAst in funcAst.Body.ParamBlock.Parameters) - { - foreach (var paramAstAttribute in paramAst.Attributes) + foreach (ParameterAst paramAst in funcAst.Body.ParamBlock.Parameters) + { + if (paramAst == null) { - if (!(paramAstAttribute is AttributeAst)) + continue; + } + + foreach (AttributeBaseAst attributeAst in paramAst.Attributes) + { + var paramAttributeAst = attributeAst as AttributeAst; + if (paramAttributeAst == null) { continue; } - var namedArguments = (paramAstAttribute as AttributeAst).NamedArguments; - + var namedArguments = paramAttributeAst.NamedArguments; if (namedArguments == null) { continue; } - + foreach (NamedAttributeArgumentAst namedArgument in namedArguments) { - if (!(namedArgument.ArgumentName.Equals("HelpMessage", StringComparison.OrdinalIgnoreCase))) + if (namedArgument == null || !(namedArgument.ArgumentName.Equals("HelpMessage", StringComparison.OrdinalIgnoreCase))) { continue; } string errCondition; - if (namedArgument.ExpressionOmitted - || namedArgument.Argument.Extent.Text.Equals("\"\"") - || namedArgument.Argument.Extent.Text.Equals("\'\'")) + if (namedArgument.ExpressionOmitted || HasEmptyStringInExpression(namedArgument.Argument)) { errCondition = "empty"; } - else if (namedArgument.Argument.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) + else if (HasNullInExpression(namedArgument.Argument)) { errCondition = "null"; } @@ -82,7 +84,6 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { errCondition = null; } - if (!String.IsNullOrEmpty(errCondition)) { string message = string.Format(CultureInfo.CurrentCulture, @@ -102,6 +103,30 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } + /// + /// Checks if the given ast is an empty string. + /// + /// + /// + private bool HasEmptyStringInExpression(ExpressionAst ast) + { + var constStrAst = ast as StringConstantExpressionAst; + return constStrAst != null && constStrAst.Value.Equals(String.Empty); + } + + /// + /// Checks if the ast contains null expression. + /// + /// + /// + private bool HasNullInExpression(Ast ast) + { + var varExprAst = ast as VariableExpressionAst; + return varExprAst != null + && varExprAst.VariablePath.IsUnqualified + && varExprAst.VariablePath.UserPath.Equals("null", StringComparison.OrdinalIgnoreCase); + } + /// /// GetName: Retrieves the name of this rule. /// From 87be819209ebbbb51d8fb12f42c1b738ea7cb56e Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Sun, 28 Feb 2016 22:11:28 -0800 Subject: [PATCH 06/29] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index be947f915..b272eeee5 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ ##### ScriptAnalyzer community meeting schedule: - - [Next Meeting - Mar 1 2016 - 11am to 12pm PDT](http://1drv.ms/1VvAaxO) + - [Next Meeting - Mar 29 2016 - 11am to 12pm PDT](http://1drv.ms/1VvAaxO) - [Notes and recordings from earlier meetings](https://github.com/PowerShell/PSScriptAnalyzer/wiki) From fc4eff4eb1e1a7ee7e1e243c64729c159d8cb390 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 2 Mar 2016 17:08:15 -0800 Subject: [PATCH 07/29] Modifies the error message of AvoidUsernameAndPasswordParams rule. --- Rules/Strings.Designer.cs | 4 ++-- Rules/Strings.resx | 4 ++-- Rules/UsePSCredentialType.cs | 4 ++-- Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index aae951eeb..22ae1b249 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -457,7 +457,7 @@ internal static string AvoidUsernameAndPasswordParamsCommonName { } /// - /// Looks up a localized string similar to Functions should only take in a credential parameter of type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute instead of username and password parameters.. + /// Looks up a localized string similar to Functions should take in a credential parameter of type PSCredential with CredentialAttribute or set the password parameter to SecureString type.. /// internal static string AvoidUsernameAndPasswordParamsDescription { get { @@ -466,7 +466,7 @@ internal static string AvoidUsernameAndPasswordParamsDescription { } /// - /// Looks up a localized string similar to Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used.. + /// Looks up a localized string similar to Function '{0}' has both username and password parameters. Either set the type of password parameter to SecureString or replace the username and password parameters by a credential parameter of type PSCredential.. /// internal static string AvoidUsernameAndPasswordParamsError { get { diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 233f99a5e..786abdebd 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -511,10 +511,10 @@ Avoid Using Username and Password Parameters - Functions should only take in a credential parameter of type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute instead of username and password parameters. + Functions should take in a credential parameter of type PSCredential with CredentialAttribute or set the password parameter to SecureString type. - Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used. + Function '{0}' has both username and password parameters. Either set the type of password parameter to SecureString or replace the username and password parameters by a credential parameter of type PSCredential. AvoidUsingUserNameAndPassWordParams diff --git a/Rules/UsePSCredentialType.cs b/Rules/UsePSCredentialType.cs index bddc3bf66..d165dd73d 100644 --- a/Rules/UsePSCredentialType.cs +++ b/Rules/UsePSCredentialType.cs @@ -24,13 +24,13 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// UsePSCredentialType: Analyzes the ast to check that cmdlets that have a Credential parameter accept PSCredential. + /// UsePSCredentialType: Checks if a credential parameter of type PSCredential has a credential attribute of type CredentialAttribute. /// [Export(typeof(IScriptRule))] public class UsePSCredentialType : IScriptRule { /// - /// AnalyzeScript: Analyzes the ast to check that cmdlets that have a Credential parameter accept PSCredential. + /// AnalyzeScript: Analyzes the ast to check if a credential parameter of type PSCredential has a credential attribute of type CredentialAttribute. /// /// The script's ast /// The script's file name diff --git a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 index 555bc4ca9..5a5f339d4 100644 --- a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 +++ b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Function 'TestFunction' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used." +$violationMessage = "Function 'TestFunction' has both username and password parameters. Either set the type of password parameter to SecureString or replace the username and password parameters by a credential parameter of type PSCredential." $violationName = "PSAvoidUsingUserNameAndPasswordParams" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName} From 3e1bc0d0c52b7931b9a56d8d6a323d3e8ca982f6 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Fri, 4 Mar 2016 10:38:50 -0800 Subject: [PATCH 08/29] Modifies the error message of UsePSCredentialType as the rule is not applicable for PSVersions > 5.0. --- Rules/Strings.Designer.cs | 8 ++++---- Rules/Strings.resx | 8 ++++---- Rules/UsePSCredentialType.cs | 4 ++-- Tests/Rules/PSCredentialType.tests.ps1 | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 22ae1b249..9baec9e63 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1834,7 +1834,7 @@ internal static string UseOutputTypeCorrectlyName { } /// - /// Looks up a localized string similar to PSCredential. + /// Looks up a localized string similar to Use PSCredential type.. /// internal static string UsePSCredentialTypeCommonName { get { @@ -1843,7 +1843,7 @@ internal static string UsePSCredentialTypeCommonName { } /// - /// Looks up a localized string similar to Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. This comes from the PowerShell teams best practices.. + /// Looks up a localized string similar to Checks if a credential parameter of type PSCredential has a CredentialAttribute attribute such that PSCredential precedes CredentialAttribute. This is not applicable to PowerShell version 5.0 or above. . /// internal static string UsePSCredentialTypeDescription { get { @@ -1852,7 +1852,7 @@ internal static string UsePSCredentialTypeDescription { } /// - /// Looks up a localized string similar to The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. + /// Looks up a localized string similar to The Credential parameter in '{0}' must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above.. /// internal static string UsePSCredentialTypeError { get { @@ -1861,7 +1861,7 @@ internal static string UsePSCredentialTypeError { } /// - /// Looks up a localized string similar to The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. + /// Looks up a localized string similar to The Credential parameter found in the script block must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above.. /// internal static string UsePSCredentialTypeErrorSB { get { diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 786abdebd..da379e8d3 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -217,16 +217,16 @@ One Char - Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. This comes from the PowerShell teams best practices. + Checks if a credential parameter of type PSCredential has a CredentialAttribute attribute such that PSCredential precedes CredentialAttribute. This is not applicable to PowerShell version 5.0 or above. - The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute. + The Credential parameter in '{0}' must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above. - The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute. + The Credential parameter found in the script block must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above. - PSCredential + Use PSCredential type. Checks for reserved characters in cmdlet names. These characters usually cause a parsing error. Otherwise they will generally cause runtime errors. diff --git a/Rules/UsePSCredentialType.cs b/Rules/UsePSCredentialType.cs index d165dd73d..65985b107 100644 --- a/Rules/UsePSCredentialType.cs +++ b/Rules/UsePSCredentialType.cs @@ -24,13 +24,13 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// UsePSCredentialType: Checks if a credential parameter of type PSCredential has a credential attribute of type CredentialAttribute. + /// UsePSCredentialType: Checks if a credential parameter of type PSCredential has a credential attribute of type CredentialAttribute such that the type precedes the attribute. This is applicable to only to PowerShell Version less than 5.0. /// [Export(typeof(IScriptRule))] public class UsePSCredentialType : IScriptRule { /// - /// AnalyzeScript: Analyzes the ast to check if a credential parameter of type PSCredential has a credential attribute of type CredentialAttribute. + /// AnalyzeScript: Analyzes the ast to check if a credential parameter of type PSCredential has a credential attribute of type CredentialAttribute such that the type precedes the attribute. This is applicable to only to PowerShell Version less than 5.0. /// /// The script's ast /// The script's file name diff --git a/Tests/Rules/PSCredentialType.tests.ps1 b/Tests/Rules/PSCredentialType.tests.ps1 index 852af655c..a9edf1906 100644 --- a/Tests/Rules/PSCredentialType.tests.ps1 +++ b/Tests/Rules/PSCredentialType.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The Credential parameter in 'Credential' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute." +$violationMessage = "The Credential parameter in 'Credential' must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above." $violationName = "PSUsePSCredentialType" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\PSCredentialType.ps1 | Where-Object {$_.RuleName -eq $violationName} From e2b77f2b9a4b7d646d94e65c5b43f693702bea15 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 10 Mar 2016 16:18:38 -0800 Subject: [PATCH 09/29] Fixes a bug that prevented rule modules with version in their path from being loaded. --- Engine/ScriptAnalyzer.cs | 55 +++------ Tests/Engine/CustomizedRule.tests.ps1 | 21 +++- .../1.0.0.0/SampleRuleWithVersion.psd1 | 112 ++++++++++++++++++ .../1.0.0.0/SampleRuleWithVersion.psm1 | 47 ++++++++ 4 files changed, 193 insertions(+), 42 deletions(-) create mode 100644 Tests/Engine/SampleRuleWithVersion/SampleRuleWithVersion/1.0.0.0/SampleRuleWithVersion.psd1 create mode 100644 Tests/Engine/SampleRuleWithVersion/SampleRuleWithVersion/1.0.0.0/SampleRuleWithVersion.psm1 diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index df11bf9d8..1e7227b47 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -706,28 +706,16 @@ private List GetExternalRule(string[] moduleNames) // Imports modules by using full path. InitialSessionState state = InitialSessionState.CreateDefault2(); - state.ImportPSModule(new string[] { moduleName }); - using (System.Management.Automation.PowerShell posh = System.Management.Automation.PowerShell.Create(state)) { - posh.AddCommand("Get-Module"); - Collection loadedModules = posh.Invoke(); - foreach (PSModuleInfo module in loadedModules) + posh.AddCommand("Import-Module").AddArgument(moduleName).AddParameter("PassThru"); + Collection loadedModules = posh.Invoke(); + if (loadedModules != null && loadedModules.Count > 0) { - string pathToCompare = moduleName; - if (!File.GetAttributes(moduleName).HasFlag(FileAttributes.Directory)) - { - pathToCompare = Path.GetDirectoryName(moduleName); - } - - if (pathToCompare == Path.GetDirectoryName(module.Path)) - { - shortModuleName = module.Name; - break; - } + shortModuleName = loadedModules.First().Name; } - + // Invokes Get-Command and Get-Help for each functions in the module. posh.Commands.Clear(); posh.AddCommand("Get-Command").AddParameter("Module", shortModuleName); @@ -987,34 +975,21 @@ public Dictionary> CheckRuleExtension(string[] path, PathIn { resolvedPath = basePath .GetResolvedPSPathFromPSPath(childPath).First().ToString(); - } + } // Import the module - InitialSessionState state = InitialSessionState.CreateDefault2(); - state.ImportPSModule(new string[] { resolvedPath }); - + InitialSessionState state = InitialSessionState.CreateDefault2(); using (System.Management.Automation.PowerShell posh = System.Management.Automation.PowerShell.Create(state)) - { - posh.AddCommand("Get-Module"); + { + posh.AddCommand("Import-Module").AddArgument(resolvedPath).AddParameter("PassThru"); Collection loadedModules = posh.Invoke(); - foreach (PSModuleInfo module in loadedModules) - { - string pathToCompare = resolvedPath; - if (!File.GetAttributes(resolvedPath).HasFlag(FileAttributes.Directory)) - { - pathToCompare = Path.GetDirectoryName(resolvedPath); - } - - if (pathToCompare == Path.GetDirectoryName(module.Path)) - { - if (module.ExportedFunctions.Count > 0) - { - validModPaths.Add(resolvedPath); - } - break; - } - } + if (loadedModules != null + && loadedModules.Count > 0 + && loadedModules.First().ExportedFunctions.Count > 0) + { + validModPaths.Add(resolvedPath); + } } } catch diff --git a/Tests/Engine/CustomizedRule.tests.ps1 b/Tests/Engine/CustomizedRule.tests.ps1 index 6c3cb10e9..937587993 100644 --- a/Tests/Engine/CustomizedRule.tests.ps1 +++ b/Tests/Engine/CustomizedRule.tests.ps1 @@ -108,7 +108,7 @@ Describe "Test importing correct customized rules" { It "will show the custom rules when given glob with recurse switch" { $customizedRulePath = Get-ScriptAnalyzerRule -RecurseCustomRulePath -CustomizedRulePath $directory\samplerule* | Where-Object {$_.RuleName -eq $measure} - $customizedRulePath.Count | Should be 3 + $customizedRulePath.Count | Should be 4 } } @@ -147,7 +147,7 @@ Describe "Test importing correct customized rules" { It "will show the custom rules when given glob with recurse switch" { $customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -RecurseCustomRulePath -CustomizedRulePath $directory\samplerule* | Where-Object {$_.Message -eq $message} - $customizedRulePath.Count | Should be 3 + $customizedRulePath.Count | Should be 4 } It "Using IncludeDefaultRules Switch with CustomRulePath" { @@ -164,6 +164,23 @@ Describe "Test importing correct customized rules" { $customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 $customizedRulePath.Count | Should Be 1 } + + It "loads custom rules that contain version in their path" { + $customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -CustomRulePath $directory\SampleRuleWithVersion\SampleRuleWithVersion + $customizedRulePath.Count | Should Be 1 + + $customizedRulePath = Get-ScriptAnalyzerRule -CustomRulePath $directory\SampleRuleWithVersion\SampleRuleWithVersion + $customizedRulePath.Count | Should Be 1 + } + + It "loads custom rules that contain version in their path with the RecurseCustomRule switch" { + $customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -CustomRulePath $directory\SampleRuleWithVersion -RecurseCustomRulePath + $customizedRulePath.Count | Should Be 1 + + $customizedRulePath = Get-ScriptAnalyzerRule -CustomRulePath $directory\SampleRuleWithVersion -RecurseCustomRulePath + $customizedRulePath.Count | Should Be 1 + + } } } diff --git a/Tests/Engine/SampleRuleWithVersion/SampleRuleWithVersion/1.0.0.0/SampleRuleWithVersion.psd1 b/Tests/Engine/SampleRuleWithVersion/SampleRuleWithVersion/1.0.0.0/SampleRuleWithVersion.psd1 new file mode 100644 index 000000000..be3c1192f --- /dev/null +++ b/Tests/Engine/SampleRuleWithVersion/SampleRuleWithVersion/1.0.0.0/SampleRuleWithVersion.psd1 @@ -0,0 +1,112 @@ +# +# Module manifest for module 'SampleRuleWithVersion' +# + +@{ + +# Script module or binary module file associated with this manifest. +RootModule = 'SampleRuleWithVersion.psm1' + +# Version number of this module. +ModuleVersion = '1.0.0.0' + +# ID used to uniquely identify this module +GUID = 'f3452359-9e01-4c64-89cc-f5bfbcee53e3' + +# Author of this module +Author = '' + +# Company or vendor of this module +CompanyName = '' + +# Copyright statement for this module +Copyright = '' + +# Description of the functionality provided by this module +Description = 'Sample PSScriptAnalyzer rule.' + +# Minimum version of the Windows PowerShell engine required by this module +PowerShellVersion = '3.0' + +# Name of the Windows PowerShell host required by this module +# PowerShellHostName = '' + +# Minimum version of the Windows PowerShell host required by this module +# PowerShellHostVersion = '' + +# Minimum version of Microsoft .NET Framework required by this module +# DotNetFrameworkVersion = '' + +# Minimum version of the common language runtime (CLR) required by this module +CLRVersion = '4.0' + +# Processor architecture (None, X86, Amd64) required by this module +# ProcessorArchitecture = '' + +# Modules that must be imported into the global environment prior to importing this module +# RequiredModules = @() + +# Assemblies that must be loaded prior to importing this module +# RequiredAssemblies = @() + +# Script files (.ps1) that are run in the caller's environment prior to importing this module. +# ScriptsToProcess = @() + +# Type files (.ps1xml) to be loaded when importing this module +# TypesToProcess = @() + +# Format files (.ps1xml) to be loaded when importing this module +# FormatsToProcess = @() + +# Modules to import as nested modules of the module specified in RootModule/ModuleToProcess +# NestedModules = @() + +# Functions to export from this module +FunctionsToExport = 'Measure*' + +# Cmdlets to export from this module +CmdletsToExport = '*' + +# Variables to export from this module +VariablesToExport = '*' + +# Aliases to export from this module +AliasesToExport = '*' + +# List of all modules packaged with this module +# ModuleList = @() + +# List of all files packaged with this module +# FileList = @() + +# Private data to pass to the module specified in RootModule/ModuleToProcess. This may also contain a PSData hashtable with additional module metadata used by PowerShell. +PrivateData = @{ + + PSData = @{ + + # Tags applied to this module. These help with module discovery in online galleries. + # Tags = @() + + # A URL to the license for this module. + # LicenseUri = '' + + # A URL to the main website for this project. + # ProjectUri = '' + + # A URL to an icon representing this module. + # IconUri = '' + + # ReleaseNotes of this module + # ReleaseNotes = '' + + } # End of PSData hashtable + +} # End of PrivateData hashtable + +# HelpInfo URI of this module +# HelpInfoURI = '' + +# Default prefix for commands exported from this module. Override the default prefix using Import-Module -Prefix. +# DefaultCommandPrefix = '' + +} \ No newline at end of file diff --git a/Tests/Engine/SampleRuleWithVersion/SampleRuleWithVersion/1.0.0.0/SampleRuleWithVersion.psm1 b/Tests/Engine/SampleRuleWithVersion/SampleRuleWithVersion/1.0.0.0/SampleRuleWithVersion.psm1 new file mode 100644 index 000000000..cc94c6b1b --- /dev/null +++ b/Tests/Engine/SampleRuleWithVersion/SampleRuleWithVersion/1.0.0.0/SampleRuleWithVersion.psm1 @@ -0,0 +1,47 @@ +#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] + $testAst + ) + + + $results = @() + + $result = [Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]]@{"Message" = "this is help"; + "Extent" = $ast.Extent; + "RuleName" = $PSCmdlet.MyInvocation.InvocationName; + "Severity" = "Warning"} + + $results += $result + + + return $results + + +} +Export-ModuleMember -Function Measure* \ No newline at end of file From a67642c36f7ac0fb7a506d4ee0204c5476dfe700 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 10 Mar 2016 17:37:37 -0800 Subject: [PATCH 10/29] Update the error messages of UsePSCredentialType and AvoidUsernameAndPasswordParams --- Rules/Strings.Designer.cs | 10 +++++----- Rules/Strings.resx | 10 +++++----- Rules/UsePSCredentialType.cs | 13 +++++++++++-- .../Rules/AvoidUserNameAndPasswordParams.tests.ps1 | 4 ++-- Tests/Rules/PSCredentialType.tests.ps1 | 4 ++-- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 9baec9e63..844c0061c 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -457,7 +457,7 @@ internal static string AvoidUsernameAndPasswordParamsCommonName { } /// - /// Looks up a localized string similar to Functions should take in a credential parameter of type PSCredential with CredentialAttribute or set the password parameter to SecureString type.. + /// Looks up a localized string similar to Functions should take in a Credential parameter of type PSCredential (with a Credential transformation attribute defined after it in PowerShell 4.0 or earlier) or set the Password parameter to type SecureString.. /// internal static string AvoidUsernameAndPasswordParamsDescription { get { @@ -466,7 +466,7 @@ internal static string AvoidUsernameAndPasswordParamsDescription { } /// - /// Looks up a localized string similar to Function '{0}' has both username and password parameters. Either set the type of password parameter to SecureString or replace the username and password parameters by a credential parameter of type PSCredential.. + /// Looks up a localized string similar to Function '{0}' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute.. /// internal static string AvoidUsernameAndPasswordParamsError { get { @@ -1843,7 +1843,7 @@ internal static string UsePSCredentialTypeCommonName { } /// - /// Looks up a localized string similar to Checks if a credential parameter of type PSCredential has a CredentialAttribute attribute such that PSCredential precedes CredentialAttribute. This is not applicable to PowerShell version 5.0 or above. . + /// Looks up a localized string similar to For PowerShell 4.0 and earlier, a parameter named Credential with type PSCredential must have a credential transformation attribute defined after the PSCredential type attribute. . /// internal static string UsePSCredentialTypeDescription { get { @@ -1852,7 +1852,7 @@ internal static string UsePSCredentialTypeDescription { } /// - /// Looks up a localized string similar to The Credential parameter in '{0}' must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above.. + /// Looks up a localized string similar to The Credential parameter in '{0}' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute.. /// internal static string UsePSCredentialTypeError { get { @@ -1861,7 +1861,7 @@ internal static string UsePSCredentialTypeError { } /// - /// Looks up a localized string similar to The Credential parameter found in the script block must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above.. + /// Looks up a localized string similar to The Credential parameter found in the script block must be of type PSCredential. For PowerShell 4.0 and earlier please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute. . /// internal static string UsePSCredentialTypeErrorSB { get { diff --git a/Rules/Strings.resx b/Rules/Strings.resx index da379e8d3..6b62986c1 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -217,13 +217,13 @@ One Char - Checks if a credential parameter of type PSCredential has a CredentialAttribute attribute such that PSCredential precedes CredentialAttribute. This is not applicable to PowerShell version 5.0 or above. + For PowerShell 4.0 and earlier, a parameter named Credential with type PSCredential must have a credential transformation attribute defined after the PSCredential type attribute. - The Credential parameter in '{0}' must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above. + The Credential parameter in '{0}' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute. - The Credential parameter found in the script block must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above. + The Credential parameter found in the script block must be of type PSCredential. For PowerShell 4.0 and earlier please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute. Use PSCredential type. @@ -511,10 +511,10 @@ Avoid Using Username and Password Parameters - Functions should take in a credential parameter of type PSCredential with CredentialAttribute or set the password parameter to SecureString type. + Functions should take in a Credential parameter of type PSCredential (with a Credential transformation attribute defined after it in PowerShell 4.0 or earlier) or set the Password parameter to type SecureString. - Function '{0}' has both username and password parameters. Either set the type of password parameter to SecureString or replace the username and password parameters by a credential parameter of type PSCredential. + Function '{0}' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute. AvoidUsingUserNameAndPassWordParams diff --git a/Rules/UsePSCredentialType.cs b/Rules/UsePSCredentialType.cs index 65985b107..9f3c17377 100644 --- a/Rules/UsePSCredentialType.cs +++ b/Rules/UsePSCredentialType.cs @@ -24,13 +24,13 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// UsePSCredentialType: Checks if a credential parameter of type PSCredential has a credential attribute of type CredentialAttribute such that the type precedes the attribute. This is applicable to only to PowerShell Version less than 5.0. + /// UsePSCredentialType: Checks if a parameter named Credential is of type PSCredential. Also checks if there is a credential transformation attribute defined after the PSCredential type attribute. The order between credential transformation attribute and PSCredential type attribute is applicable only to Poweshell 4.0 and earlier. /// [Export(typeof(IScriptRule))] public class UsePSCredentialType : IScriptRule { /// - /// AnalyzeScript: Analyzes the ast to check if a credential parameter of type PSCredential has a credential attribute of type CredentialAttribute such that the type precedes the attribute. This is applicable to only to PowerShell Version less than 5.0. + /// AnalyzeScript: Analyzes the ast to check if a parameter named Credential is of type PSCredential. Also checks if there is a credential transformation attribute defined after the PSCredential type attribute. The order between the credential transformation attribute and PSCredential type attribute is applicable only to Poweshell 4.0 and earlier. /// /// The script's ast /// The script's file name @@ -39,6 +39,15 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + var sbAst = ast as ScriptBlockAst; + if (sbAst != null + && sbAst.ScriptRequirements != null + && sbAst.ScriptRequirements.RequiredPSVersion != null + && sbAst.ScriptRequirements.RequiredPSVersion.Major == 5) + { + yield break; + } + IEnumerable funcDefAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true); IEnumerable scriptBlockAsts = ast.FindAll(testAst => testAst is ScriptBlockAst, true); diff --git a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 index 5a5f339d4..df39b6931 100644 --- a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 +++ b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Function 'TestFunction' has both username and password parameters. Either set the type of password parameter to SecureString or replace the username and password parameters by a credential parameter of type PSCredential." +$violationMessage = "Function 'TestFunction' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute." $violationName = "PSAvoidUsingUserNameAndPasswordParams" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName} @@ -13,7 +13,7 @@ Describe "AvoidUserNameAndPasswordParams" { } It "has the correct violation message" { - $violations[0].Message | Should Match $violationMessage + $violations[0].Message | Should Be $violationMessage } } diff --git a/Tests/Rules/PSCredentialType.tests.ps1 b/Tests/Rules/PSCredentialType.tests.ps1 index a9edf1906..7f4207e1d 100644 --- a/Tests/Rules/PSCredentialType.tests.ps1 +++ b/Tests/Rules/PSCredentialType.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The Credential parameter in 'Credential' must be of type PSCredential and must have a CredentialAttribute attribute such that PSCredential is placed before CredentialAttribute. This is not applicable to PowerShell version 5.0 or above." +$violationMessage = "The Credential parameter in 'Credential' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute." $violationName = "PSUsePSCredentialType" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\PSCredentialType.ps1 | Where-Object {$_.RuleName -eq $violationName} @@ -12,7 +12,7 @@ Describe "PSCredentialType" { } It "has the correct description message" { - $violations[0].Message | Should Match $violationMessage + $violations[0].Message | Should Be $violationMessage } } From 34f9aa485fb9ad8c746a236876d8371203b185d0 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Fri, 11 Mar 2016 21:27:00 -0800 Subject: [PATCH 11/29] Fix loading custom rules that do not have comment help Add conditions to check if indexing does not take place on a null valued Value property. If a PowerShell based custom rule does't have comment based help, the engine throws an error. This occurs because the lack of comment based help results in a null value for the Value property of the dynamically typed description variable. --- Engine/ScriptAnalyzer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 1e7227b47..07b891ae3 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -764,8 +764,8 @@ private List GetExternalRule(string[] moduleNames) { dynamic description = helpContent[0].Properties["Description"]; - if (null != description) - { + if (null != description && null != description.Value && description.Value.GetType().IsArray) + { desc = description.Value[0].Text; } } From 68ae3ff792481e6a35198762025511b5de0a23d9 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 14 Mar 2016 14:22:18 -0700 Subject: [PATCH 12/29] Changes the appveyor default image to Visual Studio 2015 --- appveyor.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 50865ac4c..f6a0abf1f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,5 +1,7 @@ # WS2012R2 image with April WMF5.0 -os: unstable +# os: unstable + +image: Visual Studio 2015 # clone directory clone_folder: c:\projects\psscriptanalyzer From f090fa7e0d0fbc2d44e4efc0ea0df10e939b68a6 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 14 Mar 2016 14:32:20 -0700 Subject: [PATCH 13/29] Change appveyor settings path --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index f6a0abf1f..a22b84fb1 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -8,6 +8,7 @@ clone_folder: c:\projects\psscriptanalyzer # Install Pester install: + - set PATH=C:\Program Files (x86)\MSBuild\12.0\Bin;%PATH% - cinst -y pester --version 3.3.13 # Build PSScriptAnalyzer using msbuild From eecab58688ef10fa1aa044b86ef359384a31d909 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 14 Mar 2016 15:08:24 -0700 Subject: [PATCH 14/29] Change appveyor os image to WMF 5 --- appveyor.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index a22b84fb1..6c3e376d6 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,14 +1,11 @@ -# WS2012R2 image with April WMF5.0 -# os: unstable - -image: Visual Studio 2015 +# Image with WMF5.0 RTM +os: "WMF 5" # clone directory clone_folder: c:\projects\psscriptanalyzer # Install Pester install: - - set PATH=C:\Program Files (x86)\MSBuild\12.0\Bin;%PATH% - cinst -y pester --version 3.3.13 # Build PSScriptAnalyzer using msbuild From 7d766f14d23ab3da5409ebe163f7ff6e021fcbfe Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 14 Mar 2016 15:08:24 -0700 Subject: [PATCH 15/29] Change appveyor os image to WMF 5 --- appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 50865ac4c..6c3e376d6 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,5 +1,5 @@ -# WS2012R2 image with April WMF5.0 -os: unstable +# Image with WMF5.0 RTM +os: "WMF 5" # clone directory clone_folder: c:\projects\psscriptanalyzer From 98323d06464829295bd6d0a247feb42002f8bf79 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 14 Mar 2016 17:19:42 -0700 Subject: [PATCH 16/29] Change the example in UsePSCredentialType error message --- Rules/Strings.Designer.cs | 4 ++-- Rules/Strings.resx | 4 ++-- Tests/Rules/PSCredentialType.ps1 | 2 +- Tests/Rules/PSCredentialType.tests.ps1 | 2 +- Tests/Rules/PSCredentialTypeNoViolations.ps1 | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 844c0061c..c8bc1fe79 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1852,7 +1852,7 @@ internal static string UsePSCredentialTypeDescription { } /// - /// Looks up a localized string similar to The Credential parameter in '{0}' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute.. + /// Looks up a localized string similar to The Credential parameter in '{0}' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute.. /// internal static string UsePSCredentialTypeError { get { @@ -1861,7 +1861,7 @@ internal static string UsePSCredentialTypeError { } /// - /// Looks up a localized string similar to The Credential parameter found in the script block must be of type PSCredential. For PowerShell 4.0 and earlier please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute. . + /// Looks up a localized string similar to The Credential parameter found in the script block must be of type PSCredential. For PowerShell 4.0 and earlier please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute. . /// internal static string UsePSCredentialTypeErrorSB { get { diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 6b62986c1..a7c969482 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -220,10 +220,10 @@ For PowerShell 4.0 and earlier, a parameter named Credential with type PSCredential must have a credential transformation attribute defined after the PSCredential type attribute. - The Credential parameter in '{0}' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute. + The Credential parameter in '{0}' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute. - The Credential parameter found in the script block must be of type PSCredential. For PowerShell 4.0 and earlier please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute. + The Credential parameter found in the script block must be of type PSCredential. For PowerShell 4.0 and earlier please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute. Use PSCredential type. diff --git a/Tests/Rules/PSCredentialType.ps1 b/Tests/Rules/PSCredentialType.ps1 index 3e2d9e730..f89ebdf01 100644 --- a/Tests/Rules/PSCredentialType.ps1 +++ b/Tests/Rules/PSCredentialType.ps1 @@ -14,7 +14,7 @@ function Credential2 [Parameter(Mandatory=$true, ValueFromPipelineByPropertyName=$true, Position=0)] - [System.Management.Automation.CredentialAttribute()] + [System.Management.Automation.Credential()] [pscredential] $Credential ) diff --git a/Tests/Rules/PSCredentialType.tests.ps1 b/Tests/Rules/PSCredentialType.tests.ps1 index 7f4207e1d..866e46a60 100644 --- a/Tests/Rules/PSCredentialType.tests.ps1 +++ b/Tests/Rules/PSCredentialType.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The Credential parameter in 'Credential' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, [CredentialAttribute()], after the PSCredential type attribute." +$violationMessage = "The Credential parameter in 'Credential' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute." $violationName = "PSUsePSCredentialType" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\PSCredentialType.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/PSCredentialTypeNoViolations.ps1 b/Tests/Rules/PSCredentialTypeNoViolations.ps1 index 7b8f09bc0..6f4f375ad 100644 --- a/Tests/Rules/PSCredentialTypeNoViolations.ps1 +++ b/Tests/Rules/PSCredentialTypeNoViolations.ps1 @@ -10,7 +10,7 @@ ValueFromPipelineByPropertyName=$true, Position=0)] [pscredential] - [System.Management.Automation.CredentialAttribute()] + [System.Management.Automation.Credential()] $Credential ) } \ No newline at end of file From 9ce81fe285399513f2414d3a6d2393243a842b3b Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 16 Mar 2016 13:12:00 -0700 Subject: [PATCH 17/29] Remove severity inconsistency for AvoidUsingConvertToSecureStringWithPlainText rule. The severity showed by Get-ScriptAnalyzerRule and Invoke-ScriptAnalyzer for this rule were different. This was because scriptanalyzer defines two types of severities, viz., RuleSeverity and DiagnosticSeverity and the severity levels were not consistent. This commit makes the severity level consistent by setting them both to Warning. --- Engine/Generic/AvoidParameterGeneric.cs | 12 +++++++++++- Rules/AvoidUsingComputerNameHardcoded.cs | 9 +++++++++ .../AvoidUsingConvertToSecureStringWithPlainText.cs | 11 ++++++++++- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 6 +++--- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/Engine/Generic/AvoidParameterGeneric.cs b/Engine/Generic/AvoidParameterGeneric.cs index 2f108b07d..039994c1f 100644 --- a/Engine/Generic/AvoidParameterGeneric.cs +++ b/Engine/Generic/AvoidParameterGeneric.cs @@ -44,7 +44,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ParameterCondition(cmdAst, ceAst)) { - yield return new DiagnosticRecord(GetError(fileName, cmdAst), cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + yield return new DiagnosticRecord(GetError(fileName, cmdAst), cmdAst.Extent, GetName(), GetDiagnosticSeverity(), fileName, cmdAst.GetCommandName()); } } } @@ -102,6 +102,16 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) /// The source type of the rule. public abstract SourceType GetSourceType(); + /// + /// RuleSeverity: Returns the severity of the rule. + /// + /// public abstract RuleSeverity GetSeverity(); + + /// + /// DiagnosticSeverity: Returns the severity of the rule of type DiagnosticSeverity + /// + /// + public abstract DiagnosticSeverity GetDiagnosticSeverity(); } } diff --git a/Rules/AvoidUsingComputerNameHardcoded.cs b/Rules/AvoidUsingComputerNameHardcoded.cs index 95ec0a454..58f49fe1a 100644 --- a/Rules/AvoidUsingComputerNameHardcoded.cs +++ b/Rules/AvoidUsingComputerNameHardcoded.cs @@ -149,6 +149,15 @@ public override RuleSeverity GetSeverity() return RuleSeverity.Error; } + /// + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning of information. + /// + /// + public override DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Error; + } + /// /// GetSourceName: Retrieves the module/assembly name the rule is from. /// diff --git a/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs b/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs index 0105680c8..378f5fe77 100644 --- a/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs +++ b/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs @@ -113,7 +113,16 @@ public override SourceType GetSourceType() /// public override RuleSeverity GetSeverity() { - return RuleSeverity.Error; + return RuleSeverity.Warning; + } + + /// + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning of information. + /// + /// + public override DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; } /// diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index ef2cefc31..b4c78d797 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -126,17 +126,17 @@ Describe "Test RuleExtension" { Describe "TestSeverity" { It "filters rules based on the specified rule severity" { $rules = Get-ScriptAnalyzerRule -Severity Error - $rules.Count | Should be 6 + $rules.Count | Should be 5 } It "filters rules based on multiple severity inputs"{ $rules = Get-ScriptAnalyzerRule -Severity Error,Information - $rules.Count | Should be 13 + $rules.Count | Should be 12 } It "takes lower case inputs" { $rules = Get-ScriptAnalyzerRule -Severity error - $rules.Count | Should be 6 + $rules.Count | Should be 5 } } From 159a394a608538aac719a5859850deee38b9276b Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 16 Mar 2016 17:02:48 -0700 Subject: [PATCH 18/29] Set the severity level of AvoidUsingConvertToSecureStringWithPlainText --- Rules/AvoidUsingConvertToSecureStringWithPlainText.cs | 4 ++-- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs b/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs index 378f5fe77..4371f57d7 100644 --- a/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs +++ b/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs @@ -113,7 +113,7 @@ public override SourceType GetSourceType() /// public override RuleSeverity GetSeverity() { - return RuleSeverity.Warning; + return RuleSeverity.Error; } /// @@ -122,7 +122,7 @@ public override RuleSeverity GetSeverity() /// public override DiagnosticSeverity GetDiagnosticSeverity() { - return DiagnosticSeverity.Warning; + return DiagnosticSeverity.Error; } /// diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index b4c78d797..ef2cefc31 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -126,17 +126,17 @@ Describe "Test RuleExtension" { Describe "TestSeverity" { It "filters rules based on the specified rule severity" { $rules = Get-ScriptAnalyzerRule -Severity Error - $rules.Count | Should be 5 + $rules.Count | Should be 6 } It "filters rules based on multiple severity inputs"{ $rules = Get-ScriptAnalyzerRule -Severity Error,Information - $rules.Count | Should be 12 + $rules.Count | Should be 13 } It "takes lower case inputs" { $rules = Get-ScriptAnalyzerRule -Severity error - $rules.Count | Should be 5 + $rules.Count | Should be 6 } } From e0a5cb66a44ed5dcce73f710c07514cdb0634fc8 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 21 Mar 2016 15:36:54 -0700 Subject: [PATCH 19/29] Fix the severity level of UseIdenticalParametersDSC rule. --- Rules/UseIdenticalParametersDSC.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/UseIdenticalParametersDSC.cs b/Rules/UseIdenticalParametersDSC.cs index 99b9fac5e..f7eb69453 100644 --- a/Rules/UseIdenticalParametersDSC.cs +++ b/Rules/UseIdenticalParametersDSC.cs @@ -53,7 +53,7 @@ public IEnumerable AnalyzeDSCResource(Ast ast, string fileName if (funcParamAsts.Count() != funcParamAsts2.Count()) { yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseIdenticalParametersDSCError), - firstFunc.Extent, GetName(), DiagnosticSeverity.Information, fileName); + firstFunc.Extent, GetName(), DiagnosticSeverity.Error, fileName); } foreach (ParameterAst paramAst in funcParamAsts) From 8c15ac901d35f5454edb232f9e6cdffe468d5107 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 21 Mar 2016 15:45:39 -0700 Subject: [PATCH 20/29] Add an abstract method to retrieve DiagnosticSeverity in AvoidCmdletGeneric class --- Engine/Generic/AvoidCmdletGeneric.cs | 8 +++++++- Rules/AvoidUsingInvokeExpression.cs | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Engine/Generic/AvoidCmdletGeneric.cs b/Engine/Generic/AvoidCmdletGeneric.cs index b23d85679..7a54d7331 100644 --- a/Engine/Generic/AvoidCmdletGeneric.cs +++ b/Engine/Generic/AvoidCmdletGeneric.cs @@ -45,7 +45,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (cmdletNameAndAliases.Contains(cmdAst.GetCommandName(), StringComparer.OrdinalIgnoreCase)) { - yield return new DiagnosticRecord(GetError(fileName), cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + yield return new DiagnosticRecord(GetError(fileName), cmdAst.Extent, GetName(), GetDiagnosticSeverity(), fileName); } } } @@ -97,5 +97,11 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) /// /// public abstract RuleSeverity GetSeverity(); + + /// + /// DiagnosticSeverity: Returns the severity of the rule of type DiagnosticSeverity + /// + /// + public abstract DiagnosticSeverity GetDiagnosticSeverity(); } } diff --git a/Rules/AvoidUsingInvokeExpression.cs b/Rules/AvoidUsingInvokeExpression.cs index 597383495..c91fd2ad1 100644 --- a/Rules/AvoidUsingInvokeExpression.cs +++ b/Rules/AvoidUsingInvokeExpression.cs @@ -85,6 +85,15 @@ public override RuleSeverity GetSeverity() return RuleSeverity.Warning; } + /// + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. + /// + /// + public override DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + /// /// Method: Retrieves the module/assembly name the rule is from. /// From b66e1ccf2597fbe217afc7b70d3d1036b20402a1 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 21 Mar 2016 15:46:14 -0700 Subject: [PATCH 21/29] Fix typos in source comments --- Rules/AvoidUsingComputerNameHardcoded.cs | 4 ++-- Rules/AvoidUsingConvertToSecureStringWithPlainText.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Rules/AvoidUsingComputerNameHardcoded.cs b/Rules/AvoidUsingComputerNameHardcoded.cs index 58f49fe1a..869a27f09 100644 --- a/Rules/AvoidUsingComputerNameHardcoded.cs +++ b/Rules/AvoidUsingComputerNameHardcoded.cs @@ -141,7 +141,7 @@ public override SourceType GetSourceType() } /// - /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// GetSeverity: Retrieves the severity of the rule: error, warning or information. /// /// public override RuleSeverity GetSeverity() @@ -150,7 +150,7 @@ public override RuleSeverity GetSeverity() } /// - /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning of information. + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. /// /// public override DiagnosticSeverity GetDiagnosticSeverity() diff --git a/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs b/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs index 4371f57d7..b7bafde15 100644 --- a/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs +++ b/Rules/AvoidUsingConvertToSecureStringWithPlainText.cs @@ -108,7 +108,7 @@ public override SourceType GetSourceType() } /// - /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// GetSeverity: Retrieves the severity of the rule: error, warning or information. /// /// public override RuleSeverity GetSeverity() @@ -117,7 +117,7 @@ public override RuleSeverity GetSeverity() } /// - /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning of information. + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. /// /// public override DiagnosticSeverity GetDiagnosticSeverity() From 8334c5e7543aea614ead712bc2c16957b88d7a4c Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 21 Mar 2016 15:47:56 -0700 Subject: [PATCH 22/29] Move AvoidUsingFilePaths and AvoidUnloadableModule to deprecated rules folder --- {Rules => DeprecatedRules}/AvoidUnloadableModule.cs | 0 {Rules => DeprecatedRules}/AvoidUsingFilePaths.cs | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename {Rules => DeprecatedRules}/AvoidUnloadableModule.cs (100%) rename {Rules => DeprecatedRules}/AvoidUsingFilePaths.cs (100%) diff --git a/Rules/AvoidUnloadableModule.cs b/DeprecatedRules/AvoidUnloadableModule.cs similarity index 100% rename from Rules/AvoidUnloadableModule.cs rename to DeprecatedRules/AvoidUnloadableModule.cs diff --git a/Rules/AvoidUsingFilePaths.cs b/DeprecatedRules/AvoidUsingFilePaths.cs similarity index 100% rename from Rules/AvoidUsingFilePaths.cs rename to DeprecatedRules/AvoidUsingFilePaths.cs From 6d66d7515952538de4cd06687e6ca7ca066e9987 Mon Sep 17 00:00:00 2001 From: Ryan Yates Date: Tue, 22 Mar 2016 14:18:41 +0000 Subject: [PATCH 23/29] Dirty fix for the issue raised in #473 --- Engine/Helper.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 6ba8556b1..141bd42d1 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -100,9 +100,9 @@ internal set /// private Dictionary VariableAnalysisDictionary; - private string[] functionScopes = new string[] { "global:", "local:", "script:", "private:" }; + private string[] functionScopes = new string[] { "global:", "local:", "script:", "private:", "Global:", "Local:", "Script:", "Private:" }; - private string[] variableScopes = new string[] { "global:", "local:", "script:", "private:", "variable:", ":" }; + private string[] variableScopes = new string[] { "global:", "local:", "script:", "private:", "variable:", ":", "Global:", "Local:", "Script:", "Private:", "Variable:" }; #endregion @@ -3022,4 +3022,4 @@ public object VisitUsingExpression(UsingExpressionAst usingExpressionAst) return null; } } -} \ No newline at end of file +} From 37201c85d5ebb0c3625fb43e7f3f43bedf0c8039 Mon Sep 17 00:00:00 2001 From: Ryan Yates Date: Thu, 24 Mar 2016 18:13:15 +0000 Subject: [PATCH 24/29] Correct Fix for issue #473 --- Engine/Helper.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 141bd42d1..08d324028 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -100,9 +100,9 @@ internal set /// private Dictionary VariableAnalysisDictionary; - private string[] functionScopes = new string[] { "global:", "local:", "script:", "private:", "Global:", "Local:", "Script:", "Private:" }; + private string[] functionScopes = new string[] { "global:", "local:", "script:", "private:"}; - private string[] variableScopes = new string[] { "global:", "local:", "script:", "private:", "variable:", ":", "Global:", "Local:", "Script:", "Private:", "Variable:" }; + private string[] variableScopes = new string[] { "global:", "local:", "script:", "private:", "variable:", ":"}; #endregion @@ -419,7 +419,8 @@ private string NameWithoutScope(string name, string[] scopes) foreach (string scope in scopes) { // trim the scope part - if (name.IndexOf(scope) == 0) + if (name.IndexOf(scope, StringComparison.OrdinalIgnoreCase) == 0) + { return name.Substring(scope.Length); } From 14d8a2e6e164eaa3c424a3ee2bb1a8e57f50ad62 Mon Sep 17 00:00:00 2001 From: Ryan Yates Date: Thu, 24 Mar 2016 18:37:15 +0000 Subject: [PATCH 25/29] Added Tests to compliment fix for issue #473 --- Tests/Rules/GoodCmdlet.ps1 | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Tests/Rules/GoodCmdlet.ps1 b/Tests/Rules/GoodCmdlet.ps1 index 2c9d552b0..d4d6526da 100644 --- a/Tests/Rules/GoodCmdlet.ps1 +++ b/Tests/Rules/GoodCmdlet.ps1 @@ -223,6 +223,21 @@ function global:Get-SimpleFunc* } +function Local:Get-SimpleFunc* +{ + +} + +function PRIVATE:Get-SimpleFunc* +{ + +} + +function ScRiPt:Get-SimpleFunc* +{ + +} + <# .Synopsis Short description From 16b407f5705ac3523ad5e7abffd9b4c374f30332 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sun, 27 Mar 2016 15:24:39 -0600 Subject: [PATCH 26/29] Updates the Initialize method to process the profile parameter. We need this capability in the PowerShell extension for VSCode to enable it to use an external settings file. This will allow users of the extension to be able to customize the default settings. --- Engine/ScriptAnalyzer.cs | 69 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 07b891ae3..af111b426 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -133,7 +133,8 @@ public void Initialize( string[] excludeRuleNames = null, string[] severity = null, bool includeDefaultRules = false, - bool suppressedOnly = false) + bool suppressedOnly = false, + string profile = null) { if (runspace == null) { @@ -149,7 +150,8 @@ public void Initialize( excludeRuleNames, severity, includeDefaultRules, - suppressedOnly); + suppressedOnly, + profile); } /// @@ -476,13 +478,70 @@ private void Initialize( #region Initializes Rules + var includeRuleList = new List(); + var excludeRuleList = new List(); + var severityList = new List(); + + if (profile != null) + { + ParseProfileString(profile, path, outputWriter, severityList, includeRuleList, excludeRuleList); + } + + if (includeRuleNames != null) + { + foreach (string includeRuleName in includeRuleNames.Where(rule => !includeRuleList.Contains(rule, StringComparer.OrdinalIgnoreCase))) + { + includeRuleList.Add(includeRuleName); + } + } + + if (excludeRuleNames != null) + { + foreach (string excludeRuleName in excludeRuleNames.Where(rule => !excludeRuleList.Contains(rule, StringComparer.OrdinalIgnoreCase))) + { + excludeRuleList.Add(excludeRuleName); + } + } + + if (severity != null) + { + foreach (string sev in severity.Where(s => !severityList.Contains(s, StringComparer.OrdinalIgnoreCase))) + { + severityList.Add(sev); + } + } + this.suppressedOnly = suppressedOnly; - this.severity = this.severity == null ? severity : this.severity.Union(severity ?? new String[0]).ToArray(); - this.includeRule = this.includeRule == null ? includeRuleNames : this.includeRule.Union(includeRuleNames ?? new String[0]).ToArray(); - this.excludeRule = this.excludeRule == null ? excludeRuleNames : this.excludeRule.Union(excludeRuleNames ?? new String[0]).ToArray(); this.includeRegexList = new List(); this.excludeRegexList = new List(); + if (this.severity == null) + { + this.severity = severityList.Count == 0 ? null : severityList.ToArray(); + } + else + { + this.severity = this.severity.Union(severityList).ToArray(); + } + + if (this.includeRule == null) + { + this.includeRule = includeRuleList.Count == 0 ? null : includeRuleList.ToArray(); + } + else + { + this.includeRule = this.includeRule.Union(includeRuleList).ToArray(); + } + + if (this.excludeRule == null) + { + this.excludeRule = excludeRuleList.Count == 0 ? null : excludeRuleList.ToArray(); + } + else + { + this.excludeRule = this.excludeRule.Union(excludeRuleList).ToArray(); + } + //Check wild card input for the Include/ExcludeRules and create regex match patterns if (this.includeRule != null) { From c597849ac37758859a5452c05f8e4bc28a93cb36 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 28 Mar 2016 23:42:43 -0700 Subject: [PATCH 27/29] Fix severity filtering while invoking script analyzer Filters the rules based on their severity before passing them the AST for analysis. Prior to the fix, the severity filter was applied on diagnostic records. This was inefficient as Invoke-ScriptAnalyzer would run all the rules irrespective of the given severity and then filter the diagnostic records to extract the results that corresponded to the given severity. --- Engine/ScriptAnalyzer.cs | 135 ++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 72 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index af111b426..710479df7 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1320,6 +1320,64 @@ private IEnumerable AnalyzeFile(string filePath) return this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath); } + private bool IsSeverityAllowed(IEnumerable allowedSeverities, IRule rule) + { + return severity == null + || (allowedSeverities != null + && rule != null + && HasGetSeverity(rule) + && allowedSeverities.Contains((uint)rule.GetSeverity())); + } + + IEnumerable GetAllowedSeveritiesInInt() + { + return severity != null + ? severity.Select(item => (uint)Enum.Parse(typeof(DiagnosticSeverity), item, true)) + : null; + } + + bool HasMethod(T obj, string methodName) + { + var type = obj.GetType(); + return type.GetMethod(methodName) != null; + } + + bool HasGetSeverity(T obj) + { + return HasMethod(obj, "GetSeverity"); + } + + bool IsRuleAllowed(IRule rule) + { + IEnumerable allowedSeverities = GetAllowedSeveritiesInInt(); + bool includeRegexMatch = false; + bool excludeRegexMatch = false; + foreach (Regex include in includeRegexList) + { + if (include.IsMatch(rule.GetName())) + { + includeRegexMatch = true; + break; + } + } + + foreach (Regex exclude in excludeRegexList) + { + if (exclude.IsMatch(rule.GetName())) + { + excludeRegexMatch = true; + break; + } + } + + bool helpRule = String.Equals(rule.GetName(), "PSUseUTF8EncodingForHelpFile", StringComparison.OrdinalIgnoreCase); + bool includeSeverity = IsSeverityAllowed(allowedSeverities, rule); + + return (includeRule == null || includeRegexMatch) + && (excludeRule == null || !excludeRegexMatch) + && IsSeverityAllowed(allowedSeverities, rule); + } + /// /// Analyzes the syntax tree of a script file that has already been parsed. /// @@ -1373,39 +1431,17 @@ public IEnumerable AnalyzeSyntaxTree( Helper.Instance.Tokens = scriptTokens; } - + #region Run ScriptRules //Trim down to the leaf element of the filePath and pass it to Diagnostic Record string fileName = filePathIsNullOrWhiteSpace ? String.Empty : System.IO.Path.GetFileName(filePath); - if (this.ScriptRules != null) { var tasks = this.ScriptRules.Select(scriptRule => Task.Factory.StartNew(() => { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - - foreach (Regex include in includeRegexList) - { - if (include.IsMatch(scriptRule.GetName())) - { - includeRegexMatch = true; - break; - } - } - - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(scriptRule.GetName())) - { - excludeRegexMatch = true; - break; - } - } - bool helpRule = String.Equals(scriptRule.GetName(), "PSUseUTF8EncodingForHelpFile", StringComparison.OrdinalIgnoreCase); - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + if (IsRuleAllowed(scriptRule)) { List result = new List(); @@ -1475,25 +1511,7 @@ public IEnumerable AnalyzeSyntaxTree( { foreach (ITokenRule tokenRule in this.TokenRules) { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - foreach (Regex include in includeRegexList) - { - if (include.IsMatch(tokenRule.GetName())) - { - includeRegexMatch = true; - break; - } - } - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(tokenRule.GetName())) - { - excludeRegexMatch = true; - break; - } - } - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + if (IsRuleAllowed(tokenRule)) { this.outputWriter.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, tokenRule.GetName())); @@ -1592,24 +1610,7 @@ public IEnumerable AnalyzeSyntaxTree( // Run all DSC Rules foreach (IDSCResourceRule dscResourceRule in this.DSCResourceRules) { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - foreach (Regex include in includeRegexList) - { - if (include.IsMatch(dscResourceRule.GetName())) - { - includeRegexMatch = true; - break; - } - } - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(dscResourceRule.GetName())) - { - excludeRegexMatch = true; - } - } - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + if (IsRuleAllowed(dscResourceRule)) { this.outputWriter.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, dscResourceRule.GetName())); @@ -1646,8 +1647,7 @@ public IEnumerable AnalyzeSyntaxTree( foreach (ExternalRule exRule in this.ExternalRules) { - if ((includeRule == null || includeRule.Contains(exRule.GetName(), StringComparer.OrdinalIgnoreCase)) && - (excludeRule == null || !excludeRule.Contains(exRule.GetName(), StringComparer.OrdinalIgnoreCase))) + if (IsRuleAllowed(exRule)) { string ruleName = string.Format(CultureInfo.CurrentCulture, "{0}\\{1}", exRule.GetSourceName(), exRule.GetName()); this.outputWriter.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, ruleName)); @@ -1676,15 +1676,6 @@ public IEnumerable AnalyzeSyntaxTree( // Need to reverse the concurrentbag to ensure that results are sorted in the increasing order of line numbers IEnumerable diagnosticsList = diagnostics.Reverse(); - if (severity != null) - { - var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true)); - if (diagSeverity.Count() != 0) - { - diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity)); - } - } - return this.suppressedOnly ? suppressed.OfType() : diagnosticsList; From 27d9d2b660477fb238e2eb9f76598d058bc5ff73 Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Tue, 29 Mar 2016 10:46:43 -0700 Subject: [PATCH 28/29] Fix for hang issue --- Engine/Helper.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 6ba8556b1..b3fea76e0 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -32,6 +32,7 @@ public class Helper private CommandInvocationIntrinsics invokeCommand; private IOutputWriter outputWriter; + private Object getCommandLock = new object(); #endregion @@ -567,7 +568,10 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio /// public CommandInfo GetCommandInfo(string name, CommandTypes commandType = CommandTypes.Alias | CommandTypes.Cmdlet | CommandTypes.Configuration | CommandTypes.ExternalScript | CommandTypes.Filter | CommandTypes.Function | CommandTypes.Script | CommandTypes.Workflow) { - return this.invokeCommand.GetCommand(name, commandType); + lock (getCommandLock) + { + return this.invokeCommand.GetCommand(name, commandType); + } } /// From 57f4efc0606fa8faf4c5c5e6ab677c5c5aad8f7d Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 29 Mar 2016 13:00:38 -0700 Subject: [PATCH 29/29] Run rules as tasks only if they are allowed Rules are filtered prior to creating tasks (threads). This avoids creating unnecessary tasks as opposed to the earlier approach where rule invocation was decided within their corresponding tasks. --- Engine/ScriptAnalyzer.cs | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 710479df7..8b4df52b7 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1437,14 +1437,14 @@ public IEnumerable AnalyzeSyntaxTree( string fileName = filePathIsNullOrWhiteSpace ? String.Empty : System.IO.Path.GetFileName(filePath); if (this.ScriptRules != null) { - var tasks = this.ScriptRules.Select(scriptRule => Task.Factory.StartNew(() => - { - bool helpRule = String.Equals(scriptRule.GetName(), "PSUseUTF8EncodingForHelpFile", StringComparison.OrdinalIgnoreCase); + var allowedRules = this.ScriptRules.Where(IsRuleAllowed); - if (IsRuleAllowed(scriptRule)) + if (allowedRules.Any()) + { + var tasks = allowedRules.Select(scriptRule => Task.Factory.StartNew(() => { + bool helpRule = String.Equals(scriptRule.GetName(), "PSUseUTF8EncodingForHelpFile", StringComparison.OrdinalIgnoreCase); List result = new List(); - result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); // Ensure that any unhandled errors from Rules are converted to non-terminating errors @@ -1478,26 +1478,26 @@ public IEnumerable AnalyzeSyntaxTree( } verboseOrErrors.Add(result); - } - })); + })); - Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding()); + Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding()); - while (!verboseOrErrors.IsCompleted) - { - List data = null; - try + while (!verboseOrErrors.IsCompleted) { - data = verboseOrErrors.Take(); - } - catch (InvalidOperationException) { } + List data = null; + try + { + data = verboseOrErrors.Take(); + } + catch (InvalidOperationException) { } - if (data != null) - { - this.outputWriter.WriteVerbose(data[0] as string); - if (data.Count == 2) + if (data != null) { - this.outputWriter.WriteError(data[1] as ErrorRecord); + this.outputWriter.WriteVerbose(data[0] as string); + if (data.Count == 2) + { + this.outputWriter.WriteError(data[1] as ErrorRecord); + } } } }