tidy: running of eslint, tsc, and es-check are now an 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,18 @@ fn check_impl(
         shellcheck_runner(&merge_args(&cfg_args, &file_args_shc))?;
     }
 
+    if js_lint {
+        rustdoc_js::lint(librustdoc_path, tools_path, src_path)?;
+    }
+
+    if js_typecheck {
+        rustdoc_js::typecheck(librustdoc_path)?;
+    }
+
+    if js_es_check {
+        rustdoc_js::es_check(librustdoc_path)?;
+    }
+
     Ok(())
 }
 

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

@@ -0,0 +1,142 @@
+//! Tidy check to ensure that rustdoc templates didn't forget a `{# #}` to strip extra whitespace
+//! characters.
+
+use std::ffi::OsStr;
+use std::path::{Path, PathBuf};
+use std::process::Command;
+
+use ignore::DirEntry;
+
+use crate::walk::walk_no_read;
+
+fn rustdoc_js_files(librustdoc_path: &Path) -> Vec<PathBuf> {
+    let mut files = Vec::new();
+    walk_no_read(
+        &[&librustdoc_path.join("html/static/js")],
+        |path, is_dir| is_dir || !path.extension().is_some_and(|ext| ext == OsStr::new("js")),
+        &mut |path: &DirEntry| {
+            files.push(path.path().into());
+        },
+    );
+    return files;
+}
+
+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()?;
+    match child.wait() {
+        Ok(exit_status) => {
+            if exit_status.success() {
+                return Ok(());
+            }
+            Err(super::Error::FailedCheck("eslint command failed"))
+        }
+        Err(error) => Err(super::Error::Generic(format!("eslint command failed: {error:?}"))),
+    }
+}
+
+fn get_eslint_version_inner(global: bool) -> Option<String> {
+    let mut command = Command::new("npm");
+    command.arg("list").arg("--parseable").arg("--long").arg("--depth=0");
+    if global {
+        command.arg("--global");
+    }
+    let output = command.output().ok()?;
+    let lines = String::from_utf8_lossy(&output.stdout);
+    lines.lines().find_map(|l| l.split(':').nth(1)?.strip_prefix("eslint@")).map(|v| v.to_owned())
+}
+
+fn get_eslint_version() -> Option<String> {
+    get_eslint_version_inner(false).or_else(|| get_eslint_version_inner(true))
+}
+
+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) => {
+            eprintln!("failed to read `{}`: {error:?}", eslint_version_path.display());
+            return Err(error.into());
+        }
+    };
+    // Having the correct `eslint` version installed via `npm` isn't strictly necessary, since we're invoking it via `npx`,
+    // but this check allows the vast majority that is not working on the rustdoc frontend to avoid the penalty of running
+    // `eslint` in tidy. See also: https://github.com/rust-lang/rust/pull/142851
+    match get_eslint_version() {
+        Some(version) => {
+            if version != eslint_version {
+                // 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}`)\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 Err(super::Error::MissingReq(
+                "eslint",
+                "js lint checks",
+                Some(format!("You can install it using `npm install eslint@{eslint_version}`")),
+            ));
+        }
+    }
+    let files_to_check = rustdoc_js_files(librustdoc_path);
+    println!("Running eslint on rustdoc JS files");
+    run_eslint(&files_to_check, librustdoc_path.join("html/static"))?;
+
+    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(())
+}
+
+pub(super) fn typecheck(librustdoc_path: &Path) -> Result<(), super::Error> {
+    // use npx to ensure correct version
+    let mut child = Command::new("npx")
+        .arg("tsc")
+        .arg("-p")
+        .arg(librustdoc_path.join("html/static/js/tsconfig.json"))
+        .spawn()?;
+    match child.wait() {
+        Ok(exit_status) => {
+            if exit_status.success() {
+                return Ok(());
+            }
+            Err(super::Error::FailedCheck("tsc command failed"))
+        }
+        Err(error) => Err(super::Error::Generic(format!("tsc command failed: {error:?}"))),
+    }
+}
+
+pub(super) fn es_check(librustdoc_path: &Path) -> Result<(), super::Error> {
+    let files_to_check = rustdoc_js_files(librustdoc_path);
+    // use npx to ensure correct version
+    let mut cmd = Command::new("npx");
+    cmd.arg("es-check").arg("es2019");
+    for f in files_to_check {
+        cmd.arg(f);
+    }
+    match cmd.spawn()?.wait() {
+        Ok(exit_status) => {
+            if exit_status.success() {
+                return Ok(());
+            }
+            Err(super::Error::FailedCheck("es-check command failed"))
+        }
+        Err(error) => Err(super::Error::Generic(format!("es-check command failed: {error:?}"))),
+    }
+}

src/tools/tidy/src/lib.rs

@@ -143,7 +143,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;

src/tools/tidy/src/main.rs

@@ -111,7 +111,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, &ci_info);
         check!(known_bug, &crashes_path);
         check!(unknown_revision, &tests_path);
@@ -173,7 +172,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) {