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-12311] Improve diagnostic for read-only properties #54741

Closed
typesanitizer opened this issue Mar 3, 2020 · 5 comments
Closed

[SR-12311] Improve diagnostic for read-only properties #54741

typesanitizer opened this issue Mar 3, 2020 · 5 comments
Labels
compiler The Swift compiler in itself good first issue Good for newcomers improvement

Comments

@typesanitizer
Copy link

Previous ID SR-12311
Radar rdar://problem/60013553
Original Reporter @typesanitizer
Type Improvement
Status Closed
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Improvement, StarterBug
Assignee hassaneldesouky (JIRA)
Priority Medium

md5: 5490ca2514b7d3781b548e117ac40d68

Issue Description:

If you write some code like

protocol P {
  let x : Int // error: immutable property requirement must be declared as 'var' with a '{ get }' specifier
}

This diagnostic is not quite accurate. There is 1 of 2 things the user is trying to do:

1. They really want an immutable property. This is impossible given current restrictions, because `let` implies that the property is a stored property (it is not possible today to determine if the computation is pure, if computed `let` properties were to be allowed) and protocols cannot enforce whether the witnesses are stored or computed.
2. They want a read-only property. This can be solved with `var { get }`.

The diagnostic is saying that if you want to get 1., use the solution for 2., which is not quite right. Instead, we should be clearer about potential things they want and what the solutions are. At the very least, mixing those two things up doesn’t feel right.

We should change the diagnostic to something like:

protocol P {
  let x : Int // protocols cannot require properties to be immutable; declare read-only properties by using 'var' with a '{ get }' specifier
}

You can grep the codebase to find where the error text is and then replace it with the suggested text (or if you have a better idea, feel free to suggest that 🙂).

Please make sure that you update any test cases or add one if we are missing a test case for this diagnostic.

You can @ me (varungandhi-apple) on GitHub for review.

@typesanitizer
Copy link
Author

@swift-ci create

@theblixguy
Copy link
Collaborator

Hmm, I think the reason why the diagnostic is worded in that way is because the user can still implement it as a let which makes it immutable and which is what I believe most of them would do with a var foo: Type {get} requirement anyway. So, I think it might be worth keeping that in mind when changing the diagnostic wording.

@theblixguy
Copy link
Collaborator

Maybe we can keep the wording as what you’ve suggested, but perhaps add further explanation as part of educational notes.

@typesanitizer
Copy link
Author

Yeah, I don't think this diagnostic specifically is the right place for that (because the diagnostic is about the protocol requirement, not the witness), but if we are adding extra informative notes, we can add the information about different potential witnesses.

@swift-ci
Copy link
Collaborator

swift-ci commented Mar 9, 2020

Comment by Hassan ElDesouky (JIRA)

Successfully merged at #30287

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler The Swift compiler in itself good first issue Good for newcomers improvement
Projects
None yet
Development

No branches or pull requests

3 participants