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

Add E0609 #42585

Merged
merged 2 commits into from
Jun 12, 2017
Merged

Add E0609 #42585

merged 2 commits into from
Jun 12, 2017

Conversation

GuillaumeGomez
Copy link
Member

Part of #42229.

cc @Susurrus

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -4095,6 +4095,33 @@ assert_eq!(!Question::No, true);
```
"##,

E0609: r##"
An attempt to get a field which doesn't exist was performed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "Attempted to access a non-existent field in a struct" reads better?

Copy link
Member Author

Choose a reason for hiding this comment

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

What about "An attempt to access a non-existent field in a struct"?

println!("{}", s.foo); // error: no field `foo` on type `StructWithFields`
```

To fix this error, check that if you didn't mispell the field's name or that
Copy link
Contributor

Choose a reason for hiding this comment

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

"misspell"

Remove the "if" as well.

@GuillaumeGomez
Copy link
Member Author

Updated.

@Susurrus
Copy link
Contributor

I really don't like the phrasing "An attempt ... was performed". It reads awkwardly. I think using "Attempted to ..." reads better overall.

Additionally it appears that this error covers two situations: One where they improperly use field syntax on a type (currently non-structs-or-tuples) and another error when the struct does not contain the given field. And my understanding is that error is separate from when that field is private as well. I think these errors would be more clear if E0609 was split into two errors: one for when it's invalid for the type to have field access performed on it and another for when a struct or tuple doesn't have the field being referenced.

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 11, 2017
@GuillaumeGomez
Copy link
Member Author

I splitted the error into two.

println!("{}", s.foo); // error: no field `foo` on type `StructWithFields`
```

To fix this error, check if you didn't misspell the field's name or that the
Copy link
Contributor

Choose a reason for hiding this comment

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

"check that you didn't misspell the field's name or that the field actually exists"


```compile_fail,E0610
let x: u32 = 0;
println!("{}", x.foo); // error: `{interger}` is a primitive type, therefore
Copy link
Contributor

Choose a reason for hiding this comment

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

"integer"

```

Primitive types are the most basic types available in Rust and don't have
fields. If you want to use fields, take a look to the structs. Example:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know how the user would accidentally access a field of a variable when coding. I would think the most likely reason would be they a) didn't know the datatype of the variable they used or b) accidentally used the wrong variable. I don't know if suggesting the user looks at structs will be helpful in them resolving this error. I think this extra description may be unnecessary.

In case you want to keep it, I'd reword the second sentence as it reads as being very informal. Maybe instead "To access data via named fields, struct types are used."?


// We create an instance of this struct:
let variable = Foo { x: 0, y: -12 };
// And we can now call its fields:
Copy link
Contributor

Choose a reason for hiding this comment

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

"call" is usually reserved for use with functions/methods. I'd suggest "access" instead.

println!("x: {}, y: {}", variable.x, variable.y);
```

For more information, go read The Rust Book: https://doc.rust-lang.org/book/
Copy link
Contributor

Choose a reason for hiding this comment

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

"go read" sounds like a derogatory command. I'd suggest "see" instead (and drop the comma).

err
} else {
type_error_struct!(self.tcx().sess, field.span, expr_t, E0610,
"`{}` is a primitive type, therefore doesn't have fields",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest replacing the comma with "and"

@GuillaumeGomez
Copy link
Member Author

Updated.

println!("x: {}, y: {}", variable.x, variable.y);
```

For more information, see The Rust Book: https://doc.rust-lang.org/book/
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the comma as it's unnecessary.

@@ -4096,7 +4096,7 @@ assert_eq!(!Question::No, true);
"##,

E0609: r##"
An attempt to access a non-existent field in a struct was performed.
Attempted to access a non-existent field in a struct was performed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "was performed", the sentence doesn't make sense given that it now starts with "Attempted"

@GuillaumeGomez
Copy link
Member Author

Updated.

@Susurrus
Copy link
Contributor

LGTM 👍

@GuillaumeGomez
Copy link
Member Author

@bors: r=Susurrus

@bors
Copy link
Contributor

bors commented Jun 11, 2017

📌 Commit 2f37894 has been approved by Susurrus

@bors
Copy link
Contributor

bors commented Jun 11, 2017

⌛ Testing commit 2f37894 with merge 29ef412...

bors added a commit that referenced this pull request Jun 11, 2017
@bors
Copy link
Contributor

bors commented Jun 12, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: Susurrus
Pushing 29ef412 to master...

@bors bors merged commit 2f37894 into rust-lang:master Jun 12, 2017
This was referenced Jun 12, 2017
@GuillaumeGomez GuillaumeGomez deleted the E0609 branch June 12, 2017 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants