rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.66k stars 12.75k forks source link

Sometimes `DiagnosticBuilder::span_suggestion` doesn't print the suggestion #60979

Open flip1995 opened 5 years ago

flip1995 commented 5 years ago

In Clippy we have a convenience function for DiagnosticBuilder::span_suggestion: https://github.com/rust-lang/rust-clippy/blob/fd563810158425c20f27768da4ef54c54230bbbf/clippy_lints/src/utils/diagnostics.rs#L169-L177

Until now we had 3 cases where the suggestion wasn't emitted: rust-lang/rust-clippy#4119, https://github.com/rust-lang/rust-clippy/pull/3582#discussion_r246656947, rust-lang/rust-clippy#3913

This issue still occurs, when replacing the convenience functions with the normal DiagnosticBuilder methods.

Until now I couldn't figure out a pattern when or why this happens.

estebank commented 5 years ago

@flip1995 does the suggestion appear in the json output? The only situation where we hide suggestions implicitly is when span == DUMMY_SP. There's suggestion hiding logic, but it is gated behind the API in a backwards compatible way.

flip1995 commented 5 years ago

Minimal example: (with rust-lang/rust-clippy#4119)

#[warn(clippy::non_ascii_literal)]
fn main() {
    let _ = "Üben!";
    print!("Üben!");
}

Output:

warning: literal non-ASCII character detected
 --> src/main.rs:3:13
  |
3 |     let _ = "Üben!";
  |             ^^^^^^^ help: consider replacing the string with: `"\u{dc}ben!"`
  |
note: lint level defined here
 --> src/main.rs:1:8
  |
1 | #[warn(clippy::non_ascii_literal)]
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal

warning: literal non-ASCII character detected
 --> src/main.rs:4:12
  |
4 |     print!("Üben!");
  |            ^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal
json output With suggestion ```json { "reason": "compiler-message", "package_id": "test_sugg 0.1.0 (path+file:///home/pkrones/test_sugg)", "target": { "kind": [ "bin" ], "crate_types": [ "bin" ], "name": "test_sugg", "src_path": "/home/pkrones/test_sugg/src/main.rs", "edition": "2018" }, "message": { "message": "literal non-ASCII character detected", "code": { "code": "clippy::non_ascii_literal", "explanation": null }, "level": "warning", "spans": [ { "file_name": "src/main.rs", "byte_start": 59, "byte_end": 67, "line_start": 3, "line_end": 3, "column_start": 13, "column_end": 20, "is_primary": true, "text": [ { "text": " let _ = \"Üben!\";", "highlight_start": 13, "highlight_end": 20 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": null } ], "children": [ { "message": "lint level defined here", "code": null, "level": "note", "spans": [ { "file_name": "src/main.rs", "byte_start": 7, "byte_end": 32, "line_start": 1, "line_end": 1, "column_start": 8, "column_end": 33, "is_primary": true, "text": [ { "text": "#[warn(clippy::non_ascii_literal)]", "highlight_start": 8, "highlight_end": 33 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": null } ], "children": [], "rendered": null }, { "message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal", "code": null, "level": "help", "spans": [], "children": [], "rendered": null }, { "message": "consider replacing the string with", "code": null, "level": "help", "spans": [ { "file_name": "src/main.rs", "byte_start": 59, "byte_end": 67, "line_start": 3, "line_end": 3, "column_start": 13, "column_end": 20, "is_primary": true, "text": [ { "text": " let _ = \"Üben!\";", "highlight_start": 13, "highlight_end": 20 } ], "label": null, "suggested_replacement": "\"\\u{dc}ben!\"", "suggestion_applicability": "MachineApplicable", "expansion": null } ], "children": [], "rendered": null } ], "rendered": "warning: literal non-ASCII character detected\n --> src/main.rs:3:13\n |\n3 | let _ = \"Üben!\";\n | ^^^^^^^ help: consider replacing the string with: `\"\\u{dc}ben!\"`\n |\nnote: lint level defined here\n --> src/main.rs:1:8\n |\n1 | #[warn(clippy::non_ascii_literal)]\n | ^^^^^^^^^^^^^^^^^^^^^^^^^\n = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal\n\n" } } ``` Without suggestion (`print!` macro) ```json { "reason": "compiler-message", "package_id": "test_sugg 0.1.0 (path+file:///home/pkrones/test_sugg)", "target": { "kind": [ "bin" ], "crate_types": [ "bin" ], "name": "test_sugg", "src_path": "/home/pkrones/test_sugg/src/main.rs", "edition": "2018" }, "message": { "message": "literal non-ASCII character detected", "code": { "code": "clippy::non_ascii_literal", "explanation": null }, "level": "warning", "spans": [ { "file_name": "src/main.rs", "byte_start": 80, "byte_end": 88, "line_start": 4, "line_end": 4, "column_start": 12, "column_end": 19, "is_primary": true, "text": [ { "text": " print!(\"Üben!\");", "highlight_start": 12, "highlight_end": 19 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": { "span": { "file_name": "<::std::macros::print macros>", "byte_start": 54, "byte_end": 85, "line_start": 2, "line_end": 2, "column_start": 27, "column_end": 58, "is_primary": false, "text": [ { "text": "$ crate :: io :: _print ( format_args ! ( $ ( $ arg ) * ) ) ) ;", "highlight_start": 27, "highlight_end": 58 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": { "span": { "file_name": "src/main.rs", "byte_start": 73, "byte_end": 90, "line_start": 4, "line_end": 4, "column_start": 5, "column_end": 21, "is_primary": false, "text": [ { "text": " print!(\"Üben!\");", "highlight_start": 5, "highlight_end": 21 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": null }, "macro_decl_name": "print!", "def_site_span": { "file_name": "<::std::macros::print macros>", "byte_start": 0, "byte_end": 91, "line_start": 1, "line_end": 2, "column_start": 1, "column_end": 64, "is_primary": false, "text": [ { "text": "( $ ( $ arg : tt ) * ) => (", "highlight_start": 1, "highlight_end": 28 }, { "text": "$ crate :: io :: _print ( format_args ! ( $ ( $ arg ) * ) ) ) ;", "highlight_start": 1, "highlight_end": 64 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": null } } }, "macro_decl_name": "format_args!", "def_site_span": null } } ], "children": [ { "message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal", "code": null, "level": "help", "spans": [], "children": [], "rendered": null } ], "rendered": "warning: literal non-ASCII character detected\n --> src/main.rs:4:12\n |\n4 | print!(\"Üben!\");\n | ^^^^^^^\n |\n = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal\n\n" } } ```

On the non-expanded code with the &str literal the suggestion is emitted. But when the code comes from an expansion (macro or desugaring), the suggestion is not emitted. Neither in human-readable, nor in json output.

In the Clippy convenience functions the span for struct_span_lint and span_suggestion are exactly the same. That means, that span != DUMMY_SP, otherwise the error message would already be empty and not only the suggestion. (?)

estebank commented 5 years ago

I think this is on purpose. We've faced multiple bugs when pointing at a span that corresponds to macros. In this case the second span points at the inside of the print macro, so rustc avoids showing suggestions for it. There are other recent tickets with problems caused by this. We can remove the compiler desugaring from the span and that would cause the suggestion to be emitted, but this is a situation that no matter where we settle we'll likely end up with either too few suggestions or incorrect suggestions.