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

Importing libraries leads to fully qualified types being shown unnecessarily #113933

Open
Wilfred opened this issue Jul 21, 2023 · 3 comments
Open
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Wilfred
Copy link
Contributor

Wilfred commented Jul 21, 2023

Code

use clap::Parser;

fn wrong_type() -> Option<String> {
    return true;
}

Current output

error[E0308]: mismatched types
 --> src/lib.rs:4:12
  |
3 | fn wrong_type() -> Option<String> {
  |                    -------------- expected `Option<std::string::String>` because of return type
4 |     return true;
  |            ^^^^ expected `Option<String>`, found `bool`
  |
  = note: expected enum `Option<std::string::String>`
             found type `bool`

Desired output

error[E0308]: mismatched types
 --> src/lib.rs:4:12
  |
3 | fn wrong_type() -> Option<String> {
  |                    -------------- expected `Option<String>` because of return type
4 |     return true;
  |            ^^^^ expected `Option<String>`, found `bool`
  |
  = note: expected enum `Option<String>`
             found type `bool`

Rationale and extra context

Adding imports, even though they don't introduce another String type in the current file, is making rustc print fully qualified type names.

Clap doesn't define any types called String, although it does have a variant named String elsewhere in the library: https://docs.rs/clap/latest/clap/error/enum.ContextValue.html#variant.String

Other cases

There's a similar issue with serde and option:

use serde::Deserialize;

fn wrong_type() -> Option<String> {
    return true;
}

rustc output:

error[E0308]: mismatched types
 --> src/lib.rs:4:12
  |
3 | fn wrong_type() -> Option<String> {
  |                    -------------- expected `std::option::Option<std::string::String>` because of return type
4 |     return true;
  |            ^^^^ expected `Option<String>`, found `bool`
  |
  = note: expected enum `std::option::Option<std::string::String>`
             found type `bool`

Anything else?

Playground links:

@Wilfred Wilfred added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 21, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 21, 2023
@davidbarsky
Copy link
Contributor

davidbarsky commented Jul 21, 2023

Using the same program that @Wilfred wrote, env RUSTFLAGS="-Ztrim-diagnostic-paths=true" cargo +nightly check results in fully-qualified types showing up in diagnostics as well.

(I'm using rustc 1.73.0-nightly (399b06823 2023-07-20))

@Wilfred Wilfred changed the title Importing libraries leads to fully qualified types being shown unnecessairly Importing libraries leads to fully qualified types being shown unnecessarily Jul 21, 2023
@saethlin saethlin added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Jul 22, 2023
@Noratrieb
Copy link
Member

I think the issue here is that clap contains an item called String, so just using String is not fully unambiguous anymore. The fact that it's an enum variant which cannot be a type is not considered by the compiler (and I'm not sure whether it should be, erring on the side of "no").

@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 8, 2023
@moxian
Copy link
Contributor

moxian commented Mar 20, 2025

I think the issue here is that clap contains an item called String

This appears to be the case. Minimized:

mod xx {
    struct String;   // doesn't need to be pub
}

fn wrong_type() -> Option<String> {
    return true;
}
or, two-crate version, more faithful to the original issue
// crate1
pub struct Foo;
pub struct String;
// crate2
use crate1::Foo;  // important to have *an* import from crate1

fn wrong_type() -> Option<String> {
    return true;
}

This is not a regression, the fully-qualified std::string::String is in the output all the way since 1.0.0. Although the core::option::Option did get shortened to just Option in #73996

I believe there is no need to fully qualify String here because it is unambiguous in this case. After all, the diagnostics machinery was able to correctly identify the std::string::String that we meant. If we wanted to use any String not from prelude we would have to either fully qualify it, or add a use for it.

Now, whether or not the extra qualification is useful (and thus whether rustc is working as intended here), I can't say.

@rustbot label: -E-needs-bisection +D-verbose

@rustbot rustbot added D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. and removed E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants