Skip to content

[CSDiagnostics] Emit fix-its to insert requirement stubs for missing protocols in context #33628

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

Merged
merged 2 commits into from
Aug 26, 2020
Merged

[CSDiagnostics] Emit fix-its to insert requirement stubs for missing protocols in context #33628

merged 2 commits into from
Aug 26, 2020

Conversation

theblixguy
Copy link
Collaborator

This is a follow-up to #27026.


At the moment, we emit a fix-it to insert the missing conformance. For example:

protocol P {
  func method()
}

class C {
  var p: P?
  
  func assign() {
    p = self
  }
}

Here, we emit fix-it to add : P to C. With this change, we now also emit the requirement stub (func method()) if we're in editor mode.

@theblixguy theblixguy requested review from xedin and hborla August 25, 2020 16:07
@hborla
Copy link
Member

hborla commented Aug 25, 2020

Will this still add the requirement stub if the type already implements the requirement?

@theblixguy
Copy link
Collaborator Author

theblixguy commented Aug 25, 2020

Yeah, it will, which is unfortunate. I am assuming that in most cases there won't be any conflicts. There is filterProtocolRequirements which seems to remove conflicting declarations. Let me give that a try.

…missing protocols in context

The conforming type may already have declarations that could satisfy a requirement, so we shouldn't include it in the fix-it
@theblixguy
Copy link
Collaborator Author

Oh never mind, I misunderstood the role of filterProtocolRequirements (it filters conflicting requirements, it doesn't filter requirements that are already implemented).

I figured out another way using ConformanceChecker which seems to work. Let me know what you think of this approach.

@xedin
Copy link
Contributor

xedin commented Aug 25, 2020

@swift-ci please build toolchain macOS

@theblixguy
Copy link
Collaborator Author

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just wanted to give it a try and see where it would insert these stubs

@theblixguy
Copy link
Collaborator Author

@swift-ci please smoke test

@theblixguy
Copy link
Collaborator Author

@swift-ci please smoke test macOS

@theblixguy theblixguy merged commit 79b0a7e into swiftlang:master Aug 26, 2020
@theblixguy theblixguy deleted the chore/improve-protocol-conformance-fixits branch August 26, 2020 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants