Skip to content

Fix bugs in importing external rule #90

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.

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 6 commits into from
May 5, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions Engine/Generic/ExternalRule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ internal class ExternalRule : IExternalRule
string param = string.Empty;
string srcName = string.Empty;
string modPath = string.Empty;

string paramType = string.Empty;

public string GetName()
{
Expand All @@ -55,6 +55,11 @@ public SourceType GetSourceType()
return SourceType.Module;
}

public string GetParameterType()
{
return this.paramType;
}

//Set the community rule level as warning as the current implementation does not require user to specify rule severity when defining their functions in PS scripts
public RuleSeverity GetSeverity()
{
Expand All @@ -80,14 +85,15 @@ public ExternalRule()

}

public ExternalRule(string name, string commonName, string desc, string param, string srcName, string modPath)
public ExternalRule(string name, string commonName, string desc, string param, string paramType, string srcName, string modPath)
{
this.name = name;
this.commonName = commonName;
this.desc = desc;
this.param = param;
this.srcName = srcName;
this.modPath = modPath;
this.paramType = paramType;
}

#endregion
Expand Down
102 changes: 58 additions & 44 deletions Engine/ScriptAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public void Initilaize(Dictionary<string, List<string>> result)
paths = result.ContainsKey("ValidDllPaths") ? result["ValidDllPaths"] : result["ValidPaths"];
foreach (string path in paths)
{
if (Path.GetExtension(path).ToLower(CultureInfo.CurrentCulture) == ".dll")
if (String.Equals(Path.GetExtension(path),".dll",StringComparison.OrdinalIgnoreCase))
{
catalog.Catalogs.Add(new AssemblyCatalog(path));
}
Expand Down Expand Up @@ -241,8 +241,8 @@ public List<ExternalRule> GetExternalRule(string[] moduleNames)

FunctionInfo funcInfo = (FunctionInfo)psobject.ImmediateBaseObject;
ParameterMetadata param = funcInfo.Parameters.Values
.First<ParameterMetadata>(item => item.Name.ToLower(CultureInfo.CurrentCulture).EndsWith("ast", StringComparison.CurrentCulture) ||
item.Name.ToLower(CultureInfo.CurrentCulture).EndsWith("token", StringComparison.CurrentCulture));
.First<ParameterMetadata>(item => item.Name.EndsWith("ast", StringComparison.OrdinalIgnoreCase) ||
item.Name.EndsWith("token", StringComparison.OrdinalIgnoreCase));

//Only add functions that are defined as rules.
if (param != null)
Expand All @@ -251,7 +251,7 @@ public List<ExternalRule> GetExternalRule(string[] moduleNames)
string desc =posh.AddScript(script).Invoke()[0].ImmediateBaseObject.ToString()
.Replace("\r\n", " ").Trim();

rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.Name,
rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.Name,param.ParameterType.FullName,
funcInfo.ModuleName, funcInfo.Module.Path));
}
}
Expand Down Expand Up @@ -289,13 +289,13 @@ public IEnumerable<DiagnosticRecord> GetExternalRecord(Ast ast, Token[] token, E

// Groups rules by AstType and Tokens.
Dictionary<string, List<ExternalRule>> astRuleGroups = rules
.Where<ExternalRule>(item => item.GetParameter().EndsWith("ast", true, CultureInfo.CurrentCulture))
.GroupBy<ExternalRule, string>(item => item.GetParameter())
.Where<ExternalRule>(item => item.GetParameter().EndsWith("ast", StringComparison.OrdinalIgnoreCase))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the key of this dictionary is still the name of the parameter instead of the type of the parameter.

.GroupBy<ExternalRule, string>(item => item.GetParameterType())
.ToDictionary(item => item.Key, item => item.ToList());

Dictionary<string, List<ExternalRule>> tokenRuleGroups = rules
.Where<ExternalRule>(item => item.GetParameter().EndsWith("token", true, CultureInfo.CurrentCulture))
.GroupBy<ExternalRule, string>(item => item.GetParameter())
.Where<ExternalRule>(item => item.GetParameter().EndsWith("token", StringComparison.OrdinalIgnoreCase))
.GroupBy<ExternalRule, string>(item => item.GetParameterType())
.ToDictionary(item => item.Key, item => item.ToList());

using (rsp)
Expand Down Expand Up @@ -337,7 +337,7 @@ public IEnumerable<DiagnosticRecord> GetExternalRecord(Ast ast, Token[] token, E
{
// Find all AstTypes that appeared in rule groups.
IEnumerable<Ast> childAsts = ast.FindAll(new Func<Ast, bool>((testAst) =>
(testAst.GetType().Name.ToLower(CultureInfo.CurrentCulture) == astRuleGroup.Key.ToLower(CultureInfo.CurrentCulture))), false);
(astRuleGroup.Key.IndexOf(testAst.GetType().FullName,StringComparison.OrdinalIgnoreCase) != -1)), false);

foreach (Ast childAst in childAsts)
{
Expand Down Expand Up @@ -365,49 +365,63 @@ public IEnumerable<DiagnosticRecord> GetExternalRecord(Ast ast, Token[] token, E
}

#endregion

#region Collects the results from commands.

for (int i = 0; i < powerShellCommands.Count; i++)
List<DiagnosticRecord> diagnostics = new List<DiagnosticRecord>();
try
{
// EndInvoke will wait for each command to finish, so we will be getting the commands
// in the same order that they have been invoked withy BeginInvoke.
PSDataCollection<PSObject> psobjects = powerShellCommands[i].EndInvoke(powerShellCommandResults[i]);

foreach (var psobject in psobjects)
for (int i = 0; i < powerShellCommands.Count; i++)
{
DiagnosticSeverity severity;
IScriptExtent extent;
string message = string.Empty;
string ruleName = string.Empty;

// Because error stream is merged to output stream,
// we need to handle the error records.
if (psobject.ImmediateBaseObject is ErrorRecord)
{
ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject;
command.WriteError(record);
continue;
}
// EndInvoke will wait for each command to finish, so we will be getting the commands
// in the same order that they have been invoked withy BeginInvoke.
PSDataCollection<PSObject> psobjects = powerShellCommands[i].EndInvoke(powerShellCommandResults[i]);

// DiagnosticRecord may not be correctly returned from external rule.
try
foreach (var psobject in psobjects)
{
Enum.TryParse<DiagnosticSeverity>(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity);
message = psobject.Properties["Message"].Value.ToString();
extent = (IScriptExtent)psobject.Properties["Extent"].Value;
ruleName = psobject.Properties["RuleName"].Value.ToString();
DiagnosticSeverity severity;
IScriptExtent extent;
string message = string.Empty;
string ruleName = string.Empty;

if (psobject != null && psobject.ImmediateBaseObject != null)
{
// Because error stream is merged to output stream,
// we need to handle the error records.
if (psobject.ImmediateBaseObject is ErrorRecord)
{
ErrorRecord record = (ErrorRecord)psobject.ImmediateBaseObject;
command.WriteError(record);
continue;
}

// DiagnosticRecord may not be correctly returned from external rule.
try
{
Enum.TryParse<DiagnosticSeverity>(psobject.Properties["Severity"].Value.ToString().ToUpper(), out severity);
message = psobject.Properties["Message"].Value.ToString();
extent = (IScriptExtent)psobject.Properties["Extent"].Value;
ruleName = psobject.Properties["RuleName"].Value.ToString();
}
catch (Exception ex)
{
command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this));
continue;
}

if (!string.IsNullOrEmpty(message))
{
diagnostics.Add(new DiagnosticRecord(message, extent, ruleName, severity, null));
}
}
}
catch (Exception ex)
{
command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this));
continue;
}

if (!string.IsNullOrEmpty(message)) yield return new DiagnosticRecord(message, extent, ruleName, severity, null);
}
}
//Catch exception where customized defined rules have exceptins when doing invoke
catch(Exception ex)
{
command.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this));
}

return diagnostics;
#endregion
}
}
Expand Down Expand Up @@ -478,7 +492,7 @@ public Dictionary<string, List<string>> CheckRuleExtension(string[] path, PSCmdl

cmdlet.WriteDebug(string.Format(CultureInfo.CurrentCulture, Strings.CheckAssemblyFile, resolvedPath));

if (Path.GetExtension(resolvedPath).ToLower(CultureInfo.CurrentCulture) == ".dll")
if (String.Equals(Path.GetExtension(resolvedPath),".dll", StringComparison.OrdinalIgnoreCase))
{
if (!File.Exists(resolvedPath))
{
Expand Down
40 changes: 40 additions & 0 deletions Tests/Engine/CustomizedRule.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
Import-Module PSScriptAnalyzer
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
$message = "this is help"
$measure = "Measure-RequiresRunAsAdministrator"

Describe "Test importing customized rules with null return results" {
Context "Test Get-ScriptAnalyzer with customized rules" {
It "will not terminate the engine" {
$customizedRulePath = Get-ScriptAnalyzerRule -CustomizedRulePath $directory\samplerule\SampleRulesWithErrors.psm1 | Where-Object {$_.RuleName -eq $measure}
$customizedRulePath.Count | Should Be 1
}

}

Context "Test Invoke-ScriptAnalyzer with customized rules" {
It "will not terminate the engine" {
$customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -CustomizedRulePath $directory\samplerule\SampleRulesWithErrors.psm1 | Where-Object {$_.RuleName -eq $measure}
$customizedRulePath.Count | Should Be 0
}
}

}

Describe "Test importing correct customized rules" {
Context "Test Get-ScriptAnalyzer with customized rules" {
It "will show the customized rule" {
$customizedRulePath = Get-ScriptAnalyzerRule -CustomizedRulePath $directory\samplerule\samplerule.psm1 | Where-Object {$_.RuleName -eq $measure}
$customizedRulePath.Count | Should Be 1
}

}

Context "Test Invoke-ScriptAnalyzer with customized rules" {
It "will show the customized rule in the results" {
$customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -CustomizedRulePath $directory\samplerule\samplerule.psm1 | Where-Object {$_.Message -eq $message}
$customizedRulePath.Count | Should Be 1
}
}

}
40 changes: 40 additions & 0 deletions Tests/Engine/samplerule/SampleRulesWithErrors.psm1
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#Requires -Version 3.0

<#
.SYNOPSIS
Uses #Requires -RunAsAdministrator instead of your own methods.
.DESCRIPTION
The #Requires statement prevents a script from running unless the Windows PowerShell version, modules, snap-ins, and module and snap-in version prerequisites are met.
From Windows PowerShell 4.0, the #Requires statement let script developers require that sessions be run with elevated user rights (run as Administrator).
Script developers does not need to write their own methods any more.
To fix a violation of this rule, please consider to use #Requires -RunAsAdministrator instead of your own methods.
.EXAMPLE
Measure-RequiresRunAsAdministrator -ScriptBlockAst $ScriptBlockAst
.INPUTS
[System.Management.Automation.Language.ScriptBlockAst]
.OUTPUTS
[OutputType([PSCustomObject[])]
.NOTES
None
#>
function Measure-RequiresRunAsAdministrator
{
[CmdletBinding()]
[OutputType([Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]])]
Param
(
[Parameter(Mandatory = $true)]
[ValidateNotNullOrEmpty()]
[System.Management.Automation.Language.ScriptBlockAst]
$ScriptBlockAst
)


$results = @()

$results += $null
return $results


}
Export-ModuleMember -Function Measure*
47 changes: 47 additions & 0 deletions Tests/Engine/samplerule/samplerule.psm1
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#Requires -Version 3.0

<#
.SYNOPSIS
Uses #Requires -RunAsAdministrator instead of your own methods.
.DESCRIPTION
The #Requires statement prevents a script from running unless the Windows PowerShell version, modules, snap-ins, and module and snap-in version prerequisites are met.
From Windows PowerShell 4.0, the #Requires statement let script developers require that sessions be run with elevated user rights (run as Administrator).
Script developers does not need to write their own methods any more.
To fix a violation of this rule, please consider to use #Requires -RunAsAdministrator instead of your own methods.
.EXAMPLE
Measure-RequiresRunAsAdministrator -ScriptBlockAst $ScriptBlockAst
.INPUTS
[System.Management.Automation.Language.ScriptBlockAst]
.OUTPUTS
[OutputType([PSCustomObject[])]
.NOTES
None
#>
function Measure-RequiresRunAsAdministrator
{
[CmdletBinding()]
[OutputType([Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]])]
Param
(
[Parameter(Mandatory = $true)]
[ValidateNotNullOrEmpty()]
[System.Management.Automation.Language.ScriptBlockAst]
$testAst
)


$results = @()

$result = [Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]]@{"Message" = "this is help";
"Extent" = $ast.Extent;
"RuleName" = $PSCmdlet.MyInvocation.InvocationName;
"Severity" = "Warning"}

$results += $result


return $results


}
Export-ModuleMember -Function Measure*