Skip to content

New Best practice formatting #136

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 5 commits into from
May 11, 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
133 changes: 133 additions & 0 deletions PowerShellBestPractices.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
#PowerShell Best Practices

The following guidelines come from a combined effort from both the PowerShell team and the community. We will use this guideline to define rules for PSScriptAnalyzer. Please feel free to propose additional guidelines and rules for PSScriptAnalyzer.
**Note: The hyperlink next to each guidelines will redirect to documentation page for the rule that is already implemented.

##Cmdlet Design Rules
###Severity: Error
###Severity: Warning
- Use Only Approved Verbs [UseApprovedVerbs](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseApprovedVerbs.md)
- Cmdlets Names: Characters that cannot be Used [AvoidReservedCharInCmdlet](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidReservedCharInCmdlet.md)
- Parameter Names that cannot be Used [AvoidReservedParams](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidReservedParams.md)
- Support Confirmation Requests [UseShouldProcessCorrectly](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseShouldProcessCorrectly.md) and [UseShouldProcessForStateChangingFunctions](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseShouldProcessForStateChangingFunctions.md)
- Nouns should be singular [UseSingularNouns](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseSingularNouns.md)
- Module Manifest Fields [MissingModuleManifestField](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/MissingModuleManifestField.md)
- Version
- Author
- Description
- LicenseUri (for PowerShell Gallery)
- Must call ShouldProcess when ShouldProcess attribute is present and vice versa.[UseShouldProcessCorrectly](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseShouldProcessCorrectly.md)
- Switch parameters should not default to true  [AvoidDefaultTrueValueSwtichParameter](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidDefaultTrueValueSwitchParameter.md)

###Severity: Information

###Severity: TBD
- Support Force Parameter for Interactive Session
- If your cmdlet is used interactively, always provide a Force parameter to override the interactive actions, such as prompts or reading lines of input). This is important because it allows your cmdlet to be used in non-interactive scripts and hosts. The following methods can be implemented by an interactive host.
- Document Output Objects
- Module must be loadable
- No syntax errors
- Unresolved dependencies are an error
- Derive from the Cmdlet or PSCmdlet Classes
- Specify the Cmdlet Attribute
- Override an Input Processing Method
- Specify the OutputType Attribute
- Write Single Records to the Pipeline
- Make Cmdlets Case-Insensitive and Case-Preserving

##Script Functions
###Severity: Error

###Severity: Warning
- Avoid using alias [AvoidAlias](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidAlias.md)
- Avoid using deprecated WMI cmdlets [AvoidUsingWMICmdlet](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingWMICmdlet.md)
- Empty catch block should not be used [AvoidEmptyCatchBlock](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidEmptyCatchBlock.md)
- Invoke existing cmdlet with correct parameters [UseCmdletCorrectly](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseCmdletCorrectly.md)
- Cmdlets should have ShouldProcess/ShouldContinue and Force param if certain system-modding verbs are present (Update, Set, Remove, New)[UseShouldProcessForStateChangingFunctions](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseShouldProcessForStateChangingFunctions.md)
- Positional parameters should be avoided [AvoidUsingPositionalParameters](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingPositionalParameters.md)
- Non-global variables must be initialized. Those that are supposed to be global and not initialized must have “global:” (includes for loop initializations)[AvoidUninitializedVariable](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUninitializedVariable.md)
- Global variables should be avoided. [AvoidGlobalVars](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidGlobalVars.md)
- Declared variables must be used in more than just their assignment. [UseDeclaredVarsMoreThanAssignments](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseDeclaredVarsMoreThanAssignments.md)
- No trap statments should be used [AvoidTrapStatement](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidTrapStatement.md)
- No Invoke-Expression [AvoidUsingInvokeExpression](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingInvokeExpression.md)

###Severity: Information

###Severity: TBD
- Clear-Host should not be used
- File paths should not be used (UNC)
- Error Handling
- Use -ErrorAction Stop when calling cmdlets
- Use $ErrorActionPreference = 'Stop'/' Continue' when calling non-cmdlets
- Avoid using flags to handle errors
- Avoid using $?
- Avoid testing for a null variable as an error condition
- Copy $Error[0] to your own variable
- Avoid using pipelines in scripts
- If a return type is declared, the cmdlet must return that type. If a type is returned, a return type must be declared.



##Scripting Style
###Severity: Error

###Severity: Warning
- Don't use write-host unless writing to the host is all you want to do [AvoidUsingWriteHost](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingWriteHost.md)

