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

Invalid RA diagnostic error: expected 2 arguments, found 1 #17497

Open
mckenfra opened this issue Jun 26, 2024 · 0 comments
Open

Invalid RA diagnostic error: expected 2 arguments, found 1 #17497

mckenfra opened this issue Jun 26, 2024 · 0 comments
Labels
A-ty type system / type inference / traits / method resolution C-bug Category: bug

Comments

@mckenfra
Copy link

rust-analyzer version: 0.3.2011-standalone (2fd803cc1 2024-06-22)
rustc 1.78.0 (9b00956e5 2024-04-29)
VSCode, Version 0.3.2011, Server Version 0.3.2011-standalone (2fd803cc1 2024-06-22)

While working on a rust project in VSCode, I noticed that RA was reporting errors with perfectly valid code. After stripping back all surrounding code, I have come up with the below minimal example that reproduces the error.

The error message I see in VSCode is: expected 2 arguments, found 1 rust-analyzer E0107

This occurs on the line vec.push("hello");

#![allow(dead_code)]
use core::ops::Deref;

trait Trait1 {
    type Assoc: Deref<Target = String>;
}
trait Trait2: Trait1 {
}
trait Trait3 {
    type T1: Trait1;
    type T2: Trait2;
    fn push(&self, x: bool, y: bool);
}

fn problem_method<T: Trait3>() {
    let mut vec = Vec::new();
    vec.push("hello"); // Rustc ok, RA errors (mismatched args)
}
@mckenfra mckenfra added the C-bug Category: bug label Jun 26, 2024
@lnicola lnicola added the A-ty type system / type inference / traits / method resolution label Jun 26, 2024
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 26, 2024
…xpected 2 arguments, found 1

The issue occurs because RA discards the `mut` qualifier that exists in the code being analyzed,
in the case where a variable is an owned value, not a ref.

E.g. `let mut vec = Vec::new();`

Then, when resolving candidate methods against this variable as a receiver, RA incorrectly
rejects all `&mut self` methods, because it has lost the information that the variable is mutable.

The fix involves preserving the `mut` qualifier information when carrying out method resolution.
At the point that the code fails to unify the receiver's type with a candidate method's expected receiver type,
an additional chalk request is done but this time with the receiver type coerced to &mut.

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 26, 2024
…xpected 2 arguments, found 1

The issue occurs because RA discards the `mut` qualifier that exists in the code being analyzed,
in the case where a variable is an owned value, not a ref.

E.g. `let mut vec = Vec::new();`

Then, when resolving candidate methods against this variable as a receiver, RA incorrectly
rejects all `&mut self` methods, because it has lost the information that the variable is mutable.

The fix involves preserving the `mut` qualifier information when carrying out method resolution.
At the point that the code fails to unify the receiver's type with a candidate method's expected receiver type,
an additional chalk request is done but this time with the receiver type coerced to &mut.

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 27, 2024
…xpected 2 arguments, found 1

The issue occurs because in some configurations of traits where one of them has Deref
as a supertrait, RA's type inference algorithm fails to fully resolve a trait's
self type, and instead uses a TyKind::BoundVar. This then erroneously matches
with types that do not implement the trait. In other words, unconnected types
appear to inherit the trait's methods.

The fix is to insert an optional heuristic that tells the method-lookup algorithm to
continue looking for other possibly better matches, in the case where the initial match
is affected by the above situation. It may find a better match, or fall back
to returning the initial match.

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 28, 2024
…uments, found 1

Also fixes rust-lang#17233

The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to fully resolve a trait's self type, and instead uses a TyKind::BoundVar. This then erroneously matches with types that do not implement the trait. In other words, unconnected types appear to inherit the trait's methods.

The fix is to insert a heuristic when invoking lookup_method() that allows the caller to determine if the returned method is a good match. If the returned method would result in diagnostic errors, the caller instructs the algorithm to continue iterating the possible methods to see if a better one exists (i.e. one that won't result in diagnostic errors).

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 28, 2024
…uments, found 1

Also fixes rust-lang#17233

The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to fully resolve a trait's self type, and instead uses a TyKind::BoundVar. This then erroneously matches with types that do not implement the trait. In other words, unconnected types appear to inherit the trait's methods.

The fix is to insert a heuristic when invoking lookup_method() that allows the caller to determine if the returned method is a good match. If the returned method would result in diagnostic errors, the caller instructs the algorithm to continue iterating the possible methods to see if a better one exists (i.e. one that won't result in diagnostic errors).

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 28, 2024
…uments, found 1

Also fixes rust-lang#17233

The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to fully resolve a trait's self type, and instead uses a TyKind::BoundVar. This then erroneously matches with types that do not implement the trait. In other words, unconnected types appear to inherit the trait's methods.

The fix is to insert a heuristic when invoking lookup_method() that allows the caller to determine if the returned method is a good match. If the returned method would result in diagnostic errors, the caller instructs the algorithm to continue iterating the possible methods to see if a better one exists (i.e. one that won't result in diagnostic errors).

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 29, 2024
…uments, found 1

The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to resolve the Deref::Target type, and instead uses a TyKind::BoundVar (i.e. an unknown type). This "autoderefed" type then incorrectly acts as if it implements all traits in scope.

The fix is to re-apply the same sanity-check that is done in iterate_method_candidates_with_autoref(), that is: don't try to resolve methods on unknown types. This same sanity-check is now done on each autoderefed type for which trait methods are about to be checked. If the autoderefed type is unknown, then the iterating of the trait methods for that type is skipped.

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 29, 2024
…uments, found 1

The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to resolve the Deref::Target type, and instead uses a TyKind::BoundVar (i.e. an unknown type). This "autoderefed" type then incorrectly acts as if it implements all traits in scope.

The fix is to re-apply the same sanity-check that is done in iterate_method_candidates_with_autoref(), that is: don't try to resolve methods on unknown types. This same sanity-check is now done on each autoderefed type for which trait methods are about to be checked. If the autoderefed type is unknown, then the iterating of the trait methods for that type is skipped.

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 29, 2024
…uments, found 1

The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to resolve the Deref::Target type, and instead uses a TyKind::BoundVar (i.e. an unknown type). This "autoderefed" type then incorrectly acts as if it implements all traits in scope.

The fix is to re-apply the same sanity-check that is done in iterate_method_candidates_with_autoref(), that is: don't try to resolve methods on unknown types. This same sanity-check is now done on each autoderefed type for which trait methods are about to be checked. If the autoderefed type is unknown, then the iterating of the trait methods for that type is skipped.

However, if the eventual receiver of the candidate method is also unknown then we have incomplete information, so no extra sanity-check is done and the trait method traversal is allowed to proceed.

Includes a unit test that only passes after applying the fixes in this commit.
mckenfra added a commit to mckenfra/rust-analyzer that referenced this issue Jun 29, 2024
…uments, found 1

The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to resolve the Deref::Target type, and instead uses a TyKind::BoundVar (i.e. an unknown type). This "autoderefed" type then incorrectly acts as if it implements all traits in scope.

The fix is to re-apply the same sanity-check that is done in iterate_method_candidates_with_autoref(), that is: don't try to resolve methods on unknown types. This same sanity-check is now done on each autoderefed type for which trait methods are about to be checked. If the autoderefed type is unknown, then the iterating of the trait methods for that type is skipped.

However, if the eventual receiver of the candidate method is also unknown then we have incomplete information, so no extra sanity-check is done and the trait method traversal is allowed to proceed.

Includes a unit test that only passes after applying the fixes in this commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ty type system / type inference / traits / method resolution C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

2 participants