diff --git a/CHANGELOG.MD b/CHANGELOG.MD index 4bec47113..b81273723 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -1,3 +1,30 @@ +## Released v1.0.2 (June.24, 2015) +###Features: +- Perf improvements in the Engine to execute rules concurrently. + + +###Rules: +- New rule to validate the presence of deprecated module manifest fields. +- Removed PSAvoidTrapStatement rule from the builtin set – since this is not deprecated and using trap is a better choice in certain scenarios. + + +###Fixes: +- Verbose Message rule applies to only DSC cmdlet based resources. +- Multiple fixes to AvoidUninitializedVariable to work with non-mandatory parameters, fix in the flow graphs for throw statements; support for missing preference variables; support for automatic variables. +- PSAvoidUsingInternalsURLs to work with xPath expressions. +- UseSingularNouns rule to raise warnings for plural phrases. +- Added .gitignore to exclude certain files from being reported as untracked. +- Revisited severity for DSC rules. +- PSUseOutputTypeCorrectly rule not to get triggered for functions returning system.void type. +- PSAvoidDefaultTrueValueSwitchParameter to work with switch attribute when supplied as fully qualified. +- Ignore PSObject and PSCustomObject for UseOutputTypeCorrectly rule. +- Only raise NullComparisonRule if the RHS is an array or has unknown type. +- PSUseDeclaredVarsMoreThanAssignments rule to be raised for script variables and for setting the property of a variable. +- Support for using PSUseCmdletCorrectly rule when mandatory parameters are supplied in a splatted hashtable. +- AvoidUsingPlainTextForPassword rule to be raised only strings or object types. +- Fix for PositionalParameterUsed method (Helper.cs) uses unsafe method to exclude ForEach-Object and Where-Object. + + ## Released v1.0.1 (May.8, 2015) ###Features: - Integrated with waffle.io for Project Management. @@ -18,7 +45,6 @@ ##Released v1.0.0 on Apr.24, 2015 - ###Features: - Finalized three levels of Severity - Error/Warning/Information. - Improved PSScriptAnalyzer engine behavior: emits non-terminating errors (Ex: for failed ast parse) and continues rule application when running on multiple scripts. diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 02ef89a75..2fa611404 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -12,6 +12,7 @@ using System.Text.RegularExpressions; using System; +using System.ComponentModel; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ -20,6 +21,9 @@ using System.Management.Automation.Language; using System.IO; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System.Threading.Tasks; +using System.Collections.Concurrent; +using System.Threading; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands { @@ -251,12 +255,12 @@ private void ProcessPath(string path) } } else if (File.Exists(path)) - { - WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, path)); + { if ((path.Length > ps1Suffix.Length && path.Substring(path.Length - ps1Suffix.Length).Equals(ps1Suffix, StringComparison.OrdinalIgnoreCase)) || (path.Length > psm1Suffix.Length && path.Substring(path.Length - psm1Suffix.Length).Equals(psm1Suffix, StringComparison.OrdinalIgnoreCase)) || (path.Length > psd1Suffix.Length && path.Substring(path.Length - psd1Suffix.Length).Equals(psd1Suffix, StringComparison.OrdinalIgnoreCase))) { + WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, path)); AnalyzeFile(path); } } @@ -267,6 +271,13 @@ private void ProcessPath(string path) } + ConcurrentBag diagnostics; + ConcurrentBag suppressed; + Dictionary> ruleSuppressions; + List includeRegexList; + List excludeRegexList; + ConcurrentDictionary> ruleDictionary; + /// /// Analyzes a single script file. /// @@ -275,15 +286,16 @@ private void AnalyzeFile(string filePath) { Token[] tokens = null; ParseError[] errors = null; - List diagnostics = new List(); - List suppressed = new List(); + ConcurrentBag diagnostics = new ConcurrentBag(); + ConcurrentBag suppressed = new ConcurrentBag(); + BlockingCollection> verboseOrErrors = new BlockingCollection>(); // Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash List> cmdInfoTable = new List>(); //Check wild card input for the Include/ExcludeRules and create regex match patterns - List includeRegexList = new List(); - List excludeRegexList = new List(); + includeRegexList = new List(); + excludeRegexList = new List(); if (includeRule != null) { foreach (string rule in includeRule) @@ -331,7 +343,7 @@ private void AnalyzeFile(string filePath) return; } - Dictionary> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); + ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); foreach (List ruleSuppressionsList in ruleSuppressions.Values) { @@ -360,43 +372,75 @@ private void AnalyzeFile(string filePath) if (ScriptAnalyzer.Instance.ScriptRules != null) { - foreach (IScriptRule scriptRule in ScriptAnalyzer.Instance.ScriptRules) - { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - foreach (Regex include in includeRegexList) + var tasks = ScriptAnalyzer.Instance.ScriptRules.Select(scriptRule => Task.Factory.StartNew(() => { - if (include.IsMatch(scriptRule.GetName())) + bool includeRegexMatch = false; + bool excludeRegexMatch = false; + + foreach (Regex include in includeRegexList) { - includeRegexMatch = true; - break; + if (include.IsMatch(scriptRule.GetName())) + { + includeRegexMatch = true; + break; + } } - } - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(scriptRule.GetName())) + foreach (Regex exclude in excludeRegexList) { - excludeRegexMatch = true; - break; + if (exclude.IsMatch(scriptRule.GetName())) + { + excludeRegexMatch = true; + break; + } } - } - - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) - { - WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); - // Ensure that any unhandled errors from Rules are converted to non-terminating errors - // We want the Engine to continue functioning even if one or more Rules throws an exception - try + if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) { - var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + 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 + // We want the Engine to continue functioning even if one or more Rules throws an exception + try + { + var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList()); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } + } + catch (Exception scriptRuleException) + { + result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File)); + } + + verboseOrErrors.Add(result); } - catch (Exception scriptRuleException) + })); + + Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding()); + + while (!verboseOrErrors.IsCompleted) + { + List data = null; + try + { + data = verboseOrErrors.Take(); + } + catch (InvalidOperationException) { } + + if (data != null) + { + WriteVerbose(data[0] as string); + if (data.Count == 2) { - WriteError(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath)); + WriteError(data[1] as ErrorRecord); } } } @@ -437,8 +481,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception tokenRuleException) { @@ -489,8 +539,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception dscResourceRuleException) { @@ -532,8 +588,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception dscResourceRuleException) { @@ -573,15 +635,20 @@ private void AnalyzeFile(string filePath) } } - diagnostics.AddRange(ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName)); + foreach (var record in ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName)) + { + diagnostics.Add(record); + } } #endregion + IEnumerable diagnosticsList = diagnostics; + if (severity != null) { var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true)); - diagnostics = diagnostics.Where(item => diagSeverity.Contains(item.Severity)).ToList(); + diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity)); } //Output through loggers @@ -596,7 +663,7 @@ private void AnalyzeFile(string filePath) } else { - foreach (DiagnosticRecord diagnostic in diagnostics) + foreach (DiagnosticRecord diagnostic in diagnosticsList) { logger.LogObject(diagnostic, this); } diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 373f4bd58..66105b1b3 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -219,6 +219,21 @@ item is TypeDefinitionAst return false; } + /// + /// Given a commandast, checks whether it uses splatted variable + /// + /// + /// + public bool HasSplattedVariable(CommandAst cmdAst) + { + if (cmdAst == null || cmdAst.CommandElements == null) + { + return false; + } + + return cmdAst.CommandElements.Any(cmdElem => cmdElem is VariableExpressionAst && (cmdElem as VariableExpressionAst).Splatted); + } + /// /// Given a commandast, checks whether positional parameters are used or not. /// @@ -234,23 +249,17 @@ public bool PositionalParameterUsed(CommandAst cmdAst) CommandInfo commandInfo = GetCommandInfo(GetCmdletNameFromAlias(cmdAst.GetCommandName())) ?? GetCommandInfo(cmdAst.GetCommandName()); IEnumerable switchParams = null; - IEnumerable scriptBlocks = null; - bool hasScriptBlockSet = false; + + if (HasSplattedVariable(cmdAst)) + { + return false; + } if (commandInfo != null && commandInfo.CommandType == System.Management.Automation.CommandTypes.Cmdlet) { try { switchParams = commandInfo.Parameters.Values.Where(pm => pm.SwitchParameter); - scriptBlocks = commandInfo.ParameterSets; - foreach (CommandParameterSetInfo cmdParaset in scriptBlocks) - { - if (String.Equals(cmdParaset.Name, "ScriptBlockSet", StringComparison.OrdinalIgnoreCase)) - { - hasScriptBlockSet = true; - } - } - } catch (Exception) { @@ -264,8 +273,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst) foreach (CommandElementAst ceAst in cmdAst.CommandElements) { - if (!hasScriptBlockSet) - { if (ceAst is CommandParameterAst) { // Skip if it's a switch parameter @@ -286,17 +293,17 @@ public bool PositionalParameterUsed(CommandAst cmdAst) } else { - //Skip if splatting "@" is used - if (ceAst is VariableExpressionAst) - { - if ((ceAst as VariableExpressionAst).Splatted) - { - continue; - } - } arguments += 1; } - } + + } + + // if not the first element in a pipeline, increase the number of arguments by 1 + PipelineAst parent = cmdAst.Parent as PipelineAst; + + if (parent != null && parent.PipelineElements.Count > 1 && parent.PipelineElements[0] != cmdAst) + { + arguments += 1; } return arguments > parameters; @@ -672,7 +679,7 @@ internal string GetTypeFromMemberExpressionAstHelper(MemberExpressionAst memberA /// /// /// - internal Type GetTypeFromAnalysis(VariableExpressionAst varAst, Ast ast) + public Type GetTypeFromAnalysis(VariableExpressionAst varAst, Ast ast) { try { diff --git a/Engine/PSScriptAnalyzer.psd1 b/Engine/PSScriptAnalyzer.psd1 index 2057d6ca3..8d26cea21 100644 --- a/Engine/PSScriptAnalyzer.psd1 +++ b/Engine/PSScriptAnalyzer.psd1 @@ -11,7 +11,7 @@ Author = 'Microsoft Corporation' RootModule = 'Microsoft.Windows.PowerShell.ScriptAnalyzer.dll' # Version number of this module. -ModuleVersion = '1.0.1' +ModuleVersion = '1.0.2' # ID used to uniquely identify this module GUID = '324fc715-36bf-4aee-8e58-72e9b4a08ad9' diff --git a/Engine/SpecialVars.cs b/Engine/SpecialVars.cs index 6c41f57fb..633657e2d 100644 --- a/Engine/SpecialVars.cs +++ b/Engine/SpecialVars.cs @@ -34,6 +34,8 @@ internal class SpecialVars internal const string PSScriptRoot = "PSScriptRoot"; internal const string PSCommandPath = "PSCommandPath"; internal const string ExecutionContext = "ExecutionContext"; + internal const string Matches = "Matches"; + internal const string PSVersionTable = "PSVersionTable"; internal static readonly string[] InitializedVariables; @@ -63,7 +65,9 @@ static SpecialVars() MyInvocation, PSScriptRoot, PSCommandPath, - ExecutionContext + ExecutionContext, + Matches, + PSVersionTable }; internal static readonly Type[] AutomaticVariableTypes = new Type[] { @@ -76,7 +80,9 @@ static SpecialVars() /* MyInvocation */ typeof(InvocationInfo), /* PSScriptRoot */ typeof(string), /* PSCommandPath */ typeof(string), - /* ExecutionContext */ typeof(EngineIntrinsics), + /* ExecutionContext */ typeof(EngineIntrinsics), + /* Matches */ typeof(System.Collections.Hashtable), + /* PSVersionTable */ typeof(System.Collections.Hashtable) }; diff --git a/Rules/AvoidDefaultTrueValueSwitchParameter.cs b/Rules/AvoidDefaultTrueValueSwitchParameter.cs index a2f892571..932da65e6 100644 --- a/Rules/AvoidDefaultTrueValueSwitchParameter.cs +++ b/Rules/AvoidDefaultTrueValueSwitchParameter.cs @@ -39,7 +39,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) // Iterrates all ParamAsts and check if any are switch. foreach (ParameterAst paramAst in paramAsts) { - if (paramAst.Attributes.Any(attr => string.Equals(attr.TypeName.GetReflectionType().FullName, "system.management.automation.switchparameter", StringComparison.OrdinalIgnoreCase)) + if (paramAst.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(System.Management.Automation.SwitchParameter)) && paramAst.DefaultValue != null && String.Equals(paramAst.DefaultValue.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase)) { yield return new DiagnosticRecord( diff --git a/Rules/AvoidPositionalParameters.cs b/Rules/AvoidPositionalParameters.cs index 248dc4f71..a52ac550b 100644 --- a/Rules/AvoidPositionalParameters.cs +++ b/Rules/AvoidPositionalParameters.cs @@ -47,8 +47,23 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (Helper.Instance.GetCommandInfo(cmdAst.GetCommandName()) != null && Helper.Instance.PositionalParameterUsed(cmdAst)) { - yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), - cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + PipelineAst parent = cmdAst.Parent as PipelineAst; + + if (parent != null && parent.PipelineElements.Count > 1) + { + // raise if it's the first element in pipeline. otherwise no. + if (parent.PipelineElements[0] == cmdAst) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), + cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + } + } + // not in pipeline so just raise it normally + else + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), + cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + } } } } diff --git a/Rules/AvoidUsingDeprecatedManifestFields.cs b/Rules/AvoidUsingDeprecatedManifestFields.cs index 964522490..8ec58ca60 100644 --- a/Rules/AvoidUsingDeprecatedManifestFields.cs +++ b/Rules/AvoidUsingDeprecatedManifestFields.cs @@ -41,7 +41,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase)) { - var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace); + var ps = System.Management.Automation.PowerShell.Create(); IEnumerable result = null; try { @@ -73,6 +73,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } + ps.Dispose(); } } diff --git a/Rules/AvoidUsingInternalURLs.cs b/Rules/AvoidUsingInternalURLs.cs index 99844de4f..90c880909 100644 --- a/Rules/AvoidUsingInternalURLs.cs +++ b/Rules/AvoidUsingInternalURLs.cs @@ -11,8 +11,10 @@ // using System; +using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Linq.Expressions; using System.Management.Automation.Language; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; using System.ComponentModel.Composition; @@ -44,8 +46,19 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { foreach (StringConstantExpressionAst expressionAst in expressionAsts) { - //Check if XPath is used. If XPath is used, then we don't throw warnings. + Ast parentAst = expressionAst.Parent; + //Check if -replace is used, if it is string replace, we don't throw warnings. + Ast grandParentAst = parentAst.Parent; + if (grandParentAst is BinaryExpressionAst) + { + if ((grandParentAst as BinaryExpressionAst).Operator.Equals(TokenKind.Ireplace)) + { + continue; + } + } + + //Check if XPath is used. If XPath is used, then we don't throw warnings. if (parentAst is InvokeMemberExpressionAst) { InvokeMemberExpressionAst invocation = parentAst as InvokeMemberExpressionAst; @@ -55,7 +68,9 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) String.Equals(invocation.Member.ToString(), "SelectNodes",StringComparison.OrdinalIgnoreCase) || String.Equals(invocation.Member.ToString(), "Select", StringComparison.OrdinalIgnoreCase) || String.Equals(invocation.Member.ToString(), "Evaluate",StringComparison.OrdinalIgnoreCase) || - String.Equals(invocation.Member.ToString(), "Matches",StringComparison.OrdinalIgnoreCase)) + String.Equals(invocation.Member.ToString(), "Matches",StringComparison.OrdinalIgnoreCase) || + String.Equals(invocation.Expression.ToString(), "[System.String]",StringComparison.OrdinalIgnoreCase) || + String.Equals(invocation.Expression.ToString(), "[String]", StringComparison.OrdinalIgnoreCase)) { continue; } diff --git a/Rules/MissingModuleManifestField.cs b/Rules/MissingModuleManifestField.cs index 9b0dd5954..6df37980c 100644 --- a/Rules/MissingModuleManifestField.cs +++ b/Rules/MissingModuleManifestField.cs @@ -38,7 +38,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase)) { - var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace); + var ps = System.Management.Automation.PowerShell.Create(); try { @@ -68,6 +68,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } + ps.Dispose(); } } diff --git a/Rules/PossibleIncorrectComparisonWithNull.cs b/Rules/PossibleIncorrectComparisonWithNull.cs index da97c13f6..cc589f28a 100644 --- a/Rules/PossibleIncorrectComparisonWithNull.cs +++ b/Rules/PossibleIncorrectComparisonWithNull.cs @@ -11,6 +11,7 @@ // using System; +using System.Linq; using System.Collections.Generic; using System.Management.Automation.Language; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; @@ -33,17 +34,67 @@ public class PossibleIncorrectComparisonWithNull : IScriptRule { public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - IEnumerable binExpressionAsts = ast.FindAll(testAst => testAst is BinaryExpressionAst, true); + IEnumerable binExpressionAsts = ast.FindAll(testAst => testAst is BinaryExpressionAst, false); - if (binExpressionAsts != null) { - foreach (BinaryExpressionAst binExpressionAst in binExpressionAsts) { - if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) - || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq)) - && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) { - yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + foreach (BinaryExpressionAst binExpressionAst in binExpressionAsts) { + if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) + || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq)) + && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) + { + if (IncorrectComparisonWithNull(binExpressionAst, ast)) + { + yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); } } } + + IEnumerable funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true).Union(ast.FindAll(item => item is FunctionMemberAst, true)); + foreach (Ast funcAst in funcAsts) + { + IEnumerable binAsts = funcAst.FindAll(item => item is BinaryExpressionAst, true); + foreach (BinaryExpressionAst binAst in binAsts) + { + if (IncorrectComparisonWithNull(binAst, funcAst)) + { + yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + } + } + } + } + + private bool IncorrectComparisonWithNull(BinaryExpressionAst binExpressionAst, Ast ast) + { + if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) + || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq)) + && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) + { + if (binExpressionAst.Left.StaticType.IsArray) + { + return true; + } + else if (binExpressionAst.Left is VariableExpressionAst) + { + // ignores if the variable is a special variable + if (!Helper.Instance.HasSpecialVars((binExpressionAst.Left as VariableExpressionAst).VariablePath.UserPath)) + { + Type lhsType = Helper.Instance.GetTypeFromAnalysis(binExpressionAst.Left as VariableExpressionAst, ast); + if (lhsType == null) + { + return true; + } + else if (lhsType.IsArray || lhsType == typeof(object) || lhsType == typeof(Undetermined) || lhsType == typeof(Unreached)) + { + return true; + } + } + } + else if (binExpressionAst.Left.StaticType == typeof(object)) + { + return true; + } + } + + return false; } /// diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index e9b8a56f6..32697b07f 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -75,7 +75,7 @@ - + True True diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 54175ef88..8c76e720b 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.35317 +// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -1167,51 +1167,6 @@ internal static string ProvideDefaultParameterValueName { } } - /// - /// Looks up a localized string similar to Verbose. - /// - internal static string ProvideVerboseMessageCommonName { - get { - return ResourceManager.GetString("ProvideVerboseMessageCommonName", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Checks that Write-Verbose is called at least once in every cmdlet or script. This is in line with the PowerShell best practices.. - /// - internal static string ProvideVerboseMessageDescription { - get { - return ResourceManager.GetString("ProvideVerboseMessageDescription", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to There is no call to Write-Verbose in the function ‘{0}’.. - /// - internal static string ProvideVerboseMessageErrorFunction { - get { - return ResourceManager.GetString("ProvideVerboseMessageErrorFunction", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to ProvideVerboseMessage. - /// - internal static string ProvideVerboseMessageName { - get { - return ResourceManager.GetString("ProvideVerboseMessageName", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to There is no call to Write-Verbose in the script.. - /// - internal static string ProvideVerboseMessageScript { - get { - return ResourceManager.GetString("ProvideVerboseMessageScript", resourceCulture); - } - } - /// /// Looks up a localized string similar to Reserved Cmdlet Chars. /// @@ -1913,5 +1868,41 @@ internal static string UseTypeAtVariableAssignmentName { return ResourceManager.GetString("UseTypeAtVariableAssignmentName", resourceCulture); } } + + /// + /// Looks up a localized string similar to Use verbose message in DSC resource. + /// + internal static string UseVerboseMessageInDSCResourceCommonName { + get { + return ResourceManager.GetString("UseVerboseMessageInDSCResourceCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to It is a best practice to emit informative, verbose messages in DSC resource functions. This helps in debugging issues when a DSC configuration is executed.. + /// + internal static string UseVerboseMessageInDSCResourceDescription { + get { + return ResourceManager.GetString("UseVerboseMessageInDSCResourceDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to There is no call to Write-Verbose in DSC function ‘{0}’. If you are using Write-Verbose in a helper function, suppress this rule application.. + /// + internal static string UseVerboseMessageInDSCResourceErrorFunction { + get { + return ResourceManager.GetString("UseVerboseMessageInDSCResourceErrorFunction", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to UseVerboseMessageInDSCResource. + /// + internal static string UseVerboseMessageInDSCResourceName { + get { + return ResourceManager.GetString("UseVerboseMessageInDSCResourceName", resourceCulture); + } + } } } diff --git a/Rules/Strings.resx b/Rules/Strings.resx index c16fa35e3..dd02abedf 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -276,17 +276,14 @@ PS - - Checks that Write-Verbose is called at least once in every cmdlet or script. This is in line with the PowerShell best practices. + + It is a best practice to emit informative, verbose messages in DSC resource functions. This helps in debugging issues when a DSC configuration is executed. - - There is no call to Write-Verbose in the function ‘{0}’. + + There is no call to Write-Verbose in DSC function ‘{0}’. If you are using Write-Verbose in a helper function, suppress this rule application. - - Verbose - - - There is no call to Write-Verbose in the script. + + Use verbose message in DSC resource Some fields of the module manifest (such as ModuleVersion) are required. @@ -459,8 +456,8 @@ MissingModuleManifestField - - ProvideVerboseMessage + + UseVerboseMessageInDSCResource Command Not Found diff --git a/Rules/UseCmdletCorrectly.cs b/Rules/UseCmdletCorrectly.cs index 430e9a2d4..691f4f743 100644 --- a/Rules/UseCmdletCorrectly.cs +++ b/Rules/UseCmdletCorrectly.cs @@ -92,6 +92,12 @@ private bool IsMandatoryParameterExisted(CommandAst cmdAst) return true; } + // ignores if splatted variable is used + if (Helper.Instance.HasSplattedVariable(cmdAst)) + { + return true; + } + // Gets parameters from command elements. ceAsts = cmdAst.CommandElements.Where(foundParamASTs); diff --git a/Rules/UseDeclaredVarsMoreThanAssignments.cs b/Rules/UseDeclaredVarsMoreThanAssignments.cs index 1f1ecf089..f90302500 100644 --- a/Rules/UseDeclaredVarsMoreThanAssignments.cs +++ b/Rules/UseDeclaredVarsMoreThanAssignments.cs @@ -35,8 +35,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - IEnumerable assignmentAsts = ast.FindAll(testAst => testAst is AssignmentStatementAst, true); - IEnumerable assingmentVarAsts; + IEnumerable assignmentAsts = ast.FindAll(testAst => testAst is AssignmentStatementAst, true); IEnumerable varAsts = ast.FindAll(testAst => testAst is VariableExpressionAst, true); IEnumerable varsInAssignment; diff --git a/Rules/UseOutputTypeCorrectly.cs b/Rules/UseOutputTypeCorrectly.cs index a41c3a23c..93d8353be 100644 --- a/Rules/UseOutputTypeCorrectly.cs +++ b/Rules/UseOutputTypeCorrectly.cs @@ -102,15 +102,20 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun List> returnTypes = FindPipelineOutput.OutputTypes(funcAst, _classes); + HashSet specialTypes = new HashSet(StringComparer.OrdinalIgnoreCase); + specialTypes.Add(typeof(Unreached).FullName); + specialTypes.Add(typeof(Undetermined).FullName); + specialTypes.Add(typeof(object).FullName); + specialTypes.Add(typeof(void).FullName); + specialTypes.Add(typeof(PSCustomObject).FullName); + specialTypes.Add(typeof(PSObject).FullName); + foreach (Tuple returnType in returnTypes) { string typeName = returnType.Item1; if (String.IsNullOrEmpty(typeName) - || String.Equals(typeof(Unreached).FullName, typeName, StringComparison.OrdinalIgnoreCase) - || String.Equals(typeof(Undetermined).FullName, typeName, StringComparison.OrdinalIgnoreCase) - || String.Equals(typeof(object).FullName, typeName, StringComparison.OrdinalIgnoreCase) - || String.Equals(typeof(void).FullName, typeName, StringComparison.OrdinalIgnoreCase) + || specialTypes.Contains(typeName) || outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase)) { continue; diff --git a/Rules/ProvideVerboseMessage.cs b/Rules/UseVerboseMessageInDSCResource.cs similarity index 53% rename from Rules/ProvideVerboseMessage.cs rename to Rules/UseVerboseMessageInDSCResource.cs index b3d191b56..df556d97c 100644 --- a/Rules/ProvideVerboseMessage.cs +++ b/Rules/UseVerboseMessageInDSCResource.cs @@ -22,70 +22,56 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// ProvideVerboseMessage: Analyzes the ast to check that Write-Verbose is called at least once in every cmdlet or script. + /// ProvideVerboseMessage: Analyzes the ast to check that Write-Verbose is called for DSC Resources /// - [Export(typeof(IScriptRule))] - public class ProvideVerboseMessage : SkipNamedBlock, IScriptRule + [Export(typeof(IDSCResourceRule))] + public class UseVerboseMessageInDSCResource : SkipNamedBlock, IDSCResourceRule { /// - /// AnalyzeScript: Analyzes the ast to check that Write-Verbose is called at least once in every cmdlet or script. + /// AnalyzeDSCResource: Analyzes the ast to check that Write-Verbose is called for DSC Resources /// The script's ast /// The script's file name /// - public IEnumerable AnalyzeScript(Ast ast, string fileName) + public IEnumerable AnalyzeDSCResource(Ast ast, string fileName) { - if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - - ClearList(); - this.AddNames(new List() { "Configuration", "Workflow" }); - DiagnosticRecords.Clear(); - - this.fileName = fileName; - //We only check that advanced functions should have Write-Verbose - ast.Visit(this); - - return DiagnosticRecords; - } - - /// - /// Visit function and checks that it has write verbose - /// - /// - /// - public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) - { - if (funcAst == null) + if (ast == null) { - return AstVisitAction.SkipChildren; - } + throw new ArgumentNullException(Strings.NullAstErrorMessage); + } + + IEnumerable functionDefinitionAsts = Helper.Instance.DscResourceFunctions(ast); - //Write-Verbose is not required for non-advanced functions - if (funcAst.Body == null || funcAst.Body.ParamBlock == null - || funcAst.Body.ParamBlock.Attributes == null || - funcAst.Body.ParamBlock.Parameters == null || - !funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute))) + foreach (FunctionDefinitionAst functionDefinitionAst in functionDefinitionAsts) { - return AstVisitAction.Continue; - } + var commandAsts = functionDefinitionAst.Body.FindAll(testAst => testAst is CommandAst, false); + bool hasVerbose = false; - var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false); - bool hasVerbose = false; + if (null != commandAsts) + { + foreach (CommandAst commandAst in commandAsts) + { + hasVerbose |= String.Equals(commandAst.GetCommandName(), "Write-Verbose", StringComparison.OrdinalIgnoreCase); + } + } - if (commandAsts != null) - { - foreach (CommandAst commandAst in commandAsts) + if (!hasVerbose) { - hasVerbose |= String.Equals(commandAst.GetCommandName(), "Write-Verbose", StringComparison.OrdinalIgnoreCase); + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseVerboseMessageInDSCResourceErrorFunction, functionDefinitionAst.Name), + functionDefinitionAst.Extent, GetName(), DiagnosticSeverity.Information, fileName); } - } - if (!hasVerbose) - { - DiagnosticRecords.Add(new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ProvideVerboseMessageErrorFunction, funcAst.Name), - funcAst.Extent, GetName(), DiagnosticSeverity.Information, fileName)); } - - return AstVisitAction.Continue; + } + + /// + /// AnalyzeDSCClass: This function returns nothing in the case of dsc class. + /// + /// + /// + /// + public IEnumerable AnalyzeDSCClass(Ast ast, string fileName) + { + return Enumerable.Empty(); } /// @@ -93,7 +79,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun /// public string GetName() { - return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.ProvideVerboseMessageName); + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseVerboseMessageInDSCResourceName); } /// @@ -102,7 +88,7 @@ public string GetName() /// The common name of this rule public string GetCommonName() { - return string.Format(CultureInfo.CurrentCulture, Strings.ProvideVerboseMessageCommonName); + return string.Format(CultureInfo.CurrentCulture, Strings.UseVerboseMessageInDSCResourceCommonName); } /// @@ -111,7 +97,7 @@ public string GetCommonName() /// The description of this rule public string GetDescription() { - return string.Format(CultureInfo.CurrentCulture, Strings.ProvideVerboseMessageDescription); + return string.Format(CultureInfo.CurrentCulture, Strings.UseVerboseMessageInDSCResourceDescription); } /// @@ -136,7 +122,7 @@ public RuleSeverity GetSeverity() /// public string GetSourceName() { - return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + return string.Format(CultureInfo.CurrentCulture, Strings.DSCSourceName); } } -} +} \ No newline at end of file diff --git a/Tests/Rules/ProvideVerboseMessage.tests.ps1 b/Tests/Disabled Rules/ProvideVerboseMessage.tests.ps1 similarity index 100% rename from Tests/Rules/ProvideVerboseMessage.tests.ps1 rename to Tests/Disabled Rules/ProvideVerboseMessage.tests.ps1 diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index fa5346b6a..2606aab63 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -130,11 +130,11 @@ Describe "TestSeverity" { Describe "TestWildCard" { It "filters rules based on the -Name wild card input" { $rules = Get-ScriptAnalyzerRule -Name PSDSC* - $rules.Count | Should be 6 + $rules.Count | Should be 7 } It "filters rules based on wild card input and severity"{ $rules = Get-ScriptAnalyzerRule -Name PSDSC* -Severity Information - $rules.Count | Should be 3 + $rules.Count | Should be 4 } } diff --git a/Tests/Engine/RuleSuppression.ps1 b/Tests/Engine/RuleSuppression.ps1 index d5cd4e055..b63aef84c 100644 --- a/Tests/Engine/RuleSuppression.ps1 +++ b/Tests/Engine/RuleSuppression.ps1 @@ -5,7 +5,7 @@ Param( function SuppressMe () { - [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideVerboseMessage")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideCommentHelp")] [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideDefaultParameterValue", "unused1")] Param([string]$unUsed1, [int] $unUsed2) { diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index bb6ff4a77..17d8a719d 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -5,7 +5,7 @@ $violations = Invoke-ScriptAnalyzer $directory\RuleSuppression.ps1 Describe "RuleSuppressionWithoutScope" { Context "Function" { It "Does not raise violations" { - $suppression = $violations | Where-Object { $_.RuleName -eq "PSProvideVerboseMessage" } + $suppression = $violations | Where-Object { $_.RuleName -eq "PSProvideCommentHelp" } $suppression.Count | Should Be 0 } } diff --git a/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 b/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 index 0b90d1b71..47a709f27 100644 --- a/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 +++ b/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 @@ -2,14 +2,14 @@ $globalMessage = "Found global variable 'Global:1'." $globalName = "PSAvoidGlobalVars" $nonInitializedName = "PSAvoidUninitializedVariable" -$nonInitializedMessage = "Variable 'a' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables." +$nonInitializedMessage = "Variable 'globalVars' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables." $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVars.ps1 $dscResourceViolations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAny\MSFT_WaitForAny.psm1 | Where-Object {$_.RuleName -eq $nonInitializedName} $globalViolations = $violations | Where-Object {$_.RuleName -eq $globalName} $nonInitializedViolations = $violations | Where-Object {$_.RuleName -eq $nonInitializedName} $noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVarsNoViolations.ps1 -$noGlobalViolations = $noViolations | Where-Object {$_.RuleName -eq $violationName} +$noGlobalViolations = $noViolations | Where-Object {$_.RuleName -eq $globalName} $noUninitializedViolations = $noViolations | Where-Object {$_.RuleName -eq $nonInitializedName} Describe "AvoidGlobalVars" { @@ -23,7 +23,7 @@ Describe "AvoidGlobalVars" { } It "has the correct description message" { - $violations[0].Message | Should Match $globalMessage + $globalViolations[0].Message | Should Match $globalMessage } } diff --git a/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 b/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 index 42c137872..b2f61dfb8 100644 --- a/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 +++ b/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 @@ -9,6 +9,11 @@ function Test { $a = 3; +"hi there!" -match "hi" | Out-Null; +$matches[0]; + +$PSVersionTable; + if ($true) { $a = 4; $c = 3; diff --git a/Tests/Rules/AvoidPositionalParameters.tests.ps1 b/Tests/Rules/AvoidPositionalParameters.tests.ps1 index 82be8a726..2e5156edd 100644 --- a/Tests/Rules/AvoidPositionalParameters.tests.ps1 +++ b/Tests/Rules/AvoidPositionalParameters.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Cmdlet 'Get-Content' has positional parameter. Please use named parameters instead of positional parameters when calling a command." +$violationMessage = "Cmdlet 'Write-Host' has positional parameter. Please use named parameters instead of positional parameters when calling a command." $violationName = "PSAvoidUsingPositionalParameters" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidPositionalParameters.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 b/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 index 5ae0bd222..41562ffd8 100644 --- a/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 +++ b/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 @@ -3,4 +3,5 @@ Get-ChildItem -Path "Tests" Clear-Host Split-Path -Path "Random" -leaf Get-Process | Where-Object {$_.handles -gt 200} -get-service-computername localhost | where {($_.status -eq "Running") -and ($_.CanStop -eq $true)} \ No newline at end of file +get-service-computername localhost | where {($_.status -eq "Running") -and ($_.CanStop -eq $true)} +1, 2, $null, 4 | ForEach-Object {"Hello"} \ No newline at end of file diff --git a/Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1 b/Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1 index 4fab2e16b..9daf1cf3e 100644 --- a/Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1 +++ b/Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Function 'Verb-Noun' in file 'AvoidShouldContinueWithoutForce.ps1' uses ShouldContinue but does not have a boolean force parameter. The force parameter will allow users of the script to bypass ShouldContinue prompt" +$violationMessage = "Function 'Verb-Noun2' in file 'AvoidShouldContinueWithoutForce.ps1' uses ShouldContinue but does not have a boolean force parameter. The force parameter will allow users of the script to bypass ShouldContinue prompt" $violationName = "PSAvoidShouldContinueWithoutForce" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidShouldContinueWithoutForce.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 index e50e9fcdc..14f567160 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 should be used." +$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential should be used." $violationName = "PSAvoidUsingUserNameAndPasswordParams" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidUsingAlias.tests.ps1 b/Tests/Rules/AvoidUsingAlias.tests.ps1 index 7c9d74db1..63e8f2780 100644 --- a/Tests/Rules/AvoidUsingAlias.tests.ps1 +++ b/Tests/Rules/AvoidUsingAlias.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "'iex' is an alias of 'Invoke-Expression'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content." +$violationMessage = "'cls' is an alias of 'Clear-Host'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content." $violationName = "PSAvoidUsingCmdletAliases" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingAlias.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidUsingInternalURLsNoViolations.ps1 b/Tests/Rules/AvoidUsingInternalURLsNoViolations.ps1 index 337bb0832..1d28e5baa 100644 --- a/Tests/Rules/AvoidUsingInternalURLsNoViolations.ps1 +++ b/Tests/Rules/AvoidUsingInternalURLsNoViolations.ps1 @@ -5,4 +5,6 @@ function Test { $filesNode = $infoXml.SelectSingleNode("//files") } -$sd = "O:BAG:BAD:(A;;0x800;;;WD)(A;;0x120fff;;;SY)(A;;0x120fff;;;LS)(A;;0x120fff;;;NS)(A;;0x120fff;;;BA)(A;;0xee5;;;LU)(A;;LC;;;MU)(A;;0x800;;;AG)" \ No newline at end of file +$sd = "O:BAG:BAD:(A;;0x800;;;WD)(A;;0x120fff;;;SY)(A;;0x120fff;;;LS)(A;;0x120fff;;;NS)(A;;0x120fff;;;BA)(A;;0xee5;;;LU)(A;;LC;;;MU)(A;;0x800;;;AG)" +$msg = [System.String]::Format("1:{0}", "test") + $internalId = $Id -replace '^([^/])','/$1' -as $Id.GetType() \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 b/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 index 2c298a7cf..3dd6ff7c7 100644 --- a/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 +++ b/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer -$violationMessage = [regex]::Escape("Parameter '`$passphrases' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.") +$violationMessage = [regex]::Escape("Parameter '`$password' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.") $violationName = "PSAvoidUsingPlainTextForPassword" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingPlainTextForPassword.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 b/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 index 3fb75b360..4f499ed04 100644 --- a/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 +++ b/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer $AvoidUninitializedVariable = "PSAvoidUninitializedVariable" -$violationMessage = "Variable 'MyProgressPreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables." +$violationMessage = "Variable 'MyVerbosePreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables." $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariable.ps1 -IncludeRule $AvoidUninitializedVariable $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariableNoViolations.ps1 -IncludeRule $AvoidUninitializedVariable diff --git a/Tests/Rules/DSCResources/MSFT_WaitForAny/MSFT_WaitForAny.psm1 b/Tests/Rules/DSCResources/MSFT_WaitForAny/MSFT_WaitForAny.psm1 index 096cf8b54..5b8ee83d9 100644 --- a/Tests/Rules/DSCResources/MSFT_WaitForAny/MSFT_WaitForAny.psm1 +++ b/Tests/Rules/DSCResources/MSFT_WaitForAny/MSFT_WaitForAny.psm1 @@ -29,6 +29,8 @@ function Get-TargetResource [Uint32] $ThrottleLimit = 32 #Powershell New-CimSession default throttle value ) + Write-Verbose "In Get-TargetResource" + Import-Module $PSScriptRoot\..\..\PSDSCxMachine.psm1 $b = @{"hash" = "table"} @@ -75,10 +77,14 @@ function Set-TargetResource [Uint32] $ThrottleLimit = 32 #Powershell New-CimSession default throttle value ) + Write-Verbose "In Set-TargetResource" + Import-Module $PSScriptRoot\..\..\PSDSCxMachine.psm1 if ($PSBoundParameters["Verbose"]) { + Write-Verbose "Calling xMachine with Verbose parameter" + PSDSCxMachine\Set-_InternalPSDscXMachineTR ` -RemoteResourceId $ResourceName ` -RemoteMachine $NodeName ` @@ -130,6 +136,8 @@ function Test-TargetResource [Uint32] $ThrottleLimit = 32 #Powershell New-CimSession default throttle value ) + Write-Verbose "In Test-TargetResource" + Import-Module $PSScriptRoot\..\..\PSDSCxMachine.psm1 $a = $true diff --git a/Tests/Rules/GoodCmdlet.ps1 b/Tests/Rules/GoodCmdlet.ps1 index 24106a3f5..93958ebf7 100644 --- a/Tests/Rules/GoodCmdlet.ps1 +++ b/Tests/Rules/GoodCmdlet.ps1 @@ -28,7 +28,7 @@ function Get-File HelpUri = 'http://www.microsoft.com/', ConfirmImpact='Medium')] [Alias()] - [OutputType([String], [System.Double], [Hashtable])] + [OutputType([String], [System.Double], [Hashtable], "MyCustom.OutputType")] [OutputType("System.Int32", ParameterSetName="ID")] Param @@ -85,6 +85,18 @@ function Get-File $a } + $a | Write-Warning + + $b = @{"hash"="table"} + + Write-Debug @b + + [pscustomobject]@{ + PSTypeName = 'MyCustom.OutputType' + Prop1 = 'SomeValue' + Prop2 = 'OtherValue' + } + return @{"hash"="true"} } End diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 index a2b520739..fbfffd93d 100644 --- a/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 +++ b/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 @@ -1,4 +1,23 @@ function CompareWithNull { if ($DebugPreference -eq $null) { } +} + +if (@("dfd", "eee") -eq $null) +{ +} + +if ($randomUninitializedVariable -eq $null) +{ +} + +function Test +{ + $b = "dd", "ddfd"; + if ($b -ceq $null) + { + if ("dd","ee" -eq $null) + { + } + } } \ No newline at end of file diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 index 4ef850c55..bdcb40902 100644 --- a/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 +++ b/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 @@ -7,8 +7,8 @@ $noViolations = Invoke-ScriptAnalyzer $directory\PossibleIncorrectComparisonWith Describe "PossibleIncorrectComparisonWithNull" { Context "When there are violations" { - It "has 1 possible incorrect comparison with null violation" { - $violations.Count | Should Be 1 + It "has 4 possible incorrect comparison with null violation" { + $violations.Count | Should Be 4 } It "has the correct description message" { diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 index 5ac741e0a..6a34d3c80 100644 --- a/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 +++ b/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 @@ -1,4 +1,15 @@ function CompareWithNull { if ($null -eq $DebugPreference) { } + if ($DebugPreference -eq $null) { + } +} + +$a = 3 + +if ($a -eq $null) +{ + if (3 -eq $null) + { + } } \ No newline at end of file diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index 30a183710..a0ab714d4 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The cmdlet 'Verb-Files' does not have a help comment." +$violationMessage = "The cmdlet 'Comment' does not have a help comment." $violationName = "PSProvideCommentHelp" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/ProvideDefaultParameterValue.tests.ps1 b/Tests/Rules/ProvideDefaultParameterValue.tests.ps1 index c731d7974..5289fdc72 100644 --- a/Tests/Rules/ProvideDefaultParameterValue.tests.ps1 +++ b/Tests/Rules/ProvideDefaultParameterValue.tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer $violationName = "PSProvideDefaultParameterValue" -$violationMessage = "Parameter 'Param2' is not initialized. Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters" +$violationMessage = "Parameter 'Param1' is not initialized. Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValue.ps1 | Where-Object {$_.RuleName -match $violationName} $noViolations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValueNoViolations.ps1 diff --git a/Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1 b/Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1 index f45163f88..4ba05667d 100644 --- a/Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1 +++ b/Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1 @@ -1,7 +1,7 @@ Import-Module -Verbose PSScriptAnalyzer $violationMessageDSCResource = "Test-TargetResource function in DSC Resource should return object of type System.Boolean instead of System.Collections.Hashtable" -$violationMessageDSCClass = "Test function in DSC Class FileResource should return object of type System.Boolean instead of type System.Int32" +$violationMessageDSCClass = "Get function in DSC Class FileResource should return object of type FileResource instead of type System.Collections.Hashtable" $violationName = "PSDSCReturnCorrectTypesForDSCFunctions" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAll\MSFT_WaitForAll.psm1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/UseCmdletCorrectly.ps1 b/Tests/Rules/UseCmdletCorrectly.ps1 index 8fb8392c2..373078af8 100644 --- a/Tests/Rules/UseCmdletCorrectly.ps1 +++ b/Tests/Rules/UseCmdletCorrectly.ps1 @@ -2,4 +2,4 @@ Wrong-Cmd Write-Verbose -Message "Write Verbose" Write-Verbose "Warning" -OutVariable $test -Write-Verbose "Warning" \ No newline at end of file +Write-Verbose "Warning" | PipeLineCmdlet \ No newline at end of file diff --git a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 index 61b5b34a2..f1d818893 100644 --- a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 +++ b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The variable 'declaredVar' is assigned but never used." +$violationMessage = "The variable 'declaredVar2' is assigned but never used." $violationName = "PSUseDeclaredVarsMoreThanAssigments" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\UseDeclaredVarsMoreThanAssignments.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 b/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 index 666d281d0..236a5faa1 100644 --- a/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 +++ b/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The cmdlet 'Verb-Files' returns an object of type 'System.Double' but this type is not declared in the OutputType attribute." +$violationMessage = "The cmdlet 'Verb-Files' returns an object of type 'System.Collections.Hashtable' but this type is not declared in the OutputType attribute." $violationName = "PSUseOutputTypeCorrectly" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 b/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 new file mode 100644 index 000000000..bca6904e7 --- /dev/null +++ b/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 @@ -0,0 +1,26 @@ +Import-Module PSScriptAnalyzer + +$violationMessage = "There is no call to Write-Verbose in DSC function ‘Test-TargetResource’. If you are using Write-Verbose in a helper function, suppress this rule application." +$violationName = "PSDSCUseVerboseMessageInDSCResource" +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$violations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAll\MSFT_WaitForAll.psm1 | Where-Object {$_.RuleName -eq $violationName} +$noViolations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAny\MSFT_WaitForAny.psm1 | Where-Object {$_.RuleName -eq $violationName} +$noClassViolations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $directory\DSCResources\MyDscResource\MyDscResource.psm1 | Where-Object {$_.RuleName -eq $violationName} + +Describe "UseVerboseMessageInDSCResource" { + Context "When there are violations" { + It "has 2 Verbose Message violations" { + $violations.Count | Should Be 2 + } + + It "has the correct description message" { + $violations[0].Message | Should Match $violationMessage + } + } + + Context "When there are no violations" { + It "returns no violations" { + $noViolations.Count | Should Be 0 + } + } +} \ No newline at end of file