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

[John McCall]

> On Apr 22, 2016, at 3:33 PM, Douglas Gregor via swift-dev <swift-dev at swift.org> wrote:
> 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
>                 ^

Would a word-by-word edit-distance heuristic work better?  That is, removeFirst is not a plausible typo for removeLast because First is not a plausible typo for Last.

Admittedly, you might need to do something else if the word counts don't match.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-dev/attachments/20160422/c77b4ba1/attachment.html>