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 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/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/Engine/Helper.cs b/Engine/Helper.cs index 6ba8556b1..a15e5166b 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 @@ -100,9 +101,9 @@ internal set /// private Dictionary VariableAnalysisDictionary; - private string[] functionScopes = new string[] { "global:", "local:", "script:", "private:" }; + private string[] functionScopes = new string[] { "global:", "local:", "script:", "private:"}; - private string[] variableScopes = new string[] { "global:", "local:", "script:", "private:", "variable:", ":" }; + private string[] variableScopes = new string[] { "global:", "local:", "script:", "private:", "variable:", ":"}; #endregion @@ -419,7 +420,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); } @@ -567,7 +569,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); + } } /// @@ -3022,4 +3027,4 @@ public object VisitUsingExpression(UsingExpressionAst usingExpressionAst) return null; } } -} \ No newline at end of file +} diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index df11bf9d8..8b4df52b7 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) { @@ -706,28 +765,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); @@ -776,8 +823,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; } } @@ -987,34 +1034,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 @@ -1286,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. /// @@ -1339,42 +1431,20 @@ 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; + var allowedRules = this.ScriptRules.Where(IsRuleAllowed); - 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 (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 @@ -1408,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); + } } } } @@ -1441,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())); @@ -1558,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())); @@ -1612,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)); @@ -1642,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; 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) 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. /// diff --git a/Rules/AvoidUsingComputerNameHardcoded.cs b/Rules/AvoidUsingComputerNameHardcoded.cs index 95ec0a454..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() @@ -149,6 +149,15 @@ public override RuleSeverity GetSeverity() return RuleSeverity.Error; } + /// + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or 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..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() @@ -116,6 +116,15 @@ public override RuleSeverity GetSeverity() return RuleSeverity.Error; } + /// + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. + /// + /// + public override DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Error; + } + /// /// GetSourceName: Retrieves the module/assembly name the rule is from. /// 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. /// diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index aae951eeb..c8bc1fe79 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 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. 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 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 { @@ -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 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 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. 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 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. 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 233f99a5e..a7c969482 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. + 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 the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute. + 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 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. For PowerShell 4.0 and earlier please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute. - 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. @@ -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 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. 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 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/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) diff --git a/Rules/UsePSCredentialType.cs b/Rules/UsePSCredentialType.cs index bddc3bf66..9f3c17377 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 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 that cmdlets that have a Credential parameter accept PSCredential. + /// 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/Rules/UseToExportFieldsInManifest.cs b/Rules/UseToExportFieldsInManifest.cs index f7623c085..324c60487 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; @@ -83,6 +84,30 @@ 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; + return strConstExprAst != null && WildcardPattern.ContainsWildcardCharacters(strConstExprAst.Value); + } + + /// + /// 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); + } /// /// Checks if the *ToExport fields are explicitly set to arrays, eg. @(...), and the array entries do not contain any wildcard. @@ -97,32 +122,41 @@ 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)) + var keyStrConstAst = pair.Item1 as StringConstantExpressionAst; + if (keyStrConstAst != null && keyStrConstAst.Value.Equals(key, 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; + // Checks for wildcard character in the entry. + var astWithWildcard = pair.Item2.Find(HasWildcardInExpression, false); + if (astWithWildcard != null) + { + extent = astWithWildcard.Extent; return false; - } + } else { - //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 astWithNull = pair.Item2.Find(HasNullInExpression, false); + if (astWithNull != null) { - extent = elementWithWildcard.Extent; + extent = astWithNull.Extent; return false; - } - return true; + } + else + { + return true; + } } } } return true; } + + /// + /// Gets the error string of the rule. + /// + /// + /// public string GetError(string field) { return string.Format(CultureInfo.CurrentCulture, Strings.UseToExportFieldsInManifestError, field); 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 diff --git a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 index 555bc4ca9..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. 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 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/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 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 852af655c..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 the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute." +$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} @@ -12,7 +12,7 @@ Describe "PSCredentialType" { } It "has the correct description message" { - $violations[0].Message | Should Match $violationMessage + $violations[0].Message | Should Be $violationMessage } } 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 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