diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 2c1e2f3f4..16c2911ff 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -129,8 +129,8 @@ public SwitchParameter SuppressedOnly #region Private Members Dictionary> validationResults = new Dictionary>(); - private ScriptBlockAst ast = null; - private IEnumerable rules = null; + private ScriptBlockAst ast = null; + private IEnumerable rules = null; #endregion @@ -163,9 +163,9 @@ protected override void BeginProcessing() } else { - validationResults.Add("InvalidPaths", new List()); + validationResults.Add("InvalidPaths", new List()); validationResults.Add("ValidModPaths", new List()); - validationResults.Add("ValidDllPaths", new List()); + validationResults.Add("ValidDllPaths", new List()); } #endregion @@ -174,7 +174,7 @@ protected override void BeginProcessing() try { - if (validationResults["ValidDllPaths"].Count == 0 && + if (validationResults["ValidDllPaths"].Count == 0 && validationResults["ValidModPaths"].Count == 0) { ScriptAnalyzer.Instance.Initialize(); @@ -185,7 +185,7 @@ protected override void BeginProcessing() } } catch (Exception ex) - { + { ThrowTerminatingError(new ErrorRecord(ex, ex.HResult.ToString("X", CultureInfo.CurrentCulture), ErrorCategory.NotSpecified, this)); } @@ -228,8 +228,8 @@ private void ProcessPath(string path) if (path == null) { - ThrowTerminatingError(new ErrorRecord(new FileNotFoundException(), - string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, path), + ThrowTerminatingError(new ErrorRecord(new FileNotFoundException(), + string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, path), ErrorCategory.InvalidArgument, this)); } @@ -315,11 +315,11 @@ private void AnalyzeFile(string filePath) else { ThrowTerminatingError(new ErrorRecord(new FileNotFoundException(), - string.Format(CultureInfo.CurrentCulture, Strings.InvalidPath, filePath), + string.Format(CultureInfo.CurrentCulture, Strings.InvalidPath, filePath), ErrorCategory.InvalidArgument, filePath)); } - if (errors != null && errors.Length > 0) + if (errors != null && errors.Length > 0) { foreach (ParseError error in errors) { @@ -362,7 +362,7 @@ private void AnalyzeFile(string filePath) #region Run ScriptRules //Trim down to the leaf element of the filePath and pass it to Diagnostic Record string fileName = System.IO.Path.GetFileName(filePath); - + if (ScriptAnalyzer.Instance.ScriptRules != null) { foreach (IScriptRule scriptRule in ScriptAnalyzer.Instance.ScriptRules) @@ -433,7 +433,7 @@ private void AnalyzeFile(string filePath) break; } } - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) { WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, tokenRule.GetName())); @@ -448,7 +448,7 @@ private void AnalyzeFile(string filePath) catch (Exception tokenRuleException) { WriteError(new ErrorRecord(tokenRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, fileName)); - } + } } } } @@ -458,46 +458,50 @@ private void AnalyzeFile(string filePath) #region DSC Resource Rules if (ScriptAnalyzer.Instance.DSCResourceRules != null) { - // Run DSC Class rule - foreach (IDSCResourceRule dscResourceRule in ScriptAnalyzer.Instance.DSCResourceRules) + // Invoke AnalyzeDSCClass only if the ast is a class based resource + if (Helper.Instance.IsDscResourceClassBased(ast)) { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - - foreach (Regex include in includeRegexList) + // Run DSC Class rule + foreach (IDSCResourceRule dscResourceRule in ScriptAnalyzer.Instance.DSCResourceRules) { - if (include.IsMatch(dscResourceRule.GetName())) + bool includeRegexMatch = false; + bool excludeRegexMatch = false; + + foreach (Regex include in includeRegexList) { - includeRegexMatch = true; - break; + if (include.IsMatch(dscResourceRule.GetName())) + { + includeRegexMatch = true; + break; + } } - } - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(dscResourceRule.GetName())) + foreach (Regex exclude in excludeRegexList) { - excludeRegexMatch = true; - break; + if (exclude.IsMatch(dscResourceRule.GetName())) + { + excludeRegexMatch = true; + break; + } } - } - - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || excludeRegexMatch)) - { - WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, dscResourceRule.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(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, dscResourceRule.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(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList()); + diagnostics.AddRange(records.Item2); + suppressed.AddRange(records.Item1); + } + catch (Exception dscResourceRuleException) + { + WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath)); + } } - catch (Exception dscResourceRuleException) - { - WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath)); - } } } @@ -543,7 +547,7 @@ private void AnalyzeFile(string filePath) } } - } + } } #endregion @@ -607,4 +611,4 @@ private void AnalyzeFile(string filePath) #endregion } -} +} \ No newline at end of file diff --git a/Engine/Helper.cs b/Engine/Helper.cs index c603b20ce..8864ac136 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -191,6 +191,35 @@ public bool IsDscResourceModule(string filePath) return false; } + /// + /// Given an AST, checks whether dsc resource is class based or not + /// + /// + /// + public bool IsDscResourceClassBased(ScriptBlockAst ast) + { + if (null == ast) + { + return false; + } + + List dscResourceFunctionNames = new List(new string[] { "Test", "Get", "Set" }); + + IEnumerable dscClasses = ast.FindAll(item => + item is TypeDefinitionAst + && ((item as TypeDefinitionAst).IsClass) + && (item as TypeDefinitionAst).Attributes.Any(attr => String.Equals("DSCResource", attr.TypeName.FullName, StringComparison.OrdinalIgnoreCase)), true); + + // Found one or more classes marked with DscResource attribute + // So this might be a DscResource. Further validation will be performed by the individual rules + if (null != dscClasses && 0 < dscClasses.Count()) + { + return true; + } + + return false; + } + /// /// Given a commandast, checks whether positional parameters are used or not. /// @@ -280,7 +309,7 @@ public bool PositionalParameterUsed(CommandAst cmdAst) /// /// /// - public CommandInfo GetCommandInfo(string name, CommandTypes commandType=CommandTypes.All) + public CommandInfo GetCommandInfo(string name, CommandTypes commandType = CommandTypes.All) { return Helper.Instance.MyCmdlet.InvokeCommand.GetCommand(name, commandType); } @@ -294,7 +323,7 @@ public IEnumerable DscResourceFunctions(Ast ast) { List resourceFunctionNames = new List(new string[] { "Set-TargetResource", "Get-TargetResource", "Test-TargetResource" }); return ast.FindAll(item => item is FunctionDefinitionAst - && resourceFunctionNames.Contains((item as FunctionDefinitionAst).Name, StringComparer.OrdinalIgnoreCase), true); + && resourceFunctionNames.Contains((item as FunctionDefinitionAst).Name, StringComparer.OrdinalIgnoreCase), true); } /// @@ -481,7 +510,7 @@ public bool AllCodePathReturns(Ast ast) var analysis = VariableAnalysisDictionary[ast]; return analysis.Exit._predecessors.All(block => block._returns || block._unreachable || block._throws); } - + /// /// Initialize variable analysis on the script ast /// @@ -701,7 +730,7 @@ public Dictionary> GetRuleSuppression(Ast ast) { ruleSuppressionList.AddRange(RuleSuppression.GetSuppressions(sbAst.ParamBlock.Attributes, sbAst.Extent.StartOffset, sbAst.Extent.EndOffset, sbAst)); } - + // Get rule suppression from functions IEnumerable funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true).Cast(); @@ -727,13 +756,13 @@ public Dictionary> GetRuleSuppression(Ast ast) List ruleSuppressions = new List(); results.Add(ruleSuppression.RuleName, ruleSuppressions); } - + results[ruleSuppression.RuleName].Add(ruleSuppression); } return results; } - + /// /// Returns a list of rule suppressions from the function /// @@ -943,11 +972,11 @@ public int Compare(Tuple t1, Tuple t2) } } } - + /// /// Class used to do variable analysis on the whole script /// - public class ScriptAnalysis: ICustomAstVisitor + public class ScriptAnalysis : ICustomAstVisitor { private VariableAnalysis OuterAnalysis; @@ -1148,7 +1177,7 @@ public object VisitBreakStatement(BreakStatementAst breakStatementAst) { return null; } - + /// /// Visits body /// @@ -2396,7 +2425,7 @@ public object VisitArrayExpression(ArrayExpressionAst arrayExprAst) { return typeof(System.Array).FullName; } - + /// /// Returns type of array /// @@ -2499,7 +2528,7 @@ public object VisitIndexExpression(IndexExpressionAst indexAst) return null; } - + /// /// Only returns boolean type for unary operator that returns boolean /// @@ -2540,4 +2569,4 @@ public object VisitUsingExpression(UsingExpressionAst usingExpressionAst) return null; } } -} +} \ No newline at end of file