From 59d5a58af4483b3785143902872a9530b738c6a2 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Fri, 24 Apr 2015 14:48:37 -0700 Subject: [PATCH 01/19] Implement UseOutputTypeCorrectly --- Engine/Helper.cs | 15 ++ Rules/ScriptAnalyzerBuiltinRules.csproj | 1 + Rules/Strings.Designer.cs | 36 ++++ Rules/Strings.resx | 12 ++ Rules/UseOutputTypeCorrectly.cs | 179 +++++++++++++++++++ Tests/Rules/BadCmdlet.ps1 | 11 ++ Tests/Rules/GoodCmdlet.ps1 | 13 +- Tests/Rules/UseOutputTypeCorrectly.tests.ps1 | 29 +++ 8 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 Rules/UseOutputTypeCorrectly.cs create mode 100644 Tests/Rules/UseOutputTypeCorrectly.tests.ps1 diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 0e59ad803..16304ae86 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -2085,6 +2085,11 @@ public object VisitConvertExpression(ConvertExpressionAst convAst) { if (convAst != null) { + if (convAst.Type.TypeName.GetReflectionType() != null) + { + return convAst.Type.TypeName.GetReflectionType().FullName; + } + return convAst.Type.TypeName.FullName; } @@ -2135,6 +2140,11 @@ public object VisitTypeConstraint(TypeConstraintAst typeAst) { if (typeAst != null) { + if (typeAst.TypeName.GetReflectionType() != null) + { + return typeAst.TypeName.GetReflectionType().FullName; + } + return typeAst.TypeName.FullName; } @@ -2160,6 +2170,11 @@ public object VisitTypeExpression(TypeExpressionAst typeExpressionAst) { if (typeExpressionAst != null) { + if (typeExpressionAst.TypeName.GetReflectionType() != null) + { + return typeExpressionAst.TypeName.GetReflectionType().FullName; + } + return typeExpressionAst.TypeName.FullName; } diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index e85c892d0..eef54841b 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -68,6 +68,7 @@ + diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 1ec732308..3f1eb4469 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1545,6 +1545,42 @@ internal static string UseIdenticalParametersDSCName { } } + /// + /// Looks up a localized string similar to Use OutputType Correctly. + /// + internal static string UseOutputTypeCorrectlyCommonName { + get { + return ResourceManager.GetString("UseOutputTypeCorrectlyCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The return types of a cmdlet should be declared using the OutputType attribute.. + /// + internal static string UseOutputTypeCorrectlyDescription { + get { + return ResourceManager.GetString("UseOutputTypeCorrectlyDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The cmdlet '{0}' returns an object of type '{1}' but this type is not declared in the OutputType attribute.. + /// + internal static string UseOutputTypeCorrectlyError { + get { + return ResourceManager.GetString("UseOutputTypeCorrectlyError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to UseOutputTypeCorrectly. + /// + internal static string UseOutputTypeCorrectlyName { + get { + return ResourceManager.GetString("UseOutputTypeCorrectlyName", resourceCulture); + } + } + /// /// Looks up a localized string similar to PSCredential. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index f7501a45c..1d548b83a 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -678,4 +678,16 @@ AvoidUsingWMIObjectCmdlet + + Use OutputType Correctly + + + The return types of a cmdlet should be declared using the OutputType attribute. + + + The cmdlet '{0}' returns an object of type '{1}' but this type is not declared in the OutputType attribute. + + + UseOutputTypeCorrectly + \ No newline at end of file diff --git a/Rules/UseOutputTypeCorrectly.cs b/Rules/UseOutputTypeCorrectly.cs new file mode 100644 index 000000000..5a2d8eb75 --- /dev/null +++ b/Rules/UseOutputTypeCorrectly.cs @@ -0,0 +1,179 @@ +// +// Copyright (c) Microsoft Corporation. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// + +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 +{ + /// + /// ProvideCommentHelp: Analyzes ast to check that cmdlets have help. + /// + [Export(typeof(IScriptRule))] + public class UseOutputTypeCorrectly : SkipTypeDefinition, IScriptRule + { + private IEnumerable _classes; + + /// + /// AnalyzeScript: Analyzes the ast to check that cmdlets have help. + /// + /// The script's ast + /// The name of the script + /// A List of diagnostic results of this rule + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + DiagnosticRecords.Clear(); + this.fileName = fileName; + + _classes = ast.FindAll(item => item is TypeDefinitionAst && ((item as TypeDefinitionAst).IsClass), true).Cast(); + + ast.Visit(this); + + return DiagnosticRecords; + } + + /// + /// Visit function and checks that it is a cmdlet. If yes, then checks that any object returns must have a type declared + /// in the output type (the only exception is if the type is object) + /// + /// + /// + public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) + { + if (funcAst == null || funcAst.Body == null || funcAst.Body.ParamBlock == null + || funcAst.Body.ParamBlock.Attributes == null || funcAst.Body.ParamBlock.Attributes.Count == 0 + || !funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute))) + { + return AstVisitAction.Continue; + } + + HashSet outputTypes = new HashSet(); + + foreach (AttributeAst attrAst in funcAst.Body.ParamBlock.Attributes) + { + if (attrAst.TypeName != null && attrAst.TypeName.GetReflectionType() == typeof(OutputTypeAttribute) + && attrAst.PositionalArguments != null) + { + foreach (ExpressionAst expAst in attrAst.PositionalArguments) + { + if (expAst is StringConstantExpressionAst) + { + Type type = Type.GetType((expAst as StringConstantExpressionAst).Value); + if (type != null) + { + outputTypes.Add(type.FullName); + } + } + else + { + TypeExpressionAst typeAst = expAst as TypeExpressionAst; + if (typeAst != null && typeAst.TypeName != null) + { + if (typeAst.TypeName.GetReflectionType() != null) + { + outputTypes.Add(typeAst.TypeName.GetReflectionType().FullName); + } + else + { + outputTypes.Add(typeAst.TypeName.FullName); + } + } + } + } + } + } + + List> returnTypes = FindPipelineOutput.OutputTypes(funcAst, _classes); + + foreach (Tuple 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) + || outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase)) + { + continue; + } + else + { + DiagnosticRecords.Add(new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyError, + funcAst.Name, typeName), returnType.Item2.Extent, GetName(), DiagnosticSeverity.Warning, fileName)); + } + } + + return AstVisitAction.Continue; + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseOutputTypeCorrectlyName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyDescription); + } + + /// + /// Method: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// + /// + public RuleSeverity GetSeverity() + { + return RuleSeverity.Information; + } + + /// + /// Method: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} diff --git a/Tests/Rules/BadCmdlet.ps1 b/Tests/Rules/BadCmdlet.ps1 index 546dc31a8..2958a0778 100644 --- a/Tests/Rules/BadCmdlet.ps1 +++ b/Tests/Rules/BadCmdlet.ps1 @@ -7,6 +7,8 @@ ConfirmImpact='Medium')] [Alias()] [OutputType([String])] + [OutputType("System.Int32", ParameterSetName="ID")] + Param ( # Param1 help description @@ -46,6 +48,14 @@ } Process { + $a = 4.5 + + if ($true) + { + $a + } + + return @{"hash"="true"} } End { @@ -53,6 +63,7 @@ } # Provide comment help should not be raised here because this is not exported +#Output type rule should also not be raised here as this is not a cmdlet function NoComment { Write-Verbose "No Comment" diff --git a/Tests/Rules/GoodCmdlet.ps1 b/Tests/Rules/GoodCmdlet.ps1 index 7c1dfa240..73ea8399c 100644 --- a/Tests/Rules/GoodCmdlet.ps1 +++ b/Tests/Rules/GoodCmdlet.ps1 @@ -28,7 +28,9 @@ function Get-File HelpUri = 'http://www.microsoft.com/', ConfirmImpact='Medium')] [Alias()] - [OutputType([String])] + [OutputType([String], [System.Double], [Hashtable])] + [OutputType("System.Int32", ParameterSetName="ID")] + Param ( # Param1 help description @@ -75,6 +77,15 @@ function Get-File Write-Verbose "Write Verbose" Get-Process } + + $a = 4.5 + + if ($true) + { + $a + } + + return @{"hash"="true"} } End { diff --git a/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 b/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 new file mode 100644 index 000000000..666d281d0 --- /dev/null +++ b/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 @@ -0,0 +1,29 @@ +Import-Module PSScriptAnalyzer +$violationMessage = "The cmdlet 'Verb-Files' returns an object of type 'System.Double' but this type is not declared in the OutputType attribute." +$violationName = "PSUseOutputTypeCorrectly" +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} +$dscViolations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $directory\DSCResources\MyDscResource\MyDscResource.psm1 | Where-Object {$_.RuleName -eq $violationName} +$noViolations = Invoke-ScriptAnalyzer $directory\GoodCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} + +Describe "UseOutputTypeCorrectly" { + Context "When there are violations" { + It "has 2 Use OutputType Correctly violations" { + $violations.Count | Should Be 2 + } + + It "has the correct description message" { + $violations[0].Message | Should Match $violationMessage + } + + It "Does not count violation in DSC class" { + $dscViolations.Count | Should Be 0 + } + } + + Context "When there are no violations" { + It "returns no violations" { + $noViolations.Count | Should Be 0 + } + } +} \ No newline at end of file From 70a48d0190e066b3105362ce79e8459571078d95 Mon Sep 17 00:00:00 2001 From: quoctruong Date: Fri, 24 Apr 2015 15:21:47 -0700 Subject: [PATCH 02/19] Fix comment and change severity to warning --- Rules/UseOutputTypeCorrectly.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Rules/UseOutputTypeCorrectly.cs b/Rules/UseOutputTypeCorrectly.cs index 5a2d8eb75..489f3637d 100644 --- a/Rules/UseOutputTypeCorrectly.cs +++ b/Rules/UseOutputTypeCorrectly.cs @@ -22,7 +22,7 @@ namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules { /// - /// ProvideCommentHelp: Analyzes ast to check that cmdlets have help. + /// ProvideCommentHelp: Checks that objects return in a cmdlet have their types declared in OutputType Attribute /// [Export(typeof(IScriptRule))] public class UseOutputTypeCorrectly : SkipTypeDefinition, IScriptRule @@ -30,7 +30,7 @@ public class UseOutputTypeCorrectly : SkipTypeDefinition, IScriptRule private IEnumerable _classes; /// - /// AnalyzeScript: Analyzes the ast to check that cmdlets have help. + /// AnalyzeScript: Checks that objects return in a cmdlet have their types declared in OutputType Attribute /// /// The script's ast /// The name of the script @@ -165,7 +165,7 @@ public SourceType GetSourceType() /// public RuleSeverity GetSeverity() { - return RuleSeverity.Information; + return RuleSeverity.Warning; } /// From 44130cec6f0564f3f23b35d8b4b4de558a902443 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Mon, 27 Apr 2015 10:27:33 -0700 Subject: [PATCH 03/19] Add rule documentation for useoutputtypecorrectly --- RuleDocumentation/UseOutputTypeCorrectly.md | 37 +++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 RuleDocumentation/UseOutputTypeCorrectly.md diff --git a/RuleDocumentation/UseOutputTypeCorrectly.md b/RuleDocumentation/UseOutputTypeCorrectly.md new file mode 100644 index 000000000..64ccfebbd --- /dev/null +++ b/RuleDocumentation/UseOutputTypeCorrectly.md @@ -0,0 +1,37 @@ +#UseOutputTypeCorrectly +**Severity Level: Warning** + + +##Description + +If a return type is declared, the cmdlet must return that type. If a type is returned, a return type must be declared. + +##How to Fix + +To fix a violation of this rule, please check the OuputType attribute lists and the types that are returned in your code. You can get more details by running “Get-Help about_Functions_OutputTypeAttribute” command in Windows PowerShell. + +##Example + +##Example +Wrong: + + function Get-Foo + { + [CmdletBinding()] + [OutputType([String])] + Param( + ) + return "4 + } + +Correct: + + function Get-Foo + { + [CmdletBinding()] + [OutputType([String])] + Param( + ) + + return "bar" + } From 4f9972e76169a129e4c9e24273c495ed2985f7aa Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Mon, 27 Apr 2015 10:53:56 -0700 Subject: [PATCH 04/19] Change severity of useoutputtypecorrectly to information --- RuleDocumentation/UseOutputTypeCorrectly.md | 6 +++--- Rules/UseOutputTypeCorrectly.cs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/RuleDocumentation/UseOutputTypeCorrectly.md b/RuleDocumentation/UseOutputTypeCorrectly.md index 64ccfebbd..577f430cb 100644 --- a/RuleDocumentation/UseOutputTypeCorrectly.md +++ b/RuleDocumentation/UseOutputTypeCorrectly.md @@ -1,5 +1,5 @@ #UseOutputTypeCorrectly -**Severity Level: Warning** +**Severity Level: Information** ##Description @@ -17,7 +17,7 @@ Wrong: function Get-Foo { - [CmdletBinding()] + [CmdletBinding()] [OutputType([String])] Param( ) @@ -28,7 +28,7 @@ Correct: function Get-Foo { - [CmdletBinding()] + [CmdletBinding()] [OutputType([String])] Param( ) diff --git a/Rules/UseOutputTypeCorrectly.cs b/Rules/UseOutputTypeCorrectly.cs index 489f3637d..f79ccea15 100644 --- a/Rules/UseOutputTypeCorrectly.cs +++ b/Rules/UseOutputTypeCorrectly.cs @@ -117,7 +117,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun else { DiagnosticRecords.Add(new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyError, - funcAst.Name, typeName), returnType.Item2.Extent, GetName(), DiagnosticSeverity.Warning, fileName)); + funcAst.Name, typeName), returnType.Item2.Extent, GetName(), DiagnosticSeverity.Information, fileName)); } } @@ -165,7 +165,7 @@ public SourceType GetSourceType() /// public RuleSeverity GetSeverity() { - return RuleSeverity.Warning; + return RuleSeverity.Information; } /// From ae0b374faad06b74d053257a0370e9e24b59569b Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 28 Apr 2015 15:05:39 -0700 Subject: [PATCH 05/19] Adding DscExamplesPresent rule scaffolding --- Rules/DscExamplesPresent.cs | 105 ++++++++++++++++++++++++++++++++++++ Rules/Strings.Designer.cs | 27 ++++++++++ Rules/Strings.resx | 9 ++++ 3 files changed, 141 insertions(+) create mode 100644 Rules/DscExamplesPresent.cs diff --git a/Rules/DscExamplesPresent.cs b/Rules/DscExamplesPresent.cs new file mode 100644 index 000000000..573a8bc82 --- /dev/null +++ b/Rules/DscExamplesPresent.cs @@ -0,0 +1,105 @@ +// +// Copyright (c) Microsoft Corporation. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// + +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; + +namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules +{ + /// + /// DscExamplesExist: Checks that DSC examples for given resource are present + /// + [Export(typeof(IDSCResourceRule))] + public class DscExamplesPresent : IDSCResourceRule + { + /// + /// AnalyzeDSCResource: Analyzes given DSC Resource + /// + /// The script's ast + /// The name of the script file being analyzed + /// The results of the analysis + public IEnumerable AnalyzeDSCResource(Ast ast, string fileName) + { + + } + + /// + /// AnalyzeDSCClass: Analyzes given DSC class + /// + /// + /// + /// + public IEnumerable AnalyzeDSCClass(Ast ast, string fileName) + { + + } + + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.DscExamplesPresent); + } + + /// + /// GetCommonName: Retrieves the Common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning or information. + /// + /// + public RuleSeverity GetSeverity() + { + return RuleSeverity.Information; + } + + /// + /// GetSourceName: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.DSCSourceName); + } + } + +} \ No newline at end of file diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 3f1eb4469..d8bb1ac40 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -825,6 +825,33 @@ internal static string CommandNotFoundName { } } + /// + /// Looks up a localized string similar to DscExamplesPresent. + /// + internal static string DscExamplesPresent { + get { + return ResourceManager.GetString("DscExamplesPresent", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to DSC examples are present. + /// + internal static string DscExamplesPresentCommonName { + get { + return ResourceManager.GetString("DscExamplesPresentCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Every DSC resource module should contain folder "Examples" with sample configurations for every resource. Sample configurations should have resource name they are demonstrating in the title.. + /// + internal static string DscExamplesPresentDescription { + get { + return ResourceManager.GetString("DscExamplesPresentDescription", resourceCulture); + } + } + /// /// Looks up a localized string similar to PSDSC. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 1d548b83a..f06b0a2da 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -690,4 +690,13 @@ UseOutputTypeCorrectly + + DscExamplesPresent + + + DSC examples are present + + + Every DSC resource module should contain folder "Examples" with sample configurations for every resource. Sample configurations should have resource name they are demonstrating in the title. + \ No newline at end of file From 6ff3193367f8bca4c1b3b1993dc8670968e37cb2 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 28 Apr 2015 15:07:35 -0700 Subject: [PATCH 06/19] Fix typo --- Rules/ReturnCorrectTypesForDSCFunctions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/ReturnCorrectTypesForDSCFunctions.cs b/Rules/ReturnCorrectTypesForDSCFunctions.cs index 2890660fb..90aeeeabc 100644 --- a/Rules/ReturnCorrectTypesForDSCFunctions.cs +++ b/Rules/ReturnCorrectTypesForDSCFunctions.cs @@ -210,7 +210,7 @@ public SourceType GetSourceType() } /// - /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// GetSeverity: Retrieves the severity of the rule: error, warning or information. /// /// public RuleSeverity GetSeverity() From d94733cc7da41ae1007287e99dde170f5fb10194 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Mon, 4 May 2015 17:16:00 -0700 Subject: [PATCH 07/19] Adding implementation for class based resources --- Rules/DscExamplesPresent.cs | 67 ++++++++++++++++++++++++- Rules/ScriptAnalyzerBuiltinRules.csproj | 1 + Rules/Strings.Designer.cs | 9 ++++ Rules/Strings.resx | 3 ++ 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/Rules/DscExamplesPresent.cs b/Rules/DscExamplesPresent.cs index 573a8bc82..84f6d5268 100644 --- a/Rules/DscExamplesPresent.cs +++ b/Rules/DscExamplesPresent.cs @@ -17,11 +17,17 @@ using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; using System.ComponentModel.Composition; using System.Globalization; +using System.IO; +using System.Management.Automation; namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules { /// - /// DscExamplesExist: Checks that DSC examples for given resource are present + /// DscExamplesPresent: Checks that DSC examples for given resource are present. + /// Rule expects directory Examples to be present: + /// For non-class based resources it should exist at the same folder level as DSCResources folder. + /// For class based resources it should be present at the same folder level as resource psm1 file. + /// Examples folder should contain sample configuration for given resource - file name should contain resource's name. /// [Export(typeof(IDSCResourceRule))] public class DscExamplesPresent : IDSCResourceRule @@ -34,7 +40,29 @@ public class DscExamplesPresent : IDSCResourceRule /// The results of the analysis public IEnumerable AnalyzeDSCResource(Ast ast, string fileName) { + String fileNameOnly = fileName.Substring(fileName.LastIndexOf("\\", StringComparison.Ordinal) + 1); + String resourceName = fileNameOnly.Substring(0, fileNameOnly.Length - ".psm1".Length); + String examplesQuery = "*" + resourceName + "*"; + Boolean examplesPresent = false; + String expectedExamplesPath = fileName + "\\..\\..\\..\\Examples"; + // Verify examples are present + if (Directory.Exists(expectedExamplesPath)) + { + DirectoryInfo examplesFolder = new DirectoryInfo(expectedExamplesPath); + FileInfo[] exampleFiles = examplesFolder.GetFiles(examplesQuery); + if (exampleFiles.Length != 0) + { + examplesPresent = true; + } + } + + // Return error if no examples present + if (!examplesPresent) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentNoExamplesError, resourceName), + null, GetName(), DiagnosticSeverity.Information, fileName); + } } /// @@ -45,9 +73,44 @@ public IEnumerable AnalyzeDSCResource(Ast ast, string fileName /// public IEnumerable AnalyzeDSCClass(Ast ast, string fileName) { + String resourceName = null; - } + // Obtain class based resource name + IEnumerable typeDefinitionAsts = (ast.FindAll(dscAst => dscAst is TypeDefinitionAst, true)); + foreach (TypeDefinitionAst typeDefinitionAst in typeDefinitionAsts) + { + var attributes = typeDefinitionAst.Attributes; + foreach(var attribute in attributes) + { + if (attribute.TypeName.FullName.Equals("DscResource")) + { + resourceName = typeDefinitionAst.Name; + } + } + } + + String examplesQuery = "*" + resourceName + "*"; + Boolean examplesPresent = false; + String expectedExamplesPath = fileName + "\\..\\Examples"; + // Verify examples are present + if (Directory.Exists(expectedExamplesPath)) + { + DirectoryInfo examplesFolder = new DirectoryInfo(expectedExamplesPath); + FileInfo[] exampleFiles = examplesFolder.GetFiles(examplesQuery); + if (exampleFiles.Length != 0) + { + examplesPresent = true; + } + } + + // Return error if no examples present + if (!examplesPresent) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentNoExamplesError, resourceName), + null, GetName(), DiagnosticSeverity.Information, fileName); + } + } /// /// GetName: Retrieves the name of this rule. diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index eef54841b..a11c83100 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -68,6 +68,7 @@ + diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index d8bb1ac40..ef44c1fcd 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -852,6 +852,15 @@ internal static string DscExamplesPresentDescription { } } + /// + /// Looks up a localized string similar to No examples found for resource '{0}'. + /// + internal static string DscExamplesPresentNoExamplesError { + get { + return ResourceManager.GetString("DscExamplesPresentNoExamplesError", resourceCulture); + } + } + /// /// Looks up a localized string similar to PSDSC. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index f06b0a2da..c57821d3e 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -699,4 +699,7 @@ Every DSC resource module should contain folder "Examples" with sample configurations for every resource. Sample configurations should have resource name they are demonstrating in the title. + + No examples found for resource '{0}' + \ No newline at end of file From 823ac98ef3a282a7a55f79cfb7911bf3efcaaa26 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Mon, 4 May 2015 17:42:54 -0700 Subject: [PATCH 08/19] Adding tests for class based resource --- Tests/Rules/DscExamplesPresent.tests.ps1 | 37 ++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Tests/Rules/DscExamplesPresent.tests.ps1 diff --git a/Tests/Rules/DscExamplesPresent.tests.ps1 b/Tests/Rules/DscExamplesPresent.tests.ps1 new file mode 100644 index 000000000..763d0281b --- /dev/null +++ b/Tests/Rules/DscExamplesPresent.tests.ps1 @@ -0,0 +1,37 @@ +Import-Module -Verbose PSScriptAnalyzer + +$currentPath = Split-Path -Parent $MyInvocation.MyCommand.Path +$ruleName = "PSDSCDscExamplesPresent" + +Describe "DscExamplesPresent rule in class based resource" { + + $examplesPath = "$currentPath\DSCResources\MyDscResource\Examples" + $classResourcePath = "$currentPath\DSCResources\MyDscResource\MyDscResource.psm1" + + Context "When examples absent" { + + $violations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $classResourcePath | Where-Object {$_.RuleName -eq $ruleName} + $violationMessage = "No examples found for resource 'FileResource'" + + It "has 1 missing examples violation" { + $violations.Count | Should Be 1 + } + + It "has the correct description message" { + $violations[0].Message | Should Match $violationMessage + } + } + + Context "When examples present" { + New-Item -Path $examplesPath -ItemType Directory + New-Item -Path "$examplesPath\FileResource_Example.psm1" -ItemType File + + $noViolations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $classResourcePath | Where-Object {$_.RuleName -eq $ruleName} + + It "returns no violations" { + $noViolations.Count | Should Be 0 + } + + Remove-Item -Path $examplesPath -Recurse -Force + } +} \ No newline at end of file From da42209ff9d403e2cca0369e1c4740e328a4c2ef Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 5 May 2015 14:18:03 -0700 Subject: [PATCH 09/19] Changes in AnalyzeDSCClass to be called only for class based resource --- Rules/DscExamplesPresent.cs | 50 +++++++++++------------- Tests/Rules/DscExamplesPresent.tests.ps1 | 33 ++++++++++++++++ 2 files changed, 56 insertions(+), 27 deletions(-) diff --git a/Rules/DscExamplesPresent.cs b/Rules/DscExamplesPresent.cs index 84f6d5268..a946d0622 100644 --- a/Rules/DscExamplesPresent.cs +++ b/Rules/DscExamplesPresent.cs @@ -75,41 +75,37 @@ public IEnumerable AnalyzeDSCClass(Ast ast, string fileName) { String resourceName = null; - // Obtain class based resource name - IEnumerable typeDefinitionAsts = (ast.FindAll(dscAst => dscAst is TypeDefinitionAst, true)); - foreach (TypeDefinitionAst typeDefinitionAst in typeDefinitionAsts) + IEnumerable 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); + + foreach (TypeDefinitionAst dscClass in dscClasses) { - var attributes = typeDefinitionAst.Attributes; - foreach(var attribute in attributes) + resourceName = dscClass.Name; + + String examplesQuery = "*" + resourceName + "*"; + Boolean examplesPresent = false; + String expectedExamplesPath = fileName + "\\..\\Examples"; + + // Verify examples are present + if (Directory.Exists(expectedExamplesPath)) { - if (attribute.TypeName.FullName.Equals("DscResource")) + DirectoryInfo examplesFolder = new DirectoryInfo(expectedExamplesPath); + FileInfo[] exampleFiles = examplesFolder.GetFiles(examplesQuery); + if (exampleFiles.Length != 0) { - resourceName = typeDefinitionAst.Name; + examplesPresent = true; } } - } - - String examplesQuery = "*" + resourceName + "*"; - Boolean examplesPresent = false; - String expectedExamplesPath = fileName + "\\..\\Examples"; - // Verify examples are present - if (Directory.Exists(expectedExamplesPath)) - { - DirectoryInfo examplesFolder = new DirectoryInfo(expectedExamplesPath); - FileInfo[] exampleFiles = examplesFolder.GetFiles(examplesQuery); - if (exampleFiles.Length != 0) + // Return error if no examples present + if (!examplesPresent) { - examplesPresent = true; + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentNoExamplesError, resourceName), + null, GetName(), DiagnosticSeverity.Information, fileName); } - } - - // Return error if no examples present - if (!examplesPresent) - { - yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentNoExamplesError, resourceName), - null, GetName(), DiagnosticSeverity.Information, fileName); - } + } } /// diff --git a/Tests/Rules/DscExamplesPresent.tests.ps1 b/Tests/Rules/DscExamplesPresent.tests.ps1 index 763d0281b..e27459a9a 100644 --- a/Tests/Rules/DscExamplesPresent.tests.ps1 +++ b/Tests/Rules/DscExamplesPresent.tests.ps1 @@ -32,6 +32,39 @@ Describe "DscExamplesPresent rule in class based resource" { $noViolations.Count | Should Be 0 } + Remove-Item -Path $examplesPath -Recurse -Force + } +} + +Describe "DscExamplesPresent rule in regular (non-class) based resource" { + + $examplesPath = "$currentPath\Examples" + $resourcePath = "$currentPath\DSCResources\MSFT_WaitForAll\MSFT_WaitForAll.psm1" + + Context "When examples absent" { + + $violations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $resourcePath | Where-Object {$_.RuleName -eq $ruleName} + $violationMessage = "No examples found for resource 'MSFT_WaitForAll'" + + It "has 1 missing examples violation" { + $violations.Count | Should Be 1 + } + + It "has the correct description message" { + $violations[0].Message | Should Match $violationMessage + } + } + + Context "When examples present" { + New-Item -Path $examplesPath -ItemType Directory + New-Item -Path "$examplesPath\MSFT_WaitForAll_Example.psm1" -ItemType File + + $noViolations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $resourcePath | Where-Object {$_.RuleName -eq $ruleName} + + It "returns no violations" { + $noViolations.Count | Should Be 0 + } + Remove-Item -Path $examplesPath -Recurse -Force } } \ No newline at end of file From 110b53c92a4530ef2e8dcbc41bf09af66a6c61ac Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 5 May 2015 15:06:01 -0700 Subject: [PATCH 10/19] Replacing string operations with functions --- Rules/DscExamplesPresent.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Rules/DscExamplesPresent.cs b/Rules/DscExamplesPresent.cs index a946d0622..1a90379f3 100644 --- a/Rules/DscExamplesPresent.cs +++ b/Rules/DscExamplesPresent.cs @@ -40,11 +40,11 @@ public class DscExamplesPresent : IDSCResourceRule /// The results of the analysis public IEnumerable AnalyzeDSCResource(Ast ast, string fileName) { - String fileNameOnly = fileName.Substring(fileName.LastIndexOf("\\", StringComparison.Ordinal) + 1); - String resourceName = fileNameOnly.Substring(0, fileNameOnly.Length - ".psm1".Length); - String examplesQuery = "*" + resourceName + "*"; + String fileNameOnly = Path.GetFileName(fileName); + String resourceName = Path.GetFileNameWithoutExtension(fileNameOnly); + String examplesQuery = String.Format("*{0}*", resourceName); Boolean examplesPresent = false; - String expectedExamplesPath = fileName + "\\..\\..\\..\\Examples"; + String expectedExamplesPath = Path.Combine(new String[] {fileName, "..", "..", "..", "Examples"}); // Verify examples are present if (Directory.Exists(expectedExamplesPath)) @@ -84,9 +84,9 @@ item is TypeDefinitionAst { resourceName = dscClass.Name; - String examplesQuery = "*" + resourceName + "*"; + String examplesQuery = String.Format("*{0}*", resourceName); Boolean examplesPresent = false; - String expectedExamplesPath = fileName + "\\..\\Examples"; + String expectedExamplesPath = Path.Combine(new String[] {fileName, "..", "Examples"}); // Verify examples are present if (Directory.Exists(expectedExamplesPath)) From 96342d95eb91c0bedd7828d410a3e77a6aa6c56d Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 5 May 2015 15:35:23 -0700 Subject: [PATCH 11/19] Populating extent --- Rules/DscExamplesPresent.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Rules/DscExamplesPresent.cs b/Rules/DscExamplesPresent.cs index 1a90379f3..4d3129c4c 100644 --- a/Rules/DscExamplesPresent.cs +++ b/Rules/DscExamplesPresent.cs @@ -61,7 +61,7 @@ public IEnumerable AnalyzeDSCResource(Ast ast, string fileName if (!examplesPresent) { yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentNoExamplesError, resourceName), - null, GetName(), DiagnosticSeverity.Information, fileName); + ast.Extent, GetName(), DiagnosticSeverity.Information, fileName); } } @@ -103,7 +103,7 @@ item is TypeDefinitionAst if (!examplesPresent) { yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentNoExamplesError, resourceName), - null, GetName(), DiagnosticSeverity.Information, fileName); + dscClass.Extent, GetName(), DiagnosticSeverity.Information, fileName); } } } From 0d539eef5238f608787178ffc263816424ef5686 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 5 May 2015 16:00:01 -0700 Subject: [PATCH 12/19] Adding scaffolding for DscTestsPresent rule --- Rules/DscTestsPresent.cs | 110 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 Rules/DscTestsPresent.cs diff --git a/Rules/DscTestsPresent.cs b/Rules/DscTestsPresent.cs new file mode 100644 index 000000000..ed6067e33 --- /dev/null +++ b/Rules/DscTestsPresent.cs @@ -0,0 +1,110 @@ +// +// Copyright (c) Microsoft Corporation. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// + +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.IO; +using System.Management.Automation; + +namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules +{ + /// + /// DscTestsPresent: Checks that DSC tests for given resource are present. + /// Rule expects directory Tests to be present: + /// For non-class based resources it should exist at the same folder level as DSCResources folder. + /// For class based resources it should be present at the same folder level as resource psm1 file. + /// Tests folder should contain test script for given resource - file name should contain resource's name. + /// + [Export(typeof(IDSCResourceRule))] + public class DscExamplesPresent : IDSCResourceRule + { + /// + /// AnalyzeDSCResource: Analyzes given DSC Resource + /// + /// The script's ast + /// The name of the script file being analyzed + /// The results of the analysis + public IEnumerable AnalyzeDSCResource(Ast ast, string fileName) + { + + } + + /// + /// AnalyzeDSCClass: Analyzes given DSC class + /// + /// + /// + /// + public IEnumerable AnalyzeDSCClass(Ast ast, string fileName) + { + + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.DscExamplesPresent); + } + + /// + /// GetCommonName: Retrieves the Common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning or information. + /// + /// + public RuleSeverity GetSeverity() + { + return RuleSeverity.Information; + } + + /// + /// GetSourceName: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.DSCSourceName); + } + } + +} \ No newline at end of file From 5ca074ba050b19deb9f93ddca03697991b091049 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 5 May 2015 16:09:23 -0700 Subject: [PATCH 13/19] Adding implementation for non-class based resources --- Rules/DscTestsPresent.cs | 32 +++++++++++++++++++++++++++----- Rules/Strings.Designer.cs | 36 ++++++++++++++++++++++++++++++++++++ Rules/Strings.resx | 12 ++++++++++++ 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/Rules/DscTestsPresent.cs b/Rules/DscTestsPresent.cs index ed6067e33..3de793ec8 100644 --- a/Rules/DscTestsPresent.cs +++ b/Rules/DscTestsPresent.cs @@ -30,7 +30,7 @@ namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules /// Tests folder should contain test script for given resource - file name should contain resource's name. /// [Export(typeof(IDSCResourceRule))] - public class DscExamplesPresent : IDSCResourceRule + public class DscTestsPresent : IDSCResourceRule { /// /// AnalyzeDSCResource: Analyzes given DSC Resource @@ -40,7 +40,29 @@ public class DscExamplesPresent : IDSCResourceRule /// The results of the analysis public IEnumerable AnalyzeDSCResource(Ast ast, string fileName) { - + String fileNameOnly = Path.GetFileName(fileName); + String resourceName = Path.GetFileNameWithoutExtension(fileNameOnly); + String testsQuery = String.Format("*{0}*", resourceName); + Boolean testsPresent = false; + String expectedTestsPath = Path.Combine(new String[] { fileName, "..", "..", "..", "Tests" }); + + // Verify tests are present + if (Directory.Exists(expectedTestsPath)) + { + DirectoryInfo testsFolder = new DirectoryInfo(expectedTestsPath); + FileInfo[] testFiles = testsFolder.GetFiles(testsQuery); + if (testFiles.Length != 0) + { + testsPresent = true; + } + } + + // Return error if no tests present + if (!testsPresent) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.DscTestsPresentNoTestsError, resourceName), + ast.Extent, GetName(), DiagnosticSeverity.Information, fileName); + } } /// @@ -60,7 +82,7 @@ public IEnumerable AnalyzeDSCClass(Ast ast, string fileName) /// The name of this rule public string GetName() { - return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.DscExamplesPresent); + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.DscTestsPresent); } /// @@ -69,7 +91,7 @@ public string GetName() /// The common name of this rule public string GetCommonName() { - return string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentCommonName); + return string.Format(CultureInfo.CurrentCulture, Strings.DscTestsPresentCommonName); } /// @@ -78,7 +100,7 @@ public string GetCommonName() /// The description of this rule public string GetDescription() { - return string.Format(CultureInfo.CurrentCulture, Strings.DscExamplesPresentDescription); + return string.Format(CultureInfo.CurrentCulture, Strings.DscTestsPresentDescription); } /// diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 3f1eb4469..8064dc86b 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -834,6 +834,42 @@ internal static string DSCSourceName { } } + /// + /// Looks up a localized string similar to DscTestsPresent. + /// + internal static string DscTestsPresent { + get { + return ResourceManager.GetString("DscTestsPresent", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Dsc tests are present. + /// + internal static string DscTestsPresentCommonName { + get { + return ResourceManager.GetString("DscTestsPresentCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Every DSC resource module should contain folder "Tests" with tests for every resource. Test scripts should have resource name they are testing in the file name.. + /// + internal static string DscTestsPresentDescription { + get { + return ResourceManager.GetString("DscTestsPresentDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to No tests found for resource '{0}'. + /// + internal static string DscTestsPresentNoTestsError { + get { + return ResourceManager.GetString("DscTestsPresentNoTestsError", resourceCulture); + } + } + /// /// Looks up a localized string similar to Module Manifest Fields. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 1d548b83a..7692a7949 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -690,4 +690,16 @@ UseOutputTypeCorrectly + + DscTestsPresent + + + Dsc tests are present + + + Every DSC resource module should contain folder "Tests" with tests for every resource. Test scripts should have resource name they are testing in the file name. + + + No tests found for resource '{0}' + \ No newline at end of file From d0996875167a487454d932bc51adc4031ffdf184 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 5 May 2015 16:21:33 -0700 Subject: [PATCH 14/19] Adding rule to project file --- Rules/ScriptAnalyzerBuiltinRules.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index eef54841b..d0e48eb90 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -68,6 +68,7 @@ + From ed2f2916194b42d0a24cf7e8c02e3d4ba44a4f13 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 5 May 2015 16:33:05 -0700 Subject: [PATCH 15/19] Adding implementation for class based resources --- Rules/DscTestsPresent.cs | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/Rules/DscTestsPresent.cs b/Rules/DscTestsPresent.cs index 3de793ec8..15fbbda23 100644 --- a/Rules/DscTestsPresent.cs +++ b/Rules/DscTestsPresent.cs @@ -73,7 +73,39 @@ public IEnumerable AnalyzeDSCResource(Ast ast, string fileName /// public IEnumerable AnalyzeDSCClass(Ast ast, string fileName) { - + String resourceName = null; + + IEnumerable 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); + + foreach (TypeDefinitionAst dscClass in dscClasses) + { + resourceName = dscClass.Name; + + String testsQuery = String.Format("*{0}*", resourceName); + Boolean testsPresent = false; + String expectedTestsPath = Path.Combine(new String[] { fileName, "..", "Tests" }); + + // Verify tests are present + if (Directory.Exists(expectedTestsPath)) + { + DirectoryInfo testsFolder = new DirectoryInfo(expectedTestsPath); + FileInfo[] testFiles = testsFolder.GetFiles(testsQuery); + if (testFiles.Length != 0) + { + testsPresent = true; + } + } + + // Return error if no tests present + if (!testsPresent) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.DscTestsPresentNoTestsError, resourceName), + dscClass.Extent, GetName(), DiagnosticSeverity.Information, fileName); + } + } } /// From bf204bd1b9e0b0f54f5e7c53c730948aa391e126 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Tue, 5 May 2015 16:37:13 -0700 Subject: [PATCH 16/19] Adding tests --- Tests/Rules/DscTestsPresent.tests.ps1 | 70 +++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 Tests/Rules/DscTestsPresent.tests.ps1 diff --git a/Tests/Rules/DscTestsPresent.tests.ps1 b/Tests/Rules/DscTestsPresent.tests.ps1 new file mode 100644 index 000000000..f7a866cf4 --- /dev/null +++ b/Tests/Rules/DscTestsPresent.tests.ps1 @@ -0,0 +1,70 @@ +Import-Module -Verbose PSScriptAnalyzer + +$currentPath = Split-Path -Parent $MyInvocation.MyCommand.Path +$ruleName = "PSDSCDscTestsPresent" + +Describe "DscTestsPresent rule in class based resource" { + + $testsPath = "$currentPath\DSCResources\MyDscResource\Tests" + $classResourcePath = "$currentPath\DSCResources\MyDscResource\MyDscResource.psm1" + + Context "When tests absent" { + + $violations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $classResourcePath | Where-Object {$_.RuleName -eq $ruleName} + $violationMessage = "No tests found for resource 'FileResource'" + + It "has 1 missing test violation" { + $violations.Count | Should Be 1 + } + + It "has the correct description message" { + $violations[0].Message | Should Match $violationMessage + } + } + + Context "When tests present" { + New-Item -Path $testsPath -ItemType Directory + New-Item -Path "$testsPath\FileResource_Test.psm1" -ItemType File + + $noViolations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $classResourcePath | Where-Object {$_.RuleName -eq $ruleName} + + It "returns no violations" { + $noViolations.Count | Should Be 0 + } + + Remove-Item -Path $testsPath -Recurse -Force + } +} + +Describe "DscTestsPresent rule in regular (non-class) based resource" { + + $testsPath = "$currentPath\Tests" + $resourcePath = "$currentPath\DSCResources\MSFT_WaitForAll\MSFT_WaitForAll.psm1" + + Context "When tests absent" { + + $violations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $resourcePath | Where-Object {$_.RuleName -eq $ruleName} + $violationMessage = "No tests found for resource 'MSFT_WaitForAll'" + + It "has 1 missing tests violation" { + $violations.Count | Should Be 1 + } + + It "has the correct description message" { + $violations[0].Message | Should Match $violationMessage + } + } + + Context "When tests present" { + New-Item -Path $testsPath -ItemType Directory + New-Item -Path "$testsPath\MSFT_WaitForAll_Test.psm1" -ItemType File + + $noViolations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $resourcePath | Where-Object {$_.RuleName -eq $ruleName} + + It "returns no violations" { + $noViolations.Count | Should Be 0 + } + + Remove-Item -Path $testsPath -Recurse -Force + } +} \ No newline at end of file From 8fd01d17d2b979055b73ab2380114edf57851f24 Mon Sep 17 00:00:00 2001 From: KarolKaczmarek Date: Tue, 5 May 2015 17:05:30 -0700 Subject: [PATCH 17/19] Create documentation in DscExamplesPresent.md --- RuleDocumentation/DscExamplesPresent.md | 55 +++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 RuleDocumentation/DscExamplesPresent.md diff --git a/RuleDocumentation/DscExamplesPresent.md b/RuleDocumentation/DscExamplesPresent.md new file mode 100644 index 000000000..ab2743642 --- /dev/null +++ b/RuleDocumentation/DscExamplesPresent.md @@ -0,0 +1,55 @@ +#DscExamplesPresent +**Severity Level: Information** + + +##Description + +Checks that DSC examples for given resource are present. + +##How to Fix + +To fix a violation of this rule, please make sure Examples directory is present: +* For non-class based resources it should exist at the same folder level as DSCResources folder. +* For class based resources it should be present at the same folder level as resource psm1 file. + +Examples folder should contain sample configuration for given resource - file name should contain resource's name. + +##Example + +### Non-class based resource + +Let's assume we have non-class based resource with a following file structure: + +* xAzure + * DSCResources + * MSFT_xAzureSubscription + * MSFT_xAzureSubscription.psm1 + * MSFT_xAzureSubscription.schema.mof + +In this case, to fix this warning, we should add examples in a following way: + +* xAzure + * DSCResources + * MSFT_xAzureSubscription + * MSFT_xAzureSubscription.psm1 + * MSFT_xAzureSubscription.schema.mof + * Examples + * MSFT_xAzureSubscription_AddSubscriptionExample.ps1 + * MSFT_xAzureSubscription_RemoveSubscriptionExample.ps1 + +### Class based resource + +Let's assume we have class based resource with a following file structure: + +* MyDscResource + * MyDscResource.psm1 + * MyDscresource.psd1 + +In this case, to fix this warning, we should add examples in a following way: + +* MyDscResource + * MyDscResource.psm1 + * MyDscresource.psd1 + * Tests + * MyDscResource_Example1.ps1 + * MyDscResource_Example2.ps1 From 87301516a8f35e41e694184a18a9f008b6e8fd51 Mon Sep 17 00:00:00 2001 From: KarolKaczmarek Date: Wed, 6 May 2015 11:48:16 -0700 Subject: [PATCH 18/19] Create documentation for DscTestsPresent rule --- RuleDocumentation/DscTestsPresent.md | 54 ++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 RuleDocumentation/DscTestsPresent.md diff --git a/RuleDocumentation/DscTestsPresent.md b/RuleDocumentation/DscTestsPresent.md new file mode 100644 index 000000000..0ed054337 --- /dev/null +++ b/RuleDocumentation/DscTestsPresent.md @@ -0,0 +1,54 @@ +#DscTestsPresent +**Severity Level: Information** + + +##Description + +Checks that DSC tests for given resource are present. + +##How to Fix + +To fix a violation of this rule, please make sure Tests directory is present: +* For non-class based resources it should exist at the same folder level as DSCResources folder. +* For class based resources it should be present at the same folder level as resource psm1 file. + +Tests folder should contain test script for given resource - file name should contain resource's name. + +##Example + +### Non-class based resource + +Let's assume we have non-class based resource with a following file structure: + +* xAzure + * DSCResources + * MSFT_xAzureSubscription + * MSFT_xAzureSubscription.psm1 + * MSFT_xAzureSubscription.schema.mof + +In this case, to fix this warning, we should add tests in a following way: + +* xAzure + * DSCResources + * MSFT_xAzureSubscription + * MSFT_xAzureSubscription.psm1 + * MSFT_xAzureSubscription.schema.mof + * Tests + * MSFT_xAzureSubscription_Tests.ps1 + +### Class based resource + +Let's assume we have class based resource with a following file structure: + +* MyDscResource + * MyDscResource.psm1 + * MyDscresource.psd1 + +In this case, to fix this warning, we should add tests in a following way: + +* MyDscResource + * MyDscResource.psm1 + * MyDscresource.psd1 + * Tests + * MyDscResource_Tests.ps1 + From 8a6e39499382043e13caf41432846d7f765f0f71 Mon Sep 17 00:00:00 2001 From: Karol Kaczmarek Date: Wed, 6 May 2015 14:14:27 -0700 Subject: [PATCH 19/19] Fixing tests --- Tests/Rules/DscTestsPresent.tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Rules/DscTestsPresent.tests.ps1 b/Tests/Rules/DscTestsPresent.tests.ps1 index f7a866cf4..03fbeaa66 100644 --- a/Tests/Rules/DscTestsPresent.tests.ps1 +++ b/Tests/Rules/DscTestsPresent.tests.ps1 @@ -18,7 +18,7 @@ Describe "DscTestsPresent rule in class based resource" { } It "has the correct description message" { - $violations[0].Message | Should Match $violationMessage + $violations[0].Message | Should Be $violationMessage } } @@ -51,7 +51,7 @@ Describe "DscTestsPresent rule in regular (non-class) based resource" { } It "has the correct description message" { - $violations[0].Message | Should Match $violationMessage + $violations[0].Message | Should Be $violationMessage } }