-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
format! .into() suggestion deletes the format macro #110017
Comments
It cannot be reproduced on play. |
I'm able to reproduce it locally. It seems to be suggesting a change in
(note the path) I assume that this is due to macro expansion. It might not be working on the playground because the playground doesn't have the |
The presence of Ideally it would be something closer to what it looks like with {
"children": [],
"code": null,
"level": "help",
"message": "call `Into::into` on this expression to convert `String` into `Box<dyn std::error::Error>`",
"rendered": null,
"spans": [
{
"byte_end": 85,
"byte_start": 85,
"column_end": 22,
"column_start": 22,
"expansion": null,
"file_name": "src/lib.rs",
"is_primary": true,
"label": null,
"line_end": 2,
"line_start": 2,
"suggested_replacement": ".into()",
"suggestion_applicability": "MaybeIncorrect",
"text": [
{
"highlight_end": 22,
"highlight_start": 22,
"text": " Err(String::new())"
}
]
}
]
} |
It seems to affect all macro expansions (play): macro_rules! s {
($e: expr) => {
String::from($e)
}
}
pub fn foo(x: &str) -> Result<(), Box<dyn std::error::Error>> {
Err(s!(x))
} |
Is the solution to just disable this suggestion for macro-expanded code? As an aside, perhaps the playground should get |
I think it's possible to achieve the desired output found in the description, although I'm not sure how exactly. Since we don't have a parent (afaiaa), earlier I briefly tried to create a parent for |
Yeah, would that be the desired solution though? Can this be generalized to all the other type mismatch suggestions? |
That's a good question. It was mostly an experiment to see if this specific issue can be solved with |
Code
Current output
Desired output
Rationale and extra context
The suggestion converts the code to:
which is invalid.
It should suggest:
The suggestion works correctly for non-macro string values.
This suggestion was introduced in #102496.
Other cases
No response
Anything else?
The text was updated successfully, but these errors were encountered: