From 88a77b9d553055648240b92062e7aeb180b25323 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 6 Apr 2016 16:45:41 -0700 Subject: [PATCH 1/2] Fix extents of UseSingularNoun and UseApprovedVerbs Fixes search logic to find the function name in a function definition. Prior to the fix, if the function definition started on the first line of a script, the extent would include the entire function definition. This commit fixes that bug. --- Engine/Helper.cs | 46 ++++++++++++++----- Rules/UseApprovedVerbs.cs | 2 - .../UseSingularNounsReservedVerbs.tests.ps1 | 8 ++++ 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index a15e5166b..75f0a9d32 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -656,22 +656,46 @@ public IScriptExtent GetScriptExtentForFunctionName(FunctionDefinitionAst functi if (null == functionDefinitionAst) { return null; + } + var funcNameToken = Tokens.Where( + token => ContainsExtent(functionDefinitionAst.Extent, token.Extent) + && token.Text.Equals(functionDefinitionAst.Name)); + if (funcNameToken.Any()) + { + return funcNameToken.First().Extent; } - - // Obtain the index where the function name is in Tokens - int funcTokenIndex = Tokens.Select((s, index) => new { s, index }) - .Where(x => x.s.Extent.StartOffset == functionDefinitionAst.Extent.StartOffset) - .Select(x => x.index).FirstOrDefault(); - - if (funcTokenIndex > 0 && funcTokenIndex < Helper.Instance.Tokens.Count()) + else { - // return the extent of the next token - this is the extent for the function name - return Tokens[++funcTokenIndex].Extent; + return null; } + } - return null; + /// + /// Return true of subset is contained in set + /// + /// + /// + /// True or False + private bool ContainsExtent(IScriptExtent set, IScriptExtent subset) + { + if (set == null) + { + throw new ArgumentNullException("set"); + } + if (subset == null) + { + throw new ArgumentNullException("subset"); + } + if (set.StartOffset <= subset.StartOffset + && set.EndOffset >= subset.EndOffset) + { + return true; + } + else + { + return false; + } } - private void FindClosingParenthesis(string keyword) { if (Tokens == null || Tokens.Length == 0) diff --git a/Rules/UseApprovedVerbs.cs b/Rules/UseApprovedVerbs.cs index e0ac84b98..93c883c31 100644 --- a/Rules/UseApprovedVerbs.cs +++ b/Rules/UseApprovedVerbs.cs @@ -64,12 +64,10 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (!approvedVerbs.Contains(verb, StringComparer.OrdinalIgnoreCase)) { IScriptExtent extent = Helper.Instance.GetScriptExtentForFunctionName(funcAst); - if (null == extent) { extent = funcAst.Extent; } - yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseApprovedVerbsError, funcName), extent, GetName(), DiagnosticSeverity.Warning, fileName); } diff --git a/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 b/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 index 25e30b445..6685bd0d4 100644 --- a/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 +++ b/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 @@ -20,6 +20,10 @@ Describe "UseSingularNouns" { It "has the correct description message" { $nounViolations[0].Message | Should Match $nounViolationMessage } + + It "has the correct extent" { + $nounViolations[0].Extent.Text | Should be "Verb-Files" + } } Context "When there are no violations" { @@ -38,6 +42,10 @@ Describe "UseApprovedVerbs" { It "has the correct description message" { $verbViolations[0].Message | Should Match $verbViolationMessage } + + It "has the correct extent" { + $verbViolations[0].Extent.Text | Should be "Verb-Files" + } } Context "When there are no violations" { From 1b7bfb033671132e70d287093367647ea05e7d28 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 7 Apr 2016 09:43:47 -0700 Subject: [PATCH 2/2] Remove some redundancy from GetScriptExtentForFunctionName method --- Engine/Helper.cs | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 75f0a9d32..cfd75a937 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -657,44 +657,28 @@ public IScriptExtent GetScriptExtentForFunctionName(FunctionDefinitionAst functi { return null; } - var funcNameToken = Tokens.Where( - token => ContainsExtent(functionDefinitionAst.Extent, token.Extent) + var funcNameTokens = Tokens.Where( + token => + ContainsExtent(functionDefinitionAst.Extent, token.Extent) && token.Text.Equals(functionDefinitionAst.Name)); - if (funcNameToken.Any()) - { - return funcNameToken.First().Extent; - } - else - { - return null; - } + var funcNameToken = funcNameTokens.FirstOrDefault(); + return funcNameToken == null ? null : funcNameToken.Extent; } /// - /// Return true of subset is contained in set + /// Return true if subset is contained in set /// /// /// /// True or False private bool ContainsExtent(IScriptExtent set, IScriptExtent subset) { - if (set == null) - { - throw new ArgumentNullException("set"); - } - if (subset == null) - { - throw new ArgumentNullException("subset"); - } - if (set.StartOffset <= subset.StartOffset - && set.EndOffset >= subset.EndOffset) - { - return true; - } - else + if (set == null || subset == null) { return false; } + return set.StartOffset <= subset.StartOffset + && set.EndOffset >= subset.EndOffset; } private void FindClosingParenthesis(string keyword) {