###Severity: Information
- Write comment-based help [ProvideCommentHelp](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/ProvideCommentHelp.md)
- Use write-verbose to give information to someone running your script [ProvideVerboseMessage](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/ProvideVerboseMessage.md)
###Severity: TBD
- Provide usage Examples
- Use the Notes section for detail on how the tool work
- Should have help on every exported command (including parameter documentation
- Document the version of PowerShell that script was written for
- Indent your code
- Avoid backticks


##Script Security
###Severity: Error
- Password should be secure string [AvoidUsingPlainTextForPassword](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingPlainTextForPassword.md)- Should never have both -Username and -Password parameters (should take credentials)[UsePSCredentialType](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UsePSCredentialType.md)
- -ComputerName hardcoded should not be used (information disclosure)[AvoidUsingComputerNameHardcoded](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingComputerNameHardcoded.md)
- ConvertTo-SecureString with plaintext should not be used (information disclosure) [AvoidUsingConvertToSecureStringWithPlainText](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingConvertToSecureStringWithPlainText.md)

###Severity: Warning
Copy link
Member

Choose a reason for hiding this comment

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

Markdown is not rendered correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

- Password = 'string' should not be used. (information disclosure) [AvoidUsingUsernameAndPasswordParams](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingUsernameAndPasswordParams.md)
- Internal URLs should not be used (information disclosure)[AvoidUsingFilePath](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidUsingFilePath.md)

###Severity: Information

###Severity: TBD
- APIKey and Credentials variables that are initialized (information disclosure)


##DSC Related Rules
###Severity: Error
- Use standard DSC methods [UseStandardDSCFunctionsInResource](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseStandardDSC FunctionsInResource.md)
- Use identical mandatory parameters for all DSC methods [UseIdenticalMandatoryParametersDSC](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseIdenticalMandatoryParametersDSC.md)
- Use identical parameters for Set and Test DSC methods [UseIdenticalParametersDSC](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/UseIdenticalParametersDSC.md)

###Severity: Warning

###Severity: Information
- All of the following three rule are grouped by: [ReturnCorrectTypeDSCFunctions](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/ReturnCorrectTypeDSCFunctions.md)
- Avoid return any object from a Set-TargetResource function
- Returning a Boolean object from a Test-TargetResource function
- Returning an object from a Get-TargetResource function
- DSC resources should have DSC tests [DSCTestsPresent](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/DscTestsPresent.md)
- DSC resources should have DSC examples [DSCExamplesPresent](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/DscExamplesPresent.md)

###Severity: TBD
- For PowerShell V4: Resource module contains .psd1 file and schema.mof for every resource
- MOF has description for each element [IssueOpened](https://github.com/PowerShell/PSScriptAnalyzer/issues/131)
- Resource module must alwasy contain .psd1 file and schema.mof (for non-class resource).
- Use ShouldProcess for a Set DSC method
- Resource module contains Resources folder which contains the resources [IssueOpened](https://github.com/PowerShell/PSScriptAnalyzer/issues/130)



###Reference:
Cmdlet Development Guidelines from MSDN site (Cmdlet Development Guidelines)

The Community Book of PowerShell Practices (Compiled by Don Jones and Matt Penny and the Windows PowerShell Community)
4 changes: 2 additions & 2 deletions Rules/UseIdenticalMandatoryParametersDSC.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeDSCResource(Ast ast, string fileName
{
List<string> functionsNotContainingParam = expectedTargetResourceFunctionNames.Except(mandatoryParameters[paramName]).ToList();
yield return new DiagnosticRecord(string.Format(CultureInfo.InvariantCulture, Strings.UseIdenticalMandatoryParametersDSCError, paramName, string.Join(", ", functionsNotContainingParam.ToArray())),
ast.Extent, GetName(), DiagnosticSeverity.Information, fileName);
ast.Extent, GetName(), DiagnosticSeverity.Error, fileName);
}

}
Expand Down Expand Up @@ -159,7 +159,7 @@ public SourceType GetSourceType()
/// <returns></returns>
public RuleSeverity GetSeverity()
{
return RuleSeverity.Information;
return RuleSeverity.Error;
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions Rules/UseIdenticalParametersDSC.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeDSCResource(Ast ast, string fileName
|| !CompareParamAsts(paramAst, paramNames[paramAst.Name.VariablePath.UserPath]))
{
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseIdenticalParametersDSCError),
paramAst.Extent, GetName(), DiagnosticSeverity.Information, fileName);
paramAst.Extent, GetName(), DiagnosticSeverity.Error, fileName);
}
}
}
Expand Down Expand Up @@ -166,7 +166,7 @@ public SourceType GetSourceType()
/// <returns></returns>
public RuleSeverity GetSeverity()
{
return RuleSeverity.Warning;
return RuleSeverity.Error;
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions Rules/UseStandardDSCFunctionsInResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeDSCResource(Ast ast, string fileName
if (!targetResourceFunctionNamesInAst.Contains(expectedTargetResourceFunctionName, StringComparer.CurrentCultureIgnoreCase))
{
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseStandardDSCFunctionsInResourceError, expectedTargetResourceFunctionName),
ast.Extent, GetName(), DiagnosticSeverity.Information, fileName);
ast.Extent, GetName(), DiagnosticSeverity.Error, fileName);
}
}
}
Expand Down Expand Up @@ -85,7 +85,7 @@ item is TypeDefinitionAst
if (!functions.Any(function => String.Equals(resourceFunctionName, (function as FunctionMemberAst).Name)))
{
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseStandardDSCFunctionsInClassError, resourceFunctionName),
dscClass.Extent, GetName(), DiagnosticSeverity.Information, fileName);
dscClass.Extent, GetName(), DiagnosticSeverity.Error, fileName);
}
}
}
Expand Down