[swift-dev] RFC: "Near-miss" checking for defaulted protocol requirements

Douglas Gregor dgregor at apple.com
Fri Apr 22 17:33:43 CDT 2016 

Hi all,

A common complaint with protocol conformance checking is that it’s easy to make a little mistake when trying to implement a protocol requirement that has a default implementation. Here is a silly example:

protocol P { 
  func foo(value: Float)
}

extension P {
  func foo(value: Float) { } // default implementation of P.foo(value:)
}

struct S { }

extension S : P {
  func foo(value: Double) { } // intended-but-incorrect attempt to satisfy the requirement P.foo(value:)
}

S satisfies the requirement for P.foo(value:) using the implementation from the protocol extension, although it certainly looks like the user intended to provide a different implementation.

This kind of problem has prompted repeated calls for some kind of “implements” keyword to indicate that one is intending to implement a protocol requirement. I, personally, *really* don’t want yet another decorator keyword to indicate the intent here, because I believe the user has already indicated intent by stating conformance to the protocol P. I’ve recently committed a number of changes that provide “near-miss” checking for optional requirements of @objc protocols (which have the same problem).

It might be worth enabling this functionality for cases like the above as well. The Swift compiler patch to do so is attached, and will produce the following warning for the code above:

t2.swift:12:8: warning: instance method 'foo(value:)' nearly matches optional requirement 'foo(value:)' of protocol 'P'
  func foo(value: Double) { }
       ^
t2.swift:12:8: note: candidate has non-matching type '(value: Double) -> ()'
  func foo(value: Double) { }
       ^
t2.swift:12:8: note: move 'foo(value:)' to another extension to silence this warning
  func foo(value: Double) { }
       ^
t2.swift:12:8: note: make 'foo(value:)' private to silence this warning
  func foo(value: Double) { }
       ^
  private 
t2.swift:2:8: note: requirement 'foo(value:)' declared here
  func foo(value: Float)
       ^

It’s unfortunate that it’s a lot of notes. The first says what is wrong with the candidate (and there is much we could do to improve the precision of this diagnostic!), while the next two are mitigation strategies: move it to another extension (which implies that it’s not part of the conformance to P) or explicitly mark it as having less visibility than the conformance (in this case, private), which feels like a good way to state “I intend this to be a helper”. This might nudge Swift developers toward a style where they write one conformance per extension, but I don’t think that’s necessarily a bad thing: it’s a fine way to organize code.

Naturally, this handles typos as well, e.g.,

t2.swift:12:8: warning: instance method 'foob(value:)' nearly matches optional requirement 'foo(value:)' of protocol 'P'
  func foob(value: Float) { }
       ^
t2.swift:12:8: note: rename to 'foo(value:)' to satisfy this requirement
  func foob(value: Float) { }
       ^~~~
       foo

Running this on the standard library produces a number of results:

/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: warning: instance method 'removeLast()' nearly matches optional requirement 'removeFirst()' of protocol 'RangeReplaceableCollection'
  public mutating func removeLast() -> Element {
                       ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: note: rename to 'removeFirst()' to satisfy this requirement
  public mutating func removeLast() -> Element {
                       ^~~~~~~~~~
                       removeFirst
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: note: move 'removeLast()' to another extension to silence this warning
  public mutating func removeLast() -> Element {
                       ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/RangeReplaceableCollection.swift:158:17: note: requirement 'removeFirst()' declared here
  mutating func removeFirst() -> Iterator.Element
                ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: warning: instance method 'removeLast()' nearly matches optional requirement 'removeFirst()' of protocol 'RangeReplaceableCollection'
  public mutating func removeLast() -> Element {
                       ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: note: rename to 'removeFirst()' to satisfy this requirement
  public mutating func removeLast() -> Element {
                       ^~~~~~~~~~
                       removeFirst
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: note: move 'removeLast()' to another extension to silence this warning
  public mutating func removeLast() -> Element {
                       ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/RangeReplaceableCollection.swift:158:17: note: requirement 'removeFirst()' declared here
  mutating func removeFirst() -> Iterator.Element
                ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: warning: instance method 'removeLast()' nearly matches optional requirement 'removeFirst()' of protocol 'RangeReplaceableCollection'
  public mutating func removeLast() -> Element {
                       ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: note: rename to 'removeFirst()' to satisfy this requirement
  public mutating func removeLast() -> Element {
                       ^~~~~~~~~~
                       removeFirst
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: note: move 'removeLast()' to another extension to silence this warning
  public mutating func removeLast() -> Element {
                       ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/RangeReplaceableCollection.swift:158:17: note: requirement 'removeFirst()' declared here
  mutating func removeFirst() -> Iterator.Element
                ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/HashedCollections.swift.gyb:1077:10: warning: subscript 'subscript' nearly matches optional requirement 'subscript' of protocol 'Collection'
  public subscript(key: Key) -> Value? {
         ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/HashedCollections.swift.gyb:1077:10: note: candidate has non-matching type 'Key -> Value?' [with Iterator = DictionaryIterator<Key, Value>, SubSequence = Slice<Dictionary<Key, Value>>]
  public subscript(key: Key) -> Value? {
         ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/HashedCollections.swift.gyb:1077:10: note: move 'subscript' to an extension to silence this warning
  public subscript(key: Key) -> Value? {
         ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Collection.swift:147:3: note: requirement 'subscript' declared here
  subscript(bounds: Range<Index>) -> SubSequence { get }
  ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Range.swift:116:10: warning: subscript 'subscript' nearly matches optional requirement 'subscript' of protocol 'Collection'
  public subscript(_: Element._DisabledRangeIndex) -> Element {
         ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Range.swift:116:10: note: candidate has non-matching type 'Element._DisabledRangeIndex -> Element' [with Iterator = RangeIterator<Element>, SubSequence = Slice<Range<Element>>]
  public subscript(_: Element._DisabledRangeIndex) -> Element {
         ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Range.swift:116:10: note: move 'subscript' to an extension to silence this warning
  public subscript(_: Element._DisabledRangeIndex) -> Element {
         ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Collection.swift:147:3: note: requirement 'subscript' declared here
  subscript(bounds: Range<Index>) -> SubSequence { get }
  ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: warning: instance method '_distance(to:)' nearly matches optional requirement 'distance(to:)' of protocol 'ForwardIndex'
  public func _distance(to other: AnyForwardIndex) -> AnyForwardIndex.Distance {
              ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: note: rename to 'distance(to:)' to satisfy this requirement [with _DisabledRangeIndex = _DisabledRangeIndex_]
  public func _distance(to other: AnyForwardIndex) -> AnyForwardIndex.Distance {
              ^~~~~~~~~
              distance
/Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: note: move '_distance(to:)' to an extension to silence this warning
  public func _distance(to other: AnyForwardIndex) -> AnyForwardIndex.Distance {
              ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Index.swift:180:8: note: requirement 'distance(to:)' declared here
  func distance(to end: Self) -> Distance
       ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: warning: instance method '_distance(to:)' nearly matches optional requirement 'distance(to:)' of protocol 'ForwardIndex'
  public func _distance(to other: AnyBidirectionalIndex) -> AnyBidirectionalIndex.Distance {
              ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: note: rename to 'distance(to:)' to satisfy this requirement [with _DisabledRangeIndex = _DisabledRangeIndex_]
  public func _distance(to other: AnyBidirectionalIndex) -> AnyBidirectionalIndex.Distance {
              ^~~~~~~~~
              distance
/Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: note: move '_distance(to:)' to an extension to silence this warning
  public func _distance(to other: AnyBidirectionalIndex) -> AnyBidirectionalIndex.Distance {
              ^
/Users/dgregor/Projects/swift/swift/stdlib/public/core/Index.swift:180:8: note: requirement 'distance(to:)' declared here
  func distance(to end: Self) -> Distance
       ^


It’s somewhat frustrating that these are *all* false positives. However, they seem like “reasonable” false positives, in the sense that they’re close enough to the requirement to be justifiable, and the suggested recovery strategies look acceptable.

Thoughts? Should we turn this on?

	- Doug

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-dev/attachments/20160422/a0726059/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: default-impl-near-miss.patch
Type: application/octet-stream
Size: 627 bytes
Desc: not available
URL: <https://lists.swift.org/pipermail/swift-dev/attachments/20160422/a0726059/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-dev/attachments/20160422/a0726059/attachment-0001.html>

More information about the swift-dev mailing list