tidy: running of eslint is now and extra check

Author: lolbinarycat

src/bootstrap/src/core/config/flags.rs

@@ -382,7 +382,7 @@ pub enum Subcommand {
         bless: bool,
         #[arg(long)]
         /// comma-separated list of other files types to check (accepts py, py:lint,
-        /// py:fmt, shell)
+        /// py:fmt, shell, cpp, cpp:fmt, js, js:lint, js:typecheck, js:es-check)
         extra_checks: Option<String>,
         #[arg(long)]
         /// rerun tests even if the inputs are unchanged

src/etc/completions/x.fish

@@ -293,7 +293,7 @@ complete -c x -n "__fish_x_using_subcommand doc" -l skip-std-check-if-no-downloa
 complete -c x -n "__fish_x_using_subcommand doc" -s h -l help -d 'Print help (see more with \'--help\')'
 complete -c x -n "__fish_x_using_subcommand test" -l test-args -d 'extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)' -r
 complete -c x -n "__fish_x_using_subcommand test" -l compiletest-rustc-args -d 'extra options to pass the compiler when running compiletest tests' -r
-complete -c x -n "__fish_x_using_subcommand test" -l extra-checks -d 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell)' -r
+complete -c x -n "__fish_x_using_subcommand test" -l extra-checks -d 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell, cpp, cpp:fmt, js, js:lint, js:typecheck, js:es-check)' -r
 complete -c x -n "__fish_x_using_subcommand test" -l compare-mode -d 'mode describing what file the actual ui output will be compared to' -r
 complete -c x -n "__fish_x_using_subcommand test" -l pass -d 'force {check,build,run}-pass tests to this mode' -r
 complete -c x -n "__fish_x_using_subcommand test" -l run -d 'whether to execute run-* tests' -r

src/etc/completions/x.ps1

@@ -339,7 +339,7 @@ Register-ArgumentCompleter -Native -CommandName 'x' -ScriptBlock {
         'x;test' {
             [CompletionResult]::new('--test-args', '--test-args', [CompletionResultType]::ParameterName, 'extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)')
             [CompletionResult]::new('--compiletest-rustc-args', '--compiletest-rustc-args', [CompletionResultType]::ParameterName, 'extra options to pass the compiler when running compiletest tests')
-            [CompletionResult]::new('--extra-checks', '--extra-checks', [CompletionResultType]::ParameterName, 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell)')
+            [CompletionResult]::new('--extra-checks', '--extra-checks', [CompletionResultType]::ParameterName, 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell, cpp, cpp:fmt, js, js:lint, js:typecheck, js:es-check)')
             [CompletionResult]::new('--compare-mode', '--compare-mode', [CompletionResultType]::ParameterName, 'mode describing what file the actual ui output will be compared to')
             [CompletionResult]::new('--pass', '--pass', [CompletionResultType]::ParameterName, 'force {check,build,run}-pass tests to this mode')
             [CompletionResult]::new('--run', '--run', [CompletionResultType]::ParameterName, 'whether to execute run-* tests')

src/etc/completions/x.py.fish

@@ -293,7 +293,7 @@ complete -c x.py -n "__fish_x.py_using_subcommand doc" -l skip-std-check-if-no-d
 complete -c x.py -n "__fish_x.py_using_subcommand doc" -s h -l help -d 'Print help (see more with \'--help\')'
 complete -c x.py -n "__fish_x.py_using_subcommand test" -l test-args -d 'extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)' -r
 complete -c x.py -n "__fish_x.py_using_subcommand test" -l compiletest-rustc-args -d 'extra options to pass the compiler when running compiletest tests' -r
-complete -c x.py -n "__fish_x.py_using_subcommand test" -l extra-checks -d 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell)' -r
+complete -c x.py -n "__fish_x.py_using_subcommand test" -l extra-checks -d 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell, cpp, cpp:fmt, js, js:lint, js:typecheck, js:es-check)' -r
 complete -c x.py -n "__fish_x.py_using_subcommand test" -l compare-mode -d 'mode describing what file the actual ui output will be compared to' -r
 complete -c x.py -n "__fish_x.py_using_subcommand test" -l pass -d 'force {check,build,run}-pass tests to this mode' -r
 complete -c x.py -n "__fish_x.py_using_subcommand test" -l run -d 'whether to execute run-* tests' -r

