fix: check original type for replace_arith_op - #22225
Conversation
rustbot
added
the
S-waiting-on-review
fdb5df7 to
409bab5
Compare
This comment has been minimized.
This comment has been minimized.
A4-Tacks
changed the title
This comment has been minimized.
This comment has been minimized.
| check_assist( | ||
| replace_arith_with_checked, | ||
| r#" | ||
| //- minicore: add, builtin_impls |
There was a problem hiding this comment.
Dumb question: shouldn't the test behaviour actually change? Why is it sufficient to to just change the minicore comments here?
There was a problem hiding this comment.
struct Foo;
struct Bar;
impl std::ops::AddAssign<i8> for Foo {}
fn main() {
let foo = Foo;
let bar = Bar;
foo += 2;
//^^^ original: Foo, adjusted: Some(&mut Foo)
bar += 2;
//^^^ original: Bar, adjusted: None
}
| fn is_primitive_int(ctx: &AssistContext<'_, '_>, expr: &ast::Expr) -> bool { | ||
| match ctx.sema.type_of_expr(expr) { | ||
| Some(ty) => ty.adjusted().is_int_or_uint(), | ||
| Some(ty) => ty.adjusted().strip_references().is_int_or_uint(), |
There was a problem hiding this comment.
| Some(ty) => ty.adjusted().strip_references().is_int_or_uint(), | |
| Some(ty) => ty.adjusted().strip_reference().is_int_or_uint(), |
You cannot add &&int to &&int.
There was a problem hiding this comment.
Oh I see this is because adjusted is the input to add() - I think this means we need to take original() instead (but also strip one reference since Add<&int> for &int is implemented). And please add a test for adding references.
There was a problem hiding this comment.
I think this means we need to take
original()
Are there some strange cases where converting from non-integer or non-integer-references to integers has been rejected
There was a problem hiding this comment.
Adjustments can only change due to coercion (besides taking a reference for add()), which is not applicable here.
There was a problem hiding this comment.
And please add a test for adding references.
Does #23109 include these tests?
Example
---
```rust
//- minicore: add, builtin_impls
fn main() {
let mut x = 1;
x $0+= 2;
}
```
**Before this PR**
Assist not applicable
**After this PR**
```rust
fn main() {
let mut x = 1;
x = x.saturating_add(2);
}
```
364c23e to
8a29cd0
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
rustbot
removed
the
S-waiting-on-review
4 participants
Fixup #22180
The old test did not include minicore, which resulted in the inability to actually apply it
Example
Before this PR
Assist not applicable
After this PR