mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #12190 - harpsword:fix_diagostics_map_incorrectly, r=harpsword

Browse Source 
fix cargo check diagnostics are mapped incorrectly with non-BMP codepoints

fix #11945
This commit is contained in:
bors 2022-05-15 09:48:51 +00:00
commit fa133d065b
3 changed files with 338 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

184
crates/rust-analyzer/src/diagnostics/test_data/rustc_range_map_lsp_position.txt Normal file
View File

@ -0,0 +1,184 @@
[
MappedRustDiagnostic {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "/test/crates/test_diagnostics/src/main.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 3,
character: 17,
},
end: Position {
line: 3,
character: 27,
},
},
severity: Some(
Error,
),
code: Some(
String(
"E0308",
),
),
code_description: Some(
CodeDescription {
href: Url {
scheme: "https",
cannot_be_a_base: false,
username: "",
password: None,
host: Some(
Domain(
"doc.rust-lang.org",
),
),
port: None,
path: "/error-index.html",
query: None,
fragment: Some(
"E0308",
),
},
},
),
source: Some(
"rustc",
),
message: "mismatched types\nexpected `u32`, found `&str`",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "/test/crates/test_diagnostics/src/main.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 3,
character: 11,
},
end: Position {
line: 3,
character: 14,
},
},
},
message: "expected due to this",
},
],
),
tags: None,
data: None,
},
fix: None,
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "/test/crates/test_diagnostics/src/main.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 3,
character: 11,
},
end: Position {
line: 3,
character: 14,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"E0308",
),
),
code_description: Some(
CodeDescription {
href: Url {
scheme: "https",
cannot_be_a_base: false,
username: "",
password: None,
host: Some(
Domain(
"doc.rust-lang.org",
),
),
port: None,
path: "/error-index.html",
query: None,
fragment: Some(
"E0308",
),
},
},
),
source: Some(
"rustc",
),
message: "expected due to this",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "/test/crates/test_diagnostics/src/main.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 3,
character: 17,
},
end: Position {
line: 3,
character: 27,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,
data: None,
},
fix: None,
},
]

177
crates/rust-analyzer/src/diagnostics/to_proto.rs
View File