src/etc/completions/x.py.ps1

@@ -339,7 +339,7 @@ Register-ArgumentCompleter -Native -CommandName 'x.py' -ScriptBlock {
         'x.py;test' {
             [CompletionResult]::new('--test-args', '--test-args', [CompletionResultType]::ParameterName, 'extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)')
             [CompletionResult]::new('--compiletest-rustc-args', '--compiletest-rustc-args', [CompletionResultType]::ParameterName, 'extra options to pass the compiler when running compiletest tests')
-            [CompletionResult]::new('--extra-checks', '--extra-checks', [CompletionResultType]::ParameterName, 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell)')
+            [CompletionResult]::new('--extra-checks', '--extra-checks', [CompletionResultType]::ParameterName, 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell, cpp, cpp:fmt, js, js:lint, js:typecheck, js:es-check)')
             [CompletionResult]::new('--compare-mode', '--compare-mode', [CompletionResultType]::ParameterName, 'mode describing what file the actual ui output will be compared to')
             [CompletionResult]::new('--pass', '--pass', [CompletionResultType]::ParameterName, 'force {check,build,run}-pass tests to this mode')
             [CompletionResult]::new('--run', '--run', [CompletionResultType]::ParameterName, 'whether to execute run-* tests')

src/etc/completions/x.py.zsh

@@ -338,7 +338,7 @@ _arguments "${_arguments_options[@]}" : \
 _arguments "${_arguments_options[@]}" : \
 '*--test-args=[extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)]:ARGS:_default' \
 '*--compiletest-rustc-args=[extra options to pass the compiler when running compiletest tests]:ARGS:_default' \
-'--extra-checks=[comma-separated list of other files types to check (accepts py, py\:lint, py\:fmt, shell)]:EXTRA_CHECKS:_default' \
+'--extra-checks=[comma-separated list of other files types to check (accepts py, py\:lint, py\:fmt, shell, cpp, cpp\:fmt, js, js\:lint, js\:typecheck, js\:es-check)]:EXTRA_CHECKS:_default' \
 '--compare-mode=[mode describing what file the actual ui output will be compared to]:COMPARE MODE:_default' \
 '--pass=[force {check,build,run}-pass tests to this mode]:check | build | run:_default' \
 '--run=[whether to execute run-* tests]:auto | always | never:_default' \

src/etc/completions/x.zsh

@@ -338,7 +338,7 @@ _arguments "${_arguments_options[@]}" : \
 _arguments "${_arguments_options[@]}" : \
 '*--test-args=[extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)]:ARGS:_default' \
 '*--compiletest-rustc-args=[extra options to pass the compiler when running compiletest tests]:ARGS:_default' \
-'--extra-checks=[comma-separated list of other files types to check (accepts py, py\:lint, py\:fmt, shell)]:EXTRA_CHECKS:_default' \
+'--extra-checks=[comma-separated list of other files types to check (accepts py, py\:lint, py\:fmt, shell, cpp, cpp\:fmt, js, js\:lint, js\:typecheck, js\:es-check)]:EXTRA_CHECKS:_default' \
 '--compare-mode=[mode describing what file the actual ui output will be compared to]:COMPARE MODE:_default' \
 '--pass=[force {check,build,run}-pass tests to this mode]:check | build | run:_default' \
 '--run=[whether to execute run-* tests]:auto | always | never:_default' \

src/tools/tidy/src/ext_tool_checks.rs

@@ -22,6 +22,8 @@ use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::{fmt, fs, io};
 
+mod rustdoc_js;
+
 const MIN_PY_REV: (u32, u32) = (3, 9);
 const MIN_PY_REV_STR: &str = "≥3.9";
 
@@ -39,19 +41,34 @@ const PIP_REQ_PATH: &[&str] = &["src", "tools", "tidy", "config", "requirements.
 pub fn check(
     root_path: &Path,
     outdir: &Path,
+    librustdoc_path: &Path,
+    tools_path: &Path,
+    src_path: &Path,
     bless: bool,
     extra_checks: Option<&str>,
     pos_args: &[String],
     bad: &mut bool,
 ) {
-    if let Err(e) = check_impl(root_path, outdir, bless, extra_checks, pos_args) {
+    if let Err(e) = check_impl(
+        root_path,
+        outdir,
+        librustdoc_path,
+        tools_path,
+        src_path,
+        bless,
+        extra_checks,
+        pos_args,
+    ) {
         tidy_error!(bad, "{e}");
     }
 }
 
 fn check_impl(
     root_path: &Path,
     outdir: &Path,
+    librustdoc_path: &Path,
+    tools_path: &Path,
+    src_path: &Path,
     bless: bool,
     extra_checks: Option<&str>,
     pos_args: &[String],
@@ -65,13 +82,20 @@ fn check_impl(
         None => vec![],
     };
 
+    // FIXME(lolbinarycat): this is getting complex, we should probably
+    // have more proper handling, including a warning/error
+    // for unknown extra check names.
     let python_all = lint_args.contains(&"py");
     let python_lint = lint_args.contains(&"py:lint") || python_all;
     let python_fmt = lint_args.contains(&"py:fmt") || python_all;
     let shell_all = lint_args.contains(&"shell");
     let shell_lint = lint_args.contains(&"shell:lint") || shell_all;
     let cpp_all = lint_args.contains(&"cpp");
     let cpp_fmt = lint_args.contains(&"cpp:fmt") || cpp_all;
+    let js_all = lint_args.contains(&"js");
+    let js_lint = js_all || lint_args.contains(&"js:lint");
+    let js_typecheck = js_all || lint_args.contains(&"js:typecheck");
+    let js_es_check = js_all || lint_args.contains(&"js:es-check");
 
     let mut py_path = None;
 
@@ -224,6 +248,10 @@ fn check_impl(
         shellcheck_runner(&merge_args(&cfg_args, &file_args_shc))?;
     }
 
+    if js_lint {
+        rustdoc_js::lint(librustdoc_path, tools_path, src_path)?;
+    }
+
     Ok(())
 }
 

src/tools/tidy/src/rustdoc_js.rs renamed to src/tools/tidy/src/ext_tool_checks/rustdoc_js.rs

@@ -9,31 +9,22 @@ use ignore::DirEntry;
 
 use crate::walk::walk_no_read;
 
-fn run_eslint(args: &[PathBuf], config_folder: PathBuf, bad: &mut bool) {
-    let mut child = match Command::new("npx")
+fn run_eslint(args: &[PathBuf], config_folder: PathBuf) -> Result<(), super::Error> {
+    let mut child = Command::new("npx")
         .arg("eslint")
         .arg("-c")
         .arg(config_folder.join(".eslintrc.js"))
         .args(args)
-        .spawn()
-    {
-        Ok(child) => child,
-        Err(error) => {
-            *bad = true;
-            eprintln!("failed to run eslint: {error:?}");
-            return;
-        }
-    };
+        .spawn()?;
     match child.wait() {
         Ok(exit_status) => {
             if exit_status.success() {
-                return;
+                return Ok(());
             }
-            eprintln!("eslint command failed");
+            Err(super::Error::FailedCheck("eslint command failed"))
         }
-        Err(error) => eprintln!("eslint command failed: {error:?}"),
+        Err(error) => Err(super::Error::Generic(format!("eslint command failed: {error:?}"))),
     }
-    *bad = true;
 }
 
 fn get_eslint_version_inner(global: bool) -> Option<String> {
@@ -51,15 +42,18 @@ fn get_eslint_version() -> Option<String> {
     get_eslint_version_inner(false).or_else(|| get_eslint_version_inner(true))
 }
 
-pub fn check(librustdoc_path: &Path, tools_path: &Path, src_path: &Path, bad: &mut bool) {
+pub(super) fn lint(
+    librustdoc_path: &Path,
+    tools_path: &Path,
+    src_path: &Path,
+) -> Result<(), super::Error> {
     let eslint_version_path =
         src_path.join("ci/docker/host-x86_64/mingw-check-tidy/eslint.version");
     let eslint_version = match std::fs::read_to_string(&eslint_version_path) {
         Ok(version) => version.trim().to_string(),
         Err(error) => {
-            *bad = true;
             eprintln!("failed to read `{}`: {error:?}", eslint_version_path.display());
-            return;
+            return Err(error.into());
         }
     };
     // Having the correct `eslint` version installed via `npm` isn't strictly necessary, since we're invoking it via `npx`,
@@ -68,22 +62,25 @@ pub fn check(librustdoc_path: &Path, tools_path: &Path, src_path: &Path, bad: &m
     match get_eslint_version() {
         Some(version) => {
             if version != eslint_version {
-                *bad = true;
-                eprintln!(
+                // unfortunatly we can't use `Error::Version` here becuse `str::trim` isn't const and
+                // Version::required must be a static str
+                return Err(super::Error::Generic(format!(
                     "⚠️ Installed version of eslint (`{version}`) is different than the \
-                     one used in the CI (`{eslint_version}`)",
-                );
-                eprintln!(
-                    "You can install this version using `npm update eslint` or by using \
-                     `npm install eslint@{eslint_version}`",
-                );
-                return;
+                     one used in the CI (`{eslint_version}`)\n\
+                    You can install this version using `npm update eslint` or by using \
+                    `npm install eslint@{eslint_version}`\n
+"
+                )));
             }
         }
         None => {
-            eprintln!("`eslint` doesn't seem to be installed. Skipping tidy check for JS files.");
-            eprintln!("You can install it using `npm install eslint@{eslint_version}`");
-            return;
+            //eprintln!("`eslint` doesn't seem to be installed. Skipping tidy check for JS files.");
+            //eprintln!("You can install it using `npm install eslint@{eslint_version}`");
+            return Err(super::Error::MissingReq(
+                "eslint",
+                "js lint checks",
+                Some(format!("You can install it using `npm install eslint@{eslint_version}`")),
+            ));
         }
     }
     let mut files_to_check = Vec::new();
@@ -95,8 +92,9 @@ pub fn check(librustdoc_path: &Path, tools_path: &Path, src_path: &Path, bad: &m
         },
     );
     println!("Running eslint on rustdoc JS files");
-    run_eslint(&files_to_check, librustdoc_path.join("html/static"), bad);
+    run_eslint(&files_to_check, librustdoc_path.join("html/static"))?;
 
-    run_eslint(&[tools_path.join("rustdoc-js/tester.js")], tools_path.join("rustdoc-js"), bad);
-    run_eslint(&[tools_path.join("rustdoc-gui/tester.js")], tools_path.join("rustdoc-gui"), bad);
+    run_eslint(&[tools_path.join("rustdoc-js/tester.js")], tools_path.join("rustdoc-js"))?;
+    run_eslint(&[tools_path.join("rustdoc-gui/tester.js")], tools_path.join("rustdoc-gui"))?;
+    Ok(())
 }

src/tools/tidy/src/lib.rs

@@ -63,6 +63,7 @@ fn tidy_error(args: &str) -> std::io::Result<()> {
     Ok(())
 }
 
+// tidy-alphabetical-start
 pub mod alphabetical;
 pub mod bins;
 pub mod debug_artifacts;
@@ -82,7 +83,6 @@ pub mod mir_opt_tests;
 pub mod pal;
 pub mod rustdoc_css_themes;
 pub mod rustdoc_gui_tests;
-pub mod rustdoc_js;
 pub mod rustdoc_json;
 pub mod rustdoc_templates;
 pub mod style;
@@ -97,3 +97,4 @@ pub mod unknown_revision;
 pub mod unstable_book;
 pub mod walk;
 pub mod x_version;
+// tidy-alphabetical-end

src/tools/tidy/src/main.rs

@@ -109,7 +109,6 @@ fn main() {
         check!(rustdoc_gui_tests, &tests_path);
         check!(rustdoc_css_themes, &librustdoc_path);
         check!(rustdoc_templates, &librustdoc_path);
-        check!(rustdoc_js, &librustdoc_path, &tools_path, &src_path);
         check!(rustdoc_json, &src_path);
         check!(known_bug, &crashes_path);
         check!(unknown_revision, &tests_path);
@@ -171,7 +170,17 @@ fn main() {
         };
         check!(unstable_book, &src_path, collected);
 
-        check!(ext_tool_checks, &root_path, &output_directory, bless, extra_checks, pos_args);
+        check!(
+            ext_tool_checks,
+            &root_path,
+            &output_directory,
+            &librustdoc_path,
+            &tools_path,
+            &src_path,
+            bless,
+            extra_checks,
+            pos_args
+        );
     });
 
     if bad.load(Ordering::Relaxed) {