diff options
Diffstat (limited to 'cli/tools/fmt.rs')
| -rw-r--r-- | cli/tools/fmt.rs | 253 |
1 files changed, 210 insertions, 43 deletions
diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index 401fe836e..4fe99c616 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -11,6 +11,7 @@ use crate::colors; use crate::config_file::FmtConfig; use crate::config_file::FmtOptionsConfig; use crate::config_file::ProseWrap; +use crate::deno_dir::DenoDir; use crate::diff::diff; use crate::file_watcher; use crate::file_watcher::ResolutionResult; @@ -40,11 +41,14 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use std::sync::Arc; +use super::incremental_cache::IncrementalCache; + /// Format JavaScript/TypeScript files. pub async fn format( flags: &Flags, fmt_flags: FmtFlags, maybe_fmt_config: Option<FmtConfig>, + deno_dir: &DenoDir, ) -> Result<(), AnyError> { let FmtFlags { files, @@ -132,11 +136,18 @@ pub async fn format( } }; let operation = |(paths, fmt_options): (Vec<PathBuf>, FmtOptionsConfig)| async move { + let incremental_cache = Arc::new(IncrementalCache::new( + &deno_dir.fmt_incremental_cache_db_file_path(), + &fmt_options, + &paths, + )); if check { - check_source_files(paths, fmt_options).await?; + check_source_files(paths, fmt_options, incremental_cache.clone()).await?; } else { - format_source_files(paths, fmt_options).await?; + format_source_files(paths, fmt_options, incremental_cache.clone()) + .await?; } + incremental_cache.wait_completion().await; Ok(()) }; @@ -234,18 +245,18 @@ pub fn format_json( pub fn format_file( file_path: &Path, file_text: &str, - fmt_options: FmtOptionsConfig, + fmt_options: &FmtOptionsConfig, ) -> Result<Option<String>, AnyError> { let ext = get_extension(file_path).unwrap_or_default(); if matches!( ext.as_str(), "md" | "mkd" | "mkdn" | "mdwn" | "mdown" | "markdown" ) { - format_markdown(file_text, &fmt_options) + format_markdown(file_text, fmt_options) } else if matches!(ext.as_str(), "json" | "jsonc") { - format_json(file_text, &fmt_options) + format_json(file_text, fmt_options) } else { - let config = get_resolved_typescript_config(&fmt_options); + let config = get_resolved_typescript_config(fmt_options); dprint_plugin_typescript::format_text(file_path, file_text, &config) } } @@ -263,6 +274,7 @@ pub fn format_parsed_source( async fn check_source_files( paths: Vec<PathBuf>, fmt_options: FmtOptionsConfig, + incremental_cache: Arc<IncrementalCache>, ) -> Result<(), AnyError> { let not_formatted_files_count = Arc::new(AtomicUsize::new(0)); let checked_files_count = Arc::new(AtomicUsize::new(0)); @@ -277,7 +289,12 @@ async fn check_source_files( checked_files_count.fetch_add(1, Ordering::Relaxed); let file_text = read_file_contents(&file_path)?.text; - match format_file(&file_path, &file_text, fmt_options.clone()) { + // skip checking the file if we know it's formatted + if incremental_cache.is_file_same(&file_path, &file_text) { + return Ok(()); + } + + match format_file(&file_path, &file_text, &fmt_options) { Ok(Some(formatted_text)) => { not_formatted_files_count.fetch_add(1, Ordering::Relaxed); let _g = output_lock.lock(); @@ -286,7 +303,14 @@ async fn check_source_files( info!("{} {}:", colors::bold("from"), file_path.display()); info!("{}", diff); } - Ok(None) => {} + Ok(None) => { + // When checking formatting, only update the incremental cache when + // the file is the same since we don't bother checking for stable + // formatting here. Additionally, ensure this is done during check + // so that CIs that cache the DENO_DIR will get the benefit of + // incremental formatting + incremental_cache.update_file(&file_path, &file_text); + } Err(e) => { let _g = output_lock.lock(); eprintln!("Error checking: {}", file_path.to_string_lossy()); @@ -318,6 +342,7 @@ async fn check_source_files( async fn format_source_files( paths: Vec<PathBuf>, fmt_options: FmtOptionsConfig, + incremental_cache: Arc<IncrementalCache>, ) -> Result<(), AnyError> { let formatted_files_count = Arc::new(AtomicUsize::new(0)); let checked_files_count = Arc::new(AtomicUsize::new(0)); @@ -330,8 +355,19 @@ async fn format_source_files( checked_files_count.fetch_add(1, Ordering::Relaxed); let file_contents = read_file_contents(&file_path)?; - match format_file(&file_path, &file_contents.text, fmt_options.clone()) { + // skip formatting the file if we know it's formatted + if incremental_cache.is_file_same(&file_path, &file_contents.text) { + return Ok(()); + } + + match format_ensure_stable( + &file_path, + &file_contents.text, + &fmt_options, + format_file, + ) { Ok(Some(formatted_text)) => { + incremental_cache.update_file(&file_path, &formatted_text); write_file_contents( &file_path, FileContents { @@ -343,7 +379,9 @@ async fn format_source_files( let _g = output_lock.lock(); info!("{}", file_path.to_string_lossy()); } - Ok(None) => {} + Ok(None) => { + incremental_cache.update_file(&file_path, &file_contents.text); + } Err(e) => { let _g = output_lock.lock(); eprintln!("Error formatting: {}", file_path.to_string_lossy()); @@ -372,6 +410,66 @@ async fn format_source_files( Ok(()) } +/// When storing any formatted text in the incremental cache, we want +/// to ensure that anything stored when formatted will have itself as +/// the output as well. This is to prevent "double format" issues where +/// a user formats their code locally and it fails on the CI afterwards. +fn format_ensure_stable( + file_path: &Path, + file_text: &str, + fmt_options: &FmtOptionsConfig, + fmt_func: impl Fn( + &Path, + &str, + &FmtOptionsConfig, + ) -> Result<Option<String>, AnyError>, +) -> Result<Option<String>, AnyError> { + let formatted_text = fmt_func(file_path, file_text, fmt_options)?; + + match formatted_text { + Some(mut current_text) => { + let mut count = 0; + loop { + match fmt_func(file_path, ¤t_text, fmt_options) { + Ok(Some(next_pass_text)) => { + // just in case + if next_pass_text == current_text { + return Ok(Some(next_pass_text)); + } + current_text = next_pass_text; + } + Ok(None) => { + return Ok(Some(current_text)); + } + Err(err) => { + panic!( + concat!( + "Formatting succeeded initially, but failed when ensuring a ", + "stable format. This indicates a bug in the formatter where ", + "the text it produces is not syntatically correct. As a temporary ", + "workfaround you can ignore this file.\n\n{:#}" + ), + err, + ) + } + } + count += 1; + if count == 5 { + panic!( + concat!( + "Formatting not stable. Bailed after {} tries. This indicates a bug ", + "in the formatter where it formats the file differently each time. As a ", + "temporary workaround you can ignore this file." + ), + count + ) + } + } + } + None => Ok(None), + } +} + /// Format stdin and write result to stdout. /// Treats input as TypeScript or as set by `--ext` flag. /// Compatible with `--check` flag. @@ -386,7 +484,7 @@ pub fn format_stdin( let file_path = PathBuf::from(format!("_stdin.{}", fmt_flags.ext)); let fmt_options = resolve_fmt_options(&fmt_flags, fmt_options); - let formatted_text = format_file(&file_path, &source, fmt_options)?; + let formatted_text = format_file(&file_path, &source, &fmt_options)?; if fmt_flags.check { if formatted_text.is_some() { println!("Not formatted stdin"); @@ -628,37 +726,106 @@ fn is_contain_git(path: &Path) -> bool { path.components().any(|c| c.as_os_str() == ".git") } -#[test] -fn test_is_supported_ext_fmt() { - assert!(!is_supported_ext_fmt(Path::new("tests/subdir/redirects"))); - assert!(is_supported_ext_fmt(Path::new("README.md"))); - assert!(is_supported_ext_fmt(Path::new("readme.MD"))); - assert!(is_supported_ext_fmt(Path::new("readme.mkd"))); - assert!(is_supported_ext_fmt(Path::new("readme.mkdn"))); - assert!(is_supported_ext_fmt(Path::new("readme.mdwn"))); - assert!(is_supported_ext_fmt(Path::new("readme.mdown"))); - assert!(is_supported_ext_fmt(Path::new("readme.markdown"))); - assert!(is_supported_ext_fmt(Path::new("lib/typescript.d.ts"))); - assert!(is_supported_ext_fmt(Path::new("testdata/001_hello.js"))); - assert!(is_supported_ext_fmt(Path::new("testdata/002_hello.ts"))); - assert!(is_supported_ext_fmt(Path::new("foo.jsx"))); - assert!(is_supported_ext_fmt(Path::new("foo.tsx"))); - assert!(is_supported_ext_fmt(Path::new("foo.TS"))); - assert!(is_supported_ext_fmt(Path::new("foo.TSX"))); - assert!(is_supported_ext_fmt(Path::new("foo.JS"))); - assert!(is_supported_ext_fmt(Path::new("foo.JSX"))); - assert!(is_supported_ext_fmt(Path::new("foo.mjs"))); - assert!(!is_supported_ext_fmt(Path::new("foo.mjsx"))); - assert!(is_supported_ext_fmt(Path::new("foo.jsonc"))); - assert!(is_supported_ext_fmt(Path::new("foo.JSONC"))); - assert!(is_supported_ext_fmt(Path::new("foo.json"))); - assert!(is_supported_ext_fmt(Path::new("foo.JsON"))); -} +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_is_supported_ext_fmt() { + assert!(!is_supported_ext_fmt(Path::new("tests/subdir/redirects"))); + assert!(is_supported_ext_fmt(Path::new("README.md"))); + assert!(is_supported_ext_fmt(Path::new("readme.MD"))); + assert!(is_supported_ext_fmt(Path::new("readme.mkd"))); + assert!(is_supported_ext_fmt(Path::new("readme.mkdn"))); + assert!(is_supported_ext_fmt(Path::new("readme.mdwn"))); + assert!(is_supported_ext_fmt(Path::new("readme.mdown"))); + assert!(is_supported_ext_fmt(Path::new("readme.markdown"))); + assert!(is_supported_ext_fmt(Path::new("lib/typescript.d.ts"))); + assert!(is_supported_ext_fmt(Path::new("testdata/001_hello.js"))); + assert!(is_supported_ext_fmt(Path::new("testdata/002_hello.ts"))); + assert!(is_supported_ext_fmt(Path::new("foo.jsx"))); + assert!(is_supported_ext_fmt(Path::new("foo.tsx"))); + assert!(is_supported_ext_fmt(Path::new("foo.TS"))); + assert!(is_supported_ext_fmt(Path::new("foo.TSX"))); + assert!(is_supported_ext_fmt(Path::new("foo.JS"))); + assert!(is_supported_ext_fmt(Path::new("foo.JSX"))); + assert!(is_supported_ext_fmt(Path::new("foo.mjs"))); + assert!(!is_supported_ext_fmt(Path::new("foo.mjsx"))); + assert!(is_supported_ext_fmt(Path::new("foo.jsonc"))); + assert!(is_supported_ext_fmt(Path::new("foo.JSONC"))); + assert!(is_supported_ext_fmt(Path::new("foo.json"))); + assert!(is_supported_ext_fmt(Path::new("foo.JsON"))); + } + + #[test] + fn test_is_located_in_git() { + assert!(is_contain_git(Path::new("test/.git"))); + assert!(is_contain_git(Path::new(".git/bad.json"))); + assert!(is_contain_git(Path::new("test/.git/bad.json"))); + assert!(!is_contain_git(Path::new("test/bad.git/bad.json"))); + } + + #[test] + #[should_panic(expected = "Formatting not stable. Bailed after 5 tries.")] + fn test_format_ensure_stable_unstable_format() { + format_ensure_stable( + &PathBuf::from("mod.ts"), + "1", + &Default::default(), + |_, file_text, _| Ok(Some(format!("1{}", file_text))), + ) + .unwrap(); + } + + #[test] + fn test_format_ensure_stable_error_first() { + let err = format_ensure_stable( + &PathBuf::from("mod.ts"), + "1", + &Default::default(), + |_, _, _| bail!("Error formatting."), + ) + .unwrap_err(); + + assert_eq!(err.to_string(), "Error formatting."); + } -#[test] -fn test_is_located_in_git() { - assert!(is_contain_git(Path::new("test/.git"))); - assert!(is_contain_git(Path::new(".git/bad.json"))); - assert!(is_contain_git(Path::new("test/.git/bad.json"))); - assert!(!is_contain_git(Path::new("test/bad.git/bad.json"))); + #[test] + #[should_panic(expected = "Formatting succeeded initially, but failed when")] + fn test_format_ensure_stable_error_second() { + format_ensure_stable( + &PathBuf::from("mod.ts"), + "1", + &Default::default(), + |_, file_text, _| { + if file_text == "1" { + Ok(Some("11".to_string())) + } else { + bail!("Error formatting.") + } + }, + ) + .unwrap(); + } + + #[test] + fn test_format_stable_after_two() { + let result = format_ensure_stable( + &PathBuf::from("mod.ts"), + "1", + &Default::default(), + |_, file_text, _| { + if file_text == "1" { + Ok(Some("11".to_string())) + } else if file_text == "11" { + Ok(None) + } else { + unreachable!(); + } + }, + ) + .unwrap(); + + assert_eq!(result, Some("11".to_string())); + } } |