Repository: rust-analyzer/rust-analyzer (rust)

Issue:
`cargo test cli::generated_assists` fails when assist file open
This is a meta-issue related to development of assists. 

Running `cargo test cli::generated_assists` fails when any assist files are open. The cause of this issue is that my editor (vim) creates a temp file in the assists directory, which is picked up [here](https://github.com/rust-analyzer/rust-analyzer/blob/master/xtask/src/codegen/gen_assists_docs.rs#L49) and then fails to read as UTF-8 [here](https://github.com/rust-analyzer/rust-analyzer/blob/master/xtask/src/codegen/gen_assists_docs.rs#L60).

Potential fixes:

* Ignore gitignore'd files
  * This seems to be the most appropriate solution, but potentially would involve adding a dependency?
* Ignore files starting with `.`
  * This solves the problem for vim, but I'm not sure about other editors
* Provide a more descriptive error message including the file path that failed to be read as UTF-8
  * This doesn't solve the problem, but would likely provide enough information to allow the user to figure out how to move forward (likely just close the file in their editor)
* Any other options? 

I'd be happy to implement a fix, but I just wanted to discuss the options before making a PR with a particular solution. 

Pull request:
gen_assists_docs skip hidden files
Fixes #3670 

Skips hidden files when generating assist docs, which fixes an issue where the tests would fail while an editor has created a temp file in the assists directory.

There is similar logic [here](https://github.com/rust-analyzer/rust-analyzer/blob/2720e2374be951bb762ff2815dd67c7ffe3419b7/xtask/tests/tidy-tests/main.rs#L157), although in that case the `DirEntry` is a `walkdir::DirEntry` rather than a `fs::DirEntry`. Also, it's not immediately clear that it is worth moving this functionality to somewhere accessible from both of these places and creating dependencies in this way. Let me know if this is off the mark.

Patch:
diff --git a/crates/ra_hir_expand/src/db.rs b/crates/ra_hir_expand/src/db.rs
index 5a696542f6..d171d2dfd1 100644
--- a/crates/ra_hir_expand/src/db.rs
+++ b/crates/ra_hir_expand/src/db.rs
@@ -6,7 +6,7 @@ use mbe::{ExpandResult, MacroRules};
 use ra_db::{salsa, SourceDatabase};
 use ra_parser::FragmentKind;
 use ra_prof::profile;
-use ra_syntax::{algo::diff, AstNode, Parse, SyntaxKind::*, SyntaxNode};
+use ra_syntax::{AstNode, Parse, SyntaxKind::*, SyntaxNode};
 
 use crate::{
     ast_id_map::AstIdMap, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerCallLoc, EagerMacroId,
@@ -238,7 +238,7 @@ pub fn parse_macro_with_arg(
     } else {
         db.macro_expand(macro_call_id)
     };
-    if let Some(err) = &err {
+    if let Some(err) = err {
         // Note:
         // The final goal we would like to make all parse_macro success,
         // such that the following log will not call anyway.
@@ -272,25 +272,7 @@ pub fn parse_macro_with_arg(
     let fragment_kind = to_fragment_kind(db, macro_call_id);
 
     let (parse, rev_token_map) = mbe::token_tree_to_syntax_node(&tt, fragment_kind).ok()?;
-
-    if err.is_none() {
-        Some((parse, Arc::new(rev_token_map)))
-    } else {
-        // FIXME:
-        // In future, we should propagate the actual error with recovery information
-        // instead of ignore the error here.
-
-        // Safe check for recurisve identity macro
-        let node = parse.syntax_node();
-        let file: HirFileId = macro_file.into();
-        let call_node = file.call_node(db)?;
-
-        if !diff(&node, &call_node.value).is_empty() {
-            Some((parse, Arc::new(rev_token_map)))
-        } else {
-            None
-        }
-    }
+    Some((parse, Arc::new(rev_token_map)))
 }
 
 /// Given a `MacroCallId`, return what `FragmentKind` it belongs to.
diff --git a/crates/ra_syntax/src/algo.rs b/crates/ra_syntax/src/algo.rs
index ffdbdc7671..344cf0fbed 100644
--- a/crates/ra_syntax/src/algo.rs
+++ b/crates/ra_syntax/src/algo.rs
@@ -95,10 +95,6 @@ impl TreeDiff {
             builder.replace(from.text_range(), to.to_string())
         }
     }
-
-    pub fn is_empty(&self) -> bool {
-        self.replacements.is_empty()
-    }
 }
 
 /// Finds minimal the diff, which, applied to `from`, will result in `to`.
diff --git a/xtask/src/codegen/gen_assists_docs.rs b/xtask/src/codegen/gen_assists_docs.rs
index 6da5ca89e7..31d606535f 100644
--- a/xtask/src/codegen/gen_assists_docs.rs
+++ b/xtask/src/codegen/gen_assists_docs.rs
@@ -4,7 +4,7 @@ use std::{fs, path::Path};
 
 use crate::{
     codegen::{self, extract_comment_blocks_with_empty_lines, Mode},
-    project_root, Result,
+    project_root, rust_files, Result,
 };
 
 pub fn generate_assists_docs(mode: Mode) -> Result<()> {
@@ -46,12 +46,8 @@ fn reveal_hash_comments(text: &str) -> String {
 
 fn collect_assists() -> Result<Vec<Assist>> {
     let mut res = Vec::new();
-    for entry in fs::read_dir(project_root().join(codegen::ASSISTS_DIR))? {
-        let entry = entry?;
-        let path = entry.path();
-        if path.is_file() {
-            collect_file(&mut res, path.as_path())?;
-        }
+    for path in rust_files(&project_root().join(codegen::ASSISTS_DIR)) {
+        collect_file(&mut res, path.as_path())?;
     }
     res.sort_by(|lhs, rhs| lhs.id.cmp(&rhs.id));
     return Ok(res);
diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs
index e1472e85d8..4f01f84fb4 100644
--- a/xtask/src/lib.rs
+++ b/xtask/src/lib.rs
@@ -17,6 +17,7 @@ use std::{
     path::{Path, PathBuf},
     process::{Command, Stdio},
 };
+use walkdir::{DirEntry, WalkDir};
 
 use crate::{
     codegen::Mode,
@@ -37,6 +38,21 @@ pub fn project_root() -> PathBuf {
     .to_path_buf()
 }
 
+pub fn rust_files(path: &Path) -> impl Iterator<Item = PathBuf> {
+    let iter = WalkDir::new(path);
+    return iter
+        .into_iter()
+        .filter_entry(|e| !is_hidden(e))
+        .map(|e| e.unwrap())
+        .filter(|e| !e.file_type().is_dir())
+        .map(|e| e.into_path())
+        .filter(|path| path.extension().map(|it| it == "rs").unwrap_or(false));
+
+    fn is_hidden(entry: &DirEntry) -> bool {
+        entry.file_name().to_str().map(|s| s.starts_with('.')).unwrap_or(false)
+    }
+}
+
 pub fn run_rustfmt(mode: Mode) -> Result<()> {
     let _dir = pushd(project_root());
     ensure_rustfmt()?;


Test patch:
diff --git a/crates/ra_hir_ty/src/tests/regression.rs b/crates/ra_hir_ty/src/tests/regression.rs
index a02e3ee051..14c8ed3a9b 100644
--- a/crates/ra_hir_ty/src/tests/regression.rs
+++ b/crates/ra_hir_ty/src/tests/regression.rs
@@ -453,34 +453,3 @@ pub mod str {
     // should be Option<char>, but currently not because of Chalk ambiguity problem
     assert_eq!("(Option<{unknown}>, Option<{unknown}>)", super::type_at_pos(&db, pos));
 }
-
-#[test]
-fn issue_3642_bad_macro_stackover() {
-    let (db, pos) = TestDB::with_position(
-        r#"
-//- /main.rs
-#[macro_export]
-macro_rules! match_ast {
-    (match $node:ident { $($tt:tt)* }) => { match_ast!(match ($node) { $($tt)* }) };
-
-    (match ($node:expr) {
-        $( ast::$ast:ident($it:ident) => $res:expr, )*
-        _ => $catch_all:expr $(,)?
-    }) => {{
-        $( if let Some($it) = ast::$ast::cast($node.clone()) { $res } else )*
-        { $catch_all }
-    }};
-}
-
-fn main() {
-    let anchor<|> = match_ast! {
-        match parent {
-            as => {},
-            _ => return None
-        }
-    };
-}"#,
-    );
-
-    assert_eq!("()", super::type_at_pos(&db, pos));
-}
diff --git a/xtask/tests/tidy-tests/main.rs b/xtask/tests/tidy-tests/main.rs
index 5ae86c87c9..80911a68e8 100644
--- a/xtask/tests/tidy-tests/main.rs
+++ b/xtask/tests/tidy-tests/main.rs
@@ -5,13 +5,12 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use walkdir::{DirEntry, WalkDir};
-use xtask::{not_bash::fs2, project_root};
+use xtask::{not_bash::fs2, project_root, rust_files};
 
 #[test]
 fn rust_files_are_tidy() {
     let mut tidy_docs = TidyDocs::default();
-    for path in rust_files() {
+    for path in rust_files(&project_root().join("crates")) {
         let text = fs2::read_to_string(&path).unwrap();
         check_todo(&path, &text);
         check_trailing_ws(&path, &text);
@@ -142,19 +141,3 @@ fn is_exclude_dir(p: &Path, dirs_to_exclude: &[&str]) -> bool {
 
     false
 }
-
-fn rust_files() -> impl Iterator<Item = PathBuf> {
-    let crates = project_root().join("crates");
-    let iter = WalkDir::new(crates);
-    return iter
-        .into_iter()
-        .filter_entry(|e| !is_hidden(e))
-        .map(|e| e.unwrap())
-        .filter(|e| !e.file_type().is_dir())
-        .map(|e| e.into_path())
-        .filter(|path| path.extension().map(|it| it == "rs").unwrap_or(false));
-
-    fn is_hidden(entry: &DirEntry) -> bool {
-        entry.file_name().to_str().map(|s| s.starts_with('.')).unwrap_or(false)
-    }
-}