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

Correctly suggest std or core path depending if this is a no_std crate #12149

Merged
merged 3 commits into from Jan 16, 2024

Conversation

GuillaumeGomez
Copy link
Member

A few lints emit suggestions using std paths whether or not this is a no_std crate, which is an issue when running rustfix afterwards. So in case this is an item that is defined in both std and core, we need to check if the crate is no_std to emit the right path.

r? @llogiq

changelog: Correctly suggest std or core path depending if this is a no_std crate

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 14, 2024
@@ -204,7 +204,7 @@ pub(super) fn check<'tcx>(
cx,
UNNECESSARY_SORT_BY,
expr.span,
"use Vec::sort_by_key here instead",
"consider using `sort_by_key`",
Copy link
Member Author

Choose a reason for hiding this comment

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

This method comes from slices so doesn't make much sense to only write Vec::.

@@ -227,7 +227,7 @@ pub(super) fn check<'tcx>(
cx,
UNNECESSARY_SORT_BY,
expr.span,
"use Vec::sort here instead",
"consider using `sort`",
Copy link
Member Author

Choose a reason for hiding this comment

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

Same here.

@llogiq
Copy link
Contributor

llogiq commented Jan 14, 2024

I wonder if a helper method that returns std outside no_std code and its argument (which could be "core" or "alloc") inside would markedly simplify the code here.

Otherwise the change looks good to me.

@GuillaumeGomez
Copy link
Member Author

I thought about it as well, but I'm not sure what's the best course of action here for the arguments. Few ideas I had:

std_core_path("cmp::Ord") -> "core::cmp::Ord"
std_core_path(&["cmp", "Ord"]) -> "core::cmp::Ord"
std_core_path(&["std", "cmp", "Ord"]) -> "core::cmp::Ord"

A preference maybe?

@llogiq
Copy link
Contributor

llogiq commented Jan 15, 2024

Since most usages will be used in formatting anyway, a function that returns either "std" or the given argument should cater for all uses.

@GuillaumeGomez
Copy link
Member Author

Let's go for get_std_core_prefix.

@GuillaumeGomez
Copy link
Member Author

Oh there is already std_or_core. Let's use this one then!

@GuillaumeGomez
Copy link
Member Author

Updated!

@llogiq
Copy link
Contributor

llogiq commented Jan 16, 2024

Thank you!

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 16, 2024

📌 Commit 874f851 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jan 16, 2024

⌛ Testing commit 874f851 with merge ecb0311...

@bors
Copy link
Collaborator

bors commented Jan 16, 2024

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

1 similar comment
@bors
Copy link
Collaborator

bors commented Jan 16, 2024

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

@bors bors merged commit ecb0311 into rust-lang:master Jan 16, 2024
5 checks passed
@GuillaumeGomez GuillaumeGomez deleted the core-std-suggestions branch January 16, 2024 09:20
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.

None yet

4 participants