Skip to content

Commit 0515a5f

Browse files
Restore concrete FileRange overloads on FileContext for binary compat (#2319)
* Restore concrete `FileRange` overloads on `FileContext` #2306 widened the parameters of `FileContext.GetText` and `GetTextLines` from the concrete `FileRange` class to the `IFileRange` interface so that `$Context.CurrentFile.GetText($Context.SelectedRange)` would resolve (since `SelectedRange` is typed as `IFileRange`). That fixed the script-side call, but it dropped the old concrete-typed method slots from the assembly. Downstream modules that compile against PSES, most notably SeeminglyScience/EditorServicesCommandSuite, bind to the published metadata at build time. A precompiled caller of the old `GetText(FileRange)` signature would throw `MissingMethodException` at runtime even though it recompiles cleanly, so this was a binary-breaking change. Restore `GetText(FileRange)` and `GetTextLines(FileRange)` alongside the `IFileRange` overloads. Each casts to `IFileRange` and delegates to the widened implementation, so old binaries keep resolving and behavior is unchanged. The added tests declare their argument as the concrete `FileRange` so overload resolution actually exercises the compatibility shims. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Document public API binary-compatibility rules for Copilot I missed that #2306 was binary-breaking until after it merged, so add a "Public API & Binary Compatibility" section to the Copilot instructions to catch the next one at review time. The key context the agent was missing: the public types under `Microsoft.PowerShell.EditorServices` aren't only reached through PowerShell script. Downstream modules compile against the shipped assemblies, and SeeminglyScience/EditorServicesCommandSuite in particular references the extension API surface (`FileContext`, `IFileRange`, `ILspCurrentFileContext`, `IEditorScriptFile`, ...) directly from C#. So widening or renaming an existing `public` member is source-compatible at most but binary-breaking. The section spells out the failure mode (`MissingMethodException`), the fix (add an overload that delegates rather than editing the signature in place), and the expectation to bind a regression test to the old concrete signature. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 857b4ea commit 0515a5f

3 files changed

Lines changed: 85 additions & 0 deletions

File tree

.github/copilot-instructions.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,36 @@ Handlers live under `Services/<Feature>/Handlers/` and follow a consistent patte
9292
- Roslynator analyzers are enabled for formatting and code quality.
9393
- Use `Microsoft.Extensions.Logging` (`ILogger<T>` via `ILoggerFactory`) for all logging.
9494

95+
### Public API & Binary Compatibility
96+
97+
The public types under `Microsoft.PowerShell.EditorServices` are not consumed only
98+
through PowerShell script. Downstream modules **compile against the shipped PSES
99+
assemblies** and bind to that metadata at build time. The most prominent example is
100+
[`SeeminglyScience/EditorServicesCommandSuite`](https://github.com/SeeminglyScience/EditorServicesCommandSuite),
101+
which references the extension API surface (`FileContext`, `EditorContext`, the
102+
`IFileRange`/`FilePosition` types, `ILspCurrentFileContext`, `IEditorScriptFile`,
103+
etc.) directly from C#.
104+
105+
Because of that, treat any change to an existing `public` member as potentially
106+
**binary breaking**, and review for it explicitly before merging:
107+
108+
- Changing the **type of a parameter** (even widening a concrete class to an
109+
interface it implements, e.g. `FileRange` -> `IFileRange`), the **return type**,
110+
the **parameter count/order**, or **renaming/removing** a public member is
111+
source-compatible at most but **binary-breaking**. The method's signature/metadata
112+
token changes, so a precompiled caller throws `MissingMethodException` at runtime
113+
even though it would recompile cleanly.
114+
- When you need a wider or different signature, **add an overload** instead of
115+
editing the existing one. Keep the original signature in place and have it delegate
116+
to the new implementation so existing binaries keep resolving. Cast as needed to
117+
pick the new overload from the old body (e.g. `Foo((IFileRange)range)`).
118+
- Add a regression test that binds to the **old, concrete signature** (declare the
119+
argument as the original parameter type, not the widened one) so overload
120+
resolution to the compatibility shim is actually exercised.
121+
- If a breaking change is genuinely unavoidable, call it out in the PR description
122+
as a binary breaking change so maintainers can weigh it and coordinate a
123+
downstream release.
124+
95125
### Testing
96126

97127
- **Framework:** xUnit with `Xunit.SkippableFact` for conditionally skipped tests.

src/PowerShellEditorServices/Extensions/FileContext.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,18 @@ public string GetText(IFileRange bufferRange)
112112
GetTextLines(bufferRange));
113113
}
114114

115+
/// <summary>
116+
/// Gets the file content in the specified range as a string.
117+
/// </summary>
118+
/// <param name="bufferRange">The buffer range for which content will be extracted.</param>
119+
/// <returns>A string with the specified range of content.</returns>
120+
/// <remarks>
121+
/// This overload preserves binary compatibility with callers compiled
122+
/// against the concrete <see cref="FileRange"/> parameter; it delegates
123+
/// to the <see cref="GetText(IFileRange)"/> overload.
124+
/// </remarks>
125+
public string GetText(FileRange bufferRange) => GetText((IFileRange)bufferRange);
126+
115127
/// <summary>
116128
/// Gets the complete file content as an array of strings.
117129
/// </summary>
@@ -125,6 +137,18 @@ public string GetText(IFileRange bufferRange)
125137
/// <returns>An array of strings, each representing a line in the file within the specified range.</returns>
126138
public string[] GetTextLines(IFileRange fileRange) => scriptFile.GetLinesInRange(fileRange.ToBufferRange());
127139

140+
/// <summary>
141+
/// Gets the file content in the specified range as an array of strings.
142+
/// </summary>
143+
/// <param name="fileRange">The buffer range for which content will be extracted.</param>
144+
/// <returns>An array of strings, each representing a line in the file within the specified range.</returns>
145+
/// <remarks>
146+
/// This overload preserves binary compatibility with callers compiled
147+
/// against the concrete <see cref="FileRange"/> parameter; it delegates
148+
/// to the <see cref="GetTextLines(IFileRange)"/> overload.
149+
/// </remarks>
150+
public string[] GetTextLines(FileRange fileRange) => GetTextLines((IFileRange)fileRange);
151+
128152
#endregion
129153

130154
#region Text Manipulation

test/PowerShellEditorServices.Test/Extensions/FileContextTests.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,5 +64,36 @@ public void CanGetTextFromConstructedFileRange()
6464

6565
Assert.Equal("Line Three", editorContext.CurrentFile.GetText(range));
6666
}
67+
68+
// The concrete FileRange overloads exist to preserve binary compatibility
69+
// with callers compiled before GetText/GetTextLines were widened to IFileRange.
70+
// Declaring the variable as FileRange (not IFileRange) selects those overloads.
71+
[Fact]
72+
public void CanGetTextFromConcreteFileRange()
73+
{
74+
EditorContext editorContext = CreateEditorContext(
75+
"Line One\nLine Two\nLine Three",
76+
BufferRange.None);
77+
78+
FileRange range = new(
79+
new Microsoft.PowerShell.EditorServices.Extensions.FilePosition(3, 1),
80+
new Microsoft.PowerShell.EditorServices.Extensions.FilePosition(3, 11));
81+
82+
Assert.Equal("Line Three", editorContext.CurrentFile.GetText(range));
83+
}
84+
85+
[Fact]
86+
public void CanGetTextLinesFromConcreteFileRange()
87+
{
88+
EditorContext editorContext = CreateEditorContext(
89+
"Line One\nLine Two\nLine Three",
90+
BufferRange.None);
91+
92+
FileRange range = new(
93+
new Microsoft.PowerShell.EditorServices.Extensions.FilePosition(1, 1),
94+
new Microsoft.PowerShell.EditorServices.Extensions.FilePosition(2, 9));
95+
96+
Assert.Equal(new[] { "Line One", "Line Two" }, editorContext.CurrentFile.GetTextLines(range));
97+
}
6798
}
6899
}

0 commit comments

Comments
 (0)