[repo-assist] refactor: replace Regex.Replace with String.Replace for path parameter substitution + test coverage (+7 tests)#431
Open
github-actions[bot] wants to merge 3 commits into
Conversation
…bstitution
Path parameter substitution used Regex.Replace with a literal pattern like
{petId}. This required escaping '$' in the replacement value to prevent
regex back-reference interpretation ($0, $& etc.).
String.Replace is simpler, faster, and does not interpret any characters
in the replacement string specially. The behaviour is identical: both
replace every occurrence of the literal token in the path template.
Removes the System.Text.RegularExpressions open and the intermediate
escaped expression, reducing generated code complexity and eliminating
a (small) regex engine overhead per path parameter per API call.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d auto-generated names (+7 tests)
Fills three gaps in the OperationCompiler unit test suite:
1. Multiple path parameters (/users/{userId}/posts/{postId}): verify
both params appear in the signature, are required, and CancellationToken
comes last.
2. PATCH operation: verify a PATCH endpoint with path param + JSON body
generates the expected method with correct parameter order.
3. Auto-generated operation name (no operationId): verify compilation
succeeds and the generated method has the expected parameter signature
(categoryId + CancellationToken) when no operationId is specified.
Total: 389 → 396 tests (7 new).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the design-time operation code generation to use String.Replace instead of Regex.Replace when substituting {path} parameters in generated client methods, and adds new unit tests covering additional operation-compilation scenarios.
Changes:
- Replaced regex-based path parameter substitution with
String.Replace, removing the need for regex imports and$-escaping in replacements. - Added new operation-compilation tests covering multiple path parameters, PATCH operations, and missing
operationIdhandling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/SwaggerProvider.DesignTime/OperationCompiler.fs | Removes System.Text.RegularExpressions usage and switches path param substitution to String.Replace. |
| tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs | Adds 7 tests for multiple path params, PATCH signature generation, and missing operationId cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+796
to
+797
| let allMethods = types |> List.collect(fun t -> t.GetMethods() |> Array.toList) | ||
| allMethods |> List.isEmpty |> shouldEqual false |
| let lastParam = parameters |> Array.last | ||
|
|
||
| lastParam.ParameterType | ||
| |> shouldEqual typeof<Threading.CancellationToken> |
Comment on lines
+759
to
+760
| lastParam.ParameterType | ||
| |> shouldEqual typeof<Threading.CancellationToken> |
Comment on lines
+811
to
+812
| && ps[0].Name = "categoryId" | ||
| && ps[1].ParameterType = typeof<Threading.CancellationToken>) |
Member
|
@copilot fix inline comments, format code and ensure that it build and tests work |
Agent-Logs-Url: https://github.com/fsprojects/SwaggerProvider/sessions/34acd7e2-8a36-4b5a-8066-c5ee6fa4e67e Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Contributor
Done in 59fde7b. I fixed the inline review items in
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary
Two related improvements to
OperationCompiler.fsand its test suite.1. Remove
Regex.Replacefor path parameter substitutionPath parameter substitution in generated client code used
Regex.Replacewith a literal pattern like{petId}. This required:open System.Text.RegularExpressionsimport$characters in the replacement value (to avoid regex back-reference interpretation of$0,$&, etc.)Before:
After:
String.Replaceis simpler, faster, and does not interpret any characters in the replacement string specially. The behaviour is identical since the pattern is always a literal string like{petId}— no regex features are used.Net change: −4 lines; removes
open System.Text.RegularExpressions, theescapedintermediate expression, and the$ → $$escaping step.2. New OperationCompiler tests (+7 tests, 389 → 396)
Fills three gaps in the unit test suite:
/users/{userId}/posts/{postId}): verifies both params appear in the method signature, are required (not optional), and CancellationToken comes last.operationId): verifies the compiler handles a missingoperationIdgracefully and generates a method with the correct parameters.Test Status
✅ All 396 unit tests pass after the changes.