Skip to content

Commit 931f317

Browse files
bors[bot]lnicolaltentrupGabbeV
authored
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 <[email protected]> Co-authored-by: Leander Tentrup <[email protected]> Co-authored-by: Gabriel Valfridsson <[email protected]>
4 parents 09c5cfe + 2fa0b20 + 8ff91cf + 656e952 commit 931f317

10 files changed

+426
-34
lines changed

crates/ra_ide/src/snapshots/highlight_doctest.html

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,13 @@
7373
<span class="comment">///</span>
7474
<span class="comment">/// ```</span>
7575
<span class="comment">///</span>
76-
<span class="comment">/// ```</span>
76+
<span class="comment">/// ```rust,no_run</span>
7777
<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>();
7878
<span class="comment">/// ```</span>
79+
<span class="comment">///</span>
80+
<span class="comment">/// ```sh</span>
81+
<span class="comment">/// echo 1</span>
82+
<span class="comment">/// ```</span>
7983
<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> {
8084
<span class="bool_literal">true</span>
8185
}

crates/ra_ide/src/syntax_highlighting/injection.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ pub(super) fn highlight_injection(
5353
/// Mapping from extracted documentation code to original code
5454
type RangesMap = BTreeMap<TextSize, TextSize>;
5555

56+
const RUSTDOC_FENCE: &'static str = "```";
57+
const RUSTDOC_FENCE_TOKENS: &[&'static str] =
58+
&["", "rust", "should_panic", "ignore", "no_run", "compile_fail", "edition2015", "edition2018"];
59+
5660
/// Extracts Rust code from documentation comments as well as a mapping from
5761
/// the extracted source code back to the original source ranges.
5862
/// Lastly, a vector of new comment highlight ranges (spanning only the
@@ -67,6 +71,7 @@ pub(super) fn extract_doc_comments(
6771
// Mapping from extracted documentation code to original code
6872
let mut range_mapping: RangesMap = BTreeMap::new();
6973
let mut line_start = TextSize::try_from(prefix.len()).unwrap();
74+
let mut is_codeblock = false;
7075
let mut is_doctest = false;
7176
// Replace the original, line-spanning comment ranges by new, only comment-prefix
7277
// spanning comment ranges.
@@ -76,8 +81,13 @@ pub(super) fn extract_doc_comments(
7681
.filter_map(|el| el.into_token().and_then(ast::Comment::cast))
7782
.filter(|comment| comment.kind().doc.is_some())
7883
.filter(|comment| {
79-
if comment.text().contains("```") {
80-
is_doctest = !is_doctest;
84+
if let Some(idx) = comment.text().find(RUSTDOC_FENCE) {
85+
is_codeblock = !is_codeblock;
86+
// Check whether code is rust by inspecting fence guards
87+
let guards = &comment.text()[idx + RUSTDOC_FENCE.len()..];
88+
let is_rust =
89+
guards.split(',').all(|sub| RUSTDOC_FENCE_TOKENS.contains(&sub.trim()));
90+
is_doctest = is_codeblock && is_rust;
8191
false
8292
} else {
8393
is_doctest

crates/ra_ide/src/syntax_highlighting/tests.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,13 @@ impl Foo {
329329
///
330330
/// ```
331331
///
332-
/// ```
332+
/// ```rust,no_run
333333
/// let foobar = Foo::new().bar();
334334
/// ```
335+
///
336+
/// ```sh
337+
/// echo 1
338+
/// ```
335339
pub fn foo(&self) -> bool {
336340
true
337341
}

crates/rust-analyzer/src/config.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
1010
use std::{ffi::OsString, path::PathBuf};
1111

12+
use crate::diagnostics::DiagnosticsConfig;
1213
use lsp_types::ClientCapabilities;
1314
use ra_flycheck::FlycheckConfig;
1415
use ra_ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig};
@@ -20,6 +21,7 @@ pub struct Config {
2021
pub client_caps: ClientCapsConfig,
2122

2223
pub publish_diagnostics: bool,
24+
pub diagnostics: DiagnosticsConfig,
2325
pub lru_capacity: Option<usize>,
2426
pub proc_macro_srv: Option<(PathBuf, Vec<OsString>)>,
2527
pub files: FilesConfig,
@@ -136,6 +138,7 @@ impl Default for Config {
136138

137139
with_sysroot: true,
138140
publish_diagnostics: true,
141+
diagnostics: DiagnosticsConfig::default(),
139142
lru_capacity: None,
140143
proc_macro_srv: None,
141144
files: FilesConfig { watcher: FilesWatcher::Notify, exclude: Vec::new() },
@@ -184,6 +187,8 @@ impl Config {
184187

185188
set(value, "/withSysroot", &mut self.with_sysroot);
186189
set(value, "/diagnostics/enable", &mut self.publish_diagnostics);
190+
set(value, "/diagnostics/warningsAsInfo", &mut self.diagnostics.warnings_as_info);
191+
set(value, "/diagnostics/warningsAsHint", &mut self.diagnostics.warnings_as_hint);
187192
set(value, "/lruCapacity", &mut self.lru_capacity);
188193
self.files.watcher = match get(value, "/files/watcher") {
189194
Some("client") => FilesWatcher::Client,

crates/rust-analyzer/src/diagnostics.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ use crate::lsp_ext;
1010

1111
pub type CheckFixes = Arc<HashMap<FileId, Vec<Fix>>>;
1212

13+
#[derive(Debug, Default, Clone)]
14+
pub struct DiagnosticsConfig {
15+
pub warnings_as_info: Vec<String>,
16+
pub warnings_as_hint: Vec<String>,
17+
}
18+
1319
#[derive(Debug, Default, Clone)]
1420
pub struct DiagnosticCollection {
1521
pub native: HashMap<FileId, Vec<Diagnostic>>,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
---
2+
source: crates/rust-analyzer/src/diagnostics/to_proto.rs
3+
expression: diag
4+
---
5+
[
6+
MappedRustDiagnostic {
7+
location: Location {
8+
uri: "file:///test/driver/subcommand/repl.rs",
9+
range: Range {
10+
start: Position {
11+
line: 290,
12+
character: 8,
13+
},
14+
end: Position {
15+
line: 290,
16+
character: 11,
17+
},
18+
},
19+
},
20+
diagnostic: Diagnostic {
21+
range: Range {
22+
start: Position {
23+
line: 290,
24+
character: 8,
25+
},
26+
end: Position {
27+
line: 290,
28+
character: 11,
29+
},
30+
},
31+
severity: Some(
32+
Hint,
33+
),
34+
code: Some(
35+
String(
36+
"unused_variables",
37+
),
38+
),
39+
source: Some(
40+
"rustc",
41+
),
42+
message: "unused variable: `foo`\n#[warn(unused_variables)] on by default",
43+
related_information: None,
44+
tags: Some(
45+
[
46+
Unnecessary,
47+
],
48+
),
49+
},
50+
fixes: [
51+
CodeAction {
52+
title: "consider prefixing with an underscore",
53+
id: None,
54+
group: None,
55+
kind: Some(
56+
"quickfix",
57+
),
58+
command: None,
59+
edit: Some(
60+
SnippetWorkspaceEdit {
61+
changes: Some(
62+
{
63+
"file:///test/driver/subcommand/repl.rs": [
64+
TextEdit {
65+
range: Range {
66+
start: Position {
67+
line: 290,
68+
character: 8,
69+
},
70+
end: Position {
71+
line: 290,
72+
character: 11,
73+
},
74+
},
75+
new_text: "_foo",
76+
},
77+
],
78+
},
79+
),
80+
document_changes: None,
81+
},
82+
),
83+
},
84+
],
85+
},
86+
]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
---
2+
source: crates/rust-analyzer/src/diagnostics/to_proto.rs
3+
expression: diag
4+
---
5+
[
6+
MappedRustDiagnostic {
7+
location: Location {
8+
uri: "file:///test/driver/subcommand/repl.rs",
9+
range: Range {
10+
start: Position {
11+
line: 290,
12+
character: 8,
13+
},
14+
end: Position {
15+
line: 290,
16+
character: 11,
17+
},
18+
},
19+
},
20+
diagnostic: Diagnostic {
21+
range: Range {
22+
start: Position {
23+
line: 290,
24+
character: 8,
25+
},
26+
end: Position {
27+
line: 290,
28+
character: 11,
29+
},
30+
},
31+
severity: Some(
32+
Information,
33+
),
34+
code: Some(
35+
String(
36+
"unused_variables",
37+
),
38+
),
39+
source: Some(
40+
"rustc",
41+
),
42+
message: "unused variable: `foo`\n#[warn(unused_variables)] on by default",
43+
related_information: None,
44+
tags: Some(
45+
[
46+
Unnecessary,
47+
],
48+
),
49+
},
50+
fixes: [
51+
CodeAction {
52+
title: "consider prefixing with an underscore",
53+
id: None,
54+
group: None,
55+
kind: Some(
56+
"quickfix",
57+
),
58+
command: None,
59+
edit: Some(
60+
SnippetWorkspaceEdit {
61+
changes: Some(
62+
{
63+
"file:///test/driver/subcommand/repl.rs": [
64+
TextEdit {
65+
range: Range {
66+
start: Position {
67+
line: 290,
68+
character: 8,
69+
},
70+
end: Position {
71+
line: 290,
72+
character: 11,
73+
},
74+
},
75+
new_text: "_foo",
76+
},
77+
],
78+
},
79+
),
80+
document_changes: None,
81+
},
82+
),
83+
},
84+
],
85+
},
86+
]

0 commit comments

Comments
 (0)