From c555b31d40bcc374013395bd5805410fe7f77b20 Mon Sep 17 00:00:00 2001 From: CGQAQ Date: Wed, 6 Oct 2021 05:07:38 +0800 Subject: [PATCH] feat(lint): add support for --watch flag (#11983) --- cli/flags.rs | 2 + cli/main.rs | 11 +- cli/tests/integration/watcher_tests.rs | 85 +++++++++++ cli/tests/testdata/lint/watch/badly_linted.js | 1 + .../testdata/lint/watch/badly_linted.js.out | 2 + .../lint/watch/badly_linted_fixed1.js | 1 + .../lint/watch/badly_linted_fixed1.js.out | 1 + .../lint/watch/badly_linted_fixed2.js | 1 + .../lint/watch/badly_linted_fixed2.js.out | 0 cli/tools/lint.rs | 140 ++++++++++++------ 10 files changed, 189 insertions(+), 55 deletions(-) create mode 100644 cli/tests/testdata/lint/watch/badly_linted.js create mode 100644 cli/tests/testdata/lint/watch/badly_linted.js.out create mode 100644 cli/tests/testdata/lint/watch/badly_linted_fixed1.js create mode 100644 cli/tests/testdata/lint/watch/badly_linted_fixed1.js.out create mode 100644 cli/tests/testdata/lint/watch/badly_linted_fixed2.js create mode 100644 cli/tests/testdata/lint/watch/badly_linted_fixed2.js.out diff --git a/cli/flags.rs b/cli/flags.rs index dc0e932eff46f8..3075eda406e418 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1140,6 +1140,7 @@ Ignore linting a file by adding an ignore comment at the top of the file: .multiple(true) .required(false), ) + .arg(watch_arg()) } fn repl_subcommand<'a, 'b>() -> App<'a, 'b> { @@ -1964,6 +1965,7 @@ fn lsp_parse(flags: &mut Flags, _matches: &clap::ArgMatches) { fn lint_parse(flags: &mut Flags, matches: &clap::ArgMatches) { config_arg_parse(flags, matches); + flags.watch = matches.is_present("watch"); let files = match matches.values_of("files") { Some(f) => f.map(PathBuf::from).collect(), None => vec![], diff --git a/cli/main.rs b/cli/main.rs index 758681ecbd4289..dc2f2f5787c6f6 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -531,16 +531,7 @@ async fn lint_command( None }; - tools::lint::lint_files( - maybe_lint_config, - lint_flags.rules_tags, - lint_flags.rules_include, - lint_flags.rules_exclude, - lint_flags.files, - lint_flags.ignore, - lint_flags.json, - ) - .await + tools::lint::lint(maybe_lint_config, lint_flags, flags.watch).await } async fn cache_command( diff --git a/cli/tests/integration/watcher_tests.rs b/cli/tests/integration/watcher_tests.rs index 4cccfc83f83444..6148bcf2bdae0a 100644 --- a/cli/tests/integration/watcher_tests.rs +++ b/cli/tests/integration/watcher_tests.rs @@ -28,6 +28,24 @@ fn skip_restarting_line( } } +fn read_all_lints(stderr_lines: &mut impl Iterator) -> String { + let mut str = String::new(); + for t in stderr_lines { + let t = util::strip_ansi_codes(&t); + if t.starts_with("Watcher File change detected") { + continue; + } + if t.starts_with("Watcher") { + break; + } + if t.starts_with('(') { + str.push_str(&t); + str.push('\n'); + } + } + str +} + fn wait_for(s: &str, lines: &mut impl Iterator) { loop { let msg = lines.next().unwrap(); @@ -54,6 +72,73 @@ fn child_lines( (stdout_lines, stderr_lines) } +#[test] +fn lint_watch_test() { + let t = TempDir::new().expect("tempdir fail"); + let badly_linted_original = + util::testdata_path().join("lint/watch/badly_linted.js"); + let badly_linted_fixed1 = + util::testdata_path().join("lint/watch/badly_linted_fixed1.js"); + let badly_linted_fixed1_output = + util::testdata_path().join("lint/watch/badly_linted_fixed1.js.out"); + let badly_linted_fixed2 = + util::testdata_path().join("lint/watch/badly_linted_fixed2.js"); + let badly_linted_fixed2_output = + util::testdata_path().join("lint/watch/badly_linted_fixed2.js.out"); + let badly_linted = t.path().join("badly_linted.js"); + let badly_linted_output = + util::testdata_path().join("lint/watch/badly_linted.js.out"); + + std::fs::copy(&badly_linted_original, &badly_linted) + .expect("Failed to copy file"); + + let mut child = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("lint") + .arg(&badly_linted) + .arg("--watch") + .arg("--unstable") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .expect("Failed to spawn script"); + let mut stderr = child.stderr.as_mut().unwrap(); + let mut stderr_lines = std::io::BufReader::new(&mut stderr) + .lines() + .map(|r| r.unwrap()); + + // TODO(lucacasonato): remove this timeout. It seems to be needed on Linux. + std::thread::sleep(std::time::Duration::from_secs(1)); + + let mut output = read_all_lints(&mut stderr_lines); + let expected = std::fs::read_to_string(badly_linted_output).unwrap(); + assert_eq!(expected, output); + + // Change content of the file again to be badly-linted1 + std::fs::copy(&badly_linted_fixed1, &badly_linted) + .expect("Failed to copy file"); + std::thread::sleep(std::time::Duration::from_secs(1)); + + output = read_all_lints(&mut stderr_lines); + let expected = std::fs::read_to_string(badly_linted_fixed1_output).unwrap(); + assert_eq!(expected, output); + + // Change content of the file again to be badly-linted1 + std::fs::copy(&badly_linted_fixed2, &badly_linted) + .expect("Failed to copy file"); + std::thread::sleep(std::time::Duration::from_secs(1)); + + output = read_all_lints(&mut stderr_lines); + let expected = std::fs::read_to_string(badly_linted_fixed2_output).unwrap(); + assert_eq!(expected, output); + + // the watcher process is still alive + assert!(child.try_wait().unwrap().is_none()); + + child.kill().unwrap(); + drop(t); +} + #[test] fn fmt_watch_test() { let t = TempDir::new().unwrap(); diff --git a/cli/tests/testdata/lint/watch/badly_linted.js b/cli/tests/testdata/lint/watch/badly_linted.js new file mode 100644 index 00000000000000..8d1c1fe79375c3 --- /dev/null +++ b/cli/tests/testdata/lint/watch/badly_linted.js @@ -0,0 +1 @@ +let a = 5; diff --git a/cli/tests/testdata/lint/watch/badly_linted.js.out b/cli/tests/testdata/lint/watch/badly_linted.js.out new file mode 100644 index 00000000000000..5c715695aa0776 --- /dev/null +++ b/cli/tests/testdata/lint/watch/badly_linted.js.out @@ -0,0 +1,2 @@ +(no-unused-vars) `a` is never used +(prefer-const) `a` is never reassigned diff --git a/cli/tests/testdata/lint/watch/badly_linted_fixed1.js b/cli/tests/testdata/lint/watch/badly_linted_fixed1.js new file mode 100644 index 00000000000000..bfccee47dd9092 --- /dev/null +++ b/cli/tests/testdata/lint/watch/badly_linted_fixed1.js @@ -0,0 +1 @@ +let _a = 5; diff --git a/cli/tests/testdata/lint/watch/badly_linted_fixed1.js.out b/cli/tests/testdata/lint/watch/badly_linted_fixed1.js.out new file mode 100644 index 00000000000000..fe74a7a2caa1a7 --- /dev/null +++ b/cli/tests/testdata/lint/watch/badly_linted_fixed1.js.out @@ -0,0 +1 @@ +(prefer-const) `_a` is never reassigned diff --git a/cli/tests/testdata/lint/watch/badly_linted_fixed2.js b/cli/tests/testdata/lint/watch/badly_linted_fixed2.js new file mode 100644 index 00000000000000..94fe8701b988c1 --- /dev/null +++ b/cli/tests/testdata/lint/watch/badly_linted_fixed2.js @@ -0,0 +1 @@ +const _a = 5; diff --git a/cli/tests/testdata/lint/watch/badly_linted_fixed2.js.out b/cli/tests/testdata/lint/watch/badly_linted_fixed2.js.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/cli/tools/lint.rs b/cli/tools/lint.rs index 7448463bd09656..5630067ae99e6e 100644 --- a/cli/tools/lint.rs +++ b/cli/tools/lint.rs @@ -6,11 +6,13 @@ //! At the moment it is only consumed using CLI but in //! the future it can be easily extended to provide //! the same functions as ops available in JS runtime. -use crate::colors; use crate::config_file::LintConfig; +use crate::file_watcher::ResolutionResult; +use crate::flags::LintFlags; use crate::fmt_errors; use crate::fs_util::{collect_files, is_supported_ext}; use crate::tools::fmt::run_parallelized; +use crate::{colors, file_watcher}; use deno_ast::swc::parser::Syntax; use deno_ast::MediaType; use deno_core::error::{anyhow, generic_error, AnyError, JsStackFrame}; @@ -31,6 +33,7 @@ use std::sync::{Arc, Mutex}; static STDIN_FILE_NAME: &str = "_stdin.ts"; +#[derive(Clone, Debug)] pub enum LintReporterKind { Pretty, Json, @@ -43,21 +46,26 @@ fn create_reporter(kind: LintReporterKind) -> Box { } } -pub async fn lint_files( +pub async fn lint( maybe_lint_config: Option, - rules_tags: Vec, - rules_include: Vec, - rules_exclude: Vec, - args: Vec, - ignore: Vec, - json: bool, + lint_flags: LintFlags, + watch: bool, ) -> Result<(), AnyError> { + let LintFlags { + rules_tags, + rules_include, + rules_exclude, + files: args, + ignore, + json, + .. + } = lint_flags; // First, prepare final configuration. // Collect included and ignored files. CLI flags take precendence // over config file, ie. if there's `files.ignore` in config file // and `--ignore` CLI flag, only the flag value is taken into account. let mut include_files = args.clone(); - let mut exclude_files = ignore; + let mut exclude_files = ignore.clone(); if let Some(lint_config) = maybe_lint_config.as_ref() { if include_files.is_empty() { @@ -79,6 +87,13 @@ pub async fn lint_files( } } + let reporter_kind = if json { + LintReporterKind::Json + } else { + LintReporterKind::Pretty + }; + + let has_error = Arc::new(AtomicBool::new(false)); // Try to get configured rules. CLI flags take precendence // over config file, ie. if there's `rules.include` in config file // and `--rules-include` CLI flag, only the flag value is taken into account. @@ -89,27 +104,82 @@ pub async fn lint_files( rules_exclude, )?; - let has_error = Arc::new(AtomicBool::new(false)); - - let reporter_kind = if json { - LintReporterKind::Json - } else { - LintReporterKind::Pretty + let resolver = |changed: Option>| { + let files_changed = changed.is_some(); + let result = collect_files( + &*include_files.clone(), + &*exclude_files.clone(), + is_supported_ext, + ) + .map(|files| { + if let Some(paths) = changed { + files + .into_iter() + .filter(|path| paths.contains(path)) + .collect::>() + } else { + files + } + }); + let paths_to_watch = args.clone(); + async move { + if (files_changed || !watch) + && matches!(result, Ok(ref files) if files.is_empty()) + { + ResolutionResult::Ignore + } else { + ResolutionResult::Restart { + paths_to_watch, + result, + } + } + } }; - let reporter_lock = Arc::new(Mutex::new(create_reporter(reporter_kind))); - let no_of_files_linted = + let operation = |paths: Vec| async { + let target_files_len = paths.len(); + let reporter_kind = reporter_kind.clone(); + let reporter_lock = Arc::new(Mutex::new(create_reporter(reporter_kind))); + run_parallelized(paths, { + let has_error = has_error.clone(); + let lint_rules = lint_rules.clone(); + let reporter_lock = reporter_lock.clone(); + move |file_path| { + let r = lint_file(file_path.clone(), lint_rules.clone()); + handle_lint_result( + &file_path.to_string_lossy(), + r, + reporter_lock.clone(), + has_error, + ); + + Ok(()) + } + }) + .await?; + reporter_lock.lock().unwrap().close(target_files_len); + + Ok(()) + }; + if watch { if args.len() == 1 && args[0].to_string_lossy() == "-" { + return Err(generic_error( + "Lint watch on standard input is not supported.", + )); + } + file_watcher::watch_func(resolver, operation, "Lint").await?; + } else { + if args.len() == 1 && args[0].to_string_lossy() == "-" { + let reporter_lock = + Arc::new(Mutex::new(create_reporter(reporter_kind.clone()))); let r = lint_stdin(lint_rules); - handle_lint_result( STDIN_FILE_NAME, r, reporter_lock.clone(), has_error.clone(), ); - - 1 + reporter_lock.lock().unwrap().close(1); } else { let target_files = collect_files(&include_files, &exclude_files, is_supported_ext) @@ -121,32 +191,12 @@ pub async fn lint_files( } })?; debug!("Found {} files", target_files.len()); - let target_files_len = target_files.len(); - - run_parallelized(target_files, { - let reporter_lock = reporter_lock.clone(); - let has_error = has_error.clone(); - move |file_path| { - let r = lint_file(file_path.clone(), lint_rules.clone()); - handle_lint_result( - &file_path.to_string_lossy(), - r, - reporter_lock, - has_error, - ); - Ok(()) - } - }) - .await?; - - target_files_len + operation(target_files).await?; }; - - reporter_lock.lock().unwrap().close(no_of_files_linted); - let has_error = has_error.load(Ordering::Relaxed); - - if has_error { - std::process::exit(1); + let has_error = has_error.load(Ordering::Relaxed); + if has_error { + std::process::exit(1); + } } Ok(())