Repository: rust-analyzer/rust-analyzer
Pull request #4347 (2020-05-06 14:46:55)

Fix usefulness check for never type


Patch:
diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
index 871671de2b..77275156c8 100644
--- a/crates/ra_assists/src/assist_ctx.rs
+++ b/crates/ra_assists/src/assist_ctx.rs
@@ -13,7 +13,7 @@ use ra_syntax::{
 };
 use ra_text_edit::TextEditBuilder;
 
-use crate::{AssistId, AssistLabel, GroupLabel, ResolvedAssist};
+use crate::{AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist};
 
 #[derive(Clone, Debug)]
 pub(crate) struct Assist(pub(crate) Vec<AssistInfo>);
@@ -22,16 +22,16 @@ pub(crate) struct Assist(pub(crate) Vec<AssistInfo>);
 pub(crate) struct AssistInfo {
     pub(crate) label: AssistLabel,
     pub(crate) group_label: Option<GroupLabel>,
-    pub(crate) source_change: Option<SourceChange>,
+    pub(crate) action: Option<SourceChange>,
 }
 
 impl AssistInfo {
     fn new(label: AssistLabel) -> AssistInfo {
-        AssistInfo { label, group_label: None, source_change: None }
+        AssistInfo { label, group_label: None, action: None }
     }
 
-    fn resolved(self, source_change: SourceChange) -> AssistInfo {
-        AssistInfo { source_change: Some(source_change), ..self }
+    fn resolved(self, action: SourceChange) -> AssistInfo {
+        AssistInfo { action: Some(action), ..self }
     }
 
     fn with_group(self, group_label: GroupLabel) -> AssistInfo {
@@ -40,7 +40,7 @@ impl AssistInfo {
 
     pub(crate) fn into_resolved(self) -> Option<ResolvedAssist> {
         let label = self.label;
-        self.source_change.map(|source_change| ResolvedAssist { label, source_change })
+        self.action.map(|action| ResolvedAssist { label, action })
     }
 }
 
@@ -104,12 +104,12 @@ impl<'a> AssistCtx<'a> {
         let change_label = label.label.clone();
         let mut info = AssistInfo::new(label);
         if self.should_compute_edit {
-            let source_change = {
+            let action = {
                 let mut edit = ActionBuilder::new(&self);
                 f(&mut edit);
-                edit.build(change_label)
+                edit.build(change_label, self.frange.file_id)
             };
-            info = info.resolved(source_change)
+            info = info.resolved(action)
         };
 
         Some(Assist(vec![info]))
@@ -163,12 +163,12 @@ impl<'a> AssistGroup<'a> {
         let change_label = label.label.clone();
         let mut info = AssistInfo::new(label).with_group(self.group.clone());
         if self.ctx.should_compute_edit {
-            let source_change = {
+            let action = {
                 let mut edit = ActionBuilder::new(&self.ctx);
                 f(&mut edit);
-                edit.build(change_label)
+                edit.build(change_label, self.ctx.frange.file_id)
             };
-            info = info.resolved(source_change)
+            info = info.resolved(action)
         };
 
         self.assists.push(info)
@@ -186,7 +186,7 @@ impl<'a> AssistGroup<'a> {
 pub(crate) struct ActionBuilder<'a, 'b> {
     edit: TextEditBuilder,
     cursor_position: Option<TextSize>,
-    file: FileId,
+    file: AssistFile,
     ctx: &'a AssistCtx<'b>,
 }
 
@@ -195,7 +195,7 @@ impl<'a, 'b> ActionBuilder<'a, 'b> {
         Self {
             edit: TextEditBuilder::default(),
             cursor_position: None,
-            file: ctx.frange.file_id,
+            file: AssistFile::default(),
             ctx,
         }
     }
@@ -254,16 +254,20 @@ impl<'a, 'b> ActionBuilder<'a, 'b> {
         algo::diff(&node, &new).into_text_edit(&mut self.edit)
     }
 
-    pub(crate) fn set_file(&mut self, assist_file: FileId) {
-        self.file = assist_file;
+    pub(crate) fn set_file(&mut self, assist_file: AssistFile) {
+        self.file = assist_file
     }
 
-    fn build(self, change_label: String) -> SourceChange {
+    fn build(self, change_label: String, current_file: FileId) -> SourceChange {
         let edit = self.edit.finish();
         if edit.is_empty() && self.cursor_position.is_none() {
             panic!("Only call `add_assist` if the assist can be applied")
         }
+        let file = match self.file {
+            AssistFile::CurrentFile => current_file,
+            AssistFile::TargetFile(it) => it,
+        };
         SingleFileChange { label: change_label, edit, cursor_position: self.cursor_position }
-            .into_source_change(self.file)
+            .into_source_change(file)
     }
 }
diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs
index 2780796650..1d9d4e638f 100644
--- a/crates/ra_assists/src/handlers/add_function.rs
+++ b/crates/ra_assists/src/handlers/add_function.rs
@@ -3,10 +3,9 @@ use ra_syntax::{
     SyntaxKind, SyntaxNode, TextSize,
 };
 
-use crate::{Assist, AssistCtx, AssistId};
+use crate::{Assist, AssistCtx, AssistFile, AssistId};
 use ast::{edit::IndentLevel, ArgListOwner, ModuleItemOwner};
 use hir::HirDisplay;
-use ra_db::FileId;
 use rustc_hash::{FxHashMap, FxHashSet};
 
 // Assist: add_function
@@ -71,7 +70,7 @@ struct FunctionTemplate {
     insert_offset: TextSize,
     cursor_offset: TextSize,
     fn_def: ast::SourceFile,
-    file: FileId,
+    file: AssistFile,
 }
 
 struct FunctionBuilder {
@@ -79,7 +78,7 @@ struct FunctionBuilder {
     fn_name: ast::Name,
     type_params: Option<ast::TypeParamList>,
     params: ast::ParamList,
-    file: FileId,
+    file: AssistFile,
     needs_pub: bool,
 }
 
@@ -93,7 +92,7 @@ impl FunctionBuilder {
         target_module: Option<hir::InFile<hir::ModuleSource>>,
     ) -> Option<Self> {
         let needs_pub = target_module.is_some();
-        let mut file = ctx.frange.file_id;
+        let mut file = AssistFile::default();
         let target = if let Some(target_module) = target_module {
             let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, target_module)?;
             file = in_file;
@@ -254,8 +253,9 @@ fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option<GeneratedFu
 fn next_space_for_fn_in_module(
     db: &dyn hir::db::AstDatabase,
     module: hir::InFile<hir::ModuleSource>,
-) -> Option<(FileId, GeneratedFunctionTarget)> {
+) -> Option<(AssistFile, GeneratedFunctionTarget)> {
     let file = module.file_id.original_file(db);
+    let assist_file = AssistFile::TargetFile(file);
     let assist_item = match module.value {
         hir::ModuleSource::SourceFile(it) => {
             if let Some(last_item) = it.items().last() {
@@ -272,7 +272,7 @@ fn next_space_for_fn_in_module(
             }
         }
     };
-    Some((file, assist_item))
+    Some((assist_file, assist_item))
 }
 
 #[cfg(test)]
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index 13ea45ec7c..8cd8f89c42 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -18,7 +18,7 @@ pub mod utils;
 pub mod ast_transform;
 
 use hir::Semantics;
-use ra_db::FileRange;
+use ra_db::{FileId, FileRange};
 use ra_ide_db::{source_change::SourceChange, RootDatabase};
 use ra_syntax::TextRange;
 
@@ -59,7 +59,19 @@ impl AssistLabel {
 #[derive(Debug, Clone)]
 pub struct ResolvedAssist {
     pub label: AssistLabel,
-    pub source_change: SourceChange,
+    pub action: SourceChange,
+}
+
+#[derive(Debug, Clone, Copy)]
+enum AssistFile {
+    CurrentFile,
+    TargetFile(FileId),
+}
+
+impl Default for AssistFile {
+    fn default() -> Self {
+        Self::CurrentFile
+    }
 }
 
 /// Return all the assists applicable at the given position.
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs
index 779e785745..149f650424 100644
--- a/crates/ra_hir_ty/src/_match.rs
+++ b/crates/ra_hir_ty/src/_match.rs
@@ -573,14 +573,20 @@ pub(crate) fn is_useful(
     matrix: &Matrix,
     v: &PatStack,
 ) -> MatchCheckResult<Usefulness> {
-    // Handle the special case of enums with no variants. In that case, no match
-    // arm is useful.
-    if let Ty::Apply(ApplicationTy { ctor: TypeCtor::Adt(AdtId::EnumId(enum_id)), .. }) =
-        cx.infer[cx.match_expr].strip_references()
-    {
-        if cx.db.enum_data(*enum_id).variants.is_empty() {
+    // Handle two special cases:
+    // - enum with no variants
+    // - `!` type
+    // In those cases, no match arm is useful.
+    match cx.infer[cx.match_expr].strip_references() {
+        Ty::Apply(ApplicationTy { ctor: TypeCtor::Adt(AdtId::EnumId(enum_id)), .. }) => {
+            if cx.db.enum_data(*enum_id).variants.is_empty() {
+                return Ok(Usefulness::NotUseful);
+            }
+        }
+        Ty::Apply(ApplicationTy { ctor: TypeCtor::Never, .. }) => {
             return Ok(Usefulness::NotUseful);
         }
+        _ => (),
     }
 
     if v.is_empty() {
@@ -1917,6 +1923,17 @@ mod tests {
         check_no_diagnostic(content);
     }
 
+    #[test]
+    fn type_never() {
+        let content = r"
+            fn test_fn(never: !) {
+                match never {}
+            }
+        ";
+
+        check_no_diagnostic(content);
+    }
+
     #[test]
     fn enum_never_ref() {
         let content = r"
diff --git a/crates/ra_ide/src/completion/completion_item.rs b/crates/ra_ide/src/completion/completion_item.rs
index 6021f7279f..383b23ac44 100644
--- a/crates/ra_ide/src/completion/completion_item.rs
+++ b/crates/ra_ide/src/completion/completion_item.rs
@@ -2,12 +2,11 @@
 
 use std::fmt;
 
+use super::completion_config::SnippetCap;
 use hir::Documentation;
 use ra_syntax::TextRange;
 use ra_text_edit::TextEdit;
 
-use crate::completion::completion_config::SnippetCap;
-
 /// `CompletionItem` describes a single completion variant in the editor pop-up.
 /// It is basically a POD with various properties. To construct a
 /// `CompletionItem`, use `new` method and the `Builder` struct.
diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs
index 737f871091..614029de46 100644
--- a/crates/ra_ide/src/lib.rs
+++ b/crates/ra_ide/src/lib.rs
@@ -478,7 +478,7 @@ impl Analysis {
                     id: assist.label.id,
                     label: assist.label.label,
                     group_label: assist.label.group.map(|it| it.0),
-                    source_change: assist.source_change,
+                    source_change: assist.action,
                 })
                 .collect()
         })


Test patch:
diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs
index 17e3ece9f6..572462bfc4 100644
--- a/crates/ra_assists/src/tests.rs
+++ b/crates/ra_assists/src/tests.rs
@@ -57,7 +57,7 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) {
         });
 
     let actual = {
-        let change = assist.source_change.source_file_edits.pop().unwrap();
+        let change = assist.action.source_file_edits.pop().unwrap();
         let mut actual = before.clone();
         change.edit.apply(&mut actual);
         actual
@@ -94,7 +94,7 @@ fn check(assist: Handler, before: &str, expected: ExpectedResult) {
 
     match (assist(assist_ctx), expected) {
         (Some(assist), ExpectedResult::After(after)) => {
-            let mut action = assist.0[0].source_change.clone().unwrap();
+            let mut action = assist.0[0].action.clone().unwrap();
             let change = action.source_file_edits.pop().unwrap();
 
             let mut actual = db.file_text(change.file_id).as_ref().to_owned();