swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.66k stars 10.38k forks source link

[SR-14125] Provide specialized fix-its for protocol conformance errors in common cases #56506

Open typesanitizer opened 3 years ago

typesanitizer commented 3 years ago
Previous ID SR-14125
Radar rdar://problem/73742498
Original Reporter @typesanitizer
Type Improvement
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Improvement, DiagnosticsQoI, TypeChecker | |Assignee | None | |Priority | Medium | md5: 04c805de4b322dab5e8bba333a62556a

Issue Description:

struct S<T>: Equatable { // error
  var name: String
  var t: T
}

This offers to add protocol stubs, which is quite likely the wrong thing to do. Instead, it should be offering a fix-it that deletes the : Equatable and adds an extension for the synthesis:

extension S: Equatable where T: Equatable { } 

Similarly, in the case where you have code like:

struct S { // forgot to write Equatable
  var x: Int
}

struct T: Equatable { // error
  var s: S
}

This also offers to add protocol stubs, which IMO is the unlikely solution if the non-conforming property is defined in the same module. Rather, it should offer to add : Equatable to S's declaration.

typesanitizer commented 3 years ago

@swift-ci create

CodaFi commented 3 years ago

I disagree that protocol stubs are wrong here, in both cases. Consider

struct S<T>: Equatable {
  var id: UUID
  var t: T
}

Which is more rhetorical than it needs to be, as even the leading string member from before would do for an externally-imposed notion of identity. We could offer a conditional conformance in simple cases, but it becomes unclear in others. For example, suppose the user were to write their own (ordered) multi-map type

struct Pair<One, Two> { /* ... */ }

We can offer

extension Pair where One: Equatable { /* ... */ }
extension Pair where Two: Equatable { /* ... */ }
extension Pair where One: Equatable, Two: Equatable  { /* ... */ }

To you and I it is obvious which one of these is intended, but in the general case, the resulting decision space size is combinatorial in the number of generic parameters, no?

typesanitizer commented 3 years ago

I understand what you're getting at, but I feel like 80%-90% of the time, what I want is the compiler to synthesize things for me. I very rarely want to write my own Equatable. Not just in Swift, even in Haskell or Rust, I almost always want the compiler to derive Eq and Ord.

That said, I don't think it's productive for us to go back and forth on this if it's a disagreement on how frequently one case happens vs the other. I can take a look at some open source code when I have some time to spare and count when one situation happens vs the other.

ahoppen commented 3 years ago

FWIW, I think we should offer both alternatives as fix-its and let the user choose whether he wants to make all the members Equatable or add the stub.

typesanitizer commented 3 years ago

That's also a reasonable alternative IMO, but Xcode doesn't currently have a good way to surface things as alternatives in the fix-it UI (I think?), so it's difficult to convey that those are alternatives rather than multiple things that need to be fixed.

ahoppen commented 3 years ago

It does, you just need to attach multiple notes to an error/warning and each note can have its own fix-it. We already do that, e.g. when you access a member of an Optional without unwrapping it.