diff --git a/cli/flags.rs b/cli/flags.rs index 5326dc2874f2a7..445a08c0b7b899 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -37,9 +37,9 @@ pub enum DenoSubcommand { Fetch { files: Vec, }, - Format { + Fmt { check: bool, - files: Option>, + files: Vec, }, Help, Info { @@ -301,19 +301,13 @@ fn types_parse(flags: &mut DenoFlags, _matches: &clap::ArgMatches) { } fn fmt_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) { - let maybe_files = match matches.values_of("files") { - Some(f) => { - let files: Vec = f.map(PathBuf::from).collect(); - Some(files) - } - None => None, + let files = match matches.values_of("files") { + Some(f) => f.map(String::from).collect(), + None => vec![], }; - - let check = matches.is_present("check"); - - flags.subcommand = DenoSubcommand::Format { - check, - files: maybe_files, + flags.subcommand = DenoSubcommand::Fmt { + check: matches.is_present("check"), + files, } } @@ -533,7 +527,7 @@ The declaration file could be saved and used for typing information.", fn fmt_subcommand<'a, 'b>() -> App<'a, 'b> { SubCommand::with_name("fmt") - .about("Format files") + .about("Format source files") .long_about( "Auto-format JavaScript/TypeScript source code @@ -1363,12 +1357,9 @@ mod tests { assert_eq!( r.unwrap(), DenoFlags { - subcommand: DenoSubcommand::Format { + subcommand: DenoSubcommand::Fmt { check: false, - files: Some(vec![ - PathBuf::from("script_1.ts"), - PathBuf::from("script_2.ts") - ]) + files: vec!["script_1.ts".to_string(), "script_2.ts".to_string()] }, ..DenoFlags::default() } @@ -1378,9 +1369,9 @@ mod tests { assert_eq!( r.unwrap(), DenoFlags { - subcommand: DenoSubcommand::Format { + subcommand: DenoSubcommand::Fmt { check: true, - files: None + files: vec![], }, ..DenoFlags::default() } @@ -1390,9 +1381,9 @@ mod tests { assert_eq!( r.unwrap(), DenoFlags { - subcommand: DenoSubcommand::Format { + subcommand: DenoSubcommand::Fmt { check: false, - files: None + files: vec![], }, ..DenoFlags::default() } diff --git a/cli/fmt.rs b/cli/fmt.rs index f02df560a4c3fe..c9b0ef82660b5b 100644 --- a/cli/fmt.rs +++ b/cli/fmt.rs @@ -7,12 +7,9 @@ //! the future it can be easily extended to provide //! the same functions as ops available in JS runtime. -use crate::deno_error::DenoError; -use crate::deno_error::ErrorKind; use deno_core::ErrBox; use dprint_plugin_typescript as dprint; use glob; -use regex::Regex; use std::fs; use std::io::stdin; use std::io::stdout; @@ -22,16 +19,20 @@ use std::path::Path; use std::path::PathBuf; use std::time::Instant; -lazy_static! { - static ref TYPESCRIPT_LIB: Regex = Regex::new(".d.ts$").unwrap(); - static ref TYPESCRIPT: Regex = Regex::new(".tsx?$").unwrap(); - static ref JAVASCRIPT: Regex = Regex::new(".jsx?$").unwrap(); -} - fn is_supported(path: &Path) -> bool { - let path_str = path.to_string_lossy(); - !TYPESCRIPT_LIB.is_match(&path_str) - && (TYPESCRIPT.is_match(&path_str) || JAVASCRIPT.is_match(&path_str)) + if let Some(ext) = path.extension() { + if ext == "tsx" || ext == "js" || ext == "jsx" { + true + } else if ext == "ts" { + // Currently dprint does not support d.ts files. + // https://github.com/dsherret/dprint/issues/100 + !path.as_os_str().to_string_lossy().ends_with(".d.ts") + } else { + false + } + } else { + false + } } fn get_config() -> dprint::configuration::Configuration { @@ -47,22 +48,10 @@ fn get_config() -> dprint::configuration::Configuration { .build() } -fn get_supported_files(paths: Vec) -> Vec { - let mut files_to_check = vec![]; - - for path in paths { - if is_supported(&path) { - files_to_check.push(path.to_owned()); - } - } - - files_to_check -} - fn check_source_files( config: dprint::configuration::Configuration, paths: Vec, -) { +) -> Result<(), ErrBox> { let start = Instant::now(); let mut not_formatted_files = vec![]; @@ -75,7 +64,6 @@ fn check_source_files( } Ok(Some(formatted_text)) => { if formatted_text != file_contents { - println!("Not formatted {}", file_path_str); not_formatted_files.push(file_path); } } @@ -88,20 +76,20 @@ fn check_source_files( let duration = Instant::now() - start; - if !not_formatted_files.is_empty() { + if not_formatted_files.is_empty() { + Ok(()) + } else { let f = if not_formatted_files.len() == 1 { "file" } else { "files" }; - - eprintln!( + Err(crate::deno_error::other_error(format!( "Found {} not formatted {} in {:?}", not_formatted_files.len(), f, duration - ); - std::process::exit(1); + ))) } } @@ -147,56 +135,57 @@ fn format_source_files( ); } -fn get_matching_files(glob_paths: Vec) -> Vec { - let mut target_files = Vec::with_capacity(128); - - for path in glob_paths { - let files = glob::glob(&path.to_str().unwrap()) - .expect("Failed to execute glob.") - .filter_map(Result::ok); - target_files.extend(files); - } - - target_files +pub fn source_files_in_subtree(root: PathBuf) -> Vec { + assert!(root.is_dir()); + // TODO(ry) Use WalkDir instead of globs. + let g = root.join("**/*"); + glob::glob(&g.into_os_string().into_string().unwrap()) + .expect("Failed to execute glob.") + .filter_map(|result| { + if let Ok(p) = result { + if is_supported(&p) { + Some(p) + } else { + None + } + } else { + None + } + }) + .collect() } /// Format JavaScript/TypeScript files. /// /// First argument supports globs, and if it is `None` /// then the current directory is recursively walked. -pub fn format_files( - maybe_files: Option>, - check: bool, -) -> Result<(), ErrBox> { - // TODO: improve glob to look for tsx?/jsx? files only - let glob_paths = maybe_files.unwrap_or_else(|| vec![PathBuf::from("**/*")]); - - for glob_path in glob_paths.iter() { - if glob_path.to_str().unwrap() == "-" { - // If the only given path is '-', format stdin. - if glob_paths.len() == 1 { - format_stdin(check); +pub fn format_files(args: Vec, check: bool) -> Result<(), ErrBox> { + if args.len() == 1 && args[0] == "-" { + format_stdin(check); + return Ok(()); + } + + let mut target_files: Vec = vec![]; + + if args.is_empty() { + target_files + .extend(source_files_in_subtree(std::env::current_dir().unwrap())); + } else { + for arg in args { + let p = PathBuf::from(arg); + if p.is_dir() { + target_files.extend(source_files_in_subtree(p)); } else { - // Otherwise it is an error - return Err(ErrBox::from(DenoError::new( - ErrorKind::Other, - "Ambiguous filename input. To format stdin, provide a single '-' instead.".to_owned() - ))); - } - return Ok(()); + target_files.push(p); + }; } } - - let matching_files = get_matching_files(glob_paths); - let matching_files = get_supported_files(matching_files); let config = get_config(); - if check { - check_source_files(config, matching_files); + check_source_files(config, target_files)?; } else { - format_source_files(config, matching_files); + format_source_files(config, target_files); } - Ok(()) } @@ -211,10 +200,7 @@ fn format_stdin(check: bool) { let config = get_config(); match dprint::format_text("_stdin.ts", &source, &config) { - Ok(None) => { - // Should not happen. - unreachable!(); - } + Ok(None) => unreachable!(), Ok(Some(formatted_text)) => { if check { if formatted_text != source { @@ -231,3 +217,22 @@ fn format_stdin(check: bool) { } } } + +#[test] +fn test_is_supported() { + assert!(!is_supported(Path::new("tests/subdir/redirects"))); + assert!(!is_supported(Path::new("README.md"))); + assert!(!is_supported(Path::new("lib/typescript.d.ts"))); + assert!(is_supported(Path::new("cli/tests/001_hello.js"))); + assert!(is_supported(Path::new("cli/tests/002_hello.ts"))); + assert!(is_supported(Path::new("foo.jsx"))); + assert!(is_supported(Path::new("foo.tsx"))); +} + +#[test] +fn check_tests_dir() { + // Because of cli/tests/error_syntax.js the following should fail but not + // crash. + let r = format_files(vec!["./tests".to_string()], true); + assert!(r.is_err()); +} diff --git a/cli/lib.rs b/cli/lib.rs index f89eb4bd74c602..e646c4199925fe 100644 --- a/cli/lib.rs +++ b/cli/lib.rs @@ -413,7 +413,7 @@ async fn run_script( Ok(()) } -async fn fmt_command(files: Option>, check: bool) { +async fn fmt_command(files: Vec, check: bool) { if let Err(err) = fmt::format_files(files, check) { print_err_and_exit(err); } @@ -493,9 +493,7 @@ pub fn main() { } DenoSubcommand::Eval { code } => eval_command(flags, code).await, DenoSubcommand::Fetch { files } => fetch_command(flags, files).await, - DenoSubcommand::Format { check, files } => { - fmt_command(files, check).await - } + DenoSubcommand::Fmt { check, files } => fmt_command(files, check).await, DenoSubcommand::Info { file } => info_command(flags, file).await, DenoSubcommand::Install { dir, diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index a41fdd032832cc..35127c155a7e39 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -641,14 +641,6 @@ itest!(fmt_stdin { output_str: Some("const a = 1;\n"), }); -itest!(fmt_stdin_ambiguous { - args: "fmt - file.ts", - input: Some("const a = 1\n"), - check_stderr: true, - exit_code: 1, - output_str: Some("Ambiguous filename input. To format stdin, provide a single '-' instead.\n"), -}); - itest!(fmt_stdin_check_formatted { args: "fmt --check -", input: Some("const a = 1;\n"),