diff options
author | Aleksey Kladov <[email protected]> | 2021-05-31 17:06:40 +0100 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2021-05-31 17:45:50 +0100 |
commit | 341f8bb200d60caf4c0ea70738198ac8d62218b8 (patch) | |
tree | 17b534d34bf7f6da4f550678374d7da98d49c687 /crates/hir_ty/src | |
parent | 10b15b27f2f4bee6de63f66c344f741482becd21 (diff) |
fix: avoid panics in match case diagnostic
Diffstat (limited to 'crates/hir_ty/src')
-rw-r--r-- | crates/hir_ty/src/diagnostics/decl_check.rs | 141 |
1 files changed, 33 insertions, 108 deletions
diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index ef982cbcd..bfa53bdce 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs | |||
@@ -150,29 +150,11 @@ impl<'a, 'b> DeclValidator<'a, 'b> { | |||
150 | expected_case: CaseType::LowerSnakeCase, | 150 | expected_case: CaseType::LowerSnakeCase, |
151 | }); | 151 | }); |
152 | 152 | ||
153 | // Check the param names. | ||
154 | let fn_param_replacements = body | ||
155 | .params | ||
156 | .iter() | ||
157 | .filter_map(|&id| match &body[id] { | ||
158 | Pat::Bind { name, .. } => Some(name), | ||
159 | _ => None, | ||
160 | }) | ||
161 | .filter_map(|param_name| { | ||
162 | Some(Replacement { | ||
163 | current_name: param_name.clone(), | ||
164 | suggested_text: to_lower_snake_case(¶m_name.to_string())?, | ||
165 | expected_case: CaseType::LowerSnakeCase, | ||
166 | }) | ||
167 | }) | ||
168 | .collect(); | ||
169 | |||
170 | // Check the patterns inside the function body. | 153 | // Check the patterns inside the function body. |
154 | // This includes function parameters. | ||
171 | let pats_replacements = body | 155 | let pats_replacements = body |
172 | .pats | 156 | .pats |
173 | .iter() | 157 | .iter() |
174 | // We aren't interested in function parameters, we've processed them above. | ||
175 | .filter(|(pat_idx, _)| !body.params.contains(&pat_idx)) | ||
176 | .filter_map(|(id, pat)| match pat { | 158 | .filter_map(|(id, pat)| match pat { |
177 | Pat::Bind { name, .. } => Some((id, name)), | 159 | Pat::Bind { name, .. } => Some((id, name)), |
178 | _ => None, | 160 | _ => None, |
@@ -190,11 +172,10 @@ impl<'a, 'b> DeclValidator<'a, 'b> { | |||
190 | .collect(); | 172 | .collect(); |
191 | 173 | ||
192 | // If there is at least one element to spawn a warning on, go to the source map and generate a warning. | 174 | // If there is at least one element to spawn a warning on, go to the source map and generate a warning. |
193 | self.create_incorrect_case_diagnostic_for_func( | 175 | if let Some(fn_name_replacement) = fn_name_replacement { |
194 | func, | 176 | self.create_incorrect_case_diagnostic_for_func(func, fn_name_replacement); |
195 | fn_name_replacement, | 177 | } |
196 | fn_param_replacements, | 178 | |
197 | ); | ||
198 | self.create_incorrect_case_diagnostic_for_variables(func, pats_replacements); | 179 | self.create_incorrect_case_diagnostic_for_variables(func, pats_replacements); |
199 | } | 180 | } |
200 | 181 | ||
@@ -203,100 +184,34 @@ impl<'a, 'b> DeclValidator<'a, 'b> { | |||
203 | fn create_incorrect_case_diagnostic_for_func( | 184 | fn create_incorrect_case_diagnostic_for_func( |
204 | &mut self, | 185 | &mut self, |
205 | func: FunctionId, | 186 | func: FunctionId, |
206 | fn_name_replacement: Option<Replacement>, | 187 | fn_name_replacement: Replacement, |
207 | fn_param_replacements: Vec<Replacement>, | ||
208 | ) { | 188 | ) { |
209 | // XXX: only look at sources if we do have incorrect names | ||
210 | if fn_name_replacement.is_none() && fn_param_replacements.is_empty() { | ||
211 | return; | ||
212 | } | ||
213 | |||
214 | let fn_loc = func.lookup(self.db.upcast()); | 189 | let fn_loc = func.lookup(self.db.upcast()); |
215 | let fn_src = fn_loc.source(self.db.upcast()); | 190 | let fn_src = fn_loc.source(self.db.upcast()); |
216 | 191 | ||
217 | // Diagnostic for function name. | 192 | // Diagnostic for function name. |
218 | if let Some(replacement) = fn_name_replacement { | 193 | let ast_ptr = match fn_src.value.name() { |
219 | let ast_ptr = match fn_src.value.name() { | 194 | Some(name) => name, |
220 | Some(name) => name, | ||
221 | None => { | ||
222 | never!( | ||
223 | "Replacement ({:?}) was generated for a function without a name: {:?}", | ||
224 | replacement, | ||
225 | fn_src | ||
226 | ); | ||
227 | return; | ||
228 | } | ||
229 | }; | ||
230 | |||
231 | let diagnostic = IncorrectCase { | ||
232 | file: fn_src.file_id, | ||
233 | ident_type: IdentType::Function, | ||
234 | ident: AstPtr::new(&ast_ptr), | ||
235 | expected_case: replacement.expected_case, | ||
236 | ident_text: replacement.current_name.to_string(), | ||
237 | suggested_text: replacement.suggested_text, | ||
238 | }; | ||
239 | |||
240 | self.sink.push(diagnostic); | ||
241 | } | ||
242 | |||
243 | // Diagnostics for function params. | ||
244 | let fn_params_list = match fn_src.value.param_list() { | ||
245 | Some(params) => params, | ||
246 | None => { | 195 | None => { |
247 | always!( | 196 | never!( |
248 | fn_param_replacements.is_empty(), | 197 | "Replacement ({:?}) was generated for a function without a name: {:?}", |
249 | "Replacements ({:?}) were generated for a function parameters which had no parameters list: {:?}", | 198 | fn_name_replacement, |
250 | fn_param_replacements, | ||
251 | fn_src | 199 | fn_src |
252 | ); | 200 | ); |
253 | return; | 201 | return; |
254 | } | 202 | } |
255 | }; | 203 | }; |
256 | let mut fn_params_iter = fn_params_list.params(); | ||
257 | for param_to_rename in fn_param_replacements { | ||
258 | // We assume that parameters in replacement are in the same order as in the | ||
259 | // actual params list, but just some of them (ones that named correctly) are skipped. | ||
260 | let ast_ptr: ast::Name = loop { | ||
261 | match fn_params_iter.next() { | ||
262 | Some(element) => { | ||
263 | if let Some(ast::Pat::IdentPat(pat)) = element.pat() { | ||
264 | if pat.to_string() == param_to_rename.current_name.to_string() { | ||
265 | if let Some(name) = pat.name() { | ||
266 | break name; | ||
267 | } | ||
268 | // This is critical. If we consider this parameter the expected one, | ||
269 | // it **must** have a name. | ||
270 | never!( | ||
271 | "Pattern {:?} equals to expected replacement {:?}, but has no name", | ||
272 | element, | ||
273 | param_to_rename | ||
274 | ); | ||
275 | return; | ||
276 | } | ||
277 | } | ||
278 | } | ||
279 | None => { | ||
280 | never!( | ||
281 | "Replacement ({:?}) was generated for a function parameter which was not found: {:?}", | ||
282 | param_to_rename, fn_src | ||
283 | ); | ||
284 | return; | ||
285 | } | ||
286 | } | ||
287 | }; | ||
288 | 204 | ||
289 | let diagnostic = IncorrectCase { | 205 | let diagnostic = IncorrectCase { |
290 | file: fn_src.file_id, | 206 | file: fn_src.file_id, |
291 | ident_type: IdentType::Argument, | 207 | ident_type: IdentType::Function, |
292 | ident: AstPtr::new(&ast_ptr), | 208 | ident: AstPtr::new(&ast_ptr), |
293 | expected_case: param_to_rename.expected_case, | 209 | expected_case: fn_name_replacement.expected_case, |
294 | ident_text: param_to_rename.current_name.to_string(), | 210 | ident_text: fn_name_replacement.current_name.to_string(), |
295 | suggested_text: param_to_rename.suggested_text, | 211 | suggested_text: fn_name_replacement.suggested_text, |
296 | }; | 212 | }; |
297 | 213 | ||
298 | self.sink.push(diagnostic); | 214 | self.sink.push(diagnostic); |
299 | } | ||
300 | } | 215 | } |
301 | 216 | ||
302 | /// Given the information about incorrect variable names, looks up into the source code | 217 | /// Given the information about incorrect variable names, looks up into the source code |
@@ -327,20 +242,25 @@ impl<'a, 'b> DeclValidator<'a, 'b> { | |||
327 | None => continue, | 242 | None => continue, |
328 | }; | 243 | }; |
329 | 244 | ||
245 | let is_param = ast::Param::can_cast(parent.kind()); | ||
246 | |||
330 | // We have to check that it's either `let var = ...` or `var @ Variant(_)` statement, | 247 | // We have to check that it's either `let var = ...` or `var @ Variant(_)` statement, |
331 | // because e.g. match arms are patterns as well. | 248 | // because e.g. match arms are patterns as well. |
332 | // In other words, we check that it's a named variable binding. | 249 | // In other words, we check that it's a named variable binding. |
333 | let is_binding = ast::LetStmt::can_cast(parent.kind()) | 250 | let is_binding = ast::LetStmt::can_cast(parent.kind()) |
334 | || (ast::MatchArm::can_cast(parent.kind()) | 251 | || (ast::MatchArm::can_cast(parent.kind()) |
335 | && ident_pat.at_token().is_some()); | 252 | && ident_pat.at_token().is_some()); |
336 | if !is_binding { | 253 | if !(is_param || is_binding) { |
337 | // This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm. | 254 | // This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm. |
338 | continue; | 255 | continue; |
339 | } | 256 | } |
340 | 257 | ||
258 | let ident_type = | ||
259 | if is_param { IdentType::Argument } else { IdentType::Variable }; | ||
260 | |||
341 | let diagnostic = IncorrectCase { | 261 | let diagnostic = IncorrectCase { |
342 | file: source_ptr.file_id, | 262 | file: source_ptr.file_id, |
343 | ident_type: IdentType::Variable, | 263 | ident_type, |
344 | ident: AstPtr::new(&name_ast), | 264 | ident: AstPtr::new(&name_ast), |
345 | expected_case: replacement.expected_case, | 265 | expected_case: replacement.expected_case, |
346 | ident_text: replacement.current_name.to_string(), | 266 | ident_text: replacement.current_name.to_string(), |
@@ -408,7 +328,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> { | |||
408 | struct_name_replacement: Option<Replacement>, | 328 | struct_name_replacement: Option<Replacement>, |
409 | struct_fields_replacements: Vec<Replacement>, | 329 | struct_fields_replacements: Vec<Replacement>, |
410 | ) { | 330 | ) { |
411 | // XXX: only look at sources if we do have incorrect names | 331 | // XXX: Only look at sources if we do have incorrect names. |
412 | if struct_name_replacement.is_none() && struct_fields_replacements.is_empty() { | 332 | if struct_name_replacement.is_none() && struct_fields_replacements.is_empty() { |
413 | return; | 333 | return; |
414 | } | 334 | } |
@@ -1037,4 +957,9 @@ fn qualify() { | |||
1037 | "#, | 957 | "#, |
1038 | ) | 958 | ) |
1039 | } | 959 | } |
960 | |||
961 | #[test] // Issue #8809. | ||
962 | fn parenthesized_parameter() { | ||
963 | check_diagnostics(r#"fn f((O): _) {}"#) | ||
964 | } | ||
1040 | } | 965 | } |