nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-25 08:13:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge #8345

Browse Source 
8345: Add pub mod option for UnlinkedFile r=rainy-me a=rainy-me

close #8228

This is a draft that changes `Diagnostic` to contain multiple fixes. Pre analysis is in https://github.com/rust-analyzer/rust-analyzer/issues/8228#issuecomment-812887085 Because this solution is straightforward so I decided to type it out for discussion.

Currently the `check_fix` is not able to test the situation when multiple fixes available. <del>Also because `Insert 'mod x;'` and  `Insert 'pub mod x;'` are so similar, I don't know how to test them correctly and want some suggestions.</del>. I added 
 `check_fixes` to allow checking mutiple possible fixes.

In additional, instead of append after possible existing `mod y`, I think it's possible to Insert `pub mod x;` after `pub mod y`. Should I implement this too?

Co-authored-by: rainy-me <github@rainy.me>
This commit is contained in:
bors[bot] 2021-05-18 02:02:34 +00:00 committed by GitHub
commit e3d0d89d7e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 158 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
crates/ide/src/diagnostics.rs
View File

@ -28,7 +28,7 @@ use unlinked_file::UnlinkedFile;
use crate::{Assist, AssistId, AssistKind, FileId, Label, SourceChange};
use self::fixes::DiagnosticWithFix;
use self::fixes::DiagnosticWithFixes;
#[derive(Debug)]
pub struct Diagnostic {
@ -36,14 +36,14 @@ pub struct Diagnostic {
pub message: String,
pub range: TextRange,
pub severity: Severity,
pub fix: Option<Assist>,
pub fixes: Option<Vec<Assist>>,
pub unused: bool,
pub code: Option<DiagnosticCode>,
}
impl Diagnostic {
fn error(range: TextRange, message: String) -> Self {
Self { message, range, severity: Severity::Error, fix: None, unused: false, code: None }
Self { message, range, severity: Severity::Error, fixes: None, unused: false, code: None }
}
fn hint(range: TextRange, message: String) -> Self {
@ -51,14 +51,14 @@ impl Diagnostic {
message,
range,
severity: Severity::WeakWarning,
fix: None,
fixes: None,
unused: false,
code: None,
}
}
fn with_fix(self, fix: Option<Assist>) -> Self {
Self { fix, ..self }
fn with_fixes(self, fixes: Option<Vec<Assist>>) -> Self {
Self { fixes, ..self }
}
fn with_unused(self, unused: bool) -> Self {
@ -154,7 +154,7 @@ pub(crate) fn diagnostics(
// Override severity and mark as unused.
res.borrow_mut().push(
Diagnostic::hint(range, d.message())
.with_fix(d.fix(&sema, resolve))
.with_fixes(d.fixes(&sema, resolve))
.with_code(Some(d.code())),
);
})
@ -210,23 +210,23 @@ pub(crate) fn diagnostics(
res.into_inner()
}
fn diagnostic_with_fix<D: DiagnosticWithFix>(
fn diagnostic_with_fix<D: DiagnosticWithFixes>(
d: &D,
sema: &Semantics<RootDatabase>,
resolve: &AssistResolveStrategy,
) -> Diagnostic {
Diagnostic::error(sema.diagnostics_display_range(d.display_source()).range, d.message())
.with_fix(d.fix(&sema, resolve))
.with_fixes(d.fixes(&sema, resolve))
.with_code(Some(d.code()))
}
fn warning_with_fix<D: DiagnosticWithFix>(
fn warning_with_fix<D: DiagnosticWithFixes>(
d: &D,
sema: &Semantics<RootDatabase>,
resolve: &AssistResolveStrategy,
) -> Diagnostic {
Diagnostic::hint(sema.diagnostics_display_range(d.display_source()).range, d.message())
.with_fix(d.fix(&sema, resolve))
.with_fixes(d.fixes(&sema, resolve))
.with_code(Some(d.code()))
}
@ -256,12 +256,12 @@ fn check_unnecessary_braces_in_use_statement(
acc.push(
Diagnostic::hint(use_range, "Unnecessary braces in use statement".to_string())
.with_fix(Some(fix(
.with_fixes(Some(vec![fix(
"remove_braces",
"Remove unnecessary braces",
SourceChange::from_text_edit(file_id, edit),
use_range,
))),
)])),
);
}
@ -309,9 +309,23 @@ mod tests {
/// Takes a multi-file input fixture with annotated cursor positions,
/// and checks that:
/// * a diagnostic is produced
/// * this diagnostic fix trigger range touches the input cursor position
/// * the first diagnostic fix trigger range touches the input cursor position
/// * that the contents of the file containing the cursor match `after` after the diagnostic fix is applied
pub(crate) fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) {
check_nth_fix(0, ra_fixture_before, ra_fixture_after);
}
/// Takes a multi-file input fixture with annotated cursor positions,
/// and checks that:
/// * a diagnostic is produced
/// * every diagnostic fixes trigger range touches the input cursor position
/// * that the contents of the file containing the cursor match `after` after each diagnostic fix is applied
pub(crate) fn check_fixes(ra_fixture_before: &str, ra_fixtures_after: Vec<&str>) {
for (i, ra_fixture_after) in ra_fixtures_after.iter().enumerate() {
check_nth_fix(i, ra_fixture_before, ra_fixture_after)
}
}
fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) {
let after = trim_indent(ra_fixture_after);
let (analysis, file_position) = fixture::position(ra_fixture_before);
@ -324,9 +338,9 @@ mod tests {
.unwrap()
.pop()
.unwrap();
let fix = diagnostic.fix.unwrap();
let fix = &diagnostic.fixes.unwrap()[nth];
let actual = {
let source_change = fix.source_change.unwrap();
let source_change = fix.source_change.as_ref().unwrap();
let file_id = *source_change.source_file_edits.keys().next().unwrap();
let mut actual = analysis.file_text(file_id).unwrap().to_string();
@ -344,7 +358,6 @@ mod tests {
file_position.offset
);
}
/// Checks that there's a diagnostic *without* fix at `$0`.
fn check_no_fix(ra_fixture: &str) {
let (analysis, file_position) = fixture::position(ra_fixture);
@ -357,7 +370,7 @@ mod tests {
.unwrap()
.pop()
.unwrap();
assert!(diagnostic.fix.is_none(), "got a fix when none was expected: {:?}", diagnostic);
assert!(diagnostic.fixes.is_none(), "got a fix when none was expected: {:?}", diagnostic);
}
/// Takes a multi-file input fixture with annotated cursor position and checks that no diagnostics
@ -393,7 +406,7 @@ mod tests {
message: "unresolved macro `foo::bar!`",
range: 5..8,
severity: Error,
fix: None,
fixes: None,
unused: false,
code: Some(
DiagnosticCode(
@ -542,18 +555,26 @@ mod a {
#[test]
fn unlinked_file_prepend_first_item() {
cov_mark::check!(unlinked_file_prepend_before_first_item);
check_fix(
// Only tests the first one for `pub mod` since the rest are the same
check_fixes(
r#"
//- /main.rs
fn f() {}
//- /foo.rs
$0
"#,
vec![
r#"
mod foo;
fn f() {}
"#,
r#"
pub mod foo;
fn f() {}
"#,
],
);
}

13
crates/ide/src/diagnostics/field_shorthand.rs
View File

@ -46,14 +46,13 @@ fn check_expr_field_shorthand(
let field_range = record_field.syntax().text_range();
acc.push(
Diagnostic::hint(field_range, "Shorthand struct initialization".to_string()).with_fix(
Some(fix(
Diagnostic::hint(field_range, "Shorthand struct initialization".to_string())
.with_fixes(Some(vec![fix(
"use_expr_field_shorthand",
"Use struct shorthand initialization",
SourceChange::from_text_edit(file_id, edit),
field_range,
)),
),
)])),
);
}
}
@ -86,13 +85,13 @@ fn check_pat_field_shorthand(
let edit = edit_builder.finish();
let field_range = record_pat_field.syntax().text_range();
acc.push(Diagnostic::hint(field_range, "Shorthand struct pattern".to_string()).with_fix(
Some(fix(
acc.push(Diagnostic::hint(field_range, "Shorthand struct pattern".to_string()).with_fixes(
Some(vec![fix(
"use_pat_field_shorthand",
"Use struct field shorthand",
SourceChange::from_text_edit(file_id, edit),
field_range,
)),
)]),
));
}
}

8
crates/ide/src/diagnostics/fixes.rs
View File

@ -14,18 +14,18 @@ use ide_db::RootDatabase;
use crate::Assist;
/// A [Diagnostic] that potentially has a fix available.
/// A [Diagnostic] that potentially has some fixes available.
///
/// [Diagnostic]: hir::diagnostics::Diagnostic
pub(crate) trait DiagnosticWithFix: Diagnostic {
pub(crate) trait DiagnosticWithFixes: Diagnostic {
/// `resolve` determines if the diagnostic should fill in the `edit` field
/// of the assist.
///
/// If `resolve` is false, the edit will be computed later, on demand, and
/// can be omitted.
fn fix(
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Assist>;
) -> Option<Vec<Assist>>;
}

10
crates/ide/src/diagnostics/fixes/change_case.rs
View File

@ -4,16 +4,16 @@ use ide_db::{base_db::FilePosition, RootDatabase};
use syntax::AstNode;
use crate::{
diagnostics::{unresolved_fix, DiagnosticWithFix},
diagnostics::{unresolved_fix, DiagnosticWithFixes},
references::rename::rename_with_semantics,
};
impl DiagnosticWithFix for IncorrectCase {
fn fix(
impl DiagnosticWithFixes for IncorrectCase {
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
resolve: &AssistResolveStrategy,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
let root = sema.db.parse_or_expand(self.file)?;
let name_node = self.ident.to_node(&root);
@ -28,7 +28,7 @@ impl DiagnosticWithFix for IncorrectCase {
res.source_change = Some(source_change.ok().unwrap_or_default());
}
Some(res)
Some(vec![res])
}
}

19
crates/ide/src/diagnostics/fixes/create_field.rs
View File

@ -7,18 +7,17 @@ use syntax::{
use text_edit::TextEdit;
use crate::{
diagnostics::{fix, DiagnosticWithFix},
diagnostics::{fix, DiagnosticWithFixes},
Assist, AssistResolveStrategy,
};
impl DiagnosticWithFix for NoSuchField {
fn fix(
impl DiagnosticWithFixes for NoSuchField {
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
let root = sema.db.parse_or_expand(self.file)?;
missing_record_expr_field_fix(
missing_record_expr_field_fixes(
&sema,
self.file.original_file(sema.db),
&self.field.to_node(&root),
@ -26,11 +25,11 @@ impl DiagnosticWithFix for NoSuchField {
}
}
fn missing_record_expr_field_fix(
fn missing_record_expr_field_fixes(
sema: &Semantics<RootDatabase>,
usage_file_id: FileId,
record_expr_field: &ast::RecordExprField,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?;
let def_id = sema.resolve_variant(record_lit)?;
let module;
@ -89,12 +88,12 @@ fn missing_record_expr_field_fix(
TextEdit::insert(last_field_syntax.text_range().end(), new_field),
);
return Some(fix(
return Some(vec![fix(
"create_field",
"Create field",
source_change,
record_expr_field.syntax().text_range(),
));
)]);
fn record_field_list(field_def_list: ast::FieldList) -> Option<ast::RecordFieldList> {
match field_def_list {

12
crates/ide/src/diagnostics/fixes/fill_missing_fields.rs
View File

@ -5,16 +5,16 @@ use syntax::{algo, ast::make, AstNode};
use text_edit::TextEdit;
use crate::{
diagnostics::{fix, fixes::DiagnosticWithFix},
diagnostics::{fix, fixes::DiagnosticWithFixes},
Assist,
};
impl DiagnosticWithFix for MissingFields {
fn fix(
impl DiagnosticWithFixes for MissingFields {
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
// Note that although we could add a diagnostics to
// fill the missing tuple field, e.g :
// `struct A(usize);`
@ -41,12 +41,12 @@ impl DiagnosticWithFix for MissingFields {
.into_text_edit(&mut builder);
builder.finish()
};
Some(fix(
Some(vec![fix(
"fill_missing_fields",
"Fill struct fields",
SourceChange::from_text_edit(self.file.original_file(sema.db), edit),
sema.original_range(&field_list_parent.syntax()).range,
))
)])
}
}

10
crates/ide/src/diagnostics/fixes/remove_semicolon.rs
View File

@ -4,14 +4,14 @@ use ide_db::{source_change::SourceChange, RootDatabase};
use syntax::{ast, AstNode};
use text_edit::TextEdit;
use crate::diagnostics::{fix, DiagnosticWithFix};
use crate::diagnostics::{fix, DiagnosticWithFixes};
impl DiagnosticWithFix for RemoveThisSemicolon {
fn fix(
impl DiagnosticWithFixes for RemoveThisSemicolon {
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
let root = sema.db.parse_or_expand(self.file)?;
let semicolon = self
@ -26,7 +26,7 @@ impl DiagnosticWithFix for RemoveThisSemicolon {
let edit = TextEdit::delete(semicolon);
let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
Some(fix("remove_semicolon", "Remove this semicolon", source_change, semicolon))
Some(vec![fix("remove_semicolon", "Remove this semicolon", source_change, semicolon)])
}
}

12
crates/ide/src/diagnostics/fixes/replace_with_find_map.rs
View File

@ -7,14 +7,14 @@ use syntax::{
};
use text_edit::TextEdit;
use crate::diagnostics::{fix, DiagnosticWithFix};
use crate::diagnostics::{fix, DiagnosticWithFixes};
impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap {
fn fix(
impl DiagnosticWithFixes for ReplaceFilterMapNextWithFindMap {
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
let root = sema.db.parse_or_expand(self.file)?;
let next_expr = self.next_expr.to_node(&root);
let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?;
@ -32,12 +32,12 @@ impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap {
let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
Some(fix(
Some(vec![fix(
"replace_with_find_map",
"Replace filter_map(..).next() with find_map()",
source_change,
trigger_range,
))
)])
}
}

16
crates/ide/src/diagnostics/fixes/unresolved_module.rs
View File

@ -3,17 +3,17 @@ use ide_assists::{Assist, AssistResolveStrategy};
use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit, RootDatabase};
use syntax::AstNode;
use crate::diagnostics::{fix, DiagnosticWithFix};
use crate::diagnostics::{fix, DiagnosticWithFixes};
impl DiagnosticWithFix for UnresolvedModule {
fn fix(
impl DiagnosticWithFixes for UnresolvedModule {
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
let root = sema.db.parse_or_expand(self.file)?;
let unresolved_module = self.decl.to_node(&root);
Some(fix(
Some(vec![fix(
"create_module",
"Create module",
FileSystemEdit::CreateFile {
@ -25,7 +25,7 @@ impl DiagnosticWithFix for UnresolvedModule {
}
.into(),
unresolved_module.syntax().text_range(),
))
)])
}
}
@ -45,7 +45,8 @@ mod tests {
message: "unresolved module",
range: 0..8,
severity: Error,
fix: Some(
fixes: Some(
[
Assist {
id: AssistId(
"create_module",
@ -72,6 +73,7 @@ mod tests {
},
),
},
],
),
unused: false,
code: Some(

10
crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs
View File

@ -4,14 +4,14 @@ use ide_db::{source_change::SourceChange, RootDatabase};
use syntax::AstNode;
use text_edit::TextEdit;
use crate::diagnostics::{fix, DiagnosticWithFix};
use crate::diagnostics::{fix, DiagnosticWithFixes};
impl DiagnosticWithFix for MissingOkOrSomeInTailExpr {
fn fix(
impl DiagnosticWithFixes for MissingOkOrSomeInTailExpr {
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
let root = sema.db.parse_or_expand(self.file)?;
let tail_expr = self.expr.to_node(&root);
let tail_expr_range = tail_expr.syntax().text_range();
@ -19,7 +19,7 @@ impl DiagnosticWithFix for MissingOkOrSomeInTailExpr {
let edit = TextEdit::replace(tail_expr_range, replacement);
let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" };
Some(fix("wrap_tail_expr", name, source_change, tail_expr_range))
Some(vec![fix("wrap_tail_expr", name, source_change, tail_expr_range)])
}
}

48
crates/ide/src/diagnostics/unlinked_file.rs
View File

@ -18,7 +18,7 @@ use syntax::{
use text_edit::TextEdit;
use crate::{
diagnostics::{fix, fixes::DiagnosticWithFix},
diagnostics::{fix, fixes::DiagnosticWithFixes},
Assist,
};
@ -50,13 +50,13 @@ impl Diagnostic for UnlinkedFile {
}
}
impl DiagnosticWithFix for UnlinkedFile {
fn fix(
impl DiagnosticWithFixes for UnlinkedFile {
fn fixes(
&self,
sema: &hir::Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Assist> {
// If there's an existing module that could add a `mod` item to include the unlinked file,
) -> Option<Vec<Assist>> {
// If there's an existing module that could add `mod` or `pub mod` items to include the unlinked file,
// suggest that as a fix.
let source_root = sema.db.source_root(sema.db.file_source_root(self.file_id));
@ -90,7 +90,7 @@ impl DiagnosticWithFix for UnlinkedFile {
}
if module.origin.file_id() == Some(*parent_id) {
return make_fix(sema.db, *parent_id, module_name, self.file_id);
return make_fixes(sema.db, *parent_id, module_name, self.file_id);
}
}
}
@ -101,20 +101,23 @@ impl DiagnosticWithFix for UnlinkedFile {
}
}
fn make_fix(
fn make_fixes(
db: &RootDatabase,
parent_file_id: FileId,
new_mod_name: &str,
added_file_id: FileId,
) -> Option<Assist> {
) -> Option<Vec<Assist>> {
fn is_outline_mod(item: &ast::Item) -> bool {
matches!(item, ast::Item::Module(m) if m.item_list().is_none())
}
let mod_decl = format!("mod {};", new_mod_name);
let pub_mod_decl = format!("pub mod {};", new_mod_name);
let ast: ast::SourceFile = db.parse(parent_file_id).tree();
let mut builder = TextEdit::builder();
let mut mod_decl_builder = TextEdit::builder();
let mut pub_mod_decl_builder = TextEdit::builder();
// If there's an existing `mod m;` statement matching the new one, don't emit a fix (it's
// probably `#[cfg]`d out).
@ -138,30 +141,43 @@ fn make_fix(
{
Some(last) => {
cov_mark::hit!(unlinked_file_append_to_existing_mods);
builder.insert(last.syntax().text_range().end(), format!("\n{}", mod_decl));
let offset = last.syntax().text_range().end();
mod_decl_builder.insert(offset, format!("\n{}", mod_decl));
pub_mod_decl_builder.insert(offset, format!("\n{}", pub_mod_decl));
}
None => {
// Prepend before the first item in the file.
match ast.items().next() {
Some(item) => {
cov_mark::hit!(unlinked_file_prepend_before_first_item);
builder.insert(item.syntax().text_range().start(), format!("{}\n\n", mod_decl));
let offset = item.syntax().text_range().start();
mod_decl_builder.insert(offset, format!("{}\n\n", mod_decl));
pub_mod_decl_builder.insert(offset, format!("{}\n\n", pub_mod_decl));
}
None => {
// No items in the file, so just append at the end.
cov_mark::hit!(unlinked_file_empty_file);
builder.insert(ast.syntax().text_range().end(), format!("{}\n", mod_decl));
let offset = ast.syntax().text_range().end();
mod_decl_builder.insert(offset, format!("{}\n", mod_decl));
pub_mod_decl_builder.insert(offset, format!("{}\n", pub_mod_decl));
}
}
}
}
let edit = builder.finish();
let trigger_range = db.parse(added_file_id).tree().syntax().text_range();
Some(fix(
Some(vec![
fix(
"add_mod_declaration",
&format!("Insert `{}`", mod_decl),
SourceChange::from_text_edit(parent_file_id, edit),
SourceChange::from_text_edit(parent_file_id, mod_decl_builder.finish()),
trigger_range,
))
),
fix(
"add_pub_mod_declaration",
&format!("Insert `{}`", pub_mod_decl),
SourceChange::from_text_edit(parent_file_id, pub_mod_decl_builder.finish()),
trigger_range,
),
])
}

2
crates/ide/src/lib.rs
View File

@ -565,7 +565,7 @@ impl Analysis {
let diagnostic_assists = if include_fixes {
diagnostics::diagnostics(db, diagnostics_config, &resolve, frange.file_id)
.into_iter()
.filter_map(|it| it.fix)
.flat_map(|it| it.fixes.unwrap_or_default())
.filter(|it| it.target.intersect(frange.range).is_some())
.collect()
} else {