Skip to content

Optimize dsc class interface invocations #178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 19, 2015
Merged

Optimize dsc class interface invocations #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13ce922
Optimize DSC interface invocations - Class interface is only called f…
raghushantha May 18, 2015
cfbeb7b
Fixed parameter name in the comment section of the helper method
raghushantha May 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 49 additions & 45 deletions Engine/Commands/InvokeScriptAnalyzerCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ public SwitchParameter SuppressedOnly
#region Private Members

Dictionary<string, List<string>> validationResults = new Dictionary<string, List<string>>();
private ScriptBlockAst ast = null;
private IEnumerable<IRule> rules = null;
private ScriptBlockAst ast = null;
private IEnumerable<IRule> rules = null;

#endregion

Expand Down Expand Up @@ -163,9 +163,9 @@ protected override void BeginProcessing()
}
else
{
validationResults.Add("InvalidPaths", new List<string>());
validationResults.Add("InvalidPaths", new List<string>());
validationResults.Add("ValidModPaths", new List<string>());
validationResults.Add("ValidDllPaths", new List<string>());
validationResults.Add("ValidDllPaths", new List<string>());
}

#endregion
Expand All @@ -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();
Expand All @@ -185,7 +185,7 @@ protected override void BeginProcessing()
}
}
catch (Exception ex)
{
{
ThrowTerminatingError(new ErrorRecord(ex, ex.HResult.ToString("X", CultureInfo.CurrentCulture),
ErrorCategory.NotSpecified, this));
}
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()));

Expand All @@ -448,7 +448,7 @@ private void AnalyzeFile(string filePath)
catch (Exception tokenRuleException)
{
WriteError(new ErrorRecord(tokenRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, fileName));
}
}
}
}
}
Expand All @@ -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));
}
}
}

Expand Down Expand Up @@ -543,7 +547,7 @@ private void AnalyzeFile(string filePath)
}
}

}
}
}
#endregion

Expand Down Expand Up @@ -607,4 +611,4 @@ private void AnalyzeFile(string filePath)

#endregion
}
}
}
53 changes: 41 additions & 12 deletions Engine/Helper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,35 @@ public bool IsDscResourceModule(string filePath)
return false;
}

/// <summary>
/// Given an AST, checks whether dsc resource is class based or not
/// </summary>
/// <param name="ast"></param>
/// <returns></returns>
public bool IsDscResourceClassBased(ScriptBlockAst ast)
{
if (null == ast)
{
return false;
}

List<string> dscResourceFunctionNames = new List<string>(new string[] { "Test", "Get", "Set" });

IEnumerable<Ast> 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;
}

/// <summary>
/// Given a commandast, checks whether positional parameters are used or not.
/// </summary>
Expand Down Expand Up @@ -280,7 +309,7 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
/// <param name="name"></param>
/// <param name="commandType"></param>
/// <returns></returns>
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);
}
Expand All @@ -294,7 +323,7 @@ public IEnumerable<Ast> DscResourceFunctions(Ast ast)
{
List<string> resourceFunctionNames = new List<string>(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);
}

/// <summary>
Expand Down Expand Up @@ -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);
}

/// <summary>
/// Initialize variable analysis on the script ast
/// </summary>
Expand Down Expand Up @@ -701,7 +730,7 @@ public Dictionary<string, List<RuleSuppression>> GetRuleSuppression(Ast ast)
{
ruleSuppressionList.AddRange(RuleSuppression.GetSuppressions(sbAst.ParamBlock.Attributes, sbAst.Extent.StartOffset, sbAst.Extent.EndOffset, sbAst));
}

// Get rule suppression from functions
IEnumerable<FunctionDefinitionAst> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true).Cast<FunctionDefinitionAst>();

Expand All @@ -727,13 +756,13 @@ public Dictionary<string, List<RuleSuppression>> GetRuleSuppression(Ast ast)
List<RuleSuppression> ruleSuppressions = new List<RuleSuppression>();
results.Add(ruleSuppression.RuleName, ruleSuppressions);
}

results[ruleSuppression.RuleName].Add(ruleSuppression);
}

return results;
}

/// <summary>
/// Returns a list of rule suppressions from the function
/// </summary>
Expand Down Expand Up @@ -943,11 +972,11 @@ public int Compare(Tuple<int, int> t1, Tuple<int, int> t2)
}
}
}

/// <summary>
/// Class used to do variable analysis on the whole script
/// </summary>
public class ScriptAnalysis: ICustomAstVisitor
public class ScriptAnalysis : ICustomAstVisitor
{
private VariableAnalysis OuterAnalysis;

Expand Down Expand Up @@ -1148,7 +1177,7 @@ public object VisitBreakStatement(BreakStatementAst breakStatementAst)
{
return null;
}

/// <summary>
/// Visits body
/// </summary>
Expand Down Expand Up @@ -2396,7 +2425,7 @@ public object VisitArrayExpression(ArrayExpressionAst arrayExprAst)
{
return typeof(System.Array).FullName;
}

/// <summary>
/// Returns type of array
/// </summary>
Expand Down Expand Up @@ -2499,7 +2528,7 @@ public object VisitIndexExpression(IndexExpressionAst indexAst)

return null;
}

/// <summary>
/// Only returns boolean type for unary operator that returns boolean
/// </summary>
Expand Down Expand Up @@ -2540,4 +2569,4 @@ public object VisitUsingExpression(UsingExpressionAst usingExpressionAst)
return null;
}
}
}
}