Fix some issues and add tests for linting

denoland · dsherret · Jul 4, 2024 · Jun 17, 2024 · Jun 18, 2024 · Jun 18, 2024
commit 914be648c388a01375faa53ca231e661e96305eb
diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs
@@ -129,6  129,12 @@ pub async fn format(flags: Flags, fmt_flags: FmtFlags) -> Result<(), AnyError> {
   Ok(())
 }
 
 struct PathsWithOptions {
   base: PathBuf,
   paths: Vec<PathBuf>,
   options: FmtOptions,
 }
 
 fn resolve_paths_with_options_batches(
   cli_options: &CliOptions,
   fmt_flags: &FmtFlags,
@@ -154,12  160,6 @@ fn resolve_paths_with_options_batches(
   Ok(paths_with_options_batches)
 }
 
-struct PathsWithOptions {
-  base: PathBuf,
-  paths: Vec<PathBuf>,
-  options: FmtOptions,
-}
-
 async fn format_files(
   caches: &Arc<Caches>,
   fmt_flags: &FmtFlags,

diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs
@@ -91,28  91,15 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> {
         Ok(async move {
           let factory = CliFactory::from_flags(flags)?;
           let cli_options = factory.cli_options();
-          let mut linter = WorkspaceLinter::new(
-            factory.caches()?.clone(),
-            factory.module_graph_creator().await?.clone(),
-            cli_options.workspace.clone(),
-            &cli_options.resolve_workspace_lint_options(&lint_flags)?,
-          );
           let lint_config = cli_options.resolve_lint_config()?;
-          for (ctx, lint_options) in
-            cli_options.resolve_lint_options_for_members(&lint_flags)?
-          {
-            let files =
-              collect_lint_files(cli_options, lint_options.files.clone())
-                .and_then(|files| {
-                  if files.is_empty() {
-                    Err(generic_error("No target files found."))
-                  } else {
-                    Ok(files)
-                  }
-                })?;
-            _ = watcher_communicator.watch_paths(files.clone());
-
-            let lint_paths = if let Some(paths) = &changed_paths {
           let mut paths_with_options_batches =
             resolve_paths_with_options_batches(cli_options, &lint_flags)?;
           for paths_with_options in &mut paths_with_options_batches {
             _ = watcher_communicator
               .watch_paths(paths_with_options.paths.clone());
 
             let files = std::mem::take(&mut paths_with_options.paths);
             paths_with_options.paths = if let Some(paths) = &changed_paths {
               // lint all files on any changed (https://github.com/denoland/deno/issues/12446)
               files
                 .iter()
@@ -126,13  113,25 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> {
             } else {
               files
             };
           }
 
           let mut linter = WorkspaceLinter::new(
             factory.caches()?.clone(),
             factory.module_graph_creator().await?.clone(),
             cli_options.workspace.clone(),
             &cli_options.resolve_workspace_lint_options(&lint_flags)?,
           );
           for paths_with_options in paths_with_options_batches {
             linter
-              .lint_files(lint_options, lint_config.clone(), ctx, lint_paths)
               .lint_files(
                 paths_with_options.options,
                 lint_config.clone(),
                 paths_with_options.ctx,
                 paths_with_options.paths,
               )
               .await?;
           }
-          if linter.file_count == 0 {
-            return Err(generic_error("No target files found."));
-          }
 
           linter.finish();
 
           Ok(())
@@ -174,18  173,18 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> {
         cli_options.workspace.clone(),
         &workspace_lint_options,
       );
-      for (ctx, lint_options) in
-        cli_options.resolve_lint_options_for_members(&lint_flags)?
-      {
-        let files = &lint_options.files;
-        let target_files = collect_lint_files(cli_options, files.clone())?;
       let paths_with_options_batches =
         resolve_paths_with_options_batches(cli_options, &lint_flags)?;
       for paths_with_options in paths_with_options_batches {
         linter
-          .lint_files(lint_options, lint_config.clone(), ctx, target_files)
           .lint_files(
             paths_with_options.options,
             lint_config.clone(),
             paths_with_options.ctx,
             paths_with_options.paths,
           )
           .await?;
       }
-      if linter.file_count == 0 {
-        return Err(generic_error("No target files found."));
-      }
       linter.finish()
     };
     if !success {
@@ -196,6  195,36 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> {
   Ok(())
 }
 
 struct PathsWithOptions {
   ctx: WorkspaceMemberContext,
   paths: Vec<PathBuf>,
   options: LintOptions,
 }
 
 fn resolve_paths_with_options_batches(
   cli_options: &CliOptions,
   lint_flags: &LintFlags,
 ) -> Result<Vec<PathsWithOptions>, AnyError> {
   let members_lint_options =
     cli_options.resolve_lint_options_for_members(lint_flags)?;
   let mut paths_with_options_batches =
     Vec::with_capacity(members_lint_options.len());
   for (ctx, lint_options) in members_lint_options {
     let files = collect_lint_files(cli_options, lint_options.files.clone())?;
     if !files.is_empty() {
       paths_with_options_batches.push(PathsWithOptions {
         ctx,
         paths: files,
         options: lint_options,
       });
     }
   }
   if paths_with_options_batches.is_empty() {
     return Err(generic_error("No target files found."));
   }
   Ok(paths_with_options_batches)
 }
 
 type WorkspaceModuleGraphFuture =
   SharedLocal<LocalBoxFuture<'static, Result<Rc<ModuleGraph>, Rc<AnyError>>>>;
 
@@ -277,12  306,6 @@ impl WorkspaceLinter {
           .collect::<HashSet<_>>();
         futures.push(
           async move {
-            // let graph = module_graph_creator
-            //   .create_and_validate_publish_graph(
-            //     &vec![publish_config.clone()],
-            //     true,
-            //   )
-            //   .await?;
             let graph = workspace_module_graph_future
               .await
               .map_err(|err| anyhow!("{:#}", err))?;

diff --git a/tests/specs/lint/workspace/__test__.jsonc b/tests/specs/lint/workspace/__test__.jsonc
@@ -0,0  1,15 @@
 {
   "tests": {
     "root_check": {
       "args": "lint",
       "exitCode": 1,
       "output": "root.out"
     },
     "subdir_check": {
       "args": "lint",
       "cwd": "a",
       "exitCode": 1,
       "output": "a.out"
     }
   }
 }
diff --git a/tests/specs/lint/workspace/a.out b/tests/specs/lint/workspace/a.out
@@ -0,0  1,32 @@
 error[no-eval]: `eval` call is not allowed
  --> [WILDLINE]a.ts:1:1
   | 
 1 | eval("");
   | ^^^^^^^^
   = hint: Remove the use of `eval`
 
   docs: https://lint.deno.land/rules/no-eval
 
 
 error[no-await-in-loop]: Unexpected `await` inside a loop.
  --> [WILDLINE]a.ts:4:3
   | 
 4 |   await Deno.open("test");
   |   ^^^^^^^^^^^^^^^^^^^^^^^
   = hint: Remove `await` in loop body, store all promises generated and then `await Promise.all(storedPromises)` after the loop
 
   docs: https://lint.deno.land/rules/no-await-in-loop
 
 
 error[no-explicit-any]: `any` type is not allowed
  --> [WILDLINE]a.ts:9:25
   | 
 9 | export function test(): any {
   |                         ^^^
   = hint: Use a specific type other than `any`
 
   docs: https://lint.deno.land/rules/no-explicit-any
 
 
 Found 3 problems
 Checked 1 file
diff --git a/tests/specs/lint/workspace/a/a.ts b/tests/specs/lint/workspace/a/a.ts
@@ -0,0  1,11 @@
 eval("");
 
 for (let i = 0; i < 10; i  ) {
   await Deno.open("test");
 }
 
 const unused = 1;
 
 export function test(): any {
   return {};
 }
diff --git a/tests/specs/lint/workspace/a/deno.json b/tests/specs/lint/workspace/a/deno.json
@@ -0,0  1,12 @@
 {
   "lint": {
     "rules": {
       "include": [
         "no-await-in-loop"
       ],
       "exclude": [
         "no-unused-vars"
       ]
     }
   }
 }
diff --git a/tests/specs/lint/workspace/b/b.ts b/tests/specs/lint/workspace/b/b.ts
@@ -0,0  1,11 @@
 eval("");
 
 for (let i = 0; i < 10; i  ) {
   await Deno.open("test");
 }
 
 const unused = 1;
 
 export function test(): any {
   return {};
 }
diff --git a/tests/specs/lint/workspace/b/deno.json b/tests/specs/lint/workspace/b/deno.json
@@ -0,0  1,9 @@
 {
   "lint": {
     "rules": {
       "exclude": [
         "no-explicit-any"
       ]
     }
   }
 }
diff --git a/tests/specs/lint/workspace/deno.json b/tests/specs/lint/workspace/deno.json
@@ -0,0  1,11 @@
 {
   "workspace": [
     "./a",
     "./b"
   ],
   "lint": {
     "rules": {
       "include": ["no-eval"]
     }
   }
 }
diff --git a/tests/specs/lint/workspace/root.out b/tests/specs/lint/workspace/root.out
@@ -0,0  1,82 @@
 error[no-eval]: `eval` call is not allowed
  --> [WILDLINE]root.ts:1:1
   | 
 1 | eval("");
   | ^^^^^^^^
   = hint: Remove the use of `eval`
 
   docs: https://lint.deno.land/rules/no-eval
 
 
 error[no-unused-vars]: `unused` is never used
  --> [WILDLINE]root.ts:7:7
   | 
 7 | const unused = 1;
   |       ^^^^^^
   = hint: If this is intentional, prefix it with an underscore like `_unused`
 
   docs: https://lint.deno.land/rules/no-unused-vars
 
 
 error[no-explicit-any]: `any` type is not allowed
  --> [WILDLINE]root.ts:9:25
   | 
 9 | export function test(): any {
   |                         ^^^
   = hint: Use a specific type other than `any`
 
   docs: https://lint.deno.land/rules/no-explicit-any
 
 
 error[no-eval]: `eval` call is not allowed
  --> [WILDLINE]a\a.ts:1:1
   | 
 1 | eval("");
   | ^^^^^^^^
   = hint: Remove the use of `eval`
 
   docs: https://lint.deno.land/rules/no-eval
 
 
 error[no-await-in-loop]: Unexpected `await` inside a loop.
  --> [WILDLINE]a\a.ts:4:3
   | 
 4 |   await Deno.open("test");
   |   ^^^^^^^^^^^^^^^^^^^^^^^
   = hint: Remove `await` in loop body, store all promises generated and then `await Promise.all(storedPromises)` after the loop
 
   docs: https://lint.deno.land/rules/no-await-in-loop
 
 
 error[no-explicit-any]: `any` type is not allowed
  --> [WILDLINE]a.ts:9:25
   | 
 9 | export function test(): any {
   |                         ^^^
   = hint: Use a specific type other than `any`
 
   docs: https://lint.deno.land/rules/no-explicit-any
 
 
 error[no-eval]: `eval` call is not allowed
  --> [WILDLINE]b.ts:1:1
   | 
 1 | eval("");
   | ^^^^^^^^
   = hint: Remove the use of `eval`
 
   docs: https://lint.deno.land/rules/no-eval
 
 
 error[no-unused-vars]: `unused` is never used
  --> [WILDLINE]b.ts:7:7
   | 
 7 | const unused = 1;
   |       ^^^^^^
   = hint: If this is intentional, prefix it with an underscore like `_unused`
 
   docs: https://lint.deno.land/rules/no-unused-vars
 
 
 Found 8 problems
 Checked 3 files
diff --git a/tests/specs/lint/workspace/root.ts b/tests/specs/lint/workspace/root.ts
@@ -0,0  1,11 @@
 eval("");
 
 for (let i = 0; i < 10; i  ) {
   await Deno.open("test");
 }
 
 const unused = 1;
 
 export function test(): any {
   return {};
 }