Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-5858] Incorrectly implementing LocalizedError protocols does not result in an error at compile time but an incorrect error localized description #4069

Closed
swift-ci opened this issue Sep 8, 2017 · 4 comments

Comments

@swift-ci
Copy link
Contributor

swift-ci commented Sep 8, 2017

Previous ID SR-5858
Radar rdar://problem/41721949
Original Reporter nacho4d (JIRA User)
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

Reproducable everywhere I have tried:

Stable Xcode 8.3
Latests Xcode 9 beta 6
Swift Dev. 4.0 (Sep 5, 2017) Platform: Linux (x86_64)

Additional Detail from JIRA
Votes 0
Component/s Compiler, Foundation
Labels Bug, TypeChecker
Assignee None
Priority Medium

md5: b7f836ae6e1516ca27059acef1da0130

relates to:

  • SR-522 Protocol funcs cannot have covariant returns

Issue Description:

As in the summary, incorrectly implementing LocalizedError protocol leads to an incorrect localizedDescription string. Instead of an incorrect "The operation couldn’t be completed" when I call localizedDescription I would expect the compiler to complain because my implementation is incorrect.

See below two implementations:

import Foundation

// This is the bad error.
enum BadError: LocalizedError {
    case myFailure
    // Note the return type is not Optional<String> but String (Due to a typo, bad copy-pasting, whatever)
    var errorDescription: String {
        switch self {
        case .myFailure: return "BadErrorMyFailure"
        }
    }
}

// This is the good error
enum GoodError: LocalizedError {
    case myFailure
    var errorDescription: String? {
        switch self {
        case .myFailure: return "GoodErrorMyFailure"
        }
    }
}

// No compiler error nothing. This compiles without problem in Swift3 and latest 4 too 
print(BadError.myFailure.localizedDescription)
// "The operation couldn’t be completed. (__lldb_expr_32.BadError error 0.)"
// I expect a error at compile time since `errorDescription:String? { get }` is expected but I am writing `errorDescription:String { get }` (without the question mark)

print(GoodError.myFailure.localizedDescription)
// "GoodErrorMyFailure"

Same example in swift sandbox

@belkadan
Copy link

belkadan commented Sep 8, 2017

@DougGregor, do you remember why LocalizedError is designed this way?

@belkadan
Copy link

belkadan commented Sep 8, 2017

(In general, we should also consider non-optional properties to satisfy optional ones. We have a separate bug for that.)

@swift-ci
Copy link
Contributor Author

swift-ci commented Sep 9, 2017

Comment by Guillermo Ignacio Enriquez Gutierrez (JIRA)

BTW, UITableViewDatasource has a method:

@DougGregor
Copy link
Member

This has been addressed by the “near-miss” diagnostics from Swift 4.2. If you separate out your protocol conformances into separate extensions, which we generally consider good style, you will get a warning:

enum BadError {
case myFailure
}

extension BadError: LocalizedError {
// Note the return type is not Optional<String> but String (Due to a typo, bad copy-pasting, whatever)
var errorDescription: String { // warning: property 'errorDescription' nearly matches defaulted requirement 'errorDescription' of protocol 'LocalizedError'
switch self {
case .myFailure: return "BadErrorMyFailure"
}
}
}

Your original example, however, will not produce the warning. We initially did produce a warning for your example (in the earlier versions), but it was too noisy. So, we produce the warning when the extension declares the conformance and has a near-meatch with that protocol.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants