Add unstable option `self_shorthand`

[ytmimi]

trafficstars

This new option allows rustfmt to convert explicit self: Self types in method definitions to self:

• self: Self -> self
• self: &Self -> &self
• self: &'a Self -> &'a self
• mut self: Self -> mut self
• self: &mut Self -> &mut self
• self: &'a mut Self -> &'a mut self

~Note: Don't merge until a tracking issue has been added for self_shorthand.~ Closes #5257

[calebcartwright]

Note: Don't merge until a tracking issue has been added for self_shorthand.

FWIW in general I don't mind sequencing these as we can always create the issue and then update the markdown file in a subsequent PR too

[calebcartwright]

There's a few additional things we'll need to consider:

• The underlying ty can be Self or something that dereferences to it. That means we need to account for some of others, e.g. Box<Self> where I don't think we can apply the behavior in a semantics-preserving manner
• I feel like we may be re-implementing chunks of existing code that handled the MutTy formatting (e.g. lifetimes, mutability, etc.) but we weren't handling some of the edgier comment cases that covers. Instead of duplicating, what if we just extracted the relevant bits from the code that handled the rptr variant, and then reuse them from both the ty rewrite and here?
• Would be good to add some additional test cases with comments in those cases for that very reason

[calebcartwright]

Think this enumeration should serve as a good reference https://doc.rust-lang.org/stable/reference/items/associated-items.html#methods:

// Examples of methods implemented on struct `Example`.
struct Example;
type Alias = Example;
trait Trait { type Output; }
impl Trait for Example { type Output = Example; }
impl Example {
    fn by_value(self: Self) {}
    fn by_ref(self: &Self) {}
    fn by_ref_mut(self: &mut Self) {}
    fn by_box(self: Box<Self>) {}
    fn by_rc(self: Rc<Self>) {}
    fn by_arc(self: Arc<Self>) {}
    fn by_pin(self: Pin<&Self>) {}
    fn explicit_type(self: Arc<Example>) {}
    fn with_lifetime<'a>(self: &'a Self) {}
    fn nested<'a>(self: &mut &'a Arc<Rc<Box<Alias>>>) {}
    fn via_projection(self: <Example as Trait>::Output) {}
}

[ytmimi]

Thanks for the review and the great example. I'll put some time aside soon to make the updates!

[ytmimi]

@calebcartwright ready for a re-review on this one (assuming CI passes). I've gone ahead and implemented your suggestions, and to hopefully help with the review I've broken things up into smaller, more logical commits. I think the PR would now be best reviewed one commit at a time.

[calebcartwright]

Thanks. I don't have enough time to really go through this but a quick glance at b7b1083f0da6efc11c4298de5dd0417ce1434439 makes me wonder whether that commit may also be changing logic? It certainly seems to be doing more than a direct extraction which was what was expecting based on the commit message content

[ytmimi]

a quick glance at https://github.com/rust-lang/rustfmt/commit/b7b1083f0da6efc11c4298de5dd0417ce1434439 makes me wonder whether that commit may also be changing logic? It certainly seems to be doing more than a direct extraction which was what was expecting based on the commit message content

Fair point. The changes that make it more than a direct extraction of the logic are renaming cmnt_lo to comment_lo and generalizing how we find comment_lo and optionally writing the &:

    let mut comment_lo = if has_ref {
        context.snippet_provider.span_after(span, "&")
    } else if mutability == ast::Mutability::Mut {
        let lo = context.snippet_provider.span_after(span, "mut");
        lo - BytePos::from_usize(3)
    } else {
        span.lo()
    };

    if has_ref {
        result.push('&');
    }

I think what happened was that after I had worked on this I reset the head to orgin/master, and as I started grouping changes into commits those were incorrectly added. Propbably would have been more appropriate to include these changes in a7102a65d65d4b, so looks like I still have some work to do 😅.

Thanks for the quick review on this and I plan to rebase the changes to correct this.