Skip to content

Forward Integrate: Master to Development #119

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 14 commits into from
May 7, 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
46 changes: 46 additions & 0 deletions CHANGELOG.MD
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
## Unreleased (May.7, 2015)
###Features:
- Integrated with waffle.io for Project Management.
- Added documentation for writing script rules.

###Rules:
- AvoidUsingWMICmdlet rule: For PowerShell 3.0 and above, usage of WMI cmdlets is not recommended. This rule is to detect WMI cmdlet usage in scripts that are written for PS 3.0 and above.
- DSCTestsPresent rule: Resource module contains Tests folder with tests for given resource.
- UseOutputTypeCorrectly rule: If we can identify the type of an object that is outputted to the pipeline by a cmdlet, then that type must be listed in the OutputType attribute.

###Fixes:

- PSProvideVerboseMessage only throws warnings in non-advanced functions.
- Fix the issue in importing customized rule




##Relesed 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.
- Added wild card supports for rules in Invoke-ScriptAnalyzer and Get-ScriptAnalyzer. Eg. Invoke-ScriptAnalyzer -IncludeRule PSAvoid* will apply all rules starting with PSAvoid* in built in rule assemblies.
- Added -Severity to Get-ScriptAnalyzerRules. Get-ScriptAnalyzer -Severity will filter rules based on the severity given.
- Added Suppression functionality. Users are now able to specify suppression on certain parts of the scripts by specifying "SupressMessageAttribute" in the scripts. More details and documentations will be coming soon in blog posts. Also comes with this feature is the ability for users to display a list of suppressed messages.

###Rules:

- Added DSC Rules for resources including Parameter validation, Usage of standard DSC functions and return type validation. Rule checkings also support for DSC classes. Built-in DSC rules include:
+ UseStandardDSCFunctionsInResource
+ UseIdenticalParametersDSC
+ UseIdenticalMandatoryParametersDSC
+ ReturnCorrectTypesForDSCFunctions
- Added support in the engine to detect DSC configuration/resource files and disable default rule checkings on DSC configuration and resource files.
- UseShouldProcessForStateChangingFunctions - If an advanced function has Verbs like New/Start/Stop/Restart/Reset/Set- that will change system state, it should support ShouldProcess attribute.


###Fixes:

- Improved heuristics to detect usage of Username and Password instead of PSCredential type.
- Improved accuracy in the detection of uninitialized variables.
- Improved error messages to include error line numbers and file names.
- Identified usage of PSBound parameters and PowerShell supplied variables such as $MyInvocation to avoid unnecessary noise in the results returned by some of the built-in rules.
- Fixed terminating errors including "Illegal characters in Path".

90 changes: 35 additions & 55 deletions Engine/Commands/InvokeScriptAnalyzerCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -502,68 +502,48 @@ private void AnalyzeFile(string filePath)
}

// Check if the supplied artifact is indeed part of the DSC resource
// Step 1: Check if the artifact is under the "DSCResources" folder
DirectoryInfo dscResourceParent = Directory.GetParent(filePath);
if (null != dscResourceParent)
if (Helper.Instance.IsDscResourceModule(filePath))
{
DirectoryInfo dscResourcesFolder = Directory.GetParent(dscResourceParent.ToString());
if (null != dscResourcesFolder)
{
if (String.Equals(dscResourcesFolder.Name, "dscresources",StringComparison.OrdinalIgnoreCase))
// Run all DSC Rules
foreach (IDSCResourceRule dscResourceRule in ScriptAnalyzer.Instance.DSCResourceRules)
{
bool includeRegexMatch = false;
bool excludeRegexMatch = false;
foreach (Regex include in includeRegexList)
{
if (include.IsMatch(dscResourceRule.GetName()))
{
includeRegexMatch = true;
break;
}
}
foreach (Regex exclude in excludeRegexList)
{
if (exclude.IsMatch(dscResourceRule.GetName()))
{
excludeRegexMatch = true;
}
}
if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
{
// Step 2: Ensure there is a Schema.mof in the same folder as the artifact
string schemaMofParentFolder = Directory.GetParent(filePath).ToString();
string[] schemaMofFile = Directory.GetFiles(schemaMofParentFolder, "*.schema.mof");
WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, dscResourceRule.GetName()));

// Ensure Schema file exists and is the only one in the DSCResource folder
if (schemaMofFile != null && schemaMofFile.Count() == 1)
// 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
{
// Run DSC Rules only on module that matches the schema.mof file name without extension
if (schemaMofFile[0].Replace("schema.mof", "psm1") == filePath)
{
// Run all DSC Rules
foreach (IDSCResourceRule dscResourceRule in ScriptAnalyzer.Instance.DSCResourceRules)
{
bool includeRegexMatch = false;
bool excludeRegexMatch = false;
foreach (Regex include in includeRegexList)
{
if (include.IsMatch(dscResourceRule.GetName()))
{
includeRegexMatch = true;
break;
}
}
foreach (Regex exclude in excludeRegexList)
{
if (exclude.IsMatch(dscResourceRule.GetName()))
{
excludeRegexMatch = true;
}
}
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
{
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
diagnostics.AddRange(records.Item2);
suppressed.AddRange(records.Item1);
}
catch (Exception dscResourceRuleException)
{
WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
}
}
}
}
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
diagnostics.AddRange(records.Item2);
suppressed.AddRange(records.Item1);
}
catch (Exception dscResourceRuleException)
{
WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
}
}
}
}

}
}
#endregion

Expand Down
35 changes: 35 additions & 0 deletions Engine/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,41 @@ public string GetCmdletNameFromAlias(String Alias)
return String.Empty;
}

/// <summary>
/// Given a file path, checks whether the file is part of a dsc resource module
/// </summary>
/// <param name="fileName"></param>
/// <returns></returns>
public bool IsDscResourceModule(string filePath)
{
DirectoryInfo dscResourceParent = Directory.GetParent(filePath);
if (null != dscResourceParent)
{
DirectoryInfo dscResourcesFolder = Directory.GetParent(dscResourceParent.ToString());
if (null != dscResourcesFolder)
{
if (String.Equals(dscResourcesFolder.Name, "dscresources", StringComparison.OrdinalIgnoreCase))
{
// Step 2: Ensure there is a Schema.mof in the same folder as the artifact
string schemaMofParentFolder = Directory.GetParent(filePath).ToString();
string[] schemaMofFile = Directory.GetFiles(schemaMofParentFolder, "*.schema.mof");

// Ensure Schema file exists and is the only one in the DSCResource folder
if (schemaMofFile != null && schemaMofFile.Count() == 1)
{
// Run DSC Rules only on module that matches the schema.mof file name without extension
if (String.Equals(schemaMofFile[0].Replace("schema.mof", "psm1"), filePath, StringComparison.OrdinalIgnoreCase))
{
return true;
}
}
}
}
}

return false;
}

/// <summary>
/// Given a commandast, checks whether positional parameters are used or not.
/// </summary>
Expand Down
38 changes: 38 additions & 0 deletions RuleDocumentation/AvoidUsingWriteHost.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#AvoidUsingWriteHost
**Severity Level: Warning**


##Description

It is generally accepted that you should never use Write-Host to create any script output whatsoever, unless your script (or function, or whatever) uses the Show verb (as in, Show-Performance). That verb explicitly means “show on the screen, with no other possibilities.” Like Show-Command.

##How to Fix

PTo fix a violation of this rule, please replace Write-Host with Write-Output.

##Example

Wrong:

```
function Test
{
...
Write-Host "Executing.."
}
```

Correct:

```
function Test
{
...
Write-Output "Executing.."
}

function Show-Something
{
Write-Host "show something on screen";
}
```
21 changes: 19 additions & 2 deletions Rules/AvoidUninitializedVariable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//

using System;
using System.Linq;
using System.Collections.Generic;
using System.Management.Automation.Language;
using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
Expand Down Expand Up @@ -51,15 +52,31 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)

IEnumerable<Ast> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst || item is FunctionMemberAst, true);

foreach (var funcAst in funcAsts)
// Checks whether this is a dsc resource file (we don't raise this rule for get, set and test-target resource
bool isDscResourceFile = Helper.Instance.IsDscResourceModule(fileName);

List<string> targetResourcesFunctions = new List<string>( new string[] { "get-targetresource", "set-targetresource", "test-targetresource" });

foreach (FunctionDefinitionAst funcAst in funcAsts)
{
// Finds all VariableExpressionAst.
IEnumerable<Ast> varAsts = funcAst.FindAll(testAst => testAst is VariableExpressionAst, true);

HashSet<string> paramVariables = new HashSet<string>();

if (isDscResourceFile && targetResourcesFunctions.Contains(funcAst.Name, StringComparer.OrdinalIgnoreCase))
{
// don't raise the rules for variables in the param block.
if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
{
paramVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
}
}

// Iterates all VariableExpressionAst and check the command name.
foreach (VariableExpressionAst varAst in varAsts)
{
if (Helper.Instance.IsUninitialized(varAst, funcAst))
if (Helper.Instance.IsUninitialized(varAst, funcAst) && !paramVariables.Contains(varAst.VariablePath.UserPath))
{
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUninitializedVariableError, varAst.VariablePath.UserPath),
varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, varAst.VariablePath.UserPath);
Expand Down
11 changes: 11 additions & 0 deletions Rules/ProvideVerboseMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Management.Automation.Language;
using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
using System.ComponentModel.Composition;
using System.Globalization;
using System.Management.Automation;

namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
{
Expand Down Expand Up @@ -57,6 +59,15 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
return AstVisitAction.SkipChildren;
}

//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)))
{
return AstVisitAction.Continue;
}

var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false);
bool hasVerbose = false;

Expand Down
5 changes: 5 additions & 0 deletions Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ $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."
$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
Expand All @@ -17,6 +18,10 @@ Describe "AvoidGlobalVars" {
$globalViolations.Count | Should Be 1
}

It "has 4 violations for dsc resources (not counting the variables in parameters)" {
$dscResourceViolations.Count | Should Be 4
}

It "has the correct description message" {
$violations[0].Message | Should Match $globalMessage
}
Expand Down
6 changes: 6 additions & 0 deletions Tests/Rules/GoodCmdlet.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,10 @@ function Get-File
if ($pscmdlet.ShouldContinue("Yes", "No")) {
}
}
}

#Write-Verbose should not be required because this is not an advanced function
function Get-SimpleFunc
{

}