URI Templating #1198

danieleggert · 2025-03-04T10:10:15Z

This adds RFC 6570 URI templates to the Swift URL type.

Evolution proposal: #1186

jrflat · 2025-03-05T20:52:54Z

itingliu · 2025-03-24T20:17:59Z

parkera

Could you also add a benchmark suite for this functionality? You can pretty easily get a start by copying one of the existing formatting ones.

Sources/FoundationEssentials/URL/URITemplate.swift

parkera · 2025-03-25T16:29:17Z

Sources/FoundationEssentials/URL/URITemplate.swift

+    /// The template string needs to be a valid RFC 6570 template.
+    ///
+    /// This will parse the template and throw an error if the template is invalid.
+    public init(_ template: String) throws {


Do we need to add availability annotations to this, too, even though the type it is an extension of is new?

IIRC, having @available on the type is sufficient.

Sources/FoundationEssentials/URL/URITemplate_Expression.swift

parkera · 2025-03-25T16:31:01Z

Sources/FoundationEssentials/URL/URITemplate_Expression.swift

+    }
+}
+
+private struct InvalidTemplateExpression: Swift.Error {


Hm, I wonder how we can make it easier to ensure this error isn't leaked into any public functions.

None of the public functions are throwing. That should be enough, no?

Sources/FoundationEssentials/URL/URITemplate_Substitution.swift

parkera · 2025-03-25T16:33:35Z

Sources/FoundationEssentials/URL/URITemplate_VariableName.swift

+// MARK: -
+
+extension String {
+    public init(_ key: URL.Template.VariableName) {


Was this part of the proposal? Adding a public, no-arg init to String seems very broad.

Oh, I see it now. I'm not sure about it now that I see it in code. I know it is strongly typed, but it can cause confusion in documentation or at call sites. How critical is it to do this conversion in this way at a call site?

Yes, these were part of the review.

Omitting the first argument label is what’s recommended by the API Design Guidelines for initializers that perform value preserving type conversions.

I’m not sure what alternative would be better.

@parkera would removing this from the implementation require another round of review/block this PR? Or could we maybe mark it internal for now?

@danieleggert does URL.Template.VariableName conform with ExpressibleByStringLiteral?

I think the right way forward is to remove ExpressibleByStringLiteral. This is only truly useful in small demo code. In “real” code, you’d usually have a fixed set of variable names, and would probably do something like:

extension URL.Template.VariableName { static var query: URL.Template.VariableName { URL.Template.VariableName("query") } static var number: URL.Template.VariableName { URL.Template.VariableName("number") } }

and then use that when constructing the dictionary.

Agreed, or even

static var query: URL.Template.VariableName { .init("query") }

Tests/FoundationEssentialsTests/URITemplatingTests/URITemplate_ExpressionTests.swift

jrflat

Thanks for pitching and implementing this! I left some comments, some nits and some questions on whether we can merge percent-encoding functionality with implementations in URLParser.swift.

Another minor nit: would we want to rename the files to URLTemplate... given that the new type is based on URL? (I realize the RFC does name it "URI".)

I think we will also need to add the sources to CMakeLists in the URL folder.

Sources/FoundationEssentials/URL/URITemplate_PercentEncoding.swift

Sources/FoundationEssentials/URL/URITemplate.swift

danieleggert · 2025-03-26T08:33:13Z

Could you also add a benchmark suite for this functionality? You can pretty easily get a start by copying one of the existing formatting ones.

The info about building and running Benchmarks is broken / missing / misleading.

SWIFTCI_USE_LOCAL_DEPS=.. swift package benchmark

doesn’t work, it needs to be ../.. — and then (because the top-level Package.swift also uses SWIFTCI_USE_LOCAL_DEPS, the top-level project also wants to use local dependencies.

I worked around it with local modificatoins to the Package.swift, but this could use some TLC.

jrflat · 2025-03-26T17:36:15Z

@swift-ci please test

Sources/FoundationEssentials/URL/URLTemplate_VariableName.swift

Sources/FoundationEssentials/URL/URLParser.swift

jrflat · 2025-03-27T06:57:21Z

@swift-ci please test

itingliu · 2025-04-01T15:56:12Z

@swift-ci please test

…leName

parkera

Looks good to me - @itingliu can you also review?

itingliu · 2025-04-16T18:17:28Z

@swift-ci please test

itingliu

LGTM in general. I left some comments, but feel free to address them in a follow up or dismiss them if not valid.

Sources/FoundationEssentials/URL/URLTemplate.swift

itingliu · 2025-04-17T15:59:47Z

Sources/FoundationEssentials/URL/URLTemplate.swift

+                let escaped = String(literal).normalizedAddingPercentEncoding(
+                    withAllowedCharacters: .unreservedReserved
+                )
+                elements.append(.literal(escaped))


Just a thought, not blocking: In copyLiteral and the regex matching function below, you're both working on Substring rather than String. And if I read it correctly, it looks like the associated String will later be used in other places that involves some Substring processing. That makes me wonder if it'd make sense to store the associated value of case literal() as Substring instead, so we can avoid Substring-String copying until the last minute, and if this would actually make a difference performance-wise

Good point. My thinking was, that it would be better to store String because the case literal() will stay around as part of the URL.Template — and that instance is likely to remain in-memory for a long time. I don’t want that to keep the complete input String alive.

itingliu · 2025-04-17T17:01:46Z

Sources/FoundationEssentials/URL/URLTemplate_Expression.swift

+}
+
+extension URL.Template.Expression {
+    init(_ input: String) throws {


As an enhancement: perhaps it's a good place to adopt typed throw?

This may also throw errors from RegEx parsing. I took a stab at using typed throws, but it ended up making the code more complex and more difficult to read.

Sources/FoundationEssentials/URL/URLTemplate_Expression.swift

itingliu · 2025-04-17T17:14:36Z

Sources/FoundationEssentials/URL/URLTemplate_PercentEncoding.swift

+        // But any `%` that are not part of a valid escape sequence, need to be encoded.
+        guard next != UInt8(ascii: "%") || remainingInput.count < 2 else {
+            // Is this a valid escape sequence?
+            if remainingInput[remainingInput.startIndex].isValidHexDigit && remainingInput[remainingInput.startIndex + 1].isValidHexDigit {


If remainingInput.count == 1, is it still fine to do remainingInput[remainingInput.startIndex + 1]?

The remainingInput.count < 2 in the guard protects against that. The // Is this a valid escape sequence? code will only run if next == UInt8(ascii: "%") && remainingInput.count >= 2.

My bad. I kept reading it as remainingInput.count > 2 🤦‍♀️

itingliu · 2025-04-17T17:17:49Z

Sources/FoundationEssentials/URL/URLTemplate_PercentEncoding.swift

+                index += 3
+            }
+        }
+        return String(decoding: outputBuffer[..<index], as: UTF8.self)


I can't tell from the code, but is it guaranteed that index <= outputButter.count?

itingliu · 2025-04-17T22:02:12Z

@swift-ci please test

danieleggert · 2025-04-18T18:04:25Z

@itingliu Thanks for the comments / feedback. I’ve pushed 2 additional commits.

itingliu · 2025-04-18T18:25:29Z

@swift-ci please test

`TestSupport` already defines a typealias for `Expression` as `public typealias Expression = Foundation.Expression`. Therefore the top level typealias `private typealias Expression = URL.Template.Expression` caused an error when `TestSupport` is imported. Move the typealias declaration to inside the test suite.

swiftlang#1198 added most of the new files, but missed `URLTemplate_Expression.swift`, add it.

…1262) #1198 added most of the new files, but missed `URLTemplate_Expression.swift`, add it.

Following swiftlang#1198 and swiftlang#1262, this is needed to build on Windows.

`TestSupport` already defines a typealias for `Expression` as `public typealias Expression = Foundation.Expression`. Therefore the top level typealias `private typealias Expression = URL.Template.Expression` caused an error when `TestSupport` is imported. Move the typealias declaration to inside the test suite.

…RAMEWORK (swiftlang#1261)" This reverts commit 8d63523.

This reverts commit a169ceb.

vanvoorden · 2025-06-27T18:22:40Z

I'm trying to run benchmarks locally from main and seeing a lot of errors like:

only available in macOS 26.0 or newer

AFAIK the benchmarks themselves do not look gated around that requirement? Was this going to be updated soon?

Initial implementation.

ecfbdea

danieleggert marked this pull request as ready for review March 4, 2025 10:18

Cleanup and fix normalizedAddingPercentEncoding()

e99a13d

Use failing initializer on URL instead of makeURL()

a05d9fe

parkera reviewed Mar 25, 2025

View reviewed changes

jrflat reviewed Mar 26, 2025

View reviewed changes

danieleggert added 15 commits March 26, 2025 15:51

Add standard copyright header

d55dd9d

Mark helper functions as fileprivate

42a196f

URL.Template.init() needs to be failable

90f2a27

Rename InvalidTemplateExpression → URL.Template.InvalidExpression

3b8b3cd

Add missing @available

09c613a

Update copyright header

e126dcf

Convert tests XCTest → Swift Testing

f0924dd

Use UInt8.isValidHexDigit

44b6511

Use URLParser.swift methods for unreserved + reserved characters

3e80fbc

Cleanup normalizedAddingPercentEncoding()

25029fb

Use String(decoding:as:)

cefea66

guard & white-space

d7bc3d9

Cleanup “defer”

9fbff6f

Rename files URI… → URL…

ab051c8

Add new files to CMakeLists.txt

0afd988

Add benchmarks.

1ac4137

danieleggert force-pushed the uri-templating-implementation branch from 0a7c43f to 1ac4137 Compare March 26, 2025 09:00

itingliu mentioned this pull request Mar 26, 2025

Benchmark info is broken / missing / misleading #1227

Open

itingliu reviewed Mar 26, 2025

View reviewed changes

Sources/FoundationEssentials/URL/URLTemplate_VariableName.swift Show resolved Hide resolved

Add missing @available, 2

ee2a8aa

jrflat reviewed Mar 27, 2025

View reviewed changes

Sources/FoundationEssentials/URL/URLParser.swift Show resolved Hide resolved

Fix doc comment.

888032a

danieleggert added 2 commits April 12, 2025 09:02

Remove ExpressibleByStringLiteral conformance for URL.Template.Variab…

7b14d0b

…leName

Improve documentation comments

6962eb4

parkera approved these changes Apr 14, 2025

View reviewed changes

Fix for 7b14d0b

6241e1d

itingliu approved these changes Apr 17, 2025

View reviewed changes

danieleggert added 2 commits April 18, 2025 10:47

Fix doc comment

8f3b452

Do not force unwrap “maximum length”

61d15da

itingliu merged commit a169ceb into swiftlang:main Apr 21, 2025
3 checks passed

bnbarham added a commit to bnbarham/swift-foundation that referenced this pull request Apr 22, 2025

Update CMakeLists.txt with missing URLTemplate_Expression.swift

d467ffb

swiftlang#1198 added most of the new files, but missed `URLTemplate_Expression.swift`, add it.

bnbarham mentioned this pull request Apr 22, 2025

Update CMakeLists.txt with missing URLTemplate_Expression.swift #1262

Merged

bnbarham added a commit that referenced this pull request Apr 22, 2025

Update CMakeLists.txt with missing URLTemplate_Expression.swift (#…

f53d198

…1262) #1198 added most of the new files, but missed `URLTemplate_Expression.swift`, add it.

danieleggert deleted the uri-templating-implementation branch April 22, 2025 14:27

Steelskin added a commit to Steelskin/swift-foundation that referenced this pull request Apr 22, 2025

Add swiftRegexBuilder dependency on Windows

284d7dd

Following swiftlang#1198 and swiftlang#1262, this is needed to build on Windows.

Steelskin mentioned this pull request Apr 22, 2025

Add swiftRegexBuilder dependency on Windows #1264

Closed

itingliu added a commit to itingliu/swift-foundation that referenced this pull request Apr 22, 2025

Revert "Follow up for swiftlang#1198: Fix test build for FOUNDATION_F…

8c0f8e6

…RAMEWORK (swiftlang#1261)" This reverts commit 8d63523.

itingliu added a commit to itingliu/swift-foundation that referenced this pull request Apr 22, 2025

Revert "URI Templating (swiftlang#1198)"

977d7a2

This reverts commit a169ceb.

vanvoorden mentioned this pull request Jun 27, 2025

benchmarks failing from main #1386

Closed

URI Templating #1198

URI Templating #1198

Uh oh!

Conversation

danieleggert commented Mar 4, 2025

Uh oh!

jrflat commented Mar 5, 2025

Uh oh!

itingliu commented Mar 24, 2025

Uh oh!

parkera left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jrflat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danieleggert commented Mar 26, 2025

Uh oh!

jrflat commented Mar 26, 2025

Uh oh!

Uh oh!

Uh oh!

jrflat commented Mar 27, 2025

Uh oh!

itingliu commented Apr 1, 2025

Uh oh!

parkera left a comment

Choose a reason for hiding this comment

Uh oh!

itingliu commented Apr 16, 2025

Uh oh!

itingliu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

vanvoorden commented Jun 27, 2025 •

edited

Loading