Recommend fix count() -> len() on slices

[notriddle]

Fixes #87302

[rust-highfive]

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

[jackh726]

My first thought is (without reading too much of the code): what other methods do we have suggestions like this for? Also, even though this is technically compiler code, it seems more like a @rust-lang/libs decision; it reminds me somewhat of the rustdoc aliases: at what point is it "okay" and when is it "too much"

[notriddle]

My first thought is (without reading too much of the code): what other methods do we have suggestions like this for?

The same file I modified also has a suggestion for as_str(self: &str), which is another method that doesn't exist but developers are likely to try to call (and has exactly one, obvious, correction to apply).

[cuviper]

Could this use #[rustc_on_unimplemented(... from_method = "count" ...)] on Iterator?

[notriddle]

Here's what I tried, to achieve the effect using that system. It didn't work.

diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs
index a1a336a0574..bdee0e202c6 100644
--- a/library/core/src/iter/traits/iterator.rs
+++ b/library/core/src/iter/traits/iterator.rs
@@ -25,6 +25,13 @@ fn _assert_is_object_safe(_: &dyn Iterator<Item = ()>) {}
 /// [impl]: crate::iter#implementing-iterator
 #[stable(feature = "rust1", since = "1.0.0")]
 #[rustc_on_unimplemented(
+    on(
+        all(
+            any(_Self = "[]", _Self = "std::vec::Vec", _Self = "std::string::String", _Self = "str"),
+            from_method = "count",
+        ),
+        label = "consider using `.len()`"
+    ),
     on(
         _Self = "std::ops::RangeTo<Idx>",
         label = "if you meant to iterate until a value, add a starting value",

Here's the message that it gives:

error[E0599]: the method `count` exists for array `[{integer}; 4]`, but its trait bounds were not satisfied
  --> $DIR/count2len.rs:5:11
   |
LL |     slice.count();
   |           ^^^^^ method cannot be called on `[{integer}; 4]` due to unsatisfied trait bounds
   |
   = note: the following trait bounds were not satisfied:
           `[{integer}; 4]: Iterator`
           which is required by `&mut [{integer}; 4]: Iterator`
           `[{integer}]: Iterator`
           which is required by `&mut [{integer}]: Iterator`

I notice that my changes have no effect on it, but also, that the {Self} isn't an iterator language isn't being used anywhere in here, either. Maybe because there are multiple applicable trait implementations?

[cuviper]

My hunch is that the &mut I implementation is in the way. Maybe you need _Self = "&[]"?

[notriddle]

No. _Self = "&[]" doesn't do anything different. Neither does _Self = "&mut []".

[jackh726]

I'm going to defer to libs (or libs-api?) here.

r? @cuviper feel free to reassign

[bors]

☔ The latest upstream changes (presumably #88556) made this pull request unmergeable. Please resolve the merge conflicts.

[camelid]

@notriddle Please rebase instead of merging in upstream changes because of the "No-Merge Policy". Thanks!

[Mark-Simulacrum]

This feels like a fairly reasonable change to me (obviously, we can iterate further on the message in the future). I'm not entirely convinced at whether this is a particularly common case, but the extra work is not huge, and so it seems like an OK change to make in my opinion.

My cursory look at the compiler code suggests we don't consider on_unimplemented for this kind of error, so we might want to implement that in the future -- but it seems unnecessary to block this improvement on that larger goal.

I also agree with others on the "is this too much" angle, but ultimately, this is something relatively easy to back out and we're not yet seeing a huge influx of such targeted messages that I'm worried. :)

---

That said, I think the proposed error message in this PR is pretty suboptimal -- we're hiding the trait the user might've been intending to reference, not even mentioning its name:

error[E0599]: no method named `count` found for array `[{integer}; 4]` in the current scope
  --> $DIR/count2len.rs:5:11
   |
LL |     slice.count();
   |          -^^^^^--
   |          ||
   |          |method cannot be called on `[{integer}; 4]` due to unsatisfied trait bounds
   |          help: consider using `len` instead

I think it would be good to rework this to something like:

error[E0599]: no method named `count` found for array `[{integer}; 4]` in the current scope
  --> $DIR/count2len.rs:5:11
   |
LL |     slice.count();
   |          --------
   |          |
   |          help: consider using `len` instead
   |
   note: `count` is defined on `std::iter::Iterator`, which `[{integer}; 4]` does not implement

Or potentially to hide the "unsatisfied due to trait bounds" error altogether, though not sure on that front.

cc @rust-lang/wg-diagnostics -- wondering if you have opinions on this particular case or if we should instead invest in expanding rustc_on_unimplemented to cover cases like this and avoid ad-hoc code in the compiler.

[notriddle]

I agree. Rebased and pushed a new error message, that's closer to what you wrote.

error[E0599]: no method named `count` found for array `[{integer}; 4]` in the current scope
  --> $DIR/count2len.rs:5:11
   |
LL |     slice.count();
   |           ^^^^^
   |           |
   |           method cannot be called on `[{integer}; 4]` due to unsatisfied trait bounds
   |           help: consider using `len` instead
   |
   = note: `count` is defined on `Iterator`, which `[{integer}; 4]` does not implement

[Mark-Simulacrum]

r=me with nit fixed

[notriddle]

@rustbot label: -S-waiting-on-author +S-waiting-on-review

[Mark-Simulacrum]

@bors r+ rollup

[bors]

📌 Commit 6a17ee6 has been approved by Mark-Simulacrum