compiler: suggest .collect() when String is expected and Iterator is found

[thiago-fealves]

View all comments

This commit adds a diagnostic suggestion to help users who forget to call .collect() when they have an iterator and the function or variable expects a String.

The logic checks if the expected type is std::string::String and if the found type implements the Iterator trait, if so the compiler provides a suggestion to add .collect()

Includes also a UI test to verify if the suggestion appears correctly

[rustbot]

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 22 candidates

[teor2345]

The job aarch64-gnu-llvm-21-1 failed!

Looks like you need to run ./x test --bless tests/ui to update the expected test outputs for existing tests with the new suggestion.

Most of the failures are:

help: consider using .collect() to convert the Iterator into a ()

Is this a useful suggestion?
We might want to skip the unit type if other suggestions are more useful, or put it lower down in the suggestion priority list.

[thiago-fealves]

The job aarch64-gnu-llvm-21-1 failed!

Looks like you need to run ./x test --bless tests/ui to update the expected test outputs for existing tests with the new suggestion.

Most of the failures are:

help: consider using .collect() to convert the Iterator into a ()

Is this a useful suggestion? We might want to skip the unit type if other suggestions are more useful, or put it lower down in the suggestion priority list.

I didn't anticipate this could happen, i was alway thinking about more concrete types like String and int, honestly i think in this case the suggestion is not being useful for the user anymore, i can't think of a situation where you would collect into () at least. What do you suggest in this case?

I'm gonna implement more tests for non-String objects in the meantime

[teor2345]

I didn't anticipate this could happen, i was alway thinking about more concrete types like String and int, honestly i think in this case the suggestion is not being useful for the user anymore, i can't think of a situation where you would collect into () at least. What do you suggest in this case?

The docs say this is intended for cases like Result<(), E>:
https://doc.rust-lang.org/std/iter/trait.FromIterator.html#impl-FromIterator%3C()%3E-for-()

So skipping the suggestion when the expected type is just () seems like a good idea.

[thiago-fealves]

I didn't anticipate this could happen, i was alway thinking about more concrete types like String and int, honestly i think in this case the suggestion is not being useful for the user anymore, i can't think of a situation where you would collect into () at least. What do you suggest in this case?

The docs say this is intended for cases like Result<(), E>: https://doc.rust-lang.org/std/iter/trait.FromIterator.html#impl-FromIterator%3C()%3E-for-()

So skipping the suggestion when the expected type is just () seems like a good idea.

I agree, gonna finish implementing that and adding the UI tests.

[thiago-fealves]

Just added a check for the unit type, let's see if the tests pass now

[thiago-fealves]

Looks like tests succeeded!

[teor2345]

This looks good to me, but I'm not a typeck expert.

Feel free to resolve my earlier feedback, I think GitHub is having issues and dropping my attempts to do it.

View changes since this review

[estebank]

@bors r+

[rust-bors]

📌 Commit e1941ff has been approved by estebank

It is now in the queue for this repository.

[rust-bors]

💔 Test for f2434d3 failed: CI. Failed job:

• auto - dist-various-2 (web logs, enhanced plaintext logs)

[rust-log-analyzer]

A job failed! Check out the build log: (web)(plain enhanced)(plain)

Click to see the possible cause of the failure (guessed by this bot)

#13 DONE 32.2s
#14 [ 6/25] RUN apt-key adv --batch --yes --keyserver keyserver.ubuntu.com --recv-keys 74DA7924C5513486
#14 0.159 Warning: apt-key is deprecated. Manage keyring files in trusted.gpg.d instead (see apt-key(8)).
#14 0.176 Executing: /tmp/apt-key-gpghome.zToD4iKYCd/gpg.1.sh --batch --yes --keyserver keyserver.ubuntu.com --recv-keys 74DA7924C5513486
#14 1.978 gpg: key 74DA7924C5513486: public key "Igor Kozhukhov <igor@dilos.org>" imported
#14 1.982 gpg: Total number processed: 1
#14 1.982 gpg: imported: 1
#14 DONE 2.1s

[Kobzol]

@bors retry

Spurious failure fixed by #156640.

[rust-log-analyzer]

The job x86_64-gnu-llvm-21-1 failed! Check out the build log: (web)(plain enhanced)(plain)

Click to see the possible cause of the failure (guessed by this bot)

test expr_store::scope::tests::test_shadow_variable ... ok
test expr_store::tests::body::array_element_cfg ... ok
test expr_store::tests::body::async_fn_weird_param_patterns ... ok
test expr_store::tests::body::block::inner_item_smoke ... ok
error: test failed, to rerun pass `-p hir-def --lib`
Caused by:
process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/hir_def-f381e8bd2208ec8b '--skip=tests::smoke_test_real_sysroot_cargo' --skip=check_code_formatting -Z unstable-options --format json` (signal: 11, SIGSEGV: invalid memory reference)
Bootstrap failed while executing `--stage 2 test --skip tests --skip coverage-map --skip coverage-run --skip library --skip tidyselftest`
Build completed unsuccessfully in 1:04:34
local time: Sun May 17 01:26:49 UTC 2026
network time: Sun, 17 May 2026 01:26:49 GMT
##[error]Process completed with exit code 1.

[rust-bors]

💔 Test for c9ce8ae failed: CI. Failed job:

• auto - x86_64-gnu-llvm-21-1 (web logs, enhanced plaintext logs)

[JonathanBrouwer]

@bors retry