Diagnostic suggests adding type `[type error]`

[camelid]

I think the suggestion is trying to suggest a type, but gets a TyKind::Error due to the type error in the body of the const. Ideally we would just not suggest a type, but there are probably a lot of other places that this issue occurs.

const FOO = "hello" + 1;

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error[E0369]: cannot add `{integer}` to `&str`
 --> src/lib.rs:1:21
  |
1 | const FOO = "hello" + 1;
  |             ------- ^ - {integer}
  |             |
  |             &str

error: missing type for `const` item
 --> src/lib.rs:1:7
  |
1 | const FOO = "hello" + 1;
  |       ^^^ help: provide a type for the item: `FOO: [type error]`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0369`.
error: could not compile `playground`.

To learn more, run the command again with --verbose.

[oli-obk]

Fix instructions:

1. find the place where the provide a type for the item suggestion is emitted
2. prevent the suggestion from being created if the suggested type is
   • ty::Err
   • ty::FnDef (const FOO = main; fn main() {} gives that)
   • ty::Closure
   • possibly other variants of https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.TyKind.html

Hi i am interesting in tackling this issue! This would be my first time contributing.

[oli-obk]

Cool! You can mark yourself as workin on this issue by writing @rustbot claim in it. Feel free to ask away with anything that is unclear. Or actually, just anything ^^ If you prefer chat over github messages, you can also reach out in zulip by opening a new thread.

@rustbot claim

The error is emitted here:

match tcx.sess.diagnostic().steal_diagnostic(span, StashKey::ItemNoType) {
        Some(mut err) => {
            // The parser provided a sub-optimal `HasPlaceholders` suggestion for the type.
            // We are typeck and have the real type, so remove that and suggest the actual type.
            err.suggestions.clear();
            err.span_suggestion(
                span,
                "provide a type for the item",
                format!("{}: {}", item_ident, ty),
                Applicability::MachineApplicable,
            )
            .emit();
        }
        None => {
            let mut diag = bad_placeholder_type(tcx, vec![span]);
            if !matches!(ty.kind(), ty::Error(_)) {
                diag.span_suggestion(
                    span,
                    "replace `_` with the correct type",
                    ty.to_string(),
                    Applicability::MaybeIncorrect,
                );
            }
            diag.emit();
        }
    }

within fn infer_placeholder_type, but I am confused by what is happening when tcx.sess.diagnostic().steal_diagnostic(span, StashKey::ItemNoType) matches with none, since it emits a different error. If it is unrelated to this issue, should I check for ty::Err and it variants from within Some(mut err)? I was thinking of matching on ty like so:

match ty.kind() {
        TyKind::Err | TyKind::FnDef | TyKind::Closure | ... => { }
        _ =>    {
                       err.suggestions.clear();
                       err.span_suggestion(
                           span,
                           "provide a type for the item",
                           format!("{}: {}", item_ident, ty),
                           Applicability::MachineApplicable,
                           )
                           .emit();
       }
}

I've managed to block the second error:

Some(mut err) => {
            // The parser provided a sub-optimal `HasPlaceholders` suggestion for the type.
            // We are typeck and have the real type, so remove that and suggest the actual type.
            match ty.kind() {
                ty::Error(_) => {
                    err.delay_as_bug();
                },
                _ => {
                    err.suggestions.clear();
                    err.span_suggestion(
                        span,
                        "provide a type for the item",
                        format!("{}: {}", item_ident, ty),
                        Applicability::MachineApplicable,
                    )
                    .emit();
                }
            }
        }

I used err.delay_as_bug() because I was getting an internal compiler error for an unused error. I saw emit_unless(&mut self, delay: bool) which does the same thing and seems more appropriate, but is delay_as_bug() the fn that I should use?

Using emit_unless(bool):

Some(mut err) => {
            err.suggestions.clear();
            err.span_suggestion(
                span,
                "provide a type for the item",
                format!("{}: {}", item_ident, ty),
                Applicability::MachineApplicable,
            )
            .emit_unless(
                match ty.kind() {
                    ty::Error(_) => {true},
                    _ => {false}  // Is _ ok here?
                }
            );
        }

[oli-obk]

I like the delay_as_bug() variant more.

Though I think we should add an ignore method that takes a DelaySpanBugEmitted, which we get from the ty::Error. What do you think @rust-lang/wg-diagnostics

[petar-dambovaliev]

cannot add {integer} to &str

Shouldn't it also be cannot add `&str` to `{integer}, same order as in the code?