diff --git a/src/LSP/Reqnroll.IdeSupport.LSP.Core/Rename/FeatureStepTextBuilder.cs b/src/LSP/Reqnroll.IdeSupport.LSP.Core/Rename/FeatureStepTextBuilder.cs index 138876d1..e4e8dac7 100644 --- a/src/LSP/Reqnroll.IdeSupport.LSP.Core/Rename/FeatureStepTextBuilder.cs +++ b/src/LSP/Reqnroll.IdeSupport.LSP.Core/Rename/FeatureStepTextBuilder.cs @@ -231,6 +231,84 @@ public static string Build( return sb.ToString(); } + /// + /// Derives a new abstract expression from an in-place feature-file edit. and are the step's concrete text + /// (no keyword) before and after the user's edit in the rename dialog; is the binding's current abstract expression. The parameter + /// values are located in using 's static segments, then re-located verbatim in so the original parameter slots ({int}, a regex group, …) + /// can be preserved around whatever static wording the user typed. Returns when a parameter value can no longer be found verbatim in the edited + /// text — i.e. the user changed a parameter value rather than the step's wording, which + /// this flow does not support. + /// + public static string? DeriveExpressionFromEditedText(string oldExpression, string oldStepText, string newStepText) + { + var oldSegments = StepExpressionParameters.StaticSegments(oldExpression); + + // No parameters: the whole step text is static, so it becomes the new expression as-is. + if (oldSegments.Count == 1) + return newStepText; + + var prefix = oldSegments[0]; + var suffix = oldSegments[oldSegments.Count - 1]; + if (!oldStepText.StartsWith(prefix, StringComparison.Ordinal) || + !oldStepText.EndsWith(suffix, StringComparison.Ordinal)) + return null; + + var regionStart = prefix.Length; + var regionEnd = oldStepText.Length - suffix.Length; + if (regionEnd < regionStart) + return null; + + // Extract each parameter value from the original concrete text, in order. + var values = new List(); + var cursor = regionStart; + for (int i = 1; i < oldSegments.Count - 1; i++) + { + var seg = oldSegments[i]; + int idx = seg.Length == 0 + ? cursor + : oldStepText.IndexOf(seg, cursor, regionEnd - cursor, StringComparison.Ordinal); + if (idx < 0) + return null; + values.Add(oldStepText.Substring(cursor, idx - cursor)); + cursor = idx + seg.Length; + } + values.Add(oldStepText.Substring(cursor, regionEnd - cursor)); + + var slots = StepExpressionParameters.ExtractSlots(oldExpression); + if (slots.Count != values.Count) + return null; + + // Re-locate the same values, in order, within the edited text to recover the new + // static segments around them. + var searchFrom = 0; + var newSegments = new List(); + foreach (var value in values) + { + int idx = value.Length == 0 + ? searchFrom + : newStepText.IndexOf(value, searchFrom, StringComparison.Ordinal); + if (idx < 0) + return null; + newSegments.Add(newStepText.Substring(searchFrom, idx - searchFrom)); + searchFrom = idx + value.Length; + } + newSegments.Add(newStepText.Substring(searchFrom)); + + var result = new StringBuilder(); + for (int i = 0; i < newSegments.Count; i++) + { + result.Append(newSegments[i]); + if (i < slots.Count) + result.Append(slots[i]); + } + return result.ToString(); + } + private static readonly System.Text.RegularExpressions.Regex PlaceholderPattern = new(@"\<([^>]+)\>", System.Text.RegularExpressions.RegexOptions.Compiled); diff --git a/src/LSP/Reqnroll.IdeSupport.LSP.Server/Features/Rename/StepRenameHandler.cs b/src/LSP/Reqnroll.IdeSupport.LSP.Server/Features/Rename/StepRenameHandler.cs index a5e61ed6..0b375dc2 100644 --- a/src/LSP/Reqnroll.IdeSupport.LSP.Server/Features/Rename/StepRenameHandler.cs +++ b/src/LSP/Reqnroll.IdeSupport.LSP.Server/Features/Rename/StepRenameHandler.cs @@ -146,19 +146,24 @@ public StepRenameHandler( // subsequent textDocument/rename would fail with "Internal Error". if (path.EndsWith(".feature", StringComparison.OrdinalIgnoreCase)) { - var featureBindings = FindBindingsAtFeatureStep(uri, path, request.Position); + var featureBindings = FindBindingsAtFeatureStep(uri, path, request.Position, out var stepRange); if (featureBindings.Count == 0) { _logger.LogVerbose("StepRenameHandler: prepareRename — no defined binding at feature step position"); return Task.FromResult(null); } - var line = request.Position.Line; - return Task.FromResult(new LspRange + if (stepRange == null) { - Start = new Position(line, 0), - End = new Position(line, 200) - }); + // Should not happen alongside a non-empty featureBindings, but refuse rather + // than fall back to a whole-line range: that used to seed the dialog with the + // keyword/indentation, which then got duplicated when the resulting edit was + // applied at the step-text-only range HandleRenameAsync actually replaces. + _logger.LogVerbose("StepRenameHandler: prepareRename — matched a binding but could not resolve the step's text range"); + return Task.FromResult(null); + } + + return Task.FromResult(stepRange); } return Task.FromResult(null); @@ -268,14 +273,15 @@ public StepRenameHandler( bindingLocation = new SourceLocation(path, line, column); } - // ── 2. Validate new name ─────────────────────────────────────────────── var expression = binding.Expression ?? string.Empty; - var nameError = StepRenameValidator.ValidateNewName(expression, newName); - if (nameError != null) - { - _logger.LogVerbose($"StepRenameHandler: validation failed — {nameError.Message}"); - return null; - } + + // ── 2. Resolve feature step locations ────────────────────────────────── + var owners = _scopeManager.ResolveOwners(uri); + var projectFilter = owners.Count > 0 + ? owners.Select(p => new ProjectOwner(p.ProjectFullName, p.TargetFrameworkMoniker)).ToArray() + : null; + + var usages = _matchService.FindUsages(bindingLocation, projectFilter); // Resolve the live source expression once (preserves the original parameter syntax). // For a .cs-invoked rename this is the attribute string literal; otherwise it falls back @@ -284,13 +290,63 @@ public StepRenameHandler( var sourceLiteral = await FindAttributeLiteralAsync(uri, binding); var sourceExpression = sourceLiteral?.Token.ValueText ?? expression; - // ── 3. Resolve feature step locations ────────────────────────────────── - var owners = _scopeManager.ResolveOwners(uri); - var projectFilter = owners.Count > 0 - ? owners.Select(p => new ProjectOwner(p.ProjectFullName, p.TargetFrameworkMoniker)).ToArray() - : null; + // A .feature-triggered rename can arrive in two shapes, both via the same + // textDocument/rename call, with no protocol-level way to tell them apart: + // - VS Code's native F2 seeds the dialog via prepareRename's whole-line range, so + // `newName` comes back as concrete step text (real parameter values, e.g. + // "I have 10 cukes" rather than "I have {int} cukes"). Comparing that straight + // against the abstract expression always trips ValidateNewName's parameter-count + // check, silently discarding every rename of a parameterized step. + // - VS's custom "Rename Step" command builds its own prompt seeded with the binding's + // abstract expression (RenameStepCommand.cs), so `newName` already carries the + // correct placeholder syntax and needs no reconciliation — attempting it anyway would + // fail to find any parameter "value" to locate in already-abstract text and wrongly + // reject a rename that never needed fixing up. + // Try the abstract form first (matching parameter-slot count against the live source + // expression); only when that count differs do we attempt to derive the abstract + // expression by diffing the edited concrete text against the original. + var effectiveNewName = newName; + if (path.EndsWith(".feature", StringComparison.OrdinalIgnoreCase) && + StepExpressionParameters.ExtractSlots(newName).Count != StepExpressionParameters.ExtractSlots(sourceExpression).Count) + { + var currentUsage = usages.FirstOrDefault(u => + string.Equals(u.FeatureDocumentId, uri.ToString(), StringComparison.OrdinalIgnoreCase) && + u.Range != null && + request.Position.Line >= u.Range.ToLspRange().Start.Line && + request.Position.Line <= u.Range.ToLspRange().End.Line); + + var oldStepText = currentUsage?.Range != null + ? ReadStepText(uri, currentUsage.Range.ToLspRange()) + : null; + + if (oldStepText == null) + { + // Can't read the pre-edit step text (buffer and disk both unavailable) — fall + // back to treating newName as-is, same as before this reconciliation existed. + _logger.LogVerbose("StepRenameHandler: could not read original step text for the edited position — using newName as-is"); + } + else + { + var derived = FeatureStepTextBuilder.DeriveExpressionFromEditedText(sourceExpression, oldStepText, newName); + if (derived == null) + { + _logger.LogVerbose("StepRenameHandler: could not reconcile edited step text with the binding's parameter positions — the parameter values, not just the wording, appear to have changed"); + return null; + } + + effectiveNewName = derived; + _logger.LogVerbose($"StepRenameHandler: derived abstract expression '{effectiveNewName}' from edited step text '{newName}'"); + } + } + + // ── 3. Validate new name ─────────────────────────────────────────────── + var nameError = StepRenameValidator.ValidateNewName(expression, effectiveNewName); + if (nameError != null) + { + _logger.LogVerbose($"StepRenameHandler: validation failed — {nameError.Message}"); + return null; + } - var usages = _matchService.FindUsages(bindingLocation, projectFilter); var changes = new Dictionary>(); // ── 4. Build .feature file edits ─────────────────────────────────────── @@ -311,7 +367,7 @@ public StepRenameHandler( stepText = ReadStepText(featureUri, stepRange); } - var featureNewText = FeatureStepTextBuilder.Build(newName, sourceExpression, binding.Regex, stepText); + var featureNewText = FeatureStepTextBuilder.Build(effectiveNewName, sourceExpression, binding.Regex, stepText); list.Add(new TextEdit { Range = usage.Range!.ToLspRange(), @@ -322,7 +378,7 @@ public StepRenameHandler( // ── 5. Build .cs file edit ──────────────────────────────────────────── if (sourceLiteral != null) { - var csEdit = BuildCSharpEdit(sourceLiteral, newName); + var csEdit = BuildCSharpEdit(sourceLiteral, effectiveNewName); if (csEdit != null) { var csUri = path.EndsWith(".cs", StringComparison.OrdinalIgnoreCase) @@ -469,8 +525,22 @@ public StepRenameHandler( /// by querying the binding match cache for the owning projects. /// private List FindBindingsAtFeatureStep( - DocumentUri uri, string path, Position position) + DocumentUri uri, string path, Position position) => + FindBindingsAtFeatureStep(uri, path, position, out _); + + /// + /// Finds all bindings that match the feature step at the given cursor position, and the + /// matched step's own text span (excluding the keyword/indentation) via . Callers that only need the range for editing (prepareRename must + /// offer exactly the text that HandleRenameAsync will later replace at usage.Range — + /// otherwise the keyword/indentation the client seeds the dialog with gets duplicated when + /// the edit is applied) should use this overload. + /// + private List FindBindingsAtFeatureStep( + DocumentUri uri, string path, Position position, out LspRange? matchedRange) { + matchedRange = null; + var uriStr = uri.ToString(); var owners = _scopeManager.ResolveOwners(uri); if (owners.Count == 0) @@ -499,6 +569,7 @@ private List FindBindingsAtFeatureStep( var stepEndChar = (position.Line == endPos.Line) ? endPos.Character : int.MaxValue; if (position.Character >= stepStartChar && position.Character <= stepEndChar) { + matchedRange ??= step.Range.ToLspRange(); foreach (var item in step.Result.Items) { if (item.MatchedStepDefinition != null) diff --git a/src/VisualStudio/Reqnroll.IdeSupport.VisualStudio.Extension/RenameStep/WorkspaceEditApplier.cs b/src/VisualStudio/Reqnroll.IdeSupport.VisualStudio.Extension/RenameStep/WorkspaceEditApplier.cs index ffe0da2f..5b72480f 100644 --- a/src/VisualStudio/Reqnroll.IdeSupport.VisualStudio.Extension/RenameStep/WorkspaceEditApplier.cs +++ b/src/VisualStudio/Reqnroll.IdeSupport.VisualStudio.Extension/RenameStep/WorkspaceEditApplier.cs @@ -167,13 +167,26 @@ internal static string ApplyEditsToText(string fileText, IEnumerable + /// .cs files need this notification just as much as .feature files: the server's Roslyn + /// binding registry (CSharpBindingDiscoveryService) is only refreshed by textDocument/didOpen + /// or textDocument/didChange (TextDocumentSyncHandler) — there is no file-system watcher for + /// .cs content changes (WatchedFilesHandler only watches reqnroll.json/.editorconfig/output + /// assemblies, plus .cs *deletions*). Without this, a rename that rewrites a closed .cs file's + /// attribute leaves the registry stale — the renamed feature step shows unbound — until the + /// file happens to be opened, which triggers didOpen and a live re-parse. + /// + internal static bool ShouldNotifyDidChange(string localPath) => + localPath.EndsWith(".feature", StringComparison.OrdinalIgnoreCase) || + localPath.EndsWith(".cs", StringComparison.OrdinalIgnoreCase); + // ── Helpers ────────────────────────────────────────────────────────────── /// Applies a single to a VS text buffer. diff --git a/tests/LSP/Reqnroll.IdeSupport.LSP.Core.Tests/Rename/FeatureStepTextBuilderTests.cs b/tests/LSP/Reqnroll.IdeSupport.LSP.Core.Tests/Rename/FeatureStepTextBuilderTests.cs index 4d16593a..fe579be5 100644 --- a/tests/LSP/Reqnroll.IdeSupport.LSP.Core.Tests/Rename/FeatureStepTextBuilderTests.cs +++ b/tests/LSP/Reqnroll.IdeSupport.LSP.Core.Tests/Rename/FeatureStepTextBuilderTests.cs @@ -200,4 +200,48 @@ public void Build_outline_placeholder_count_mismatch_falls_through() result.Should().Be("the result is {int}"); } + + // ── DeriveExpressionFromEditedText: the inverse operation, used when a .feature-triggered + // rename hands back the edited concrete step text (real parameter values) instead of an + // abstract expression, so the rest of the rename pipeline can keep working with the + // abstract form. ───────────────────────────────────────────────────────────────────── + + [Fact] + public void DeriveExpressionFromEditedText_single_param_preserves_slot_and_wording_edit() + { + var result = FeatureStepTextBuilder.DeriveExpressionFromEditedText( + "I have {int} cukes", "I have 5 cukes", "I have 5 pickles"); + + result.Should().Be("I have {int} pickles"); + } + + [Fact] + public void DeriveExpressionFromEditedText_multiple_params_preserve_slots_in_order() + { + var result = FeatureStepTextBuilder.DeriveExpressionFromEditedText( + "the {int} was {string}", "the 3 was \"ok\"", "the 3 became \"ok\""); + + result.Should().Be("the {int} became {string}"); + } + + [Fact] + public void DeriveExpressionFromEditedText_no_parameters_returns_edited_text_verbatim() + { + var result = FeatureStepTextBuilder.DeriveExpressionFromEditedText( + "to be or not to be", "to be or not to be", "to be and not to be"); + + result.Should().Be("to be and not to be"); + } + + [Fact] + public void DeriveExpressionFromEditedText_parameter_value_changed_returns_null() + { + // The user changed the value (5 → 6), not just the wording — this rename flow only + // supports renaming the step's wording, so it should be rejected rather than silently + // renaming the binding to accept a different concrete value. + var result = FeatureStepTextBuilder.DeriveExpressionFromEditedText( + "I have {int} cukes", "I have 5 cukes", "I have 6 cukes"); + + result.Should().BeNull(); + } } diff --git a/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs/Features/Editor/RenameSteps.feature b/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs/Features/Editor/RenameSteps.feature index bc04efe6..e1a43743 100644 --- a/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs/Features/Editor/RenameSteps.feature +++ b/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs/Features/Editor/RenameSteps.feature @@ -130,6 +130,13 @@ Scenario: Renaming from a .feature file updates the attribute and all feature us """ Then the feature step "I press add" is reported as bound # Line 2 (0-based) is " When I press add" — cursor at col 9 is within the step text + # Regression: prepareRename used to return the whole line (column 0-200), so VS Code seeded + # the rename dialog with " When I press add" (keyword and indentation included). Submitting + # an edited copy of that back then duplicated the keyword when the resulting edit was applied + # only at the step-text-only range HandleRenameAsync actually replaces, producing + # " When When I choose add" in the feature file. + When prepare rename is requested at line 2 column 9 in "FeatureRename.feature" + Then the prepare rename range excludes the step keyword and indentation When rename is requested at line 2 column 9 in "FeatureRename.feature" with new name "I choose add" Then a workspace edit is returned And the workspace edit contains a change in "Steps.cs" @@ -137,6 +144,47 @@ Scenario: Renaming from a .feature file updates the attribute and all feature us And the workspace edit changes to "Steps.cs" include new text "I choose add" And the workspace edit changes to "FeatureRename.feature" include new text "I choose add" +# ── Rename from the .feature side, parameterized step ───────────────────────── +# Regression: VS Code seeds the .feature rename dialog with the step's concrete text (real +# parameter values, since prepareRename's range covers the whole line) — not the binding's +# abstract expression. The submitted "new name" is therefore concrete text too (e.g. +# "I have 5 pickles", not "I have {int} cukes"). The rename must reconcile that concrete +# edit back to an abstract expression before validating/propagating it, otherwise the +# parameter-count check always fails and the rename silently no-ops. + +Scenario: Renaming a parameterized step from the .feature file preserves the parameter slot + Given the LSP server is started + When the project is announced with output assembly "Sample.dll" for "ParamFeatureRename.feature" + # Use "opened and saved to disk with" so FindAttributeLiteralAsync can read the file from disk. + And the C# step definition file "Steps.cs" is opened and saved to disk with + """ + using Reqnroll; + namespace Sample + { + [Binding] + public class Steps + { + [Given("I have {int} cukes")] + public void GivenIHaveCukes(int count) { } + } + } + """ + And the feature file "ParamFeatureRename.feature" is opened with + """ + Feature: ParamFeatureRename + Scenario: S + Given I have 5 cukes + """ + Then the feature step "I have 5 cukes" is reported as bound + # Line 2 (0-based) is " Given I have 5 cukes" — cursor at col 20 is within "cukes". + # The dialog is seeded with the concrete line and only the static word "cukes" is edited. + When rename is requested at line 2 column 20 in "ParamFeatureRename.feature" with new name "I have 5 pickles" + Then a workspace edit is returned + And the workspace edit contains a change in "Steps.cs" + And the workspace edit contains a change in "ParamFeatureRename.feature" + And the workspace edit changes to "Steps.cs" include new text "I have {int} pickles" + And the workspace edit changes to "ParamFeatureRename.feature" include new text "I have 5 pickles" + # ── prepareRename for .feature: undefined step must block dialog ─────────────── Scenario: Rename is not available for an undefined step in a .feature file diff --git a/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs/StepDefinitions/RenameStepsSteps.cs b/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs/StepDefinitions/RenameStepsSteps.cs index 2ba57f8d..0691da6c 100644 --- a/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs/StepDefinitions/RenameStepsSteps.cs +++ b/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs/StepDefinitions/RenameStepsSteps.cs @@ -73,6 +73,18 @@ public void ThenNoPrepareRenameRangeIsReturned() "prepareRename should return null when the cursor is not on a step binding"); } + [Then("the prepare rename range excludes the step keyword and indentation")] + public void ThenThePrepareRenameRangeExcludesTheStepKeywordAndIndentation() + { + _ctx.LastPrepareRenameRange.Should().NotBeNull(); + _ctx.LastPrepareRenameRange!.Start.Character.Should().BeGreaterThan(0, + "a range starting at column 0 would seed the rename dialog with the keyword and " + + "indentation, which then duplicates when the resulting edit is applied at the " + + "step-text-only range HandleRenameAsync actually replaces"); + _ctx.LastPrepareRenameRange.End.Character.Should().NotBe(200, + "a synthetic whole-line (0-200) range was the bug this regression guards against"); + } + [Then(@"the workspace edit contains a change in ""(.*)""")] public void ThenWorkspaceEditContainsChangeIn(string fileName) { diff --git a/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Tests/Features/Rename/StepRenameHandlerTests.cs b/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Tests/Features/Rename/StepRenameHandlerTests.cs index 888939e7..5b9b639c 100644 --- a/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Tests/Features/Rename/StepRenameHandlerTests.cs +++ b/tests/LSP/Reqnroll.IdeSupport.LSP.Server.Tests/Features/Rename/StepRenameHandlerTests.cs @@ -380,6 +380,58 @@ private static LspReqnrollProject MakeTestProject() => }, Substitute.For()); + // ── Regression: prepareRename for a .feature step must return the step-text-only range + // (excluding the keyword and leading indentation), matching the range HandleRenameAsync + // later applies the edit at (usage.Range). A whole-line range used to seed the rename + // dialog with "\tThen the result should be 120"; submitting an edited version of that back + // duplicated the keyword when the edit was applied only at the step-text span, producing + // "\tThen \tThen the result should be 120" in the feature file. ────────────────────────── + + [Fact] + public async Task PrepareRename_from_feature_returns_step_text_range_excluding_keyword_and_indentation() + { + var featureUri = DocumentUri.FromFileSystemPath("/workspace/test.feature"); + var binding = MakeBinding( + ScenarioBlock.Then, + new Regex("^to be or not to be$"), + specifiedExpression: "to be or not to be", + line: 8, column: 9, + method: "Steps.ThenToBeOrNotToBe()"); + _registryLookup.GetRegistryForUri(Arg.Any()) + .Returns(ProjectBindingRegistry.FromBindings(new[] { binding })); + + var project = MakeTestProject(); + _scopeManager.ResolveOwners(featureUri).Returns(new[] { project }); + _scopeManager.GetProjectForUri(featureUri).Returns(project); + + var matchSet = MakeFeatureMatchSet( + featureUri.ToString(), binding, + "Then", "to be or not to be", stepLine: 2, stepChar: 5); + _matchService.TryGet(Arg.Any(), out Arg.Any()) + .Returns(ci => + { + ci[1] = matchSet; + return true; + }); + + var result = await CreateSut().HandlePrepareRenameAsync( + new PrepareRenameParams + { + TextDocument = new TextDocumentIdentifier { Uri = featureUri }, + Position = new Position(2, 10) + }, + CancellationToken.None); + + result.Should().NotBeNull(); + // MakeFeatureMatchSet builds the line as "\tThen to be or not to be" — the step text + // starts right after the tab + "Then " (6 chars), not at column 0. + result!.Start.Line.Should().Be(2); + result.Start.Character.Should().Be(6, + "the range must start at the step text, excluding the keyword and indentation"); + result.End.Character.Should().NotBe(200, + "a synthetic whole-line range was the bug this regression guards against"); + } + [Fact] public async Task RenameTargets_from_feature_returns_matched_binding() { @@ -593,6 +645,184 @@ public async Task Rename_from_feature_without_session_resolves_binding_via_match result.Changes.Should().ContainKey(csUri); } + // ── Regression: VS Code seeds the .feature rename dialog with the step's concrete text + // (real parameter values), not the abstract expression, since prepareRename returns a + // whole-line range. A rename submitted from the feature file must therefore be + // reconciled back to an abstract expression before validation/propagation — otherwise + // the parameter-count check always fails and the rename silently no-ops. ────────────── + + [Fact] + public async Task Rename_from_feature_with_concrete_parameter_value_updates_feature_and_csharp() + { + var featureUri = DocumentUri.FromFileSystemPath("/workspace/test.feature"); + var csUri = DocumentUri.FromFileSystemPath("/workspace/Steps.cs"); + + const string csText = + "using Reqnroll;\n" + + "namespace N\n" + + "{\n" + + " [Binding]\n" + + " public class Steps\n" + + " {\n" + + " [Given(\"I have {int} cukes\")]\n" + + " public void GivenIHaveCukes(int count) { }\n" + + " }\n" + + "}\n"; + + const string featureText = "Feature: F\nScenario: S\n\tGiven I have 5 cukes\n"; + SetupBuffers((csUri, csText), (featureUri, featureText)); + + var binding = MakeBinding( + ScenarioBlock.Given, + new Regex("^I have (-?\\d+) cukes$"), + specifiedExpression: "I have {int} cukes", + line: 8, column: 9, + method: "Steps.GivenIHaveCukes()"); + _registryLookup.GetRegistryForUri(Arg.Any()) + .Returns(ProjectBindingRegistry.FromBindings(new[] { binding })); + + var project = MakeTestProject(); + _scopeManager.ResolveOwners(featureUri).Returns(new[] { project }); + + var matchSet = MakeFeatureMatchSet( + featureUri.ToString(), binding, + "Given", "I have 5 cukes", stepLine: 2, stepChar: 5); + _matchService.TryGet(Arg.Any(), out Arg.Any()) + .Returns(ci => + { + ci[1] = matchSet; + return true; + }); + + var snapshot = new LspTextSnapshot(featureUri.ToString(), 1, featureText); + const string stepText = "I have 5 cukes"; + var stepOffset = featureText.IndexOf("\tGiven " + stepText) + "\tGiven ".Length; + var usageMatch = new StepBindingMatch( + featureUri.ToString(), + GherkinRange.FromPoint(snapshot, startOffset: stepOffset, length: stepText.Length), + MatchResult.CreateMultiMatch(new[] + { + MatchResultItem.CreateMatch(binding, ParameterMatch.NotMatch) + })); + _matchService.FindUsages(Arg.Any(), Arg.Any>()) + .Returns(new[] { usageMatch }); + + var sut = CreateSut(); + + // Simulates F2 on "cukes": VS Code seeds the dialog with the whole concrete line and the + // user edits only the static wording, keeping the parameter value (5) untouched. + var result = await sut.HandleRenameAsync( + new RenameParams + { + TextDocument = new TextDocumentIdentifier { Uri = featureUri }, + Position = new Position(2, 15), + NewName = "I have 5 pickles" + }, + CancellationToken.None); + + result.Should().NotBeNull("a parameterized step rename from the .feature file must not silently no-op"); + result!.Changes!.Should().ContainKey(featureUri); + result.Changes.Should().ContainKey(csUri); + + var featureEdit = result.Changes[featureUri].ToList(); + featureEdit.Should().ContainSingle(); + featureEdit[0].NewText.Should().Be("I have 5 pickles", + "the concrete parameter value must be preserved in the feature file"); + + var csEdit = result.Changes[csUri].ToList(); + csEdit.Should().ContainSingle(); + csEdit[0].NewText.Should().Be("\"I have {int} pickles\"", + "the {int} parameter type must be preserved in the binding attribute, not the concrete value 5"); + } + + [Fact] + public async Task Rename_from_feature_with_already_abstract_new_name_is_used_as_is() + { + // VS's custom "Rename Step" command (RenameStepCommand.cs) seeds its own prompt with the + // binding's abstract expression (placeholders intact) regardless of whether the cursor was + // in the .cs or .feature file, then submits that abstract text verbatim as `newName`. This + // must not be mistaken for VS Code's concrete-text submission and rejected/mangled by the + // parameter-value reconciliation added for that case. + var featureUri = DocumentUri.FromFileSystemPath("/workspace/test.feature"); + var csUri = DocumentUri.FromFileSystemPath("/workspace/Steps.cs"); + + const string csText = + "using Reqnroll;\n" + + "namespace N\n" + + "{\n" + + " [Binding]\n" + + " public class Steps\n" + + " {\n" + + " [Given(\"I have {int} cukes\")]\n" + + " public void GivenIHaveCukes(int count) { }\n" + + " }\n" + + "}\n"; + + const string featureText = "Feature: F\nScenario: S\n\tGiven I have 5 cukes\n"; + SetupBuffers((csUri, csText), (featureUri, featureText)); + + var binding = MakeBinding( + ScenarioBlock.Given, + new Regex("^I have (-?\\d+) cukes$"), + specifiedExpression: "I have {int} cukes", + line: 8, column: 9, + method: "Steps.GivenIHaveCukes()"); + _registryLookup.GetRegistryForUri(Arg.Any()) + .Returns(ProjectBindingRegistry.FromBindings(new[] { binding })); + + var project = MakeTestProject(); + _scopeManager.ResolveOwners(featureUri).Returns(new[] { project }); + + var matchSet = MakeFeatureMatchSet( + featureUri.ToString(), binding, + "Given", "I have 5 cukes", stepLine: 2, stepChar: 5); + _matchService.TryGet(Arg.Any(), out Arg.Any()) + .Returns(ci => + { + ci[1] = matchSet; + return true; + }); + + var snapshot = new LspTextSnapshot(featureUri.ToString(), 1, featureText); + const string stepText = "I have 5 cukes"; + var stepOffset = featureText.IndexOf("\tGiven " + stepText) + "\tGiven ".Length; + var usageMatch = new StepBindingMatch( + featureUri.ToString(), + GherkinRange.FromPoint(snapshot, startOffset: stepOffset, length: stepText.Length), + MatchResult.CreateMultiMatch(new[] + { + MatchResultItem.CreateMatch(binding, ParameterMatch.NotMatch) + })); + _matchService.FindUsages(Arg.Any(), Arg.Any>()) + .Returns(new[] { usageMatch }); + + var sut = CreateSut(); + + // Simulates VS's custom command: the prompt was seeded with, and the user edited, the + // abstract expression "I have {int} cukes" directly — not the concrete feature line. + var result = await sut.HandleRenameAsync( + new RenameParams + { + TextDocument = new TextDocumentIdentifier { Uri = featureUri }, + Position = new Position(2, 15), + NewName = "I have {int} pickles" + }, + CancellationToken.None); + + result.Should().NotBeNull("an already-abstract newName must not be rejected as an unreconcilable parameter-value change"); + result!.Changes!.Should().ContainKey(featureUri); + result.Changes.Should().ContainKey(csUri); + + var featureEdit = result.Changes[featureUri].ToList(); + featureEdit.Should().ContainSingle(); + featureEdit[0].NewText.Should().Be("I have 5 pickles", + "the concrete parameter value must still be preserved in the feature file"); + + var csEdit = result.Changes[csUri].ToList(); + csEdit.Should().ContainSingle(); + csEdit[0].NewText.Should().Be("\"I have {int} pickles\""); + } + [Fact] public async Task FindAttributeLiteralAsync_redirects_from_feature_to_csharp_source() { diff --git a/tests/VisualStudio/Reqnroll.VisualStudio.Tests/RenameStep/WorkspaceEditApplierTests.cs b/tests/VisualStudio/Reqnroll.VisualStudio.Tests/RenameStep/WorkspaceEditApplierTests.cs index 7a9b0ed3..6cbde35d 100644 --- a/tests/VisualStudio/Reqnroll.VisualStudio.Tests/RenameStep/WorkspaceEditApplierTests.cs +++ b/tests/VisualStudio/Reqnroll.VisualStudio.Tests/RenameStep/WorkspaceEditApplierTests.cs @@ -87,4 +87,18 @@ public void Crlf_line_endings_are_handled() result.Should().Be(Nl("line0", "Bye", "line2")); } + + // ── ShouldNotifyDidChange: a closed .cs file rewritten by ApplyToDisk must be reported to + // the server too, or its Roslyn binding registry goes stale until the file is reopened. ── + + [Theory] + [InlineData(@"C:\repo\Features\Calculator.feature", true)] + [InlineData(@"C:\repo\StepDefinitions\CalculatorStepDefinitions.cs", true)] + [InlineData(@"C:\repo\StepDefinitions\CalculatorStepDefinitions.CS", true)] + [InlineData(@"C:\repo\reqnroll.json", false)] + [InlineData(@"C:\repo\Notes.txt", false)] + public void ShouldNotifyDidChange_covers_feature_and_cs_files_only(string path, bool expected) + { + WorkspaceEditApplier.ShouldNotifyDidChange(path).Should().Be(expected); + } }