diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2021-03-15 01:23:29 +0000 |
---|---|---|
committer | GitHub <[email protected]> | 2021-03-15 01:23:29 +0000 |
commit | de360275416ca095102f2b17d6ca1de3bd091fdb (patch) | |
tree | 319752b9ec07b075941aeab8795242f3a92ec2c0 | |
parent | 5ba7852cf153688d5b5035a9a2a2145aa7334d79 (diff) | |
parent | 32e1ca54eac6b5d401c2f33ec1281794ef9a0a52 (diff) |
Merge #7966
7966: Diagnose files that aren't in the module tree r=jonas-schievink a=jonas-schievink
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/6377
I'm not sure if this is the best way to do this. It will cause false positives for all `include!`d files (though I'm not sure how much IDE functionality we have for these).
Co-authored-by: Jonas Schievink <[email protected]>
-rw-r--r-- | crates/ide/Cargo.toml | 1 | ||||
-rw-r--r-- | crates/ide/src/diagnostics.rs | 163 | ||||
-rw-r--r-- | crates/ide/src/diagnostics/unlinked_file.rs | 156 |
3 files changed, 317 insertions, 3 deletions
diff --git a/crates/ide/Cargo.toml b/crates/ide/Cargo.toml index 107bd8432..6ae7c9e6c 100644 --- a/crates/ide/Cargo.toml +++ b/crates/ide/Cargo.toml | |||
@@ -38,3 +38,4 @@ hir = { path = "../hir", version = "0.0.0" } | |||
38 | [dev-dependencies] | 38 | [dev-dependencies] |
39 | test_utils = { path = "../test_utils" } | 39 | test_utils = { path = "../test_utils" } |
40 | expect-test = "1.1" | 40 | expect-test = "1.1" |
41 | cov-mark = "1.1.0" | ||
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index fe32f39b6..22697a537 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs | |||
@@ -6,6 +6,7 @@ | |||
6 | 6 | ||
7 | mod fixes; | 7 | mod fixes; |
8 | mod field_shorthand; | 8 | mod field_shorthand; |
9 | mod unlinked_file; | ||
9 | 10 | ||
10 | use std::cell::RefCell; | 11 | use std::cell::RefCell; |
11 | 12 | ||
@@ -22,6 +23,7 @@ use syntax::{ | |||
22 | SyntaxNode, SyntaxNodePtr, TextRange, | 23 | SyntaxNode, SyntaxNodePtr, TextRange, |
23 | }; | 24 | }; |
24 | use text_edit::TextEdit; | 25 | use text_edit::TextEdit; |
26 | use unlinked_file::UnlinkedFile; | ||
25 | 27 | ||
26 | use crate::{FileId, Label, SourceChange}; | 28 | use crate::{FileId, Label, SourceChange}; |
27 | 29 | ||
@@ -156,6 +158,18 @@ pub(crate) fn diagnostics( | |||
156 | .with_code(Some(d.code())), | 158 | .with_code(Some(d.code())), |
157 | ); | 159 | ); |
158 | }) | 160 | }) |
161 | .on::<UnlinkedFile, _>(|d| { | ||
162 | // Override severity and mark as unused. | ||
163 | res.borrow_mut().push( | ||
164 | Diagnostic::hint( | ||
165 | sema.diagnostics_display_range(d.display_source()).range, | ||
166 | d.message(), | ||
167 | ) | ||
168 | .with_unused(true) | ||
169 | .with_fix(d.fix(&sema)) | ||
170 | .with_code(Some(d.code())), | ||
171 | ); | ||
172 | }) | ||
159 | .on::<hir::diagnostics::UnresolvedProcMacro, _>(|d| { | 173 | .on::<hir::diagnostics::UnresolvedProcMacro, _>(|d| { |
160 | // Use more accurate position if available. | 174 | // Use more accurate position if available. |
161 | let display_range = d | 175 | let display_range = d |
@@ -197,9 +211,13 @@ pub(crate) fn diagnostics( | |||
197 | ); | 211 | ); |
198 | }); | 212 | }); |
199 | 213 | ||
200 | if let Some(m) = sema.to_module_def(file_id) { | 214 | match sema.to_module_def(file_id) { |
201 | m.diagnostics(db, &mut sink); | 215 | Some(m) => m.diagnostics(db, &mut sink), |
202 | }; | 216 | None => { |
217 | sink.push(UnlinkedFile { file_id, node: SyntaxNodePtr::new(&parse.tree().syntax()) }); | ||
218 | } | ||
219 | } | ||
220 | |||
203 | drop(sink); | 221 | drop(sink); |
204 | res.into_inner() | 222 | res.into_inner() |
205 | } | 223 | } |
@@ -307,6 +325,17 @@ mod tests { | |||
307 | ); | 325 | ); |
308 | } | 326 | } |
309 | 327 | ||
328 | /// Checks that there's a diagnostic *without* fix at `$0`. | ||
329 | fn check_no_fix(ra_fixture: &str) { | ||
330 | let (analysis, file_position) = fixture::position(ra_fixture); | ||
331 | let diagnostic = analysis | ||
332 | .diagnostics(&DiagnosticsConfig::default(), file_position.file_id) | ||
333 | .unwrap() | ||
334 | .pop() | ||
335 | .unwrap(); | ||
336 | assert!(diagnostic.fix.is_none(), "got a fix when none was expected: {:?}", diagnostic); | ||
337 | } | ||
338 | |||
310 | /// Takes a multi-file input fixture with annotated cursor position and checks that no diagnostics | 339 | /// Takes a multi-file input fixture with annotated cursor position and checks that no diagnostics |
311 | /// apply to the file containing the cursor. | 340 | /// apply to the file containing the cursor. |
312 | pub(crate) fn check_no_diagnostics(ra_fixture: &str) { | 341 | pub(crate) fn check_no_diagnostics(ra_fixture: &str) { |
@@ -975,4 +1004,132 @@ impl TestStruct { | |||
975 | 1004 | ||
976 | check_fix(input, expected); | 1005 | check_fix(input, expected); |
977 | } | 1006 | } |
1007 | |||
1008 | #[test] | ||
1009 | fn unlinked_file_prepend_first_item() { | ||
1010 | cov_mark::check!(unlinked_file_prepend_before_first_item); | ||
1011 | check_fix( | ||
1012 | r#" | ||
1013 | //- /main.rs | ||
1014 | fn f() {} | ||
1015 | //- /foo.rs | ||
1016 | $0 | ||
1017 | "#, | ||
1018 | r#" | ||
1019 | mod foo; | ||
1020 | |||
1021 | fn f() {} | ||
1022 | "#, | ||
1023 | ); | ||
1024 | } | ||
1025 | |||
1026 | #[test] | ||
1027 | fn unlinked_file_append_mod() { | ||
1028 | cov_mark::check!(unlinked_file_append_to_existing_mods); | ||
1029 | check_fix( | ||
1030 | r#" | ||
1031 | //- /main.rs | ||
1032 | //! Comment on top | ||
1033 | |||
1034 | mod preexisting; | ||
1035 | |||
1036 | mod preexisting2; | ||
1037 | |||
1038 | struct S; | ||
1039 | |||
1040 | mod preexisting_bottom;) | ||
1041 | //- /foo.rs | ||
1042 | $0 | ||
1043 | "#, | ||
1044 | r#" | ||
1045 | //! Comment on top | ||
1046 | |||
1047 | mod preexisting; | ||
1048 | |||
1049 | mod preexisting2; | ||
1050 | mod foo; | ||
1051 | |||
1052 | struct S; | ||
1053 | |||
1054 | mod preexisting_bottom;) | ||
1055 | "#, | ||
1056 | ); | ||
1057 | } | ||
1058 | |||
1059 | #[test] | ||
1060 | fn unlinked_file_insert_in_empty_file() { | ||
1061 | cov_mark::check!(unlinked_file_empty_file); | ||
1062 | check_fix( | ||
1063 | r#" | ||
1064 | //- /main.rs | ||
1065 | //- /foo.rs | ||
1066 | $0 | ||
1067 | "#, | ||
1068 | r#" | ||
1069 | mod foo; | ||
1070 | "#, | ||
1071 | ); | ||
1072 | } | ||
1073 | |||
1074 | #[test] | ||
1075 | fn unlinked_file_old_style_modrs() { | ||
1076 | check_fix( | ||
1077 | r#" | ||
1078 | //- /main.rs | ||
1079 | mod submod; | ||
1080 | //- /submod/mod.rs | ||
1081 | // in mod.rs | ||
1082 | //- /submod/foo.rs | ||
1083 | $0 | ||
1084 | "#, | ||
1085 | r#" | ||
1086 | // in mod.rs | ||
1087 | mod foo; | ||
1088 | "#, | ||
1089 | ); | ||
1090 | } | ||
1091 | |||
1092 | #[test] | ||
1093 | fn unlinked_file_new_style_mod() { | ||
1094 | check_fix( | ||
1095 | r#" | ||
1096 | //- /main.rs | ||
1097 | mod submod; | ||
1098 | //- /submod.rs | ||
1099 | //- /submod/foo.rs | ||
1100 | $0 | ||
1101 | "#, | ||
1102 | r#" | ||
1103 | mod foo; | ||
1104 | "#, | ||
1105 | ); | ||
1106 | } | ||
1107 | |||
1108 | #[test] | ||
1109 | fn unlinked_file_with_cfg_off() { | ||
1110 | cov_mark::check!(unlinked_file_skip_fix_when_mod_already_exists); | ||
1111 | check_no_fix( | ||
1112 | r#" | ||
1113 | //- /main.rs | ||
1114 | #[cfg(never)] | ||
1115 | mod foo; | ||
1116 | |||
1117 | //- /foo.rs | ||
1118 | $0 | ||
1119 | "#, | ||
1120 | ); | ||
1121 | } | ||
1122 | |||
1123 | #[test] | ||
1124 | fn unlinked_file_with_cfg_on() { | ||
1125 | check_no_diagnostics( | ||
1126 | r#" | ||
1127 | //- /main.rs | ||
1128 | #[cfg(not(never))] | ||
1129 | mod foo; | ||
1130 | |||
1131 | //- /foo.rs | ||
1132 | "#, | ||
1133 | ); | ||
1134 | } | ||
978 | } | 1135 | } |
diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs new file mode 100644 index 000000000..c5741bf6b --- /dev/null +++ b/crates/ide/src/diagnostics/unlinked_file.rs | |||
@@ -0,0 +1,156 @@ | |||
1 | //! Diagnostic emitted for files that aren't part of any crate. | ||
2 | |||
3 | use hir::{ | ||
4 | db::DefDatabase, | ||
5 | diagnostics::{Diagnostic, DiagnosticCode}, | ||
6 | InFile, | ||
7 | }; | ||
8 | use ide_db::{ | ||
9 | base_db::{FileId, FileLoader, SourceDatabase, SourceDatabaseExt}, | ||
10 | source_change::SourceChange, | ||
11 | RootDatabase, | ||
12 | }; | ||
13 | use syntax::{ | ||
14 | ast::{self, ModuleItemOwner, NameOwner}, | ||
15 | AstNode, SyntaxNodePtr, | ||
16 | }; | ||
17 | use text_edit::TextEdit; | ||
18 | |||
19 | use crate::Fix; | ||
20 | |||
21 | use super::fixes::DiagnosticWithFix; | ||
22 | |||
23 | #[derive(Debug)] | ||
24 | pub(crate) struct UnlinkedFile { | ||
25 | pub(crate) file_id: FileId, | ||
26 | pub(crate) node: SyntaxNodePtr, | ||
27 | } | ||
28 | |||
29 | impl Diagnostic for UnlinkedFile { | ||
30 | fn code(&self) -> DiagnosticCode { | ||
31 | DiagnosticCode("unlinked-file") | ||
32 | } | ||
33 | |||
34 | fn message(&self) -> String { | ||
35 | "file not included in module tree".to_string() | ||
36 | } | ||
37 | |||
38 | fn display_source(&self) -> InFile<SyntaxNodePtr> { | ||
39 | InFile::new(self.file_id.into(), self.node.clone()) | ||
40 | } | ||
41 | |||
42 | fn as_any(&self) -> &(dyn std::any::Any + Send + 'static) { | ||
43 | self | ||
44 | } | ||
45 | } | ||
46 | |||
47 | impl DiagnosticWithFix for UnlinkedFile { | ||
48 | fn fix(&self, sema: &hir::Semantics<RootDatabase>) -> Option<Fix> { | ||
49 | // If there's an existing module that could add a `mod` item to include the unlinked file, | ||
50 | // suggest that as a fix. | ||
51 | |||
52 | let source_root = sema.db.source_root(sema.db.file_source_root(self.file_id)); | ||
53 | let our_path = source_root.path_for_file(&self.file_id)?; | ||
54 | let module_name = our_path.name_and_extension()?.0; | ||
55 | |||
56 | // Candidates to look for: | ||
57 | // - `mod.rs` in the same folder | ||
58 | // - we also check `main.rs` and `lib.rs` | ||
59 | // - `$dir.rs` in the parent folder, where `$dir` is the directory containing `self.file_id` | ||
60 | let parent = our_path.parent()?; | ||
61 | let mut paths = | ||
62 | vec![parent.join("mod.rs")?, parent.join("main.rs")?, parent.join("lib.rs")?]; | ||
63 | |||
64 | // `submod/bla.rs` -> `submod.rs` | ||
65 | if let Some(newmod) = (|| { | ||
66 | let name = parent.name_and_extension()?.0; | ||
67 | parent.parent()?.join(&format!("{}.rs", name)) | ||
68 | })() { | ||
69 | paths.push(newmod); | ||
70 | } | ||
71 | |||
72 | for path in &paths { | ||
73 | if let Some(parent_id) = source_root.file_for_path(path) { | ||
74 | for krate in sema.db.relevant_crates(*parent_id).iter() { | ||
75 | let crate_def_map = sema.db.crate_def_map(*krate); | ||
76 | for (_, module) in crate_def_map.modules() { | ||
77 | if module.origin.is_inline() { | ||
78 | // We don't handle inline `mod parent {}`s, they use different paths. | ||
79 | continue; | ||
80 | } | ||
81 | |||
82 | if module.origin.file_id() == Some(*parent_id) { | ||
83 | return make_fix(sema.db, *parent_id, module_name, self.file_id); | ||
84 | } | ||
85 | } | ||
86 | } | ||
87 | } | ||
88 | } | ||
89 | |||
90 | None | ||
91 | } | ||
92 | } | ||
93 | |||
94 | fn make_fix( | ||
95 | db: &RootDatabase, | ||
96 | parent_file_id: FileId, | ||
97 | new_mod_name: &str, | ||
98 | added_file_id: FileId, | ||
99 | ) -> Option<Fix> { | ||
100 | fn is_outline_mod(item: &ast::Item) -> bool { | ||
101 | matches!(item, ast::Item::Module(m) if m.item_list().is_none()) | ||
102 | } | ||
103 | |||
104 | let mod_decl = format!("mod {};", new_mod_name); | ||
105 | let ast: ast::SourceFile = db.parse(parent_file_id).tree(); | ||
106 | |||
107 | let mut builder = TextEdit::builder(); | ||
108 | |||
109 | // If there's an existing `mod m;` statement matching the new one, don't emit a fix (it's | ||
110 | // probably `#[cfg]`d out). | ||
111 | for item in ast.items() { | ||
112 | if let ast::Item::Module(m) = item { | ||
113 | if let Some(name) = m.name() { | ||
114 | if m.item_list().is_none() && name.to_string() == new_mod_name { | ||
115 | cov_mark::hit!(unlinked_file_skip_fix_when_mod_already_exists); | ||
116 | return None; | ||
117 | } | ||
118 | } | ||
119 | } | ||
120 | } | ||
121 | |||
122 | // If there are existing `mod m;` items, append after them (after the first group of them, rather). | ||
123 | match ast | ||
124 | .items() | ||
125 | .skip_while(|item| !is_outline_mod(item)) | ||
126 | .take_while(|item| is_outline_mod(item)) | ||
127 | .last() | ||
128 | { | ||
129 | Some(last) => { | ||
130 | cov_mark::hit!(unlinked_file_append_to_existing_mods); | ||
131 | builder.insert(last.syntax().text_range().end(), format!("\n{}", mod_decl)); | ||
132 | } | ||
133 | None => { | ||
134 | // Prepend before the first item in the file. | ||
135 | match ast.items().next() { | ||
136 | Some(item) => { | ||
137 | cov_mark::hit!(unlinked_file_prepend_before_first_item); | ||
138 | builder.insert(item.syntax().text_range().start(), format!("{}\n\n", mod_decl)); | ||
139 | } | ||
140 | None => { | ||
141 | // No items in the file, so just append at the end. | ||
142 | cov_mark::hit!(unlinked_file_empty_file); | ||
143 | builder.insert(ast.syntax().text_range().end(), format!("{}\n", mod_decl)); | ||
144 | } | ||
145 | } | ||
146 | } | ||
147 | } | ||
148 | |||
149 | let edit = builder.finish(); | ||
150 | let trigger_range = db.parse(added_file_id).tree().syntax().text_range(); | ||
151 | Some(Fix::new( | ||
152 | &format!("Insert `{}`", mod_decl), | ||
153 | SourceChange::from_text_edit(parent_file_id, edit), | ||
154 | trigger_range, | ||
155 | )) | ||
156 | } | ||