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

Refactor some of dereference.rs to util functions #11166

Merged
merged 1 commit into from
Jul 23, 2023

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Jul 16, 2023

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
Copy link
Collaborator

rustbot commented Jul 16, 2023

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
Member

r? @Centri3

@rustbot
Copy link
Collaborator

rustbot commented Jul 16, 2023

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.

Callee,
/// Access of a field.
FieldAccess(Ident),
}
Copy link
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
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?

Copy link
Member

@Centri3 Centri3 Jul 16, 2023

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
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
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
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
Copy link
Contributor

smoelius commented Jul 17, 2023

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
Copy link
Contributor Author

Jarcho commented Jul 17, 2023

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
Contributor

Thanks!

Copy link
Member

@Centri3 Centri3 left a comment

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

clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
Extract getting an expression's use context and the context's defined
type as util functions.
@Centri3
Copy link
Member

Centri3 commented Jul 23, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 23, 2023

📌 Commit 55dd8a9 has been approved by Centri3

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 23, 2023

⌛ Testing commit 55dd8a9 with merge a4e64ff...

@bors
Copy link
Collaborator

bors commented Jul 23, 2023

☀️ 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