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

normalize match reference style, #929 #2697

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions Configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -1885,6 +1885,52 @@ fn main() {

See also: [`match_block_trailing_comma`](#match_block_trailing_comma).

## `match_reference_style`

Normalize match reference style

- **Default value**: `manual`
- **Possible values**: `manual`, `reference`, `dereference`, `auto`
- **Stable**: No

**Note:** `auto` style requires Rust v1.26.0 or later

#### `manual`

don't touch anything

#### `reference`

```rust
fn hello(name: &Option<&str>) {
match name {
&Some(name) => println!("Hello {}!", name),
&None => println!("I don't know who you are."),
}
}
```

#### `dereference`

```rust
fn hello(name: &Option<&str>) {
match *name {
Some(name) => println!("Hello {}!", name),
None => println!("I don't know who you are."),
}
}
```

#### `auto`

```rust
fn hello(name: &Option<&str>) {
match name {
Some(name) => println!("Hello {}!", name),
None => println!("I don't know who you are."),
}
}
```

## `blank_lines_upper_bound`

Expand Down
2 changes: 2 additions & 0 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ create_config! {
threshold.";
match_arm_blocks: bool, true, false, "Wrap the body of arms in blocks when it does not fit on \
the same line with the pattern of arms";
match_reference_style: MatchReferenceStyle, MatchReferenceStyle::Manual, false,
"Normalize match reference style";
force_multiline_blocks: bool, false, false,
"Force multiline closure bodies and match arms to be wrapped in a block";
fn_args_density: Density, Density::Tall, false, "Argument density in functions";
Expand Down
19 changes: 19 additions & 0 deletions src/config/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,25 @@ configuration_option_enum! { IndentStyle:
Block,
}

configuration_option_enum! { MatchReferenceStyle:
Manual,
// match x {
// &A => ...,
// ...
// }
Reference,
// match *x {
// A => ...,
// ...
// }
Dereference,
// match x {
// A => ...,
Copy link
Member

Choose a reason for hiding this comment

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

I think this should &A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rust 1.26 has stabilized match_default_bindings

the compiler automatically references the Some, and since we’re borrowing, name is bound as ref name automatically as well. If we were mutating:

fn hello(arg: &mut Option<String>) {
    match arg {
        Some(name) => name.push_str(", world"),
        None => (),
    }
}

the compiler will automatically borrow by mutable reference, and name will be bound as ref mut too.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, then should it be *x? In any case the comments here don't seem to match the examples in the PR message. Perhaps it would be good to explain the reference/dereference modes here.

What is the motivation for having four modes? I would have thought that auto and manual would be the most widely used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before Rust 1.26, you can choose reference/dereference modes as you wish, just different code style in same semantic in most cases.

the compiler automatically references the Some, and since we’re borrowing, name is bound as ref name automatically as well.

Since Rust 1.26, the compiler could do smarter, it can automatic handle the reference/dereference modes base on the type inferencing, that's why we need a new auto mode.

fn hello(name: &Option<&str>) {
    match name {
        Some(name) => println!("Hello {}!", name),
        None => println!("I don't know who you are."),
    }
}

fn main() {
    let name = Some("world");

    hello(&name);
}

So, if we use Rust 1.26 or later, just set the auto mode and migrate the old code, let's compiler do the dirty works.

Copy link
Member

Choose a reason for hiding this comment

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

Right, but since 1.26 is stable now, I think we could just offer auto and manual, and we don't need to offer ref and deref styles. Do you think there is an advantage to continue to offer ref and deref?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is some projects may still using the old Rust version, they may need to align the code style with rustfmt for ref/deref styles.

// ...
// }
Auto
}

configuration_option_enum! { Density:
// Fit as much on one line as possible.
Compressed,
Expand Down
124 changes: 121 additions & 3 deletions src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use syntax::{ast, ptr};

use codemap::SpanUtils;
use comment::combine_strs_with_missing_comments;
use config::{Config, ControlBraceStyle, IndentStyle};
use config::{Config, ControlBraceStyle, IndentStyle, MatchReferenceStyle};
use expr::{
format_expr, is_empty_block, is_simple_block, is_unsafe_block, prefer_next_line,
rewrite_multiple_patterns, ExprType, RhsTactics, ToExpr,
Expand Down Expand Up @@ -91,7 +91,7 @@ pub fn rewrite_match(
IndentStyle::Visual => cond_shape.shrink_left(6)?,
IndentStyle::Block => cond_shape.offset_left(6)?,
};
let cond_str = cond.rewrite(context, cond_shape)?;
let mut cond_str = cond.rewrite(context, cond_shape)?;
let alt_block_sep = &shape.indent.to_string_with_newline(context.config);
let block_sep = match context.config.control_brace_style() {
ControlBraceStyle::AlwaysNextLine => alt_block_sep,
Expand Down Expand Up @@ -138,13 +138,28 @@ pub fn rewrite_match(
}
} else {
let span_after_cond = mk_sp(cond.span.hi(), span.hi());
let mut arms_str =
rewrite_match_arms(context, arms, shape, span_after_cond, open_brace_pos)?;

rewrite_match_reference_style(
context,
cond,
arms,
shape,
cond_shape,
span_after_cond,
open_brace_pos,
&mut cond_str,
&mut arms_str,
)?;

Some(format!(
"match {}{}{{\n{}{}{}\n{}}}",
cond_str,
block_sep,
inner_attrs_str,
nested_indent_str,
rewrite_match_arms(context, arms, shape, span_after_cond, open_brace_pos)?,
arms_str,
shape.indent.to_string(context.config),
))
}
Expand Down Expand Up @@ -183,6 +198,109 @@ fn collect_beginning_verts(
beginning_verts
}

fn rewrite_match_reference_style(
context: &RewriteContext,
cond: &ast::Expr,
arms: &[ast::Arm],
shape: Shape,
cond_shape: Shape,
span_after_cond: Span,
open_brace_pos: BytePos,
cond_str: &mut String,
arms_str: &mut String,
) -> Option<()> {
let match_in_reference_style = arms.iter().all(|arm| {
arm.pats.iter().all(|pat| match pat.node {
ast::PatKind::Wild | ast::PatKind::Ref(..) => true,
_ => false,
})
});
let match_in_derefrence_style = match cond.node {
ast::ExprKind::Unary(ast::UnOp::Deref, _) => true,
_ => false,
};

let prepend_arms_ref = || {
arms.iter()
.map(|arm| {
let pats = arm
.pats
.iter()
.map(|pat| {
ptr::P(ast::Pat {
id: pat.id,
node: ast::PatKind::Ref(pat.clone(), ast::Mutability::Immutable),
span: pat.span,
})
})
.collect::<Vec<_>>();

ast::Arm {
pats,
..arm.clone()
}
})
.collect::<Vec<_>>()
};

let strip_arms_ref = || {
arms.iter()
.map(|arm| {
let pats = arm
.pats
.iter()
.map(|pat| match pat.node {
ast::PatKind::Ref(ref pat, _) => pat,
_ => pat,
})
.cloned()
.collect::<Vec<_>>();

ast::Arm {
pats,
..arm.clone()
}
})
.collect::<Vec<_>>()
};

match context.config.match_reference_style() {
MatchReferenceStyle::Reference if match_in_derefrence_style => {
if let ast::ExprKind::Unary(ast::UnOp::Deref, ref cond) = cond.node {
*cond_str = cond.rewrite(context, cond_shape)?;
}

let arms = prepend_arms_ref();

*arms_str = rewrite_match_arms(context, &arms, shape, span_after_cond, open_brace_pos)?
}
MatchReferenceStyle::Dereference if match_in_reference_style => {
let cond = ast::Expr {
node: ast::ExprKind::Unary(ast::UnOp::Deref, ptr::P(cond.clone())),
..cond.clone()
};
*cond_str = cond.rewrite(context, cond_shape)?;

let arms = strip_arms_ref();

*arms_str = rewrite_match_arms(context, &arms, shape, span_after_cond, open_brace_pos)?
}
MatchReferenceStyle::Auto if match_in_derefrence_style => {
if let ast::ExprKind::Unary(ast::UnOp::Deref, ref cond) = cond.node {
*cond_str = cond.rewrite(context, cond_shape)?;
}
}
MatchReferenceStyle::Auto if match_in_reference_style => {
let arms = strip_arms_ref();

*arms_str = rewrite_match_arms(context, &arms, shape, span_after_cond, open_brace_pos)?
}
_ => {}
}

Some(())
}

fn rewrite_match_arms(
context: &RewriteContext,
arms: &[ast::Arm],
Expand Down
23 changes: 23 additions & 0 deletions tests/source/configs/match_reference_style/auto.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// rustfmt-match_reference_style: auto
// Normalize match reference style

fn hello(name: &Option<&str>) {
match name {
&Some(name) => println!("Hello {}!", name),
&None => println!("I don't know who you are."),
}
}

fn kiss(name: &Option<&str>) {
match *name {
Some(name) => println!("Kiss {}!", name),
None => println!("I don't know who you are."),
}
}

fn main() {
let name = Some("rustfmt");

hello(&name);
kiss(&name);
}
23 changes: 23 additions & 0 deletions tests/source/configs/match_reference_style/derefence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// rustfmt-match_reference_style: dereference
// Normalize match reference style

fn hello(name: &Option<&str>) {
match name {
&Some(name) => println!("Hello {}!", name),
&None => println!("I don't know who you are."),
}
}

fn kiss(name: &Option<&str>) {
match *name {
Some(name) => println!("Kiss {}!", name),
None => println!("I don't know who you are."),
}
}

fn main() {
let name = Some("rustfmt");

hello(&name);
kiss(&name);
}
23 changes: 23 additions & 0 deletions tests/source/configs/match_reference_style/reference.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// rustfmt-match_reference_style: reference
// Normalize match reference style

fn hello(name: &Option<&str>) {
match name {
&Some(name) => println!("Hello {}!", name),
&None => println!("I don't know who you are."),
}
}

fn kiss(name: &Option<&str>) {
match *name {
Some(name) => println!("Kiss {}!", name),
None => println!("I don't know who you are."),
}
}

fn main() {
let name = Some("rustfmt");

hello(&name);
kiss(&name);
}
23 changes: 23 additions & 0 deletions tests/target/configs/match_reference_style/auto.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// rustfmt-match_reference_style: auto
// Normalize match reference style

fn hello(name: &Option<&str>) {
match name {
Some(name) => println!("Hello {}!", name),
None => println!("I don't know who you are."),
}
}

fn kiss(name: &Option<&str>) {
match name {
Some(name) => println!("Kiss {}!", name),
None => println!("I don't know who you are."),
}
}

fn main() {
let name = Some("rustfmt");

hello(&name);
kiss(&name);
}
23 changes: 23 additions & 0 deletions tests/target/configs/match_reference_style/derefence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// rustfmt-match_reference_style: dereference
// Normalize match reference style

fn hello(name: &Option<&str>) {
match *name {
Some(name) => println!("Hello {}!", name),
None => println!("I don't know who you are."),
}
}

fn kiss(name: &Option<&str>) {
match *name {
Some(name) => println!("Kiss {}!", name),
None => println!("I don't know who you are."),
}
}

fn main() {
let name = Some("rustfmt");

hello(&name);
kiss(&name);
}
23 changes: 23 additions & 0 deletions tests/target/configs/match_reference_style/reference.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// rustfmt-match_reference_style: reference
// Normalize match reference style

fn hello(name: &Option<&str>) {
match name {
&Some(name) => println!("Hello {}!", name),
&None => println!("I don't know who you are."),
}
}

fn kiss(name: &Option<&str>) {
match name {
&Some(name) => println!("Kiss {}!", name),
&None => println!("I don't know who you are."),
}
}

fn main() {
let name = Some("rustfmt");

hello(&name);
kiss(&name);
}