-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Improve E0308 error message wording #70242
Merged
+36
−36
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a typo?
on the one hand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just my two cents,
on one hand
andon the other hand
may not be easily understood by non-native english speakers. Maybe it's just me but I learned that late,and
is easier to be understood.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to contribute a better message with no "on the other hand." But I got side-tracked and honestly a bit discouraged, since there's nobody to workshop good messages with, and no standards of what a good error message looks like.
I'd like there to be a working group to bounce ideas off of, and/or a style guide.
Here's what I had, roughly:
Possibly a "hint" line about dynamic typing versus static typing:
See prior discussion here if you want: https://users.rust-lang.org/t/looking-for-feedback-on-an-improved-error-message-for-e0308/40004
I actually think the phrasing can be much simplified.
Any two mis-matching types assigned to the same variable cause this error. Regardless of explicit or inferred, at or after initialization, etc... Just... two non-matching types, one variable.
(Apparently there is a thing called shadowing, but if I read correctly, I believe it just spins up a new variable with a different compiled identity, but re-using the same name in source code. New instance of the variable can have a new type. It's still static typing under the hood, though. I am afraid of over-simplifying, and ending up with a message that is inaccurate regarding shadowing.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is better than the merged one I think but still kinda long-winded, I think documentation should be short, concise and understandable. What I would say a good example is IETF RFC docs, rust docs is good enough for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reply.
I have some things to do at the moment, but I will post that version as a second PR when I get the chance. Hopefully before a new stable release.
It could probably be shortened, mostly by removing the "common example" of when this error occurs. (It is kind of arbitrary, and I think it might be overly specific when the error is more general than that.) But I don't want to be too drastic while I am still getting used to contributing to Rust.
Posting this for reference since you mentioned IETF RFCs as being a good example: https://tools.ietf.org/html/rfc7322 The IETF RFC Style Guide.
Maybe that can be an influence for a Rust docs "style guide" or best practices guide. No time now, but planning to look that over later.