diff --git a/CHANGELOG.md b/CHANGELOG.md index c5a4cd93..a7edaecb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,8 @@ ## Improvements: ## Bug fixes: - -*Contributors of this release (in alphabetical order):* +* Fix: Ambiguous steps reported wehn definition matches via more than one tag (#95) +*Contributors of this release (in alphabetical order):* @clrudolphi # v2025.1.256 - 2025-03-07 diff --git a/Reqnroll.VisualStudio/Discovery/ProjectBindingImplementationEqualityComparer.cs b/Reqnroll.VisualStudio/Discovery/ProjectBindingImplementationEqualityComparer.cs new file mode 100644 index 00000000..9dc0c773 --- /dev/null +++ b/Reqnroll.VisualStudio/Discovery/ProjectBindingImplementationEqualityComparer.cs @@ -0,0 +1,30 @@ +namespace Reqnroll.VisualStudio.Discovery; +public class ProjectBindingImplementationEqualityComparer : IEqualityComparer +{ + public bool Equals(ProjectBindingImplementation x, ProjectBindingImplementation y) + { + if (ReferenceEquals(x, y)) return true; + if (x is null || y is null) return false; + + return x.Method == y.Method && + x.ParameterTypes.SequenceEqual(y.ParameterTypes) && + Equals(x.SourceLocation, y.SourceLocation); + } + + public int GetHashCode(ProjectBindingImplementation obj) + { + if (obj is null) return 0; + + unchecked // Use unchecked to handle potential integer overflow + { + int hash = 17; + hash = hash * 23 + (obj.Method?.GetHashCode() ?? 0); + + foreach (var paramType in obj.ParameterTypes) + hash = hash * 23 + (paramType?.GetHashCode() ?? 0); + + hash = hash * 23 + (obj.SourceLocation?.GetHashCode() ?? 0); + return hash; + } + } +} \ No newline at end of file diff --git a/Reqnroll.VisualStudio/Discovery/ProjectBindingRegistry.cs b/Reqnroll.VisualStudio/Discovery/ProjectBindingRegistry.cs index d38ee9bf..103413a4 100644 --- a/Reqnroll.VisualStudio/Discovery/ProjectBindingRegistry.cs +++ b/Reqnroll.VisualStudio/Discovery/ProjectBindingRegistry.cs @@ -7,6 +7,7 @@ public record ProjectBindingRegistry private const string DocStringDefaultTypeName = TypeShortcuts.StringType; public static ProjectBindingRegistry Invalid = new(ImmutableArray.Empty, ImmutableArray.Empty); + private static ProjectBindingImplementationEqualityComparer _equalityComparerForProjectBindingImplementations = new(); private static int _versionCounter; private ProjectBindingRegistry(IEnumerable stepDefinitions, IEnumerable hooks) @@ -186,7 +187,14 @@ private MatchResultItem[] HandleScopeOverloads(MatchResultItem[] sdMatches) var matchesWithScope = sdMatches.Where(m => m.MatchedStepDefinition.Scope != null).ToArray(); if (matchesWithScope.Any()) - sdMatches = matchesWithScope; + { + // Group matches by everything except the Scope property + // and take the first item from each group + sdMatches = matchesWithScope + .GroupBy(m => m.MatchedStepDefinition.Implementation, _equalityComparerForProjectBindingImplementations) + .Select(g => g.First()) + .ToArray(); + } } return sdMatches; diff --git a/Tests/Reqnroll.VisualStudio.Specs/Features/Editor/StepAnalysis.feature b/Tests/Reqnroll.VisualStudio.Specs/Features/Editor/StepAnalysis.feature index e748b3e5..7d6b351b 100644 --- a/Tests/Reqnroll.VisualStudio.Specs/Features/Editor/StepAnalysis.feature +++ b/Tests/Reqnroll.VisualStudio.Specs/Features/Editor/StepAnalysis.feature @@ -274,6 +274,33 @@ Scenario: Matches combination scoped step definitions """ And no binding error should be highlighted +Scenario: Matches on multiple tags (without improperly highlighting as Ambiguous) + Given there is a Reqnroll project scope + And the following step definition with mulitple Tag Scopes in the project: + | type | regex | tag scope | + | When | I use a step with multiple tags | @mytag1,@mytag2 | + When the following feature file is opened in the editor + """ + Feature: Addition + + @mytag1 + @mytag2 + Scenario: Random scenario + When I use a step with multiple tags + + """ + And the project is built and the initial binding discovery is performed + Then all section of types DefinedStep should be highlighted as + """ + Feature: Addition + + @mytag1 + @mytag2 + Scenario: Random scenario + When {DefinedStep}I use a step with multiple tags{/DefinedStep} + """ + And no binding error should be highlighted + Scenario: Analyses all scopes of background steps Given there is a Reqnroll project scope And the following step definitions in the project: diff --git a/Tests/Reqnroll.VisualStudio.Specs/StepDefinitions/ProjectSystemSteps.cs b/Tests/Reqnroll.VisualStudio.Specs/StepDefinitions/ProjectSystemSteps.cs index da02879f..5ebc0b95 100644 --- a/Tests/Reqnroll.VisualStudio.Specs/StepDefinitions/ProjectSystemSteps.cs +++ b/Tests/Reqnroll.VisualStudio.Specs/StepDefinitions/ProjectSystemSteps.cs @@ -27,7 +27,7 @@ public ProjectSystemSteps(StubIdeScope stubIdeScope) action(_ideScope.BackgroundTaskTokenSource.Token)); } - private StubIdeActions ActionsMock => (StubIdeActions) _ideScope.Actions; + private StubIdeActions ActionsMock => (StubIdeActions)_ideScope.Actions; [Given(@"there is a Reqnroll project scope")] public void GivenThereIsAReqnrollProjectScope() @@ -131,6 +131,36 @@ public void WhenANewStepDefinitionIsAddedToTheProjectAs(Table stepDefinitionTabl RegisterStepDefinitions(stepDefinitions); } + [Given(@"the following step definition with mulitple Tag Scopes in the project:")] + public void GivenNewStepDefinitionsWithMultipleScopeTagsAreAddedToTheProjectAs(Table stepDefinitionTable) + { + var stepDefinitions = stepDefinitionTable.CreateSet(CreateStepDefinitionFromTableRow).ToArray(); + var resultingStepDefinitions = new List(); + // we expect that the Tag scope string in the table is a comma delimited set of tags to apply; + // So we will create a step definition for each such tag by using the built step def as a template. + foreach (var sd in stepDefinitions) + { + var taglist = sd.Scope.Tag.Split(','); + foreach (var t in taglist) + { + var stepDefToAdd = new StepDefinition + { + Type = sd.Type, + Method = sd.Method, + Regex = sd.Regex, + SourceLocation = sd.SourceLocation, + Scope = new StepScope + { + Tag = t, + FeatureTitle = sd.Scope.FeatureTitle, + ScenarioTitle = sd.Scope.ScenarioTitle + } + }; + resultingStepDefinitions.Add(stepDefToAdd); + } + } + RegisterStepDefinitions(resultingStepDefinitions.ToArray()); + } [Given("the following hooks in the project:")] public void GivenTheFollowingHooksInTheProject(DataTable hooksTable) { @@ -142,33 +172,35 @@ private StepDefinition CreateStepDefinitionFromTableRow(DataTableRow tableRow) { var filePath = @"X:\ProjectMock\CalculatorSteps.cs"; var line = _rnd.Next(1, 30); + _projectScope.AddFile(filePath, string.Empty); + + tableRow.TryGetValue("regex", out var regex); + tableRow.TryGetValue("type", out var stepType); + var stepDefinition = new StepDefinition { Method = $"M{Guid.NewGuid():N}", SourceLocation = filePath + $"|{line}|5" }; - tableRow.TryGetValue("tag scope", out var tagScope); + tableRow.TryGetValue("tag scope", out var tagScopes); tableRow.TryGetValue("feature scope", out var featureScope); tableRow.TryGetValue("scenario scope", out var scenarioScope); - if (string.IsNullOrEmpty(tagScope)) - tagScope = null; + if (string.IsNullOrEmpty(tagScopes)) + tagScopes = null; if (string.IsNullOrEmpty(featureScope)) featureScope = null; if (string.IsNullOrEmpty(scenarioScope)) scenarioScope = null; - if (tagScope != null || featureScope != null || scenarioScope != null) + if (tagScopes != null || featureScope != null || scenarioScope != null) stepDefinition.Scope = new StepScope { - Tag = tagScope, + Tag = tagScopes, FeatureTitle = featureScope, ScenarioTitle = scenarioScope }; - - _projectScope.AddFile(filePath, string.Empty); - return stepDefinition; } @@ -344,7 +376,7 @@ public void WhenIInvokeTheCommand(string commandName) [When(@"I invoke the ""(.*)"" command without waiting for the tag changes")] public void WhenIInvokeTheCommandWithoutWaitingForTagger(string commandName) { - PerformCommand(commandName, waitForTager:false); + PerformCommand(commandName, waitForTager: false); } private void PerformCommand(string commandName, string parameter = null, @@ -360,33 +392,33 @@ private void PerformCommand(string commandName, string parameter = null, switch (commandName) { case "Go To Definition": - { - _invokedCommand = new GoToDefinitionCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider); - _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); - return; - } + { + _invokedCommand = new GoToDefinitionCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider); + _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); + return; + } case "Go To Hooks": - { - _invokedCommand = new GoToHooksCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider); - _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); - return; - } + { + _invokedCommand = new GoToHooksCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider); + _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); + return; + } case "Find Step Definition Usages": - { - _invokedCommand = new FindStepDefinitionUsagesCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider); - _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); - Wait.For(() => ActionsMock.IsComplete.Should().BeTrue()); - return; - } + { + _invokedCommand = new FindStepDefinitionUsagesCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider); + _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); + Wait.For(() => ActionsMock.IsComplete.Should().BeTrue()); + return; + } case "Find Unused Step Definitions": { _invokedCommand = new FindUnusedStepDefinitionsCommand( @@ -398,73 +430,73 @@ private void PerformCommand(string commandName, string parameter = null, return; } case "Comment": - { - _invokedCommand = new CommentCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider); - _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); - break; - } + { + _invokedCommand = new CommentCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider); + _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); + break; + } case "Uncomment": - { - _invokedCommand = new UncommentCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider); - _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); - - break; - } + { + _invokedCommand = new UncommentCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider); + _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); + + break; + } case "Auto Format Document": - { - _invokedCommand = new AutoFormatDocumentCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider, - new GherkinDocumentFormatter(), - new StubEditorConfigOptionsProvider()); - _invokedCommand.PreExec(_wpfTextView, AutoFormatDocumentCommand.FormatDocumentKey); - break; - } + { + _invokedCommand = new AutoFormatDocumentCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider, + new GherkinDocumentFormatter(), + new StubEditorConfigOptionsProvider()); + _invokedCommand.PreExec(_wpfTextView, AutoFormatDocumentCommand.FormatDocumentKey); + break; + } case "Auto Format Selection": - { - _invokedCommand = new AutoFormatDocumentCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider, - new GherkinDocumentFormatter(), - new StubEditorConfigOptionsProvider()); - _invokedCommand.PreExec(_wpfTextView, AutoFormatDocumentCommand.FormatSelectionKey); - break; - } + { + _invokedCommand = new AutoFormatDocumentCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider, + new GherkinDocumentFormatter(), + new StubEditorConfigOptionsProvider()); + _invokedCommand.PreExec(_wpfTextView, AutoFormatDocumentCommand.FormatSelectionKey); + break; + } case "Auto Format Table": - { - _invokedCommand = new AutoFormatTableCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider, - new GherkinDocumentFormatter(), - new StubEditorConfigOptionsProvider()); - _wpfTextView.SimulateType((AutoFormatTableCommand) _invokedCommand, parameter?[0] ?? '|', - taggerProvider); - break; - } + { + _invokedCommand = new AutoFormatTableCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider, + new GherkinDocumentFormatter(), + new StubEditorConfigOptionsProvider()); + _wpfTextView.SimulateType((AutoFormatTableCommand)_invokedCommand, parameter?[0] ?? '|', + taggerProvider); + break; + } case "Define Steps": - { - _invokedCommand = new DefineStepsCommand(_ideScope, aggregatorFactoryService, taggerProvider); - _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); - return; - } + { + _invokedCommand = new DefineStepsCommand(_ideScope, aggregatorFactoryService, taggerProvider); + _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); + return; + } case "Complete": case "Filter Completion": - { - EnsureStubCompletionBroker(); - _invokedCommand = new CompleteCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider, - _completionBroker); + { + EnsureStubCompletionBroker(); + _invokedCommand = new CompleteCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider, + _completionBroker); if (parameter == null) { _invokedCommand.PreExec(_wpfTextView, commandTargetKey ?? _invokedCommand.Targets.First()); @@ -472,18 +504,18 @@ private void PerformCommand(string commandName, string parameter = null, } else _wpfTextView.SimulateTypeText((CompleteCommand)_invokedCommand, parameter, taggerProvider); - break; - } + break; + } case "Rename Step": - { - _invokedCommand = new RenameStepCommand( - _ideScope, - aggregatorFactoryService, - taggerProvider); - _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); + { + _invokedCommand = new RenameStepCommand( + _ideScope, + aggregatorFactoryService, + taggerProvider); + _invokedCommand.PreExec(_wpfTextView, _invokedCommand.Targets.First()); - break; - } + break; + } default: throw new NotImplementedException(commandName); } @@ -633,7 +665,7 @@ public void ThenNoBindingErrorShouldBeHighlighted() [Then(@"all (.*) section should be highlighted as")] public void ThenTheStepKeywordsShouldBeHighlightedAs(string keywordType, string expectedContent) { - ThenAllSectionOfTypesShouldBeHighlightedAs(new[] {keywordType}, expectedContent); + ThenAllSectionOfTypesShouldBeHighlightedAs(new[] { keywordType }, expectedContent); } [Then(@"the tag links should target to the following URLs")] @@ -641,7 +673,7 @@ public void ThenTheTagLinksShouldTargetToTheFollowingURLs(Table expectedTagLinks { var tagSpans = GetVsTagSpans(_wpfTextView, new DeveroomUrlTaggerProvider(CreateAggregatorFactory(), _ideScope)).ToArray(); - var actualTagLinks = tagSpans.Select(t => new {Tag = t.Span.GetText(), URL = t.Tag.Url.ToString()}); + var actualTagLinks = tagSpans.Select(t => new { Tag = t.Span.GetText(), URL = t.Tag.Url.ToString() }); expectedTagLinksTable.CompareToSet(actualTagLinks); } @@ -922,7 +954,7 @@ private void CheckCompletions(Table expectedItemsTable, Func filte _completionBroker.Should().NotBeNull(); var actualCompletions = _completionBroker.Completions .Where(c => filter?.Invoke(c.InsertionText) ?? true) - .Select(c => new {Item = c.InsertionText.Trim(), c.Description}); + .Select(c => new { Item = c.InsertionText.Trim(), c.Description }); expectedItemsTable.CompareToSet(actualCompletions); } diff --git a/Tests/Reqnroll.VisualStudio.Tests/Discovery/ProjectBindingRegistryAmbiguousTests.cs b/Tests/Reqnroll.VisualStudio.Tests/Discovery/ProjectBindingRegistryAmbiguousTests.cs index f9464cae..81ae58f3 100644 --- a/Tests/Reqnroll.VisualStudio.Tests/Discovery/ProjectBindingRegistryAmbiguousTests.cs +++ b/Tests/Reqnroll.VisualStudio.Tests/Discovery/ProjectBindingRegistryAmbiguousTests.cs @@ -1,4 +1,5 @@ #nullable disable + namespace Reqnroll.VisualStudio.Tests.Discovery; /* @@ -74,4 +75,26 @@ public void Matches_multiple_stepdefs_in_SO() result.Items.Should().HaveCount(2); result.Items.All(r => r.Type == MatchResultType.Ambiguous).Should().BeTrue(); } + + [Fact] + public void Matches_disambiguates_single_stepDef_with_multiple_matching_Scopes() + { + var methodName = "MyMethod" + Guid.NewGuid().ToString("N"); + _stepDefinitionBindings.Add(CreateStepDefinitionBindingWithScope("my .* step", "@mytag1", methodName)); + _stepDefinitionBindings.Add(CreateStepDefinitionBindingWithScope("my .* step", "@mytag2", methodName)); + _stepDefinitionBindings.Add(CreateStepDefinitionBinding("other step")); + var sut = CreateSut(); + + var result = sut.MatchStep(CreateStep(text: "my cool step"), StubGherkinDocumentWithScope.Instance); + result.HasAmbiguous.Should().BeFalse(); + result.Items.Should().HaveCount(1); + result.HasErrors.Should().BeFalse(); + result.Items[0].MatchedStepDefinition.Implementation.Method.Should().Be(methodName); + } + + private ProjectStepDefinitionBinding CreateStepDefinitionBindingWithScope(string stepRegex, string scopeText, string methodName) + { + var scope = new Scope() { Tag = new TagExpressionParser().Parse(scopeText) }; + return CreateStepDefinitionBinding(stepRegex, ScenarioBlock.Given, scope, null, methodName); + } } diff --git a/Tests/Reqnroll.VisualStudio.Tests/Discovery/StubGherkinDocument.cs b/Tests/Reqnroll.VisualStudio.Tests/Discovery/StubGherkinDocument.cs index d39e84aa..e970b772 100644 --- a/Tests/Reqnroll.VisualStudio.Tests/Discovery/StubGherkinDocument.cs +++ b/Tests/Reqnroll.VisualStudio.Tests/Discovery/StubGherkinDocument.cs @@ -12,3 +12,24 @@ private StubGherkinDocument() public IGherkinDocumentContext Parent => null!; public object Node => null!; } + +public record StubGherkinDocumentWithScope : IGherkinDocumentContext +{ + public static StubGherkinDocumentWithScope Instance { get; } = new(); + + public IGherkinDocumentContext Parent => null; + + public object Node => new Scenario( + new[] + { + new Tag(new Gherkin.Ast.Location(0, 0), "@mytag1"), + new Tag(new Gherkin.Ast.Location(0, 0), "@mytag2") + }, + null, + "Scenario ", + "Scenario with Scopes", + null, + null, + null + ); +}