feat(lint): add support for --watch flag (denoland#11983)

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![],

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(

cli/tests/integration/watcher_tests.rs

@@ -28,6 +28,24 @@ fn skip_restarting_line(
   }
 }
 
+fn read_all_lints(stderr_lines: &mut impl Iterator<Item = String>) -> 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<Item = String>) {
   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();

cli/tests/testdata/lint/watch/badly_linted.js

@@ -0,0 +1 @@
+let a = 5;

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

cli/tests/testdata/lint/watch/badly_linted_fixed1.js

@@ -0,0 +1 @@
+let _a = 5;

cli/tests/testdata/lint/watch/badly_linted_fixed1.js.out

@@ -0,0 +1 @@
+(prefer-const) `_a` is never reassigned

cli/tests/testdata/lint/watch/badly_linted_fixed2.js

@@ -0,0 +1 @@
+const _a = 5;

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<dyn LintReporter + Send> {
   }
 }
 
-pub async fn lint_files(
+pub async fn lint(
   maybe_lint_config: Option<LintConfig>,
-  rules_tags: Vec<String>,
-  rules_include: Vec<String>,
-  rules_exclude: Vec<String>,
-  args: Vec<PathBuf>,
-  ignore: Vec<PathBuf>,
-  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<Vec<PathBuf>>| {
+    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::<Vec<_>>()
+      } 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<PathBuf>| 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(())