Skip to content

Refactor some of dereference.rs to util functions#11166

Merged
bors merged 1 commit into
rust-lang:masterfrom
Jarcho:expr_use
Jul 23, 2023
Merged

Refactor some of dereference.rs to util functions#11166
bors merged 1 commit into
rust-lang:masterfrom
Jarcho:expr_use

Conversation

@Jarcho

@Jarcho Jarcho commented Jul 16, 2023

Copy link
Copy Markdown
Contributor

I've seen a few lints that need to be able to tell if changing the type of an expression would be a vaild suggestion. This extracts part of how that's done from explicit_auto_deref.

changelog: None

@rustbot

rustbot commented Jul 16, 2023

Copy link
Copy Markdown
Collaborator

r? @Manishearth

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 16, 2023
@Manishearth

Copy link
Copy Markdown
Member

r? @Centri3

@rustbot

rustbot commented Jul 16, 2023

Copy link
Copy Markdown
Collaborator

Failed to set assignee to Centri3: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

Comment thread clippy_utils/src/lib.rs
Callee,
/// Access of a field.
FieldAccess(Ident),
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}

Deref,
Reborrow,
None,
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}

Is juxtaposing the the type and its impl intentional?

@Centri3 Centri3 Jul 16, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

blank_lines_lower_bound (rustfmt) is definitely relevant here. Is it worth the conflicts though? 👀

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Formatting the type and it's impl without a blank line in between is used in other parts of clippy. It's not a consistent thing though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are the multiple impls similarly intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That part isn't.

@smoelius

smoelius commented Jul 17, 2023

Copy link
Copy Markdown
Contributor

I've seen a few lints that need to be able to tell if changing the type of an expression would be a vaild suggestion. This extracts part of how that's done from explicit_auto_deref.

Can I ask how/where you see the extracted code being applied? (Just curious.)

@Jarcho

Jarcho commented Jul 17, 2023

Copy link
Copy Markdown
Contributor Author

As a short list of recentish things:

  • single_range_in_vec_init (linked)
  • trivial_default_constructed_types
  • unnecessary_fold (to avoid adding the generic args)
  • any lint for unnecessary calls to as_slice, as_str and as_ref

@Jarcho Jarcho force-pushed the expr_use branch 2 times, most recently from 132c828 to c6cfc2c Compare July 17, 2023 17:12
@smoelius

Copy link
Copy Markdown
Contributor

Thanks!

@Centri3 Centri3 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks fine, thanks! Just two typos

Comment thread clippy_utils/src/lib.rs Outdated
Comment thread clippy_utils/src/lib.rs Outdated
Extract getting an expression's use context and the context's defined
type as util functions.
@Centri3

Centri3 commented Jul 23, 2023

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jul 23, 2023

Copy link
Copy Markdown
Contributor

📌 Commit 55dd8a9 has been approved by Centri3

It is now in the queue for this repository.

@bors

bors commented Jul 23, 2023

Copy link
Copy Markdown
Contributor

⌛ Testing commit 55dd8a9 with merge a4e64ff...

@bors

bors commented Jul 23, 2023

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Centri3
Pushing a4e64ff to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants