Warning for Implicit URL-String Comparisons

Related Projects Foundation
, ,
1

Warning for Implicit URL-String Comparisons

  • Proposal: TBD
  • Review Manager: TBD

Introduction

This proposal suggests adding a compiler warning when comparing URL objects directly with String literals, to enhance type safety and prevent potential errors.

Motivation

Currently, Swift allows implicit conversion between URL and String in comparisons due to URL's conformance to ExpressibleByStringLiteral. This can lead to unexpected behavior and potential bugs. For example:

let url = URL(string: "https://www.example.com")!
if url != "" {
    print("URL is not empty")
}

Proposed solution:

let url = URL(string: "https://www.example.com")!
if url != "" {
    // Warning: Comparing URL directly with String literal. 
    // Consider using 'url.absoluteString.isEmpty' for clearer intent.
    print("URL is not empty")
}

suggested alternatives:

if !url.absoluteString.isEmpty {
    print("URL is not empty")
}

or

if url != URL(string: "") {
    print("URL is not empty")
}

Detailed design

The compiler should emit a warning when it encounters a direct comparison between a URL and a String using the == or != operators. The warning message should guide developers towards more explicit comparisons.

Impact on existing code

This change would generate warnings in existing code that compares URLs directly with Strings. While it doesn't break existing code, it encourages developers to update their code for better clarity and type safety.

Alternatives considered

  1. Making the comparison a compiler error instead of a warning. This was deemed too severe as it would break existing code.
  2. Removing ExpressibleByStringLiteral conformance from URL. This was considered too drastic as it would significantly change how URLs can be created and used in Swift.
2

I'm not aware of any such conformance and your code doesn't compile for me even in Xcode 16.

import Foundation

let url = URL(string: "https://www.example.com")!
if url != "" { // error: referencing operator function '!=' on 'StringProtocol' requires that 'URL' conform to 'StringProtocol'
    print("URL is not empty")
}

Is it possible you're observing a retroactive conformance added by you or a teammate, or imported from a dependency? Or am I missing where this conformance was added?

3 Likes
3

Thanks for the reply, as i did not test it in xcode 16.
But in my project xcode 15 this code is compiling perfectly fine:

  private let iconURL: URL?
  private var showIcon: Bool {
    return iconURL != nil && iconURL != ""
  }
4

It doesn't work for me in Xcode 15 either. I suspect that somewhere in your project or dependency tree someone has added extension URL: ExpressibleByStringLiteral for 'convenience' which is leading to the behavior you're observing. If you do a global search for ExpressibleByStringLiteral in your project, does anything come up?

5

i have found these pieces of code from a third party pod:

extension Operand: ExpressibleByStringLiteral where T == String {
    public init(stringLiteral value: String) {
        self = Operand<String>.some(value)
    }
}

and

extension AnyCodable: ExpressibleByStringLiteral {
    public init(stringLiteral value: String) {
        self.init(value)
    }
}
6

Those don't look like they'd imbue URL with the conformance. If you invoke URL(stringLiteral: "") directly and then command-click to go to the definition of the init(stringLiteral:) for URL, where does it take you?

7

the same library but to different extension

extension URL : ExpressibleByStringLiteral {

    /// Creates an instance initialized to the given string value.
    ///
    /// - Parameter value: The value of the new instance.
    public init(stringLiteral value: StaticString)

    /// A type that represents an extended grapheme cluster literal.
    ///
    /// Valid types for `ExtendedGraphemeClusterLiteralType` are `Character`,
    /// `String`, and `StaticString`.
    public typealias ExtendedGraphemeClusterLiteralType = StaticString

    /// A type that represents a string literal.
    ///
    /// Valid types for `StringLiteralType` are `String` and `StaticString`.
    public typealias StringLiteralType = StaticString

    /// A type that represents a Unicode scalar literal.
    ///
    /// Valid types for `UnicodeScalarLiteralType` are `Unicode.Scalar`,
    /// `Character`, `String`, and `StaticString`.
    public typealias UnicodeScalarLiteralType = StaticString
}
8

Yep, that'll do it. This is 'bad behavior' by the library in question precisely because a retroactive conformance like this will 'infect' any project which imports the library (not to mention that another library could provide a different conformance of URL to ExpressibleByStringLiteral!)

Unfortunately if this is third-party code there's not gonna be a great workaround here other than 'live with the conformance', 'fork the library to remove the conformance', or 'use a different library'. It's just the nature of conformances in Swift that they act globally like this.

4 Likes
9

That conformance, frankly, is a bug in whatever library you're using.

The general rule is "don't conform types you don't own to protocols you don't own".

7 Likes
10

...and if you do, it's on you, but don't vend it to your clients!

5 Likes
11

In principle yes, but there is a compiler issue here: if you conform a public type to a public protocol, the compiler forces you to make the conformance itself public. :sob:

3 Likes
12

Only because there's (yet) no notion of a 'scoped' conformance in the language. I've always interpreted that rule as acting as a bit of an extra guardrail: indicating to the author that you are actually committing to something which has public-level effects (as well as reserving room if we ever in the future decided to add a scoped conformance feature--we won't have to either change the meaning of existing retroactive conformances or compromise on the rule that public is always explicit).

1 Like
13

Well yes, as of now I think it simply boils down to not doing this as a library author! Which of course is not the nicest thing the language can do but library authors do take on many responsibilities of that ilk that end clients do not, and this isn't nearly the most burdensome of them.

Scoped conformances is the big hammer, but merely not transitively exporting extensions would solve this problem if the library author takes care to modularize their no-good-but-internally-used retroactive conformances, and I believe we have something already pitched there don't we? If it doesn't address retroactive conformances but only the extension members when directly invoked, we ought to see if we can roll this in.

1 Like
14

You mentioned that 'not transitively exporting extensions' could be a potential solution if the library author properly modularizes their internal retroactive conformances. Could you elaborate on this approach? Is there a specific pitch or proposal for this feature that I could look into?

Also, as a developer using such a library, what would be the best practice to handle this situation in my code? Should I simply be more explicit in my use of URL and avoid relying on the problematic conformance, or are there other strategies you'd recommend?

15

I was thinking about this pitch, if the author of LibraryA separates their conformance into a LibraryARetroactiveConformances module, then imports that into their own code without using @_exported to re-export to you.

Just the options @Jumhyn writes, I'm afraid: live with it, fork it, or don't use it. Because the API here is implicitly invoked, it'll be hard to just avoid because it's not something you can easily lint against, though.

A retroactive conformance of a type you don't own to a protocol you don't own, which provides implicit functionality, that is exported to clients is close to the supreme example of what should not be done and you've found it.

5 Likes
16

In addition to solutions in the language itself such as scoped conformances and selective import of conformances or extension members, I thought about a simpler solution - give the compiler a separate file describing which functions should be hidden from the interface of an imported module. In the past, we caught too many crashes related to calls to functions like Array.init(repeating:count:) or BinaryInteger.init<T: BinaryInteger>(_ source: T). We can't remove them from the interface, but we can provide an option to hide them when compiling a specific module.
This can be achieved with a linter to some degree but some invocations are too hard to catch without a fully typed AST.