Skip to content

Commit

Permalink
Merge pull request #248 from PowerShell/BugFixes
Browse files Browse the repository at this point in the history 
Merge BugFixes to Master
  • Loading branch information
@raghushantha
raghushantha committed Jun 19, 2015
2 parents 967e956 + dc59597 commit 0805538
Show file tree
Hide file tree
Showing 21 changed files with 147 additions and 79 deletions.
149 changes: 108 additions & 41 deletions Engine/Commands/InvokeScriptAnalyzerCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

using System.Text.RegularExpressions;
using System;
using System.ComponentModel;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
Expand All @@ -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
{
Expand Down Expand Up @@ -267,6 +271,13 @@ private void ProcessPath(string path)

}

ConcurrentBag<DiagnosticRecord> diagnostics;
ConcurrentBag<SuppressedRecord> suppressed;
Dictionary<string, List<RuleSuppression>> ruleSuppressions;
List<Regex> includeRegexList;
List<Regex> excludeRegexList;
ConcurrentDictionary<string, List<object>> ruleDictionary;

/// <summary>
/// Analyzes a single script file.
/// </summary>
Expand All @@ -275,15 +286,16 @@ private void AnalyzeFile(string filePath)
{
Token[] tokens = null;
ParseError[] errors = null;
List<DiagnosticRecord> diagnostics = new List<DiagnosticRecord>();
List<SuppressedRecord> suppressed = new List<SuppressedRecord>();
ConcurrentBag<DiagnosticRecord> diagnostics = new ConcurrentBag<DiagnosticRecord>();
ConcurrentBag<SuppressedRecord> suppressed = new ConcurrentBag<SuppressedRecord>();
BlockingCollection<List<object>> verboseOrErrors = new BlockingCollection<List<object>>();

// Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash
List<KeyValuePair<CommandInfo, IScriptExtent>> cmdInfoTable = new List<KeyValuePair<CommandInfo, IScriptExtent>>();

//Check wild card input for the Include/ExcludeRules and create regex match patterns
List<Regex> includeRegexList = new List<Regex>();
List<Regex> excludeRegexList = new List<Regex>();
includeRegexList = new List<Regex>();
excludeRegexList = new List<Regex>();
if (includeRule != null)
{
foreach (string rule in includeRule)
Expand Down Expand Up @@ -331,7 +343,7 @@ private void AnalyzeFile(string filePath)
return;
}

Dictionary<string, List<RuleSuppression>> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);

foreach (List<RuleSuppression> ruleSuppressionsList in ruleSuppressions.Values)
{
Expand Down Expand Up @@ -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<object> result = new List<object>();

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<object> 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);
}
}
}
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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<DiagnosticRecord> 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
Expand All @@ -596,7 +663,7 @@ private void AnalyzeFile(string filePath)
}
else
{
foreach (DiagnosticRecord diagnostic in diagnostics)
foreach (DiagnosticRecord diagnostic in diagnosticsList)
{
logger.LogObject(diagnostic, this);
}
Expand Down
15 changes: 1 addition & 14 deletions Engine/Helper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
CommandInfo commandInfo = GetCommandInfo(GetCmdletNameFromAlias(cmdAst.GetCommandName())) ?? GetCommandInfo(cmdAst.GetCommandName());

IEnumerable<ParameterMetadata> switchParams = null;
IEnumerable<CommandParameterSetInfo> scriptBlocks = null;
bool hasScriptBlockSet = false;

if (HasSplattedVariable(cmdAst))
{
Expand All @@ -262,15 +260,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
try
{
switchParams = commandInfo.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter);
scriptBlocks = commandInfo.ParameterSets;
foreach (CommandParameterSetInfo cmdParaset in scriptBlocks)
{
if (String.Equals(cmdParaset.Name, "ScriptBlockSet", StringComparison.OrdinalIgnoreCase))
{
hasScriptBlockSet = true;
}
}

}
catch (Exception)
{
Expand All @@ -284,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
Expand All @@ -308,7 +295,7 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
{
arguments += 1;
}
}

}

// if not the first element in a pipeline, increase the number of arguments by 1
Expand Down
2 changes: 1 addition & 1 deletion Rules/AvoidDefaultTrueValueSwitchParameter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public IEnumerable<DiagnosticRecord> 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(
Expand Down
3 changes: 2 additions & 1 deletion Rules/AvoidUsingDeprecatedManifestFields.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public IEnumerable<DiagnosticRecord> 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<PSObject> result = null;
try
{
Expand Down Expand Up @@ -73,6 +73,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
}
}

ps.Dispose();
}

}
Expand Down
3 changes: 2 additions & 1 deletion Rules/MissingModuleManifestField.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public IEnumerable<DiagnosticRecord> 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
{
Expand Down Expand Up @@ -68,6 +68,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
}
}

ps.Dispose();
}

}
Expand Down
13 changes: 9 additions & 4 deletions Rules/UseOutputTypeCorrectly.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,20 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun

List<Tuple<string, StatementAst>> returnTypes = FindPipelineOutput.OutputTypes(funcAst, _classes);

HashSet<string> specialTypes = new HashSet<string>(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<string, StatementAst> 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;
Expand Down
6 changes: 3 additions & 3 deletions Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand All @@ -23,7 +23,7 @@ Describe "AvoidGlobalVars" {
}

It "has the correct description message" {
$violations[0].Message | Should Match $globalMessage
$globalViolations[0].Message | Should Match $globalMessage
}
}

Expand Down
2 changes: 1 addition & 1 deletion Tests/Rules/AvoidPositionalParameters.tests.ps1
View file
Original file line number Diff line number Diff line change
@@ -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}
Expand Down
Loading

0 comments on commit 0805538

Please sign in to comment.