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

Show trait method signature when impl differs #42362

Merged
merged 1 commit into from
Jun 4, 2017
Merged

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jun 1, 2017

When the trait's span is available, it is already being used, add a
note for the cases where the span isn't available:

error[E0053]: method `fmt` has an incompatible type for trait
  --> $DIR/trait_type.rs:17:4
   |
17 |    fn fmt(&self, x: &str) -> () { }
   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability
   |
   = note: expected type `fn(&MyType, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`
              found type `fn(&MyType, &str)`

error[E0050]: method `fmt` has 1 parameter but the declaration in trait `std::fmt::Display::fmt` has 2
  --> $DIR/trait_type.rs:21:11
   |
21 |    fn fmt(&self) -> () { }
   |           ^^^^^ expected 2 parameters, found 1
   |
   = note: `fmt` from trait: `fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`

error[E0186]: method `fmt` has a `&self` declaration in the trait, but not in the impl
  --> $DIR/trait_type.rs:25:4
   |
25 |    fn fmt() -> () { }
   |    ^^^^^^^^^^^^^^^^^^ expected `&self` in impl
   |
   = note: `fmt` from trait: `fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`

error[E0046]: not all trait items implemented, missing: `fmt`
  --> $DIR/trait_type.rs:28:1
   |
28 | impl std::fmt::Display for MyType4 {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `fmt` in implementation
   |
   = note: `fmt` from trait: `fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`

Fix #28011.

@rust-highfive
Copy link
Collaborator

r? @arielb1

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

@shepmaster
Copy link
Member

Thanks @estebank ! We'll get a wonderful reviewer like @arielb1 or someone equally awesome on the case.

@shepmaster shepmaster added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 2, 2017
pub fn signature<'a, 'tcx>(&self, tcx: &TyCtxt<'a, 'tcx, 'tcx>) -> String {
match self.kind {
ty::AssociatedKind::Method => {
format!("{}", tcx.type_of(self.def_id).fn_sig().0)
Copy link
Contributor

@arielb1 arielb1 Jun 3, 2017

Choose a reason for hiding this comment

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

why the .0? It looks like a skip_binder that makes functions show as fn(&Self) rather than for<'a> fn(&'a Self). Could it be changed to skip_binder?

@arielb1
Copy link
Contributor

arielb1 commented Jun 3, 2017

Nice! r+ with skip_binder and comment instead of .0 (yes I know the .0 is in the original).

@Mark-Simulacrum
Copy link
Member

@bors r=arielb1

@bors
Copy link
Contributor

bors commented Jun 4, 2017

📌 Commit 98885c5 has been approved by arielb1

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 4, 2017
@arielb1
Copy link
Contributor

arielb1 commented Jun 4, 2017

@bors r-

with skip_binder and comment instead of .0

There should be comments for all skip_binder calls. For example, here we would want

    pub fn signature<'a, 'tcx>(&self, tcx: &TyCtxt<'a, 'tcx, 'tcx>) -> String {
        match self.kind {
            ty::AssociatedKind::Method => {
                format!("{}",
                    // We skip the binder here because the binder would deanonymize all
                    // late-bound regions, and we don't want method signatures to show up
                    // `as for<'r> fn(&'r MyType)`. Pretty-printing handles late-bound
                    // regions just fine, showing `fn(&MyType)`.
                    tcx.type_of(self.def_id).fn_sig().skip_binder())
            }
            ty::AssociatedKind::Type => format!("type {};", self.name.to_string()),
            ty::AssociatedKind::Const => {
                format!("const {}: {:?};", self.name.to_string(), tcx.type_of(self.def_id))
            }
        }
+    }

When the trait's span is available, it is already being used, add a
`note` for the cases where the span isn't available:

```
error[E0053]: method `fmt` has an incompatible type for trait
  --> $DIR/trait_type.rs:17:4
   |
17 |    fn fmt(&self, x: &str) -> () { }
   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability
   |
   = note: expected type `fn(&MyType, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`
              found type `fn(&MyType, &str)`

error[E0050]: method `fmt` has 1 parameter but the declaration in trait `std::fmt::Display::fmt` has 2
  --> $DIR/trait_type.rs:21:11
   |
21 |    fn fmt(&self) -> () { }
   |           ^^^^^ expected 2 parameters, found 1
   |
   = note: `fmt` from trait: `fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`

error[E0186]: method `fmt` has a `&self` declaration in the trait, but not in the impl
  --> $DIR/trait_type.rs:25:4
   |
25 |    fn fmt() -> () { }
   |    ^^^^^^^^^^^^^^^^^^ expected `&self` in impl
   |
   = note: `fmt` from trait: `fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`

error[E0046]: not all trait items implemented, missing: `fmt`
  --> $DIR/trait_type.rs:28:1
   |
28 | impl std::fmt::Display for MyType4 {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `fmt` in implementation
   |
   = note: `fmt` from trait: `fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`
```
@estebank
Copy link
Contributor Author

estebank commented Jun 4, 2017

@arielb1 forgot about the comment. Added the one proposed.

@bors r=arielb1

@bors
Copy link
Contributor

bors commented Jun 4, 2017

📌 Commit e324919 has been approved by arielb1

@bors
Copy link
Contributor

bors commented Jun 4, 2017

⌛ Testing commit e324919 with merge 0418fa9...

bors added a commit that referenced this pull request Jun 4, 2017
Show trait method signature when impl differs

When the trait's span is available, it is already being used, add a
`note` for the cases where the span isn't available:

<pre>
error[E0053]: <b>method `fmt` has an incompatible type for trait</b>
  --> $DIR/trait_type.rs:17:4
   |
17 |    fn fmt(&self, x: &str) -> () { }
   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability
   |
   = note: expected type `<b>fn(&MyType, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error></b>`
              found type `<b>fn(&MyType, &str)</b>`

error[E0050]: <b>method `fmt` has 1 parameter but the declaration in trait `std::fmt::Display::fmt` has 2</b>
  --> $DIR/trait_type.rs:21:11
   |
21 |    fn fmt(&self) -> () { }
   |           ^^^^^ expected 2 parameters, found 1
   |
   = note: `fmt` from trait: `<b>fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error></b>`

error[E0186]: <b>method `fmt` has a `&self` declaration in the trait, but not in the impl</b>
  --> $DIR/trait_type.rs:25:4
   |
25 |    fn fmt() -> () { }
   |    ^^^^^^^^^^^^^^^^^^ expected `&self` in impl
   |
   = note: `fmt` from trait: `<b>fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error></b>`

error[E0046]: <b>not all trait items implemented, missing: `fmt`</b>
  --> $DIR/trait_type.rs:28:1
   |
28 | impl std::fmt::Display for MyType4 {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `fmt` in implementation
   |
   = note: `fmt` from trait: `<b>fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error></b>`
</code></pre>

Fix #28011.
@bors
Copy link
Contributor

bors commented Jun 4, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 0418fa9 to master...

@bors bors merged commit e324919 into rust-lang:master Jun 4, 2017
@estebank estebank deleted the type branch November 9, 2023 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants