4913: Remove debugging code for incremental sync r=matklad a=lnicola



4915: Inspect markdown code fences to determine whether to apply syntax highlighting r=matklad a=ltentrup

Fixes #4904 

4916: Warnings as hint or info r=matklad a=GabbeV

Fixes #4229 

This PR is my second attempt at providing a solution to the above issue. My last PR(#4721) had to be rolled back(#4862) due to it overriding behavior many users expected. This PR solves a broader problem while trying to minimize surprises for the users. 

### Problem description
The underlying problem this PR tries to solve is the mismatch between [Rustc lint levels](https://doc.rust-lang.org/rustc/lints/levels.html) and [LSP diagnostic severity](https://microsoft.github.io/language-server-protocol/specification#diagnostic). Rustc currently doesn't have a lint level less severe than warning forcing the user to disable warnings if they think they get to noisy. LSP however provides two severitys below warning, information and hint. This allows editors like VSCode to provide more fine grained control over how prominently to show different diagnostics.

Info severity shows a blue squiggly underline in code and can be filtered separately from errors and warnings in the problems panel.
![image](https://user-images.githubusercontent.com/13839236/84830640-0bb8d900-b02a-11ea-9e2f-0561b0e8f1ef.png)
![image](https://user-images.githubusercontent.com/13839236/84826931-ffca1880-b023-11ea-8080-5e5b91a6ac0d.png)

Hint severity doesn't show up in the problems panel at all and only show three dots under the affected code or just faded text if the diagnostic also has the unnecessary tag.
![image](https://user-images.githubusercontent.com/13839236/84827165-55062a00-b024-11ea-8bd6-bdbf1217c4c5.png)

### Solution
The solution provided by this PR allows the user to configure lists of of warnings to report as info severity and hint severity respectively. I purposefully only convert warnings and not errors as i believe it's a good idea to have the editor show the same severity as the compiler as much as possible.
![image](https://user-images.githubusercontent.com/13839236/84829609-50437500-b028-11ea-80a8-1bbd05680ba7.png)

### Open questions
#### Discoverability
How do we teach this to new and existing users? Should a section be added to the user manual? If so  where and what should it say?

#### Defaults
Other languages such as TypeScript report unused code as hint by default. Should rust-analyzer similarly report some problems as hint/info by default?

Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
Co-authored-by: Leander Tentrup <leander.tentrup@gmail.com>
Co-authored-by: Gabriel Valfridsson <gabriel.valfridsson@gmail.com>
This commit is contained in:
bors[bot] 2020-06-17 11:01:37 +00:00 committed by GitHub
commit 931f317399
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 426 additions and 34 deletions

View File

@ -73,9 +73,13 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
<span class="comment">///</span>
<span class="comment">/// ```</span>
<span class="comment">///</span>
<span class="comment">/// ```</span>
<span class="comment">/// ```rust,no_run</span>
<span class="comment">/// </span><span class="keyword">let</span> <span class="variable declaration">foobar</span> = <span class="struct">Foo</span>::<span class="function">new</span>().<span class="function">bar</span>();
<span class="comment">/// ```</span>
<span class="comment">///</span>
<span class="comment">/// ```sh</span>
<span class="comment">/// echo 1</span>
<span class="comment">/// ```</span>
<span class="keyword">pub</span> <span class="keyword">fn</span> <span class="function declaration">foo</span>(&<span class="self_keyword">self</span>) -&gt; <span class="builtin_type">bool</span> {
<span class="bool_literal">true</span>
}

View File

@ -53,6 +53,10 @@ pub(super) fn highlight_injection(
/// Mapping from extracted documentation code to original code
type RangesMap = BTreeMap<TextSize, TextSize>;
const RUSTDOC_FENCE: &'static str = "```";
const RUSTDOC_FENCE_TOKENS: &[&'static str] =
&["", "rust", "should_panic", "ignore", "no_run", "compile_fail", "edition2015", "edition2018"];
/// Extracts Rust code from documentation comments as well as a mapping from
/// the extracted source code back to the original source ranges.
/// Lastly, a vector of new comment highlight ranges (spanning only the
@ -67,6 +71,7 @@ pub(super) fn extract_doc_comments(
// Mapping from extracted documentation code to original code
let mut range_mapping: RangesMap = BTreeMap::new();
let mut line_start = TextSize::try_from(prefix.len()).unwrap();
let mut is_codeblock = false;
let mut is_doctest = false;
// Replace the original, line-spanning comment ranges by new, only comment-prefix
// spanning comment ranges.
@ -76,8 +81,13 @@ pub(super) fn extract_doc_comments(
.filter_map(|el| el.into_token().and_then(ast::Comment::cast))
.filter(|comment| comment.kind().doc.is_some())
.filter(|comment| {
if comment.text().contains("```") {
is_doctest = !is_doctest;
if let Some(idx) = comment.text().find(RUSTDOC_FENCE) {
is_codeblock = !is_codeblock;
// Check whether code is rust by inspecting fence guards
let guards = &comment.text()[idx + RUSTDOC_FENCE.len()..];
let is_rust =
guards.split(',').all(|sub| RUSTDOC_FENCE_TOKENS.contains(&sub.trim()));
is_doctest = is_codeblock && is_rust;
false
} else {
is_doctest

View File

@ -329,9 +329,13 @@ pub const fn new() -> Foo {
///
/// ```
///
/// ```
/// ```rust,no_run
/// let foobar = Foo::new().bar();
/// ```
///
/// ```sh
/// echo 1
/// ```
pub fn foo(&self) -> bool {
true
}

View File

@ -9,6 +9,7 @@
use std::{ffi::OsString, path::PathBuf};
use crate::diagnostics::DiagnosticsConfig;
use lsp_types::ClientCapabilities;
use ra_flycheck::FlycheckConfig;
use ra_ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig};
@ -20,6 +21,7 @@ pub struct Config {
pub client_caps: ClientCapsConfig,
pub publish_diagnostics: bool,
pub diagnostics: DiagnosticsConfig,
pub lru_capacity: Option<usize>,
pub proc_macro_srv: Option<(PathBuf, Vec<OsString>)>,
pub files: FilesConfig,
@ -136,6 +138,7 @@ fn default() -> Self {
with_sysroot: true,
publish_diagnostics: true,
diagnostics: DiagnosticsConfig::default(),
lru_capacity: None,
proc_macro_srv: None,
files: FilesConfig { watcher: FilesWatcher::Notify, exclude: Vec::new() },
@ -184,6 +187,8 @@ pub fn update(&mut self, value: &serde_json::Value) {
set(value, "/withSysroot", &mut self.with_sysroot);
set(value, "/diagnostics/enable", &mut self.publish_diagnostics);
set(value, "/diagnostics/warningsAsInfo", &mut self.diagnostics.warnings_as_info);
set(value, "/diagnostics/warningsAsHint", &mut self.diagnostics.warnings_as_hint);
set(value, "/lruCapacity", &mut self.lru_capacity);
self.files.watcher = match get(value, "/files/watcher") {
Some("client") => FilesWatcher::Client,

View File

@ -10,6 +10,12 @@
pub type CheckFixes = Arc<HashMap<FileId, Vec<Fix>>>;
#[derive(Debug, Default, Clone)]
pub struct DiagnosticsConfig {
pub warnings_as_info: Vec<String>,
pub warnings_as_hint: Vec<String>,
}
#[derive(Debug, Default, Clone)]
pub struct DiagnosticCollection {
pub native: HashMap<FileId, Vec<Diagnostic>>,

View File

@ -0,0 +1,86 @@
---
source: crates/rust-analyzer/src/diagnostics/to_proto.rs
expression: diag
---
[
MappedRustDiagnostic {
location: Location {
uri: "file:///test/driver/subcommand/repl.rs",
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"unused_variables",
),
),
source: Some(
"rustc",
),
message: "unused variable: `foo`\n#[warn(unused_variables)] on by default",
related_information: None,
tags: Some(
[
Unnecessary,
],
),
},
fixes: [
CodeAction {
title: "consider prefixing with an underscore",
id: None,
group: None,
kind: Some(
"quickfix",
),
command: None,
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
"file:///test/driver/subcommand/repl.rs": [
TextEdit {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
new_text: "_foo",
},
],
},
),
document_changes: None,
},
),
},
],
},
]

View File

@ -0,0 +1,86 @@
---
source: crates/rust-analyzer/src/diagnostics/to_proto.rs
expression: diag
---
[
MappedRustDiagnostic {
location: Location {
uri: "file:///test/driver/subcommand/repl.rs",
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
severity: Some(
Information,
),
code: Some(
String(
"unused_variables",
),
),
source: Some(
"rustc",
),
message: "unused variable: `foo`\n#[warn(unused_variables)] on by default",
related_information: None,
tags: Some(
[
Unnecessary,
],
),
},
fixes: [
CodeAction {
title: "consider prefixing with an underscore",
id: None,
group: None,
kind: Some(
"quickfix",
),
command: None,
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
"file:///test/driver/subcommand/repl.rs": [
TextEdit {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
new_text: "_foo",
},
],
},
),
document_changes: None,
},
),
},
],
},
]

View File

@ -9,14 +9,24 @@
use ra_flycheck::{Applicability, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion};
use stdx::format_to;
use super::DiagnosticsConfig;
use crate::{lsp_ext, to_proto::url_from_abs_path};
/// Converts a Rust level string to a LSP severity
fn map_level_to_severity(val: DiagnosticLevel) -> Option<DiagnosticSeverity> {
let res = match val {
/// Determines the LSP severity from a diagnostic
fn map_diagnostic_to_severity(
config: &DiagnosticsConfig,
val: &ra_flycheck::Diagnostic,
) -> Option<DiagnosticSeverity> {
let res = match val.level {
DiagnosticLevel::Ice => DiagnosticSeverity::Error,
DiagnosticLevel::Error => DiagnosticSeverity::Error,
DiagnosticLevel::Warning => DiagnosticSeverity::Warning,
DiagnosticLevel::Warning => match &val.code {
Some(code) if config.warnings_as_hint.contains(&code.code) => DiagnosticSeverity::Hint,
Some(code) if config.warnings_as_info.contains(&code.code) => {
DiagnosticSeverity::Information
}
_ => DiagnosticSeverity::Warning,
},
DiagnosticLevel::Note => DiagnosticSeverity::Information,
DiagnosticLevel::Help => DiagnosticSeverity::Hint,
DiagnosticLevel::Unknown => return None,
@ -172,6 +182,7 @@ pub(crate) struct MappedRustDiagnostic {
///
/// If the diagnostic has no primary span this will return `None`
pub(crate) fn map_rust_diagnostic_to_lsp(
config: &DiagnosticsConfig,
rd: &ra_flycheck::Diagnostic,
workspace_root: &Path,
) -> Vec<MappedRustDiagnostic> {
@ -180,7 +191,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
return Vec::new();
}
let severity = map_level_to_severity(rd.level);
let severity = map_diagnostic_to_severity(config, rd);
let mut source = String::from("rustc");
let mut code = rd.code.as_ref().map(|c| c.code.clone());
@ -328,7 +339,7 @@ fn snap_rustc_incompatible_type_for_trait() {
);
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&diag, workspace_root);
let diag = map_rust_diagnostic_to_lsp(&DiagnosticsConfig::default(), &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
@ -410,7 +421,183 @@ fn snap_rustc_unused_variable() {
);
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&diag, workspace_root);
let diag = map_rust_diagnostic_to_lsp(&DiagnosticsConfig::default(), &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
#[test]
#[cfg(not(windows))]
fn snap_rustc_unused_variable_as_info() {
let diag = parse_diagnostic(
r##"{
"message": "unused variable: `foo`",
"code": {
"code": "unused_variables",
"explanation": null
},
"level": "warning",
"spans": [
{
"file_name": "driver/subcommand/repl.rs",
"byte_start": 9228,
"byte_end": 9231,
"line_start": 291,
"line_end": 291,
"column_start": 9,
"column_end": 12,
"is_primary": true,
"text": [
{
"text": " let foo = 42;",
"highlight_start": 9,
"highlight_end": 12
}
],
"label": null,
"suggested_replacement": null,
"suggestion_applicability": null,
"expansion": null
}
],
"children": [
{
"message": "#[warn(unused_variables)] on by default",
"code": null,
"level": "note",
"spans": [],
"children": [],
"rendered": null
},
{
"message": "consider prefixing with an underscore",
"code": null,
"level": "help",
"spans": [
{
"file_name": "driver/subcommand/repl.rs",
"byte_start": 9228,
"byte_end": 9231,
"line_start": 291,
"line_end": 291,
"column_start": 9,
"column_end": 12,
"is_primary": true,
"text": [
{
"text": " let foo = 42;",
"highlight_start": 9,
"highlight_end": 12
}
],
"label": null,
"suggested_replacement": "_foo",
"suggestion_applicability": "MachineApplicable",
"expansion": null
}
],
"children": [],
"rendered": null
}
],
"rendered": "warning: unused variable: `foo`\n --> driver/subcommand/repl.rs:291:9\n |\n291 | let foo = 42;\n | ^^^ help: consider prefixing with an underscore: `_foo`\n |\n = note: #[warn(unused_variables)] on by default\n\n"
}"##,
);
let config = DiagnosticsConfig {
warnings_as_info: vec!["unused_variables".to_string()],
..DiagnosticsConfig::default()
};
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&config, &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
#[test]
#[cfg(not(windows))]
fn snap_rustc_unused_variable_as_hint() {
let diag = parse_diagnostic(
r##"{
"message": "unused variable: `foo`",
"code": {
"code": "unused_variables",
"explanation": null
},
"level": "warning",
"spans": [
{
"file_name": "driver/subcommand/repl.rs",
"byte_start": 9228,
"byte_end": 9231,
"line_start": 291,
"line_end": 291,
"column_start": 9,
"column_end": 12,
"is_primary": true,
"text": [
{
"text": " let foo = 42;",
"highlight_start": 9,
"highlight_end": 12
}
],
"label": null,
"suggested_replacement": null,
"suggestion_applicability": null,
"expansion": null
}
],
"children": [
{
"message": "#[warn(unused_variables)] on by default",
"code": null,
"level": "note",
"spans": [],
"children": [],
"rendered": null
},
{
"message": "consider prefixing with an underscore",
"code": null,
"level": "help",
"spans": [
{
"file_name": "driver/subcommand/repl.rs",
"byte_start": 9228,
"byte_end": 9231,
"line_start": 291,
"line_end": 291,
"column_start": 9,
"column_end": 12,
"is_primary": true,
"text": [
{
"text": " let foo = 42;",
"highlight_start": 9,
"highlight_end": 12
}
],
"label": null,
"suggested_replacement": "_foo",
"suggestion_applicability": "MachineApplicable",
"expansion": null
}
],
"children": [],
"rendered": null
}
],
"rendered": "warning: unused variable: `foo`\n --> driver/subcommand/repl.rs:291:9\n |\n291 | let foo = 42;\n | ^^^ help: consider prefixing with an underscore: `_foo`\n |\n = note: #[warn(unused_variables)] on by default\n\n"
}"##,
);
let config = DiagnosticsConfig {
warnings_as_hint: vec!["unused_variables".to_string()],
..DiagnosticsConfig::default()
};
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&config, &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
@ -534,7 +721,7 @@ fn snap_rustc_wrong_number_of_parameters() {
);
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&diag, workspace_root);
let diag = map_rust_diagnostic_to_lsp(&DiagnosticsConfig::default(), &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
@ -654,7 +841,7 @@ fn snap_clippy_pass_by_ref() {
);
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&diag, workspace_root);
let diag = map_rust_diagnostic_to_lsp(&DiagnosticsConfig::default(), &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
@ -697,7 +884,7 @@ fn snap_rustc_mismatched_type() {
);
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&diag, workspace_root);
let diag = map_rust_diagnostic_to_lsp(&DiagnosticsConfig::default(), &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
@ -968,7 +1155,7 @@ fn snap_handles_macro_location() {
);
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&diag, workspace_root);
let diag = map_rust_diagnostic_to_lsp(&DiagnosticsConfig::default(), &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
@ -1197,7 +1384,7 @@ fn snap_macro_compiler_error() {
);
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&diag, workspace_root);
let diag = map_rust_diagnostic_to_lsp(&DiagnosticsConfig::default(), &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
@ -1330,7 +1517,7 @@ fn snap_multi_line_fix() {
);
let workspace_root = Path::new("/test/");
let diag = map_rust_diagnostic_to_lsp(&diag, workspace_root);
let diag = map_rust_diagnostic_to_lsp(&DiagnosticsConfig::default(), &diag, workspace_root);
insta::assert_debug_snapshot!(diag);
}
}

View File

@ -669,14 +669,11 @@ fn apply_document_changes(
mut line_index: Cow<'_, LineIndex>,
content_changes: Vec<TextDocumentContentChangeEvent>,
) {
// Remove when https://github.com/rust-analyzer/rust-analyzer/issues/4263 is fixed.
let backup_text = old_text.clone();
let backup_changes = content_changes.clone();
// The changes we got must be applied sequentially, but can cross lines so we
// have to keep our line index updated.
// Some clients (e.g. Code) sort the ranges in reverse. As an optimization, we
// remember the last valid line in the index and only rebuild it if needed.
// The VFS will normalize the end of lines to `\n`.
enum IndexValid {
All,
UpToLineExclusive(u64),
@ -700,19 +697,7 @@ fn covers(&self, line: u64) -> bool {
}
index_valid = IndexValid::UpToLineExclusive(range.start.line);
let range = from_proto::text_range(&line_index, range);
let mut text = old_text.to_owned();
match std::panic::catch_unwind(move || {
text.replace_range(Range::<usize>::from(range), &change.text);
text
}) {
Ok(t) => *old_text = t,
Err(e) => {
eprintln!("Bug in incremental text synchronization. Please report the following output on https://github.com/rust-analyzer/rust-analyzer/issues/4263");
dbg!(&backup_text);
dbg!(&backup_changes);
std::panic::resume_unwind(e);
}
}
old_text.replace_range(Range::<usize>::from(range), &change.text);
}
None => {
*old_text = change.text;
@ -734,6 +719,7 @@ fn on_check_task(
CheckTask::AddDiagnostic { workspace_root, diagnostic } => {
let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp(
&global_state.config.diagnostics,
&diagnostic,
&workspace_root,
);

View File

@ -525,6 +525,24 @@
"markdownDescription": "Internal config for debugging, disables loading of sysroot crates",
"type": "boolean",
"default": true
},
"rust-analyzer.diagnostics.warningsAsInfo": {
"type": "array",
"uniqueItems": true,
"items": {
"type": "string"
},
"description": "List of warnings that should be displayed with info severity.\nThe warnings will be indicated by a blue squiggly underline in code and a blue icon in the problems panel.",
"default": []
},
"rust-analyzer.diagnostics.warningsAsHint": {
"type": "array",
"uniqueItems": true,
"items": {
"type": "string"
},
"description": "List of warnings warnings that should be displayed with hint severity.\nThe warnings will be indicated by faded text or three dots in code and will not show up in te problems panel.",
"default": []
}
}
},