@ -3,11 +3,19 @@
use std::collections::HashMap; use std::collections::HashMap;
use flycheck::{Applicability, DiagnosticLevel, DiagnosticSpan}; use flycheck::{Applicability, DiagnosticLevel, DiagnosticSpan};
use ide::TextRange;
use itertools::Itertools; use itertools::Itertools;
use stdx::format_to; use stdx::format_to;
use vfs::{AbsPath, AbsPathBuf}; use vfs::{AbsPath, AbsPathBuf};
use crate::{lsp_ext, to_proto::url_from_abs_path}; use crate::{
from_proto,
global_state::GlobalStateSnapshot,
line_index::OffsetEncoding,
lsp_ext,
to_proto::{self, url_from_abs_path},
Result,
};
use super::{DiagnosticsMapConfig, Fix}; use super::{DiagnosticsMapConfig, Fix};
@ -57,23 +65,68 @@ fn location(
config: &DiagnosticsMapConfig, config: &DiagnosticsMapConfig,
workspace_root: &AbsPath, workspace_root: &AbsPath,
span: &DiagnosticSpan, span: &DiagnosticSpan,
snap: &GlobalStateSnapshot,
) -> lsp_types::Location { ) -> lsp_types::Location {
let file_name = resolve_path(config, workspace_root, &span.file_name); let file_name = resolve_path(config, workspace_root, &span.file_name);
let uri = url_from_abs_path(&file_name); let uri = url_from_abs_path(&file_name);
// FIXME: this doesn't handle UTF16 offsets correctly let range = range(span, snap, &uri).unwrap_or_else(|_| {
let range = lsp_types::Range::new( let offset_encoding = snap.config.offset_encoding();
lsp_types::Position::new( lsp_types::Range::new(
(span.line_start as u32).saturating_sub(1), position(&offset_encoding, span, span.line_start, span.column_start),
(span.column_start as u32).saturating_sub(1), position(&offset_encoding, span, span.line_end, span.column_end),
), )
lsp_types::Position::new( });
(span.line_end as u32).saturating_sub(1), lsp_types::Location::new(uri, range)
(span.column_end as u32).saturating_sub(1), }
),
);
lsp_types::Location { uri, range } fn range(
span: &DiagnosticSpan,
snap: &GlobalStateSnapshot,
uri: &lsp_types::Url,
) -> Result<lsp_types::Range> {
let file_id = from_proto::file_id(snap, &uri)?;
let line_index = snap.file_line_index(file_id)?;
let range =
to_proto::range(&line_index, TextRange::new(span.byte_start.into(), span.byte_end.into()));
Ok(range)
}
fn position(
offset_encoding: &OffsetEncoding,
span: &DiagnosticSpan,
line_offset: usize,
column_offset: usize,
) -> lsp_types::Position {
let line_index = line_offset - span.line_start;
let mut true_column_offset = column_offset;
if let Some(line) = span.text.get(line_index) {
if line.text.chars().count() == line.text.len() {
// all utf-8
return lsp_types::Position {
line: (line_offset as u32).saturating_sub(1),
character: (column_offset as u32).saturating_sub(1),
};
}
let mut char_offset = 0;
let len_func = match offset_encoding {
OffsetEncoding::Utf8 => char::len_utf8,
OffsetEncoding::Utf16 => char::len_utf16,
};
for c in line.text.chars() {
char_offset += 1;
if char_offset > column_offset {
break;
}
true_column_offset += len_func(c) - 1;
}
}
lsp_types::Position {
line: (line_offset as u32).saturating_sub(1),
character: (true_column_offset as u32).saturating_sub(1),
}
} }
/// Extracts a suitable "primary" location from a rustc diagnostic. /// Extracts a suitable "primary" location from a rustc diagnostic.
@ -84,18 +137,19 @@ fn primary_location(
config: &DiagnosticsMapConfig, config: &DiagnosticsMapConfig,
workspace_root: &AbsPath, workspace_root: &AbsPath,
span: &DiagnosticSpan, span: &DiagnosticSpan,
snap: &GlobalStateSnapshot,
) -> lsp_types::Location { ) -> lsp_types::Location {
let span_stack = std::iter::successors(Some(span), |span| Some(&span.expansion.as_ref()?.span)); let span_stack = std::iter::successors(Some(span), |span| Some(&span.expansion.as_ref()?.span));
for span in span_stack.clone() { for span in span_stack.clone() {
let abs_path = resolve_path(config, workspace_root, &span.file_name); let abs_path = resolve_path(config, workspace_root, &span.file_name);
if !is_dummy_macro_file(&span.file_name) && abs_path.starts_with(workspace_root) { if !is_dummy_macro_file(&span.file_name) && abs_path.starts_with(workspace_root) {
return location(config, workspace_root, span); return location(config, workspace_root, span, snap);
} }
} }
// Fall back to the outermost macro invocation if no suitable span comes up. // Fall back to the outermost macro invocation if no suitable span comes up.
let last_span = span_stack.last().unwrap(); let last_span = span_stack.last().unwrap();
location(config, workspace_root, last_span) location(config, workspace_root, last_span, snap)
} }
/// Converts a secondary Rust span to a LSP related information /// Converts a secondary Rust span to a LSP related information
@ -105,9 +159,10 @@ fn diagnostic_related_information(
config: &DiagnosticsMapConfig, config: &DiagnosticsMapConfig,
workspace_root: &AbsPath, workspace_root: &AbsPath,
span: &DiagnosticSpan, span: &DiagnosticSpan,
snap: &GlobalStateSnapshot,
) -> Option<lsp_types::DiagnosticRelatedInformation> { ) -> Option<lsp_types::DiagnosticRelatedInformation> {
let message = span.label.clone()?; let message = span.label.clone()?;
let location = location(config, workspace_root, span); let location = location(config, workspace_root, span, snap);
Some(lsp_types::DiagnosticRelatedInformation { location, message }) Some(lsp_types::DiagnosticRelatedInformation { location, message })
} }
@ -142,6 +197,7 @@ fn map_rust_child_diagnostic(
config: &DiagnosticsMapConfig, config: &DiagnosticsMapConfig,
workspace_root: &AbsPath, workspace_root: &AbsPath,
rd: &flycheck::Diagnostic, rd: &flycheck::Diagnostic,
snap: &GlobalStateSnapshot,
) -> MappedRustChildDiagnostic { ) -> MappedRustChildDiagnostic {
let spans: Vec<&DiagnosticSpan> = rd.spans.iter().filter(|s| s.is_primary).collect(); let spans: Vec<&DiagnosticSpan> = rd.spans.iter().filter(|s| s.is_primary).collect();
if spans.is_empty() { if spans.is_empty() {
@ -157,7 +213,7 @@ fn map_rust_child_diagnostic(
if !suggested_replacement.is_empty() { if !suggested_replacement.is_empty() {
suggested_replacements.push(suggested_replacement); suggested_replacements.push(suggested_replacement);
} }
let location = location(config, workspace_root, span); let location = location(config, workspace_root, span, snap);
let edit = lsp_types::TextEdit::new(location.range, suggested_replacement.clone()); let edit = lsp_types::TextEdit::new(location.range, suggested_replacement.clone());
// Only actually emit a quickfix if the suggestion is "valid enough". // Only actually emit a quickfix if the suggestion is "valid enough".
@ -186,7 +242,7 @@ fn map_rust_child_diagnostic(
if edit_map.is_empty() { if edit_map.is_empty() {
MappedRustChildDiagnostic::SubDiagnostic(SubDiagnostic { MappedRustChildDiagnostic::SubDiagnostic(SubDiagnostic {
related: lsp_types::DiagnosticRelatedInformation { related: lsp_types::DiagnosticRelatedInformation {
location: location(config, workspace_root, spans[0]), location: location(config, workspace_root, spans[0], snap),
message, message,
}, },
suggested_fix: None, suggested_fix: None,
@ -194,13 +250,13 @@ fn map_rust_child_diagnostic(
} else { } else {
MappedRustChildDiagnostic::SubDiagnostic(SubDiagnostic { MappedRustChildDiagnostic::SubDiagnostic(SubDiagnostic {
related: lsp_types::DiagnosticRelatedInformation { related: lsp_types::DiagnosticRelatedInformation {
location: location(config, workspace_root, spans[0]), location: location(config, workspace_root, spans[0], snap),
message: message.clone(), message: message.clone(),
}, },
suggested_fix: Some(Fix { suggested_fix: Some(Fix {
ranges: spans ranges: spans
.iter() .iter()
.map(|&span| location(config, workspace_root, span).range) .map(|&span| location(config, workspace_root, span, snap).range)
.collect(), .collect(),
action: lsp_ext::CodeAction { action: lsp_ext::CodeAction {
title: message, title: message,
@ -242,6 +298,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
config: &DiagnosticsMapConfig, config: &DiagnosticsMapConfig,
rd: &flycheck::Diagnostic, rd: &flycheck::Diagnostic,
workspace_root: &AbsPath, workspace_root: &AbsPath,
snap: &GlobalStateSnapshot,
) -> Vec<MappedRustDiagnostic> { ) -> Vec<MappedRustDiagnostic> {
let primary_spans: Vec<&DiagnosticSpan> = rd.spans.iter().filter(|s| s.is_primary).collect(); let primary_spans: Vec<&DiagnosticSpan> = rd.spans.iter().filter(|s| s.is_primary).collect();
if primary_spans.is_empty() { if primary_spans.is_empty() {
@ -266,7 +323,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
let mut tags = Vec::new(); let mut tags = Vec::new();
for secondary_span in rd.spans.iter().filter(|s| !s.is_primary) { for secondary_span in rd.spans.iter().filter(|s| !s.is_primary) {
let related = diagnostic_related_information(config, workspace_root, secondary_span); let related = diagnostic_related_information(config, workspace_root, secondary_span, snap);
if let Some(related) = related { if let Some(related) = related {
subdiagnostics.push(SubDiagnostic { related, suggested_fix: None }); subdiagnostics.push(SubDiagnostic { related, suggested_fix: None });
} }
@ -274,7 +331,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
let mut message = rd.message.clone(); let mut message = rd.message.clone();
for child in &rd.children { for child in &rd.children {
let child = map_rust_child_diagnostic(config, workspace_root, child); let child = map_rust_child_diagnostic(config, workspace_root, child, snap);
match child { match child {
MappedRustChildDiagnostic::SubDiagnostic(sub) => { MappedRustChildDiagnostic::SubDiagnostic(sub) => {
subdiagnostics.push(sub); subdiagnostics.push(sub);
@ -318,7 +375,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
primary_spans primary_spans
.iter() .iter()
.flat_map(|primary_span| { .flat_map(|primary_span| {
let primary_location = primary_location(config, workspace_root, primary_span); let primary_location = primary_location(config, workspace_root, primary_span, snap);
let mut message = message.clone(); let mut message = message.clone();
if needs_primary_span_label { if needs_primary_span_label {
@ -348,7 +405,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
// generated that code. // generated that code.
let is_in_macro_call = i != 0; let is_in_macro_call = i != 0;
let secondary_location = location(config, workspace_root, span); let secondary_location = location(config, workspace_root, span, snap);
if secondary_location == primary_location { if secondary_location == primary_location {
continue; continue;
} }
@ -478,9 +535,12 @@ fn clippy_code_description(code: Option<&str>) -> Option<lsp_types::CodeDescript
mod tests { mod tests {
use std::{convert::TryInto, path::Path}; use std::{convert::TryInto, path::Path};
use crate::{config::Config, global_state::GlobalState};
use super::*; use super::*;
use expect_test::{expect_file, ExpectFile}; use expect_test::{expect_file, ExpectFile};
use lsp_types::ClientCapabilities;
fn check(diagnostics_json: &str, expect: ExpectFile) { fn check(diagnostics_json: &str, expect: ExpectFile) {
check_with_config(DiagnosticsMapConfig::default(), diagnostics_json, expect) check_with_config(DiagnosticsMapConfig::default(), diagnostics_json, expect)
@ -489,7 +549,13 @@ fn check(diagnostics_json: &str, expect: ExpectFile) {
fn check_with_config(config: DiagnosticsMapConfig, diagnostics_json: &str, expect: ExpectFile) { fn check_with_config(config: DiagnosticsMapConfig, diagnostics_json: &str, expect: ExpectFile) {
let diagnostic: flycheck::Diagnostic = serde_json::from_str(diagnostics_json).unwrap(); let diagnostic: flycheck::Diagnostic = serde_json::from_str(diagnostics_json).unwrap();
let workspace_root: &AbsPath = Path::new("/test/").try_into().unwrap(); let workspace_root: &AbsPath = Path::new("/test/").try_into().unwrap();
let actual = map_rust_diagnostic_to_lsp(&config, &diagnostic, workspace_root); let (sender, _) = crossbeam_channel::unbounded();
let state = GlobalState::new(
sender,
Config::new(workspace_root.to_path_buf(), ClientCapabilities::default()),
);
let snap = state.snapshot();
let actual = map_rust_diagnostic_to_lsp(&config, &diagnostic, workspace_root, &snap);
expect.assert_debug_eq(&actual) expect.assert_debug_eq(&actual)
} }
@ -1028,6 +1094,67 @@ fn clippy_pass_by_ref() {
); );
} }
#[test]
fn rustc_range_map_lsp_position() {
check(
r##"{
"message": "mismatched types",
"code": {
"code": "E0308",
"explanation": "Expected type did not match the received type.\n\nErroneous code examples:\n\n```compile_fail,E0308\nfn plus_one(x: i32) -> i32 {\n x + 1\n}\n\nplus_one(\"Not a number\");\n// ^^^^^^^^^^^^^^ expected `i32`, found `&str`\n\nif \"Not a bool\" {\n// ^^^^^^^^^^^^ expected `bool`, found `&str`\n}\n\nlet x: f32 = \"Not a float\";\n// --- ^^^^^^^^^^^^^ expected `f32`, found `&str`\n// |\n// expected due to this\n```\n\nThis error occurs when an expression was used in a place where the compiler\nexpected an expression of a different type. It can occur in several cases, the\nmost common being when calling a function and passing an argument which has a\ndifferent type than the matching type in the function declaration.\n"
},
"level": "error",
"spans": [
{
"file_name": "crates/test_diagnostics/src/main.rs",
"byte_start": 87,
"byte_end": 105,
"line_start": 4,
"line_end": 4,
"column_start": 18,
"column_end": 24,
"is_primary": true,
"text": [
{
"text": " let x: u32 = \"𐐀𐐀𐐀𐐀\"; // 17-23",
"highlight_start": 18,
"highlight_end": 24
}
],
"label": "expected `u32`, found `&str`",
"suggested_replacement": null,
"suggestion_applicability": null,
"expansion": null
},
{
"file_name": "crates/test_diagnostics/src/main.rs",
"byte_start": 81,
"byte_end": 84,
"line_start": 4,
"line_end": 4,
"column_start": 12,
"column_end": 15,
"is_primary": false,
"text": [
{
"text": " let x: u32 = \"𐐀𐐀𐐀𐐀\"; // 17-23",
"highlight_start": 12,
"highlight_end": 15
}
],
"label": "expected due to this",
"suggested_replacement": null,
"suggestion_applicability": null,
"expansion": null
}
],
"children": [],
"rendered": "error[E0308]: mismatched types\n --> crates/test_diagnostics/src/main.rs:4:18\n |\n4 | let x: u32 = \"𐐀𐐀𐐀𐐀\"; // 17-23\n | --- ^^^^^^ expected `u32`, found `&str`\n | |\n | expected due to this\n\n"
}"##,
expect_file!("./test_data/rustc_range_map_lsp_position.txt"),
)
}
#[test] #[test]
fn rustc_mismatched_type() { fn rustc_mismatched_type() {
check( check(

2
crates/rust-analyzer/src/main_loop.rs
View File

@ -370,11 +370,13 @@ fn handle_event(&mut self, event: Event) -> Result<()> {
loop { loop {
match task { match task {
flycheck::Message::AddDiagnostic { workspace_root, diagnostic } => { flycheck::Message::AddDiagnostic { workspace_root, diagnostic } => {
let snap = self.snapshot();
let diagnostics = let diagnostics =
crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp( crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp(
&self.config.diagnostics_map(), &self.config.diagnostics_map(),
&diagnostic, &diagnostic,
&workspace_root, &workspace_root,
&snap,
); );
for diag in diagnostics { for diag in diagnostics {
match url_to_file_id(&self.vfs.read().0, &diag.url) { match url_to_file_id(&self.vfs.read().0, &diag.url) {