From 17c7310f32bf6d8b80fc0639d843fa829a9d6155 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 18:01:05 -0700 Subject: [PATCH 01/22] Add corrections to get comment help snippet --- Rules/ProvideCommentHelp.cs | 132 ++++++++++++++++++++++++++++++++++-- 1 file changed, 128 insertions(+), 4 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 00411f6f7..3cc79ebf3 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -20,6 +20,7 @@ #endif using System.Globalization; using System.Management.Automation; +using System.Text; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { @@ -27,7 +28,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// ProvideCommentHelp: Analyzes ast to check that cmdlets have help. /// #if !CORECLR -[Export(typeof(IScriptRule))] + [Export(typeof(IScriptRule))] #endif public class ProvideCommentHelp : SkipTypeDefinition, IScriptRule { @@ -39,7 +40,8 @@ public class ProvideCommentHelp : SkipTypeDefinition, IScriptRule /// The script's ast /// The name of the script /// A List of diagnostic results of this rule - public IEnumerable AnalyzeScript(Ast ast, string fileName) { + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); DiagnosticRecords.Clear(); @@ -67,13 +69,18 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun { if (funcAst.GetHelpContent() == null) { + // todo create auto correction + // todo add option to add help for non exported members + // todo add option to set help location DiagnosticRecords.Add( new DiagnosticRecord( string.Format(CultureInfo.CurrentCulture, Strings.ProvideCommentHelpError, funcAst.Name), Helper.Instance.GetScriptExtentForFunctionName(funcAst), GetName(), DiagnosticSeverity.Information, - fileName)); + fileName, + null, + GetCorrection(funcAst).ToList())); } } @@ -102,7 +109,8 @@ public string GetCommonName() /// GetDescription: Retrieves the description of this rule. /// /// The description of this rule - public string GetDescription() { + public string GetDescription() + { return string.Format(CultureInfo.CurrentCulture, Strings.ProvideCommentHelpDescription); } @@ -130,6 +138,122 @@ public string GetSourceName() { return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); } + + private IEnumerable GetCorrection(FunctionDefinitionAst funcDefnAst) + { + var helpBuilder = new CommentHelpBuilder(); + + // todo replace with an extension version + var paramAsts = (funcDefnAst.Parameters ?? funcDefnAst.Body.ParamBlock?.Parameters) + ?? Enumerable.Empty(); + foreach (var paramAst in paramAsts) + { + helpBuilder.AddParameter(paramAst.Name.VariablePath.UserPath); + } + + var correctionExtents = new List(); + yield return new CorrectionExtent( + funcDefnAst.Extent.StartLineNumber, + funcDefnAst.Extent.StartLineNumber, + funcDefnAst.Extent.StartColumnNumber, + funcDefnAst.Extent.StartColumnNumber, + helpBuilder.GetCommentHelp(), + funcDefnAst.Extent.File); + } + + private class CommentHelpBuilder + { + private CommentHelpNode synopsis; + private CommentHelpNode description; + private List parameters; + private CommentHelpNode example; + private CommentHelpNode notes; + + public CommentHelpBuilder() + { + synopsis = new CommentHelpNode("Synopsis", "Short description"); + description = new CommentHelpNode("Description", "Long description"); + example = new CommentHelpNode("Example", "An example"); + parameters = new List(); + notes = new CommentHelpNode("Notes", "General notes"); + } + + public void AddParameter(string paramName) + { + parameters.Add(new ParameterHelpNode(paramName, "Parameter description")); + } + + // todo add option for comment type + public string GetCommentHelp() + { + var sb = new StringBuilder(); + sb.AppendLine("<#"); + sb.AppendLine(this.ToString()); + sb.Append("#>"); + return sb.ToString(); + } + + public override string ToString() + { + var sb = new StringBuilder(); + sb.AppendLine(synopsis.ToString()).AppendLine(); + sb.AppendLine(description.ToString()).AppendLine(); + foreach (var parameter in parameters) + { + sb.AppendLine(parameter.ToString()).AppendLine(); + } + + sb.AppendLine(example.ToString()).AppendLine(); + sb.Append(notes.ToString()); + return sb.ToString(); + } + private class CommentHelpNode + { + public CommentHelpNode(string nodeName, string description) + { + Name = nodeName; + Description = description; + } + + public string Name { get; } + public string Description { get; set; } + + public override string ToString() + { + var sb = new StringBuilder(); + sb.Append(".").AppendLine(Name.ToUpper()); + if (!String.IsNullOrWhiteSpace(Description)) + { + sb.Append(Description); + } + + return sb.ToString(); + } + } + + private class ParameterHelpNode : CommentHelpNode + { + public ParameterHelpNode(string parameterName, string parameterDescription) + : base("Parameter", parameterDescription) + { + ParameterName = parameterName; + } + + public string ParameterName { get; } + + public override string ToString() + { + var sb = new StringBuilder(); + sb.Append(".").Append(Name.ToUpper()).Append(" ").AppendLine(ParameterName); + if (!String.IsNullOrWhiteSpace(Description)) + { + sb.Append(Description); + } + + return sb.ToString(); + } + } + } } } From 4731dd1b18c247877da9630aca07c308dc09b5c5 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 18:09:43 -0700 Subject: [PATCH 02/22] Add tests to check help snippet correction --- Tests/Rules/ProvideCommentHelp.tests.ps1 | 107 +++++++++++++++++++++-- 1 file changed, 101 insertions(+), 6 deletions(-) diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index 693764cbb..4967bc4e0 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -1,17 +1,27 @@ - +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$testRootDirectory = Split-Path -Parent $directory + Import-Module PSScriptAnalyzer +Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") + $violationMessage = "The cmdlet 'Comment' does not have a help comment." $violationName = "PSProvideCommentHelp" -$directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} -if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') -{ +if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') { $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} +function Test-Correction { + param($scriptDef, $expectedCorrection) + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule $violationName + $violations.Count | Should Be 1 + $violations[0].SuggestedCorrections[0].Text | Should Be $expectedCorrection +} + Describe "ProvideCommentHelp" { Context "When there are violations" { It "has 2 provide comment help violations" { @@ -26,8 +36,93 @@ Describe "ProvideCommentHelp" { $violations[1].Extent.Text | Should Be "Comment" } - if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') - { + It "should return a help snippet correction with 0 parameters" { + $def = @' +function foo { +} + +Export-ModuleMember -Function foo +'@ + $expectedCorrection = @' +<# +.SYNOPSIS +Short description + +.DESCRIPTION +Long description + +.EXAMPLE +An example + +.NOTES +General notes +#> +'@ + Test-Correction $def $expectedCorrection + } + + It "should return a help snippet correction with 1 parameters" { + $def = @' +function foo { + param($param1) +} + +Export-ModuleMember -Function foo +'@ + $expectedCorrection = @' +<# +.SYNOPSIS +Short description + +.DESCRIPTION +Long description + +.PARAMETER param1 +Parameter description + +.EXAMPLE +An example + +.NOTES +General notes +#> +'@ + Test-Correction $def $expectedCorrection + } + + It "should return a help snippet correction with 2 parameters" { + $def = @' +function foo { + param($param1, $param2) +} + +Export-ModuleMember -Function foo +'@ + $expectedCorrection = @' +<# +.SYNOPSIS +Short description + +.DESCRIPTION +Long description + +.PARAMETER param1 +Parameter description + +.PARAMETER param2 +Parameter description + +.EXAMPLE +An example + +.NOTES +General notes +#> +'@ + Test-Correction $def $expectedCorrection + } + + if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') { It "Does not count violation in DSC class" { $dscViolations.Count | Should Be 0 } From 3eb753aa94ffb5479c61e687809cb2d7e47ea203 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 18:21:54 -0700 Subject: [PATCH 03/22] Add newline after help snippet --- Rules/ProvideCommentHelp.cs | 2 +- Tests/Rules/ProvideCommentHelp.tests.ps1 | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 3cc79ebf3..c008f8328 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -157,7 +157,7 @@ private IEnumerable GetCorrection(FunctionDefinitionAst funcDe funcDefnAst.Extent.StartLineNumber, funcDefnAst.Extent.StartColumnNumber, funcDefnAst.Extent.StartColumnNumber, - helpBuilder.GetCommentHelp(), + helpBuilder.GetCommentHelp() + Environment.NewLine, funcDefnAst.Extent.File); } diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index 4967bc4e0..e25969957 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -57,6 +57,7 @@ An example .NOTES General notes #> + '@ Test-Correction $def $expectedCorrection } @@ -86,6 +87,7 @@ An example .NOTES General notes #> + '@ Test-Correction $def $expectedCorrection } @@ -118,6 +120,7 @@ An example .NOTES General notes #> + '@ Test-Correction $def $expectedCorrection } From 72a6e7855a8d768c20118b7b8d0992156a53271f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 19:05:26 -0700 Subject: [PATCH 04/22] Add class to find functions without comment help --- Rules/ProvideCommentHelp.cs | 40 +++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index c008f8328..755dc6a82 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -161,6 +161,46 @@ private IEnumerable GetCorrection(FunctionDefinitionAst funcDe funcDefnAst.Extent.File); } + private class ViolationFinder : AstVisitor + { + private HashSet exportedFunctions; + private List functionDefinitionAsts; + + public ViolationFinder() + { + exportedFunctions = new HashSet(); + functionDefinitionAsts = new List(); + } + + public ViolationFinder(HashSet exportedFunctions) : this() + { + if (exportedFunctions == null) + { + throw new ArgumentNullException(nameof(exportedFunctions)); + } + + this.exportedFunctions = exportedFunctions; + } + + public IEnumerable FunctionDefinitionAsts { get { return functionDefinitionAsts; } } + + /// + /// Visit function and checks that it has comment help + /// + /// + /// + public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) + { + if (exportedFunctions.Contains(funcAst.Name) + && funcAst.GetHelpContent() == null) + { + functionDefinitionAsts.Add(funcAst); + } + + return AstVisitAction.Continue; + } + } + private class CommentHelpBuilder { private CommentHelpNode synopsis; From 9351878443742b42fe36afb7a374b07823825a46 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 19:06:21 -0700 Subject: [PATCH 05/22] Use ViolationFinder class to find comment help violations --- Rules/ProvideCommentHelp.cs | 60 ++++++++++++------------------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 755dc6a82..6b8e45bf0 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -30,9 +30,8 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #if !CORECLR [Export(typeof(IScriptRule))] #endif - public class ProvideCommentHelp : SkipTypeDefinition, IScriptRule + public class ProvideCommentHelp : IScriptRule { - private HashSet exportedFunctions; /// /// AnalyzeScript: Analyzes the ast to check that cmdlets have help. @@ -44,47 +43,20 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - DiagnosticRecords.Clear(); - this.fileName = fileName; - exportedFunctions = Helper.Instance.GetExportedFunction(ast); - - ast.Visit(this); - - return DiagnosticRecords; - } - - /// - /// Visit function and checks that it has comment help - /// - /// - /// - public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) - { - if (funcAst == null) - { - return AstVisitAction.SkipChildren; - } - - if (exportedFunctions.Contains(funcAst.Name)) + var exportedFunctions = Helper.Instance.GetExportedFunction(ast); + var violationFinder = new ViolationFinder(exportedFunctions); + ast.Visit(violationFinder); + foreach (var functionDefinitionAst in violationFinder.FunctionDefinitionAsts) { - if (funcAst.GetHelpContent() == null) - { - // todo create auto correction - // todo add option to add help for non exported members - // todo add option to set help location - DiagnosticRecords.Add( - new DiagnosticRecord( - string.Format(CultureInfo.CurrentCulture, Strings.ProvideCommentHelpError, funcAst.Name), - Helper.Instance.GetScriptExtentForFunctionName(funcAst), - GetName(), - DiagnosticSeverity.Information, - fileName, - null, - GetCorrection(funcAst).ToList())); - } + yield return new DiagnosticRecord( + String.Format(CultureInfo.CurrentCulture, Strings.ProvideCommentHelpError, functionDefinitionAst.Name), + Helper.Instance.GetScriptExtentForFunctionName(functionDefinitionAst), + GetName(), + GetDiagnosticSeverity(), + fileName, + null, + GetCorrection(functionDefinitionAst).ToList()); } - - return AstVisitAction.Continue; } /// @@ -139,6 +111,11 @@ public string GetSourceName() return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); } + private DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Information; + } + private IEnumerable GetCorrection(FunctionDefinitionAst funcDefnAst) { var helpBuilder = new CommentHelpBuilder(); @@ -247,6 +224,7 @@ public override string ToString() sb.Append(notes.ToString()); return sb.ToString(); } + private class CommentHelpNode { public CommentHelpNode(string nodeName, string description) From 05531bf1512e3ed617705434c5df6912bfc528d8 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 19:59:52 -0700 Subject: [PATCH 06/22] Derive ProvideCommentHelp class from ConfigurableRule --- Rules/ProvideCommentHelp.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 6b8e45bf0..3dd2f5f08 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -30,7 +30,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #if !CORECLR [Export(typeof(IScriptRule))] #endif - public class ProvideCommentHelp : IScriptRule + public class ProvideCommentHelp : ConfigurableRule { /// @@ -39,7 +39,7 @@ public class ProvideCommentHelp : IScriptRule /// The script's ast /// The name of the script /// A List of diagnostic results of this rule - public IEnumerable AnalyzeScript(Ast ast, string fileName) + public override IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); @@ -63,7 +63,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) /// GetName: Retrieves the name of this rule. /// /// The name of this rule - public string GetName() + public override string GetName() { return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.ProvideCommentHelpName); } @@ -72,7 +72,7 @@ public string GetName() /// GetCommonName: Retrieves the common name of this rule. /// /// The common name of this rule - public string GetCommonName() + public override string GetCommonName() { return string.Format(CultureInfo.CurrentCulture, Strings.ProvideCommentHelpCommonName); } @@ -81,7 +81,7 @@ public string GetCommonName() /// GetDescription: Retrieves the description of this rule. /// /// The description of this rule - public string GetDescription() + public override string GetDescription() { return string.Format(CultureInfo.CurrentCulture, Strings.ProvideCommentHelpDescription); } @@ -89,7 +89,7 @@ public string GetDescription() /// /// Method: Retrieves the type of the rule: builtin, managed or module. /// - public SourceType GetSourceType() + public override SourceType GetSourceType() { return SourceType.Builtin; } @@ -98,7 +98,7 @@ public SourceType GetSourceType() /// GetSeverity: Retrieves the severity of the rule: error, warning of information. /// /// - public RuleSeverity GetSeverity() + public override RuleSeverity GetSeverity() { return RuleSeverity.Information; } @@ -106,7 +106,7 @@ public RuleSeverity GetSeverity() /// /// Method: Retrieves the module/assembly name the rule is from. /// - public string GetSourceName() + public override string GetSourceName() { return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); } From da033a4b9a22f427375d423912ee658b184621d6 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 20:00:35 -0700 Subject: [PATCH 07/22] Enable ProvideCommentHelp by default --- Rules/ProvideCommentHelp.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 3dd2f5f08..4dd63d5b9 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -33,6 +33,12 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules public class ProvideCommentHelp : ConfigurableRule { + public ProvideCommentHelp() : base() + { + // Enable the rule by default + Enable = true; + } + /// /// AnalyzeScript: Analyzes the ast to check that cmdlets have help. /// From 1de9cfef05c9c5ac3559153c725e98ba03504ef5 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 20:11:50 -0700 Subject: [PATCH 08/22] Add a switch to analyze exported function only If the switch `ExportedOnly` is on, the rule finds help comment violations for exported functions only. If the switch is off, then the rule finds violations for all functions. --- Rules/ProvideCommentHelp.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 4dd63d5b9..5523b1a8b 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -33,6 +33,9 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules public class ProvideCommentHelp : ConfigurableRule { + [ConfigurableRuleProperty(defaultValue: true)] + public bool ExportedOnly { get; protected set; } + public ProvideCommentHelp() : base() { // Enable the rule by default @@ -50,7 +53,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); var exportedFunctions = Helper.Instance.GetExportedFunction(ast); - var violationFinder = new ViolationFinder(exportedFunctions); + var violationFinder = new ViolationFinder(exportedFunctions, ExportedOnly); ast.Visit(violationFinder); foreach (var functionDefinitionAst in violationFinder.FunctionDefinitionAsts) { @@ -146,12 +149,13 @@ private IEnumerable GetCorrection(FunctionDefinitionAst funcDe private class ViolationFinder : AstVisitor { - private HashSet exportedFunctions; + private HashSet functionWhitelist; private List functionDefinitionAsts; + private bool useFunctionWhitelist; public ViolationFinder() { - exportedFunctions = new HashSet(); + functionWhitelist = new HashSet(); functionDefinitionAsts = new List(); } @@ -162,7 +166,12 @@ public ViolationFinder(HashSet exportedFunctions) : this() throw new ArgumentNullException(nameof(exportedFunctions)); } - this.exportedFunctions = exportedFunctions; + this.functionWhitelist = exportedFunctions; + } + + public ViolationFinder(HashSet exportedFunctions, bool exportedOnly) : this(exportedFunctions) + { + this.useFunctionWhitelist = exportedOnly; } public IEnumerable FunctionDefinitionAsts { get { return functionDefinitionAsts; } } @@ -174,7 +183,7 @@ public ViolationFinder(HashSet exportedFunctions) : this() /// public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) { - if (exportedFunctions.Contains(funcAst.Name) + if ((!useFunctionWhitelist || functionWhitelist.Contains(funcAst.Name)) && funcAst.GetHelpContent() == null) { functionDefinitionAsts.Add(funcAst); From d2d9cca859372bfa883df980b45c4c4e771db9f2 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 11 May 2017 21:29:27 -0700 Subject: [PATCH 09/22] Add tests for comment help for non-exported functions --- Tests/Rules/ProvideCommentHelp.tests.ps1 | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index e25969957..215819512 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -36,6 +36,22 @@ Describe "ProvideCommentHelp" { $violations[1].Extent.Text | Should Be "Comment" } + It "should find violations in functions that are not exported" { + $def = @' +function foo { +} +'@ + $settings = @{ + IncludeRules = @("PSProvideCommentHelp") + Rules = @{ + PSProvideCommentHelp = @{ + Enable = $true + ExportedOnly = $false + } + } + } + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Get-Count | Should Be 1 + } It "should return a help snippet correction with 0 parameters" { $def = @' function foo { From 82403d25ecd5ab01e1076ba80c853278f488b5f6 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Sat, 13 May 2017 20:09:07 -0700 Subject: [PATCH 10/22] Add options to select comment style --- Rules/ProvideCommentHelp.cs | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 5523b1a8b..98d61bbac 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -33,9 +33,14 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules public class ProvideCommentHelp : ConfigurableRule { + // todo add option for comment type + // todo add option for help placement (beforeFuntion, BodyStart, BodyEnd:) [ConfigurableRuleProperty(defaultValue: true)] public bool ExportedOnly { get; protected set; } + [ConfigurableRuleProperty(defaultValue: true)] + public bool BlockComment { get; protected set; } + public ProvideCommentHelp() : base() { // Enable the rule by default @@ -143,7 +148,7 @@ private IEnumerable GetCorrection(FunctionDefinitionAst funcDe funcDefnAst.Extent.StartLineNumber, funcDefnAst.Extent.StartColumnNumber, funcDefnAst.Extent.StartColumnNumber, - helpBuilder.GetCommentHelp() + Environment.NewLine, + helpBuilder.GetCommentHelp(BlockComment) + Environment.NewLine, funcDefnAst.Extent.File); } @@ -215,13 +220,28 @@ public void AddParameter(string paramName) parameters.Add(new ParameterHelpNode(paramName, "Parameter description")); } - // todo add option for comment type - public string GetCommentHelp() + public string GetCommentHelp(bool blockComment) { var sb = new StringBuilder(); - sb.AppendLine("<#"); - sb.AppendLine(this.ToString()); - sb.Append("#>"); + if (blockComment) + { + sb.AppendLine("<#"); + sb.AppendLine(this.ToString()); + sb.Append("#>"); + } + else + { + var boundaryString = new String('#', 30); + sb.AppendLine(boundaryString); + var lines = this.ToString().Split('\n').Select(l => l.Trim('\r')); + foreach (var line in lines) + { + sb.Append("#"); + sb.AppendLine(line); + } + sb.AppendLine(boundaryString); + } + return sb.ToString(); } From 8621e1a369189231ee1f44b75975ca6b727eeb16 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Sun, 14 May 2017 10:24:39 -0700 Subject: [PATCH 11/22] Remove last newline from correction --- Rules/ProvideCommentHelp.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 98d61bbac..0b7953cf3 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -239,7 +239,8 @@ public string GetCommentHelp(bool blockComment) sb.Append("#"); sb.AppendLine(line); } - sb.AppendLine(boundaryString); + + sb.Append(boundaryString); } return sb.ToString(); From c7269d543a7b022ec06109d75a9e0d647bc7a932 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Sun, 14 May 2017 10:25:45 -0700 Subject: [PATCH 12/22] Add tests for checking line comment style --- Tests/Rules/ProvideCommentHelp.tests.ps1 | 65 +++++++++++++++++++----- 1 file changed, 51 insertions(+), 14 deletions(-) diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index 215819512..93343e665 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -6,6 +6,17 @@ Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") $violationMessage = "The cmdlet 'Comment' does not have a help comment." $violationName = "PSProvideCommentHelp" +$ruleSettings = @{ + Enable = $true + ExportedOnly = $false + BlockComment = $true +} + +$settings = @{ + IncludeRules = @("PSProvideCommentHelp") + Rules = @{ PSProvideCommentHelp = $ruleSettings } +} + $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') { @@ -15,9 +26,9 @@ if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') { $noViolations = Invoke-ScriptAnalyzer $directory\GoodCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} function Test-Correction { - param($scriptDef, $expectedCorrection) + param($scriptDef, $expectedCorrection, $settings) - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule $violationName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -Settings $settings $violations.Count | Should Be 1 $violations[0].SuggestedCorrections[0].Text | Should Be $expectedCorrection } @@ -41,15 +52,7 @@ Describe "ProvideCommentHelp" { function foo { } '@ - $settings = @{ - IncludeRules = @("PSProvideCommentHelp") - Rules = @{ - PSProvideCommentHelp = @{ - Enable = $true - ExportedOnly = $false - } - } - } + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Get-Count | Should Be 1 } It "should return a help snippet correction with 0 parameters" { @@ -75,7 +78,7 @@ General notes #> '@ - Test-Correction $def $expectedCorrection + Test-Correction $def $expectedCorrection $settings } It "should return a help snippet correction with 1 parameters" { @@ -105,7 +108,7 @@ General notes #> '@ - Test-Correction $def $expectedCorrection + Test-Correction $def $expectedCorrection $settings } It "should return a help snippet correction with 2 parameters" { @@ -138,7 +141,41 @@ General notes #> '@ - Test-Correction $def $expectedCorrection + Test-Correction $def $expectedCorrection $settings + } + + It "should return a help snippet correction with line comment style" { + $def = @' +function foo { + param($param1, $param2) +} + +Export-ModuleMember -Function foo +'@ + $expectedCorrection = @' +############################## +#.SYNOPSIS +#Short description +# +#.DESCRIPTION +#Long description +# +#.PARAMETER param1 +#Parameter description +# +#.PARAMETER param2 +#Parameter description +# +#.EXAMPLE +#An example +# +#.NOTES +#General notes +############################## + +'@ + $ruleSettings.'BlockComment' = $false + Test-Correction $def $expectedCorrection $settings } if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') { From c7e1ba1b368ad2e300dad4f7b4de1db4a7c42b70 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 15 May 2017 18:21:43 -0700 Subject: [PATCH 13/22] Add option to return correction as vscode snippet --- Rules/ProvideCommentHelp.cs | 51 ++++++++++++++++++++++-- Tests/Rules/ProvideCommentHelp.tests.ps1 | 38 +++++++++++++++++- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 0b7953cf3..4fd8c77b1 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -41,6 +41,9 @@ public class ProvideCommentHelp : ConfigurableRule [ConfigurableRuleProperty(defaultValue: true)] public bool BlockComment { get; protected set; } + [ConfigurableRuleProperty(defaultValue: false)] + public bool VSCodeSnippetCorrection { get; protected set; } + public ProvideCommentHelp() : base() { // Enable the rule by default @@ -148,7 +151,7 @@ private IEnumerable GetCorrection(FunctionDefinitionAst funcDe funcDefnAst.Extent.StartLineNumber, funcDefnAst.Extent.StartColumnNumber, funcDefnAst.Extent.StartColumnNumber, - helpBuilder.GetCommentHelp(BlockComment) + Environment.NewLine, + helpBuilder.GetCommentHelp(BlockComment, VSCodeSnippetCorrection) + Environment.NewLine, funcDefnAst.Extent.File); } @@ -220,20 +223,21 @@ public void AddParameter(string paramName) parameters.Add(new ParameterHelpNode(paramName, "Parameter description")); } - public string GetCommentHelp(bool blockComment) + public string GetCommentHelp(bool blockComment, bool snippet) { var sb = new StringBuilder(); + var helpContent = snippet ? this.ToSnippetString() : this.ToString(); if (blockComment) { sb.AppendLine("<#"); - sb.AppendLine(this.ToString()); + sb.AppendLine(helpContent); sb.Append("#>"); } else { var boundaryString = new String('#', 30); sb.AppendLine(boundaryString); - var lines = this.ToString().Split('\n').Select(l => l.Trim('\r')); + var lines = helpContent.Split('\n').Select(l => l.Trim('\r')); foreach (var line in lines) { sb.Append("#"); @@ -261,6 +265,21 @@ public override string ToString() return sb.ToString(); } + // todo remove code duplication + public string ToSnippetString() { + var sb = new StringBuilder(); + int tabStop = 1; + sb.AppendLine(synopsis.ToString(tabStop++)).AppendLine(); + sb.AppendLine(description.ToString(tabStop++)).AppendLine(); + foreach (var parameter in parameters) + { + sb.AppendLine(parameter.ToString(tabStop++)).AppendLine(); + } + + sb.AppendLine(example.ToString(tabStop++)).AppendLine(); + sb.Append(notes.ToString(tabStop++)); + return sb.ToString(); + } private class CommentHelpNode { public CommentHelpNode(string nodeName, string description) @@ -283,6 +302,18 @@ public override string ToString() return sb.ToString(); } + + public virtual string ToString(int tabStop) + { + var sb = new StringBuilder(); + sb.Append(".").AppendLine(Name.ToUpper()); + if (!String.IsNullOrWhiteSpace(Description)) + { + sb.Append($"${{{tabStop}:{Description}}}"); + } + + return sb.ToString(); + } } private class ParameterHelpNode : CommentHelpNode @@ -306,6 +337,18 @@ public override string ToString() return sb.ToString(); } + + public override string ToString(int tabStop) + { + var sb = new StringBuilder(); + sb.Append(".").Append(Name.ToUpper()).Append(" ").AppendLine(ParameterName); + if (!String.IsNullOrWhiteSpace(Description)) + { + sb.Append($"${{{tabStop}:{Description}}}"); + } + + return sb.ToString(); + } } } } diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index 93343e665..7d3a0f327 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -10,6 +10,7 @@ $ruleSettings = @{ Enable = $true ExportedOnly = $false BlockComment = $true + VSCodeSnippetCorrection = $false } $settings = @{ @@ -178,6 +179,41 @@ Export-ModuleMember -Function foo Test-Correction $def $expectedCorrection $settings } + It "should return a vscode snippet with line comment style" { + $def = @' +function foo { + param($param1, $param2) +} + +Export-ModuleMember -Function foo +'@ + $expectedCorrection = @' +############################## +#.SYNOPSIS +#${1:Short description} +# +#.DESCRIPTION +#${2:Long description} +# +#.PARAMETER param1 +#${3:Parameter description} +# +#.PARAMETER param2 +#${4:Parameter description} +# +#.EXAMPLE +#${5:An example} +# +#.NOTES +#${6:General notes} +############################## + +'@ + $ruleSettings.'BlockComment' = $false + $ruleSettings.'VSCodeSnippetCorrection' = $true + Test-Correction $def $expectedCorrection $settings + } + if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') { It "Does not count violation in DSC class" { $dscViolations.Count | Should Be 0 @@ -190,4 +226,4 @@ Export-ModuleMember -Function foo $noViolations.Count | Should Be 0 } } -} \ No newline at end of file +} From 8c1437cf596d566a684208ae73076c39dcf0d1a7 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 15 May 2017 18:51:17 -0700 Subject: [PATCH 14/22] Remove redundant for generating correction string --- Rules/ProvideCommentHelp.cs | 72 ++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 4fd8c77b1..20719ad86 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -251,35 +251,44 @@ public string GetCommentHelp(bool blockComment, bool snippet) } public override string ToString() + { + return ToString(false); + } + + // todo remove code duplication + public string ToSnippetString() { + return ToString(true); + } + + private string ToString(bool snippetify) { var sb = new StringBuilder(); - sb.AppendLine(synopsis.ToString()).AppendLine(); - sb.AppendLine(description.ToString()).AppendLine(); + var counter = new Counter(snippetify ? (int?)1 : null); + sb.AppendLine(synopsis.ToString(counter.Next())).AppendLine(); + sb.AppendLine(description.ToString(counter.Next())).AppendLine(); foreach (var parameter in parameters) { - sb.AppendLine(parameter.ToString()).AppendLine(); + sb.AppendLine(parameter.ToString(counter.Next())).AppendLine(); } - sb.AppendLine(example.ToString()).AppendLine(); - sb.Append(notes.ToString()); + sb.AppendLine(example.ToString(counter.Next())).AppendLine(); + sb.Append(notes.ToString(counter.Next())); return sb.ToString(); } - // todo remove code duplication - public string ToSnippetString() { - var sb = new StringBuilder(); - int tabStop = 1; - sb.AppendLine(synopsis.ToString(tabStop++)).AppendLine(); - sb.AppendLine(description.ToString(tabStop++)).AppendLine(); - foreach (var parameter in parameters) + private class Counter { + int? count; + + public Counter(int? start) { - sb.AppendLine(parameter.ToString(tabStop++)).AppendLine(); + count = start; } - sb.AppendLine(example.ToString(tabStop++)).AppendLine(); - sb.Append(notes.ToString(tabStop++)); - return sb.ToString(); + public int? Next() { + return count.HasValue ? count++ : null; + } } + private class CommentHelpNode { public CommentHelpNode(string nodeName, string description) @@ -293,27 +302,25 @@ public CommentHelpNode(string nodeName, string description) public override string ToString() { - var sb = new StringBuilder(); - sb.Append(".").AppendLine(Name.ToUpper()); - if (!String.IsNullOrWhiteSpace(Description)) - { - sb.Append(Description); - } - - return sb.ToString(); + return ToString(null); } - public virtual string ToString(int tabStop) + public virtual string ToString(int? tabStop) { var sb = new StringBuilder(); sb.Append(".").AppendLine(Name.ToUpper()); if (!String.IsNullOrWhiteSpace(Description)) { - sb.Append($"${{{tabStop}:{Description}}}"); + sb.Append(Snippetify(tabStop, Description)); } return sb.ToString(); } + + protected string Snippetify(int? tabStop, string defaultValue) + { + return tabStop == null ? defaultValue : $"${{{tabStop}:{defaultValue}}}"; + } } private class ParameterHelpNode : CommentHelpNode @@ -328,23 +335,16 @@ public ParameterHelpNode(string parameterName, string parameterDescription) public override string ToString() { - var sb = new StringBuilder(); - sb.Append(".").Append(Name.ToUpper()).Append(" ").AppendLine(ParameterName); - if (!String.IsNullOrWhiteSpace(Description)) - { - sb.Append(Description); - } - - return sb.ToString(); + return ToString(null); } - public override string ToString(int tabStop) + public override string ToString(int? tabStop) { var sb = new StringBuilder(); sb.Append(".").Append(Name.ToUpper()).Append(" ").AppendLine(ParameterName); if (!String.IsNullOrWhiteSpace(Description)) { - sb.Append($"${{{tabStop}:{Description}}}"); + sb.Append(Snippetify(tabStop, Description)); } return sb.ToString(); From ba172a626f188a2e5421852467b0592648e82baf Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 15 May 2017 19:22:39 -0700 Subject: [PATCH 15/22] Reuse CommentHelpNode string representation --- Rules/ProvideCommentHelp.cs | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 20719ad86..50d44f216 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -21,6 +21,7 @@ using System.Globalization; using System.Management.Automation; using System.Text; +using System.IO; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { @@ -182,7 +183,13 @@ public ViolationFinder(HashSet exportedFunctions, bool exportedOnly) : t this.useFunctionWhitelist = exportedOnly; } - public IEnumerable FunctionDefinitionAsts { get { return functionDefinitionAsts; } } + public IEnumerable FunctionDefinitionAsts + { + get + { + return functionDefinitionAsts; + } + } /// /// Visit function and checks that it has comment help @@ -256,7 +263,8 @@ public override string ToString() } // todo remove code duplication - public string ToSnippetString() { + public string ToSnippetString() + { return ToString(true); } @@ -276,7 +284,8 @@ private string ToString(bool snippetify) return sb.ToString(); } - private class Counter { + private class Counter + { int? count; public Counter(int? start) @@ -284,7 +293,8 @@ public Counter(int? start) count = start; } - public int? Next() { + public int? Next() + { return count.HasValue ? count++ : null; } } @@ -340,20 +350,12 @@ public override string ToString() public override string ToString(int? tabStop) { - var sb = new StringBuilder(); - sb.Append(".").Append(Name.ToUpper()).Append(" ").AppendLine(ParameterName); - if (!String.IsNullOrWhiteSpace(Description)) - { - sb.Append(Snippetify(tabStop, Description)); - } - - return sb.ToString(); + // todo replace with String.GetLines() extension once it is available + var lines = base.ToString(tabStop).Split('\n').Select(l => l.Trim('\r')).ToArray(); + lines[0] = $"{lines[0]} {ParameterName}"; + return String.Join(Environment.NewLine, lines); } } } } } - - - - From 6631983491bc868e24b066a268c0af48dd71ffdc Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 16 May 2017 18:31:52 -0700 Subject: [PATCH 16/22] Parse string literal as argument in settings file --- Engine/Settings.cs | 5 +++-- Tests/Engine/Settings.tests.ps1 | 8 ++++++-- .../SettingsTest/Project1/ExplicitSettings.psd1 | 11 ++++++++--- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/Engine/Settings.cs b/Engine/Settings.cs index 1cd35f448..539a97fda 100644 --- a/Engine/Settings.cs +++ b/Engine/Settings.cs @@ -416,14 +416,15 @@ private Hashtable GetHashtableFromHashTableAst(HashtableAst hashTableAst) var strConstExprAst = constExprAst as StringConstantExpressionAst; if (strConstExprAst != null) { - rhsList.Add(strConstExprAst.Value); + // it is a string literal + output[key] = strConstExprAst.Value; } else { // it is either an integer or a float output[key] = constExprAst.Value; - continue; } + continue; } else if (pureExp is HashtableAst) { diff --git a/Tests/Engine/Settings.tests.ps1 b/Tests/Engine/Settings.tests.ps1 index 638de0d3a..f4a634ad4 100644 --- a/Tests/Engine/Settings.tests.ps1 +++ b/Tests/Engine/Settings.tests.ps1 @@ -99,7 +99,7 @@ Describe "Settings Class" { } It "Should return 1 rule argument" { - $settings.RuleArguments.Count | Should Be 2 + $settings.RuleArguments.Count | Should Be 3 } It "Should parse boolean type argument" { @@ -109,5 +109,9 @@ Describe "Settings Class" { It "Should parse int type argument" { $settings.RuleArguments["PSUseConsistentIndentation"]["IndentationSize"] | Should Be 4 } + + It "Should parse string literal" { + $settings.RuleArguments["PSProvideCommentHelp"]["Placement"] | Should Be 'end' + } } -} \ No newline at end of file +} diff --git a/Tests/Engine/SettingsTest/Project1/ExplicitSettings.psd1 b/Tests/Engine/SettingsTest/Project1/ExplicitSettings.psd1 index 88d2a6b4f..3e7c8caa3 100644 --- a/Tests/Engine/SettingsTest/Project1/ExplicitSettings.psd1 +++ b/Tests/Engine/SettingsTest/Project1/ExplicitSettings.psd1 @@ -1,8 +1,8 @@ @{ "IncludeRules" = @("PSAvoidUsingCmdletAliases", "PSAvoidUsingWriteHost", "PSUseConsistentIndentation") "ExcludeRules" = @("PSShouldProcess", "PSAvoidUsingWMICmdlet", "PSUseCmdletCorrectly") - "rules" = @{ - PSAvoidUsingCmdletAliases = @{ + "rules" = @{ + PSAvoidUsingCmdletAliases = @{ WhiteList = @("cd", "cp") } @@ -10,5 +10,10 @@ Enable = $true IndentationSize = 4 } + + PSProvideCommentHelp = @{ + Enable = $true + Placement = 'end' + } } -} \ No newline at end of file +} From f99b208d8dc0d48d626b1e5fce6dae6c872617f5 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 16 May 2017 18:32:18 -0700 Subject: [PATCH 17/22] Add option for comment help placement --- Rules/ProvideCommentHelp.cs | 69 +++++++++++++++++++--- Tests/Rules/ProvideCommentHelp.tests.ps1 | 73 ++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 9 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 50d44f216..9443a18f3 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -33,9 +33,6 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class ProvideCommentHelp : ConfigurableRule { - - // todo add option for comment type - // todo add option for help placement (beforeFuntion, BodyStart, BodyEnd:) [ConfigurableRuleProperty(defaultValue: true)] public bool ExportedOnly { get; protected set; } @@ -45,6 +42,10 @@ public class ProvideCommentHelp : ConfigurableRule [ConfigurableRuleProperty(defaultValue: false)] public bool VSCodeSnippetCorrection { get; protected set; } + // possible options: before, begin, end + [ConfigurableRuleProperty(defaultValue: "before")] + public string Placement { get; protected set; } + public ProvideCommentHelp() : base() { // Enable the rule by default @@ -146,16 +147,66 @@ private IEnumerable GetCorrection(FunctionDefinitionAst funcDe helpBuilder.AddParameter(paramAst.Name.VariablePath.UserPath); } - var correctionExtents = new List(); + int startLine, endLine, startColumn, endColumn; + GetCorrectionPosition(funcDefnAst, out startLine, out endLine, out startColumn, out endColumn); yield return new CorrectionExtent( - funcDefnAst.Extent.StartLineNumber, - funcDefnAst.Extent.StartLineNumber, - funcDefnAst.Extent.StartColumnNumber, - funcDefnAst.Extent.StartColumnNumber, - helpBuilder.GetCommentHelp(BlockComment, VSCodeSnippetCorrection) + Environment.NewLine, + startLine, + endLine, + startColumn, + endColumn, + ProcessCorrectionForPlacement( + helpBuilder.GetCommentHelp( + BlockComment, + VSCodeSnippetCorrection)), funcDefnAst.Extent.File); } + private string ProcessCorrectionForPlacement(string correction) + { + switch (Placement.ToLower()) + { + case "begin": + return "{" + Environment.NewLine + correction + Environment.NewLine; + case "end": + return Environment.NewLine + correction + Environment.NewLine; + default: + return correction + Environment.NewLine; + } + } + + private void GetCorrectionPosition( + FunctionDefinitionAst funcDefnAst, + out int startLine, + out int endLine, + out int startColumn, + out int endColumn) + { + // the better thing to do is get the line/column from corresponding tokens + switch (Placement.ToLower()) + { + case "begin": + startLine = funcDefnAst.Body.Extent.StartLineNumber; + endLine = startLine; + startColumn = funcDefnAst.Body.Extent.StartColumnNumber; + endColumn = startColumn + 1; + break; + + case "end": + startLine = funcDefnAst.Body.Extent.EndLineNumber; + endLine = startLine; + startColumn = funcDefnAst.Body.Extent.EndColumnNumber - 1; + endColumn = startColumn; + break; + + default: // before + startLine = funcDefnAst.Extent.StartLineNumber; + endLine = startLine; + startColumn = funcDefnAst.Extent.StartColumnNumber; + endColumn = startColumn; + break; + } + } + private class ViolationFinder : AstVisitor { private HashSet functionWhitelist; diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index 7d3a0f327..7f514a699 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -10,6 +10,7 @@ $ruleSettings = @{ Enable = $true ExportedOnly = $false BlockComment = $true + Placement = "before" VSCodeSnippetCorrection = $false } @@ -214,6 +215,78 @@ Export-ModuleMember -Function foo Test-Correction $def $expectedCorrection $settings } + It "should return a help snippet correction with 2 parameters at the begining of function body" { + $def = @' +function foo { + param($param1, $param2) +} +'@ + $expectedCorrection = @' +{ +<# +.SYNOPSIS +Short description + +.DESCRIPTION +Long description + +.PARAMETER param1 +Parameter description + +.PARAMETER param2 +Parameter description + +.EXAMPLE +An example + +.NOTES +General notes +#> + +'@ + $ruleSettings.'ExportedOnly' = $false + $ruleSettings.'BlockComment' = $true + $ruleSettings.'VSCodeSnippetCorrection' = $false + $ruleSettings.'Placement' = 'begin' + Test-Correction $def $expectedCorrection $settings + } + + It "should return a help snippet correction with 2 parameters at the end of function body" { + $def = @' +function foo { + param($param1, $param2) +} +'@ + $expectedCorrection = @' + +<# +.SYNOPSIS +Short description + +.DESCRIPTION +Long description + +.PARAMETER param1 +Parameter description + +.PARAMETER param2 +Parameter description + +.EXAMPLE +An example + +.NOTES +General notes +#> + +'@ + $ruleSettings.'ExportedOnly' = $false + $ruleSettings.'BlockComment' = $true + $ruleSettings.'VSCodeSnippetCorrection' = $false + $ruleSettings.'Placement' = 'end' + Test-Correction $def $expectedCorrection $settings + } + if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') { It "Does not count violation in DSC class" { $dscViolations.Count | Should Be 0 From 684a8c1d2edc517ef67b142a845d6fac4db535b2 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 17 May 2017 15:23:55 -0700 Subject: [PATCH 18/22] Add Enum type for comment help placement options --- Rules/ProvideCommentHelp.cs | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 9443a18f3..6f3c667a1 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -33,6 +33,9 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class ProvideCommentHelp : ConfigurableRule { + // todo rearrange members + private CommentHelpPlacement placement; + [ConfigurableRuleProperty(defaultValue: true)] public bool ExportedOnly { get; protected set; } @@ -44,7 +47,21 @@ public class ProvideCommentHelp : ConfigurableRule // possible options: before, begin, end [ConfigurableRuleProperty(defaultValue: "before")] - public string Placement { get; protected set; } + public string Placement + { + get + { + return placement.ToString(); + } + set + { + if (String.IsNullOrWhiteSpace(value) || + !Enum.TryParse(value, true, out placement)) + { + placement = CommentHelpPlacement.Before; + } + } + } public ProvideCommentHelp() : base() { @@ -52,6 +69,8 @@ public ProvideCommentHelp() : base() Enable = true; } + private enum CommentHelpPlacement { Before, Begin, End }; + /// /// AnalyzeScript: Analyzes the ast to check that cmdlets have help. /// @@ -163,13 +182,13 @@ private IEnumerable GetCorrection(FunctionDefinitionAst funcDe private string ProcessCorrectionForPlacement(string correction) { - switch (Placement.ToLower()) + switch (placement) { - case "begin": + case CommentHelpPlacement.Begin: return "{" + Environment.NewLine + correction + Environment.NewLine; - case "end": + case CommentHelpPlacement.End: return Environment.NewLine + correction + Environment.NewLine; - default: + default: // CommentHelpPlacement.Before return correction + Environment.NewLine; } } @@ -182,23 +201,23 @@ private void GetCorrectionPosition( out int endColumn) { // the better thing to do is get the line/column from corresponding tokens - switch (Placement.ToLower()) + switch (placement) { - case "begin": + case CommentHelpPlacement.Begin: startLine = funcDefnAst.Body.Extent.StartLineNumber; endLine = startLine; startColumn = funcDefnAst.Body.Extent.StartColumnNumber; endColumn = startColumn + 1; break; - case "end": + case CommentHelpPlacement.End: startLine = funcDefnAst.Body.Extent.EndLineNumber; endLine = startLine; startColumn = funcDefnAst.Body.Extent.EndColumnNumber - 1; endColumn = startColumn; break; - default: // before + default: // CommentHelpPlacement.Before startLine = funcDefnAst.Extent.StartLineNumber; endLine = startLine; startColumn = funcDefnAst.Extent.StartColumnNumber; From 56ba0dc7941d341999f973ced81ed2dbf875bc93 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 17 May 2017 17:30:33 -0700 Subject: [PATCH 19/22] Add indentation to correction --- Rules/ProvideCommentHelp.cs | 43 +++++++++++++++++------- Tests/Rules/ProvideCommentHelp.tests.ps1 | 34 +++++++++++++++++++ 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 6f3c667a1..8015d25c6 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -93,7 +93,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file GetDiagnosticSeverity(), fileName, null, - GetCorrection(functionDefinitionAst).ToList()); + GetCorrection(ast, functionDefinitionAst).ToList()); } } @@ -154,7 +154,7 @@ private DiagnosticSeverity GetDiagnosticSeverity() return DiagnosticSeverity.Information; } - private IEnumerable GetCorrection(FunctionDefinitionAst funcDefnAst) + private IEnumerable GetCorrection(Ast ast, FunctionDefinitionAst funcDefnAst) { var helpBuilder = new CommentHelpBuilder(); @@ -173,26 +173,45 @@ private IEnumerable GetCorrection(FunctionDefinitionAst funcDe endLine, startColumn, endColumn, - ProcessCorrectionForPlacement( - helpBuilder.GetCommentHelp( - BlockComment, - VSCodeSnippetCorrection)), + GetCorrectionText( + helpBuilder.GetCommentHelp(BlockComment, VSCodeSnippetCorrection), + ast, + funcDefnAst), funcDefnAst.Extent.File); } - private string ProcessCorrectionForPlacement(string correction) + private string GetCorrectionText(string correction, Ast ast, FunctionDefinitionAst funcDefnAst) { + var indentationString = String.Empty; + if (funcDefnAst.Extent.StartColumnNumber > 1) + { + indentationString = GetLines(ast.Extent.Text) + .ElementAt(funcDefnAst.Extent.StartLineNumber - 1) + .Substring(0, funcDefnAst.Extent.StartColumnNumber - 1); + correction = String.Join( + Environment.NewLine, + GetLines(correction).Select(l => indentationString + l)); + } + switch (placement) { case CommentHelpPlacement.Begin: - return "{" + Environment.NewLine + correction + Environment.NewLine; + return $"{{{Environment.NewLine}{correction}{Environment.NewLine}"; + case CommentHelpPlacement.End: - return Environment.NewLine + correction + Environment.NewLine; + return $"{Environment.NewLine}{correction}{Environment.NewLine}{indentationString}"; + default: // CommentHelpPlacement.Before - return correction + Environment.NewLine; + return $"{correction}{Environment.NewLine}"; } } + // TODO replace with extension version + private static IEnumerable GetLines(string text) + { + return text.Split('\n').Select(l => l.Trim('\r')); + } + private void GetCorrectionPosition( FunctionDefinitionAst funcDefnAst, out int startLine, @@ -220,7 +239,7 @@ private void GetCorrectionPosition( default: // CommentHelpPlacement.Before startLine = funcDefnAst.Extent.StartLineNumber; endLine = startLine; - startColumn = funcDefnAst.Extent.StartColumnNumber; + startColumn = 1; endColumn = startColumn; break; } @@ -314,7 +333,7 @@ public string GetCommentHelp(bool blockComment, bool snippet) { var boundaryString = new String('#', 30); sb.AppendLine(boundaryString); - var lines = helpContent.Split('\n').Select(l => l.Trim('\r')); + var lines = GetLines(helpContent); foreach (var line in lines) { sb.Append("#"); diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index 7f514a699..bf33eea54 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -287,6 +287,40 @@ General notes Test-Correction $def $expectedCorrection $settings } + It "should return a help snippet correction with correct indentation" { + $def = @' + function foo { + param($param1) + } +'@ + $s = ' ' + $expectedCorrection = @" + <# + .SYNOPSIS + Short description +$s$s$s$s + .DESCRIPTION + Long description +$s$s$s$s + .PARAMETER param1 + Parameter description +$s$s$s$s + .EXAMPLE + An example +$s$s$s$s + .NOTES + General notes + #> + +"@ + $ruleSettings.'ExportedOnly' = $false + $ruleSettings.'BlockComment' = $true + $ruleSettings.'VSCodeSnippetCorrection' = $false + $ruleSettings.'Placement' = 'before' + Test-Correction $def $expectedCorrection $settings + } + + if ($PSVersionTable.PSVersion -ge [Version]'5.0.0') { It "Does not count violation in DSC class" { $dscViolations.Count | Should Be 0 From dc4e6f06978f688d466f7ebb9c96440de4aa2c4e Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 17 May 2017 17:45:25 -0700 Subject: [PATCH 20/22] Add code documentation and rearrange members --- Rules/ProvideCommentHelp.cs | 61 +++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 8015d25c6..66aa87cdb 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -33,19 +33,53 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class ProvideCommentHelp : ConfigurableRule { - // todo rearrange members private CommentHelpPlacement placement; + /// + /// Construct an object of ProvideCommentHelp type. + /// + public ProvideCommentHelp() : base() + { + // Enable the rule by default + Enable = true; + } + + /// + /// If enabled, throw violation only on functions/cmdlets that are exported using + /// the "Export-ModuleMember" cmdlet. + /// + /// Default value is true. + /// [ConfigurableRuleProperty(defaultValue: true)] public bool ExportedOnly { get; protected set; } + /// + /// If enabled, returns comment help in block comment style, i.e., `<#...#>`. Otherwise returns + /// comment help in line comment style, i.e., each comment line starts with `#`. + /// + /// Default value is true. + /// [ConfigurableRuleProperty(defaultValue: true)] public bool BlockComment { get; protected set; } + /// + /// If enabled, returns comment help in vscode snippet format. + /// + /// Default value is false. + /// [ConfigurableRuleProperty(defaultValue: false)] public bool VSCodeSnippetCorrection { get; protected set; } - // possible options: before, begin, end + /// + /// Represents the position of comment help with respect to the function definition. + /// + /// Possible values are: `before`, `begin` and `end`. If any invalid value is given, the + /// property defaults to `before`. + /// + /// `before` means the help is placed before the function definition. + /// `begin` means the help is placed at the begining of the function definition body. + /// `end` means the help is places the end of the function definition body. + /// [ConfigurableRuleProperty(defaultValue: "before")] public string Placement { @@ -63,12 +97,6 @@ public string Placement } } - public ProvideCommentHelp() : base() - { - // Enable the rule by default - Enable = true; - } - private enum CommentHelpPlacement { Before, Begin, End }; /// @@ -149,6 +177,12 @@ public override string GetSourceName() return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); } + // TODO replace with extension version + private static IEnumerable GetLines(string text) + { + return text.Split('\n').Select(l => l.Trim('\r')); + } + private DiagnosticSeverity GetDiagnosticSeverity() { return DiagnosticSeverity.Information; @@ -206,12 +240,6 @@ private string GetCorrectionText(string correction, Ast ast, FunctionDefinitionA } } - // TODO replace with extension version - private static IEnumerable GetLines(string text) - { - return text.Split('\n').Select(l => l.Trim('\r')); - } - private void GetCorrectionPosition( FunctionDefinitionAst funcDefnAst, out int startLine, @@ -280,11 +308,6 @@ public IEnumerable FunctionDefinitionAsts } } - /// - /// Visit function and checks that it has comment help - /// - /// - /// public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) { if ((!useFunctionWhitelist || functionWhitelist.Contains(funcAst.Name)) From 0ace6038c812759df6f2fed0c830f72ec86d3808 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 17 May 2017 17:55:46 -0700 Subject: [PATCH 21/22] Update ProvideCommentHelp rule documentation --- RuleDocumentation/ProvideCommentHelp.md | 44 +++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/RuleDocumentation/ProvideCommentHelp.md b/RuleDocumentation/ProvideCommentHelp.md index 8be2a1f57..ddfc914bc 100644 --- a/RuleDocumentation/ProvideCommentHelp.md +++ b/RuleDocumentation/ProvideCommentHelp.md @@ -8,9 +8,49 @@ Comment based help should be provided for all PowerShell commands. This test onl For assistance on comment based help, use the command ```Get-Help about_comment_based_help``` or the article, "How to Write Cmdlet Help" (http://go.microsoft.com/fwlink/?LinkID=123415). -## How +## Configuration -Include comment based help for each command identified. +```powershell +Rules = @{ + PSProvideCommentHelp = @{ + Enable = $true + ExportedOnly = $false + BlockComment = $true + VSCodeSnippetCorrection = $false + Placement = "before" + } +} +``` + +### Parameters + +#### Enable: bool (Default valus is `$true`) + +Enable or disable the rule during ScriptAnalyzer invocation. + +#### ExportedOnly: bool (Default value is `$true`) + +If enabled, throw violation only on functions/cmdlets that are exported using the 'Export-ModuleMember' cmdlet. + +#### BlockComment: bool (Default value is `$true`) + +If enabled, returns comment help in block comment style, i.e., `<#...#>`. Otherwise returns +comment help in line comment style, i.e., each comment line starts with `#`. + +#### VSCodeSnippetCorrection: bool (Default value is `$false`) + +If enabled, returns comment help in vscode snippet format. + +#### Placement: string (Default value is `before`) + +Represents the position of comment help with respect to the function definition. + +Possible values are: `before`, `begin` and `end`. If any invalid value is given, the +property defaults to `before`. + +`before` means the help is placed before the function definition. +`begin` means the help is placed at the begining of the function definition body. +`end` means the help is places the end of the function definition body. ## Example From 2954583f8213cf159a2044db97edaaf32cf15396 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 17 May 2017 19:35:36 -0700 Subject: [PATCH 22/22] Test each line individually instead of the entire text This should fix the appveyor test failures caused by inconsistency between expected and actual line endings. --- Tests/Rules/ProvideCommentHelp.tests.ps1 | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index bf33eea54..834765ec2 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -32,7 +32,18 @@ function Test-Correction { $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -Settings $settings $violations.Count | Should Be 1 - $violations[0].SuggestedCorrections[0].Text | Should Be $expectedCorrection + + # We split the lines because appveyor checks out files with "\n" endings + # on windows, which results in inconsistent line endings between corrections + # and result. + $resultLines = $violations[0].SuggestedCorrections[0].Text -split "\r?\n" + $expectedLines = $expectedCorrection -split "\r?\n" + $resultLines.Count | Should Be $expectedLines.Count + for ($i = 0; $i -lt $resultLines.Count; $i++) { + $resultLine = $resultLines[$i] + $expectedLine = $expectedLines[$i] + $resultLine | Should Be $expectedLine + } } Describe "ProvideCommentHelp" {