Add visibility diagnostics for private fields to the correct location

Author: Natural-selection1

crates/ide-diagnostics/src/handlers/private_field.rs

@@ -1,14 +1,16 @@
-use hir::{EditionedFileId, FileRange, HasCrate, HasSource, Semantics};
+use hir::{EditionedFileId, FieldSource, FileRange, HasCrate, HasSource, Semantics};
 use ide_db::{RootDatabase, assists::Assist, source_change::SourceChange, text_edit::TextEdit};
-use syntax::{AstNode, TextRange, TextSize, ast::HasVisibility};
+use syntax::{
+    AstNode, TextRange,
+    ast::{HasName as _, HasVisibility},
+};
 
 use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, fix};
 
 // Diagnostic: private-field
 //
 // This diagnostic is triggered if the accessed field is not visible from the current module.
 pub(crate) fn private_field(ctx: &DiagnosticsContext<'_>, d: &hir::PrivateField) -> Diagnostic {
-    // FIXME: add quickfix
     Diagnostic::new_with_syntax_node_ptr(
         ctx,
         DiagnosticCode::RustcHardError("E0616"),
@@ -40,8 +42,8 @@ pub(crate) fn field_is_private_fixes(
 
     let source = private_field.source(sema.db)?;
     let existing_visibility = match &source.value {
-        hir::FieldSource::Named(it) => it.visibility(),
-        hir::FieldSource::Pos(it) => it.visibility(),
+        FieldSource::Named(it) => it.visibility(),
+        FieldSource::Pos(it) => it.visibility(),
     };
     let range = match existing_visibility {
         Some(visibility) => {
@@ -50,11 +52,19 @@ pub(crate) fn field_is_private_fixes(
             source.with_value(visibility.syntax()).original_file_range_opt(sema.db)?.0
         }
         None => {
-            let (range, _) = source.syntax().original_file_range_opt(sema.db)?;
-            FileRange {
-                file_id: range.file_id,
-                range: TextRange::at(range.range.start(), TextSize::new(0)),
-            }
+            let (range, _) = source
+                .map(|it| {
+                    Some(match it {
+                        FieldSource::Named(it) => {
+                            it.unsafe_token().or(it.name()?.ident_token())?.text_range()
+                        }
+                        FieldSource::Pos(it) => it.ty()?.syntax().text_range(),
+                    })
+                })
+                .transpose()?
+                .original_node_file_range_opt(sema.db)?;
+
+            FileRange { file_id: range.file_id, range: TextRange::empty(range.range.start()) }
         }
     };
     let source_change = SourceChange::from_text_edit(
@@ -225,6 +235,136 @@ pub mod foo {
 
 fn foo(v: foo::bar::Struct) {
     v.field;
+}
+            "#,
+        );
+    }
+
+    #[test]
+    fn change_visibility_of_field_with_doc_comment() {
+        check_fix(
+            r#"
+pub mod foo {
+    pub struct Foo {
+        /// This is a doc comment
+        bar: u32,
+    }
+}
+
+fn main() {
+    let x = foo::Foo { bar: 0 };
+    x.bar$0;
+}
+            "#,
+            r#"
+pub mod foo {
+    pub struct Foo {
+        /// This is a doc comment
+        pub(crate) bar: u32,
+    }
+}
+
+fn main() {
+    let x = foo::Foo { bar: 0 };
+    x.bar;
+}
+            "#,
+        );
+    }
+
+    #[test]
+    fn change_visibility_of_field_with_line_comment() {
+        check_fix(
+            r#"
+pub mod foo {
+    pub struct Foo {
+        // This is a line comment
+        bar: u32,
+    }
+}
+
+fn main() {
+    let x = foo::Foo { bar: 0 };
+    x.bar$0;
+}
+            "#,
+            r#"
+pub mod foo {
+    pub struct Foo {
+        // This is a line comment
+        pub(crate) bar: u32,
+    }
+}
+
+fn main() {
+    let x = foo::Foo { bar: 0 };
+    x.bar;
+}
+            "#,
+        );
+    }
+
+    #[test]
+    fn change_visibility_of_field_with_multiple_doc_comments() {
+        check_fix(
+            r#"
+pub mod foo {
+    pub struct Foo {
+        /// First line
+        /// Second line
+        bar: u32,
+    }
+}
+
+fn main() {
+    let x = foo::Foo { bar: 0 };
+    x.bar$0;
+}
+            "#,
+            r#"
+pub mod foo {
+    pub struct Foo {
+        /// First line
+        /// Second line
+        pub(crate) bar: u32,
+    }
+}
+
+fn main() {
+    let x = foo::Foo { bar: 0 };
+    x.bar;
+}
+            "#,
+        );
+    }
+
+    #[test]
+    fn change_visibility_of_field_with_attr_and_comment() {
+        check_fix(
+            r#"
+mod foo {
+    pub struct Foo {
+        #[rustfmt::skip]
+        /// First line
+        /// Second line
+        bar: u32,
+    }
+}
+fn main() {
+    foo::Foo { $0bar: 42 };
+}
+            "#,
+            r#"
+mod foo {
+    pub struct Foo {
+        #[rustfmt::skip]
+        /// First line
+        /// Second line
+        pub(crate) bar: u32,
+    }
+}
+fn main() {
+    foo::Foo { bar: 42 };
 }
             "#,
         );