From 0527f027e8eb5000d3725d3ceb6156e36aeb7b7a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 28 May 2025 22:34:08 +0200 Subject: [PATCH 1/5] Add `eslint` as part of tidy run --- src/tools/tidy/src/lib.rs | 1 + src/tools/tidy/src/main.rs | 2 + src/tools/tidy/src/rustdoc_js.rs | 91 ++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 src/tools/tidy/src/rustdoc_js.rs diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index ca45f8bb84ba9..e8a12d563358d 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -82,6 +82,7 @@ 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_templates; pub mod style; pub mod target_policy; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 48122129b0174..671d796dba959 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -35,6 +35,7 @@ fn main() { let library_path = root_path.join("library"); let compiler_path = root_path.join("compiler"); let librustdoc_path = src_path.join("librustdoc"); + let tools_path = src_path.join("tools"); let crashes_path = tests_path.join("crashes"); let args: Vec = env::args().skip(1).collect(); @@ -108,6 +109,7 @@ 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); check!(known_bug, &crashes_path); check!(unknown_revision, &tests_path); diff --git a/src/tools/tidy/src/rustdoc_js.rs b/src/tools/tidy/src/rustdoc_js.rs new file mode 100644 index 0000000000000..73a654f51e6d1 --- /dev/null +++ b/src/tools/tidy/src/rustdoc_js.rs @@ -0,0 +1,91 @@ +//! 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 run_eslint(args: &[PathBuf], config_folder: PathBuf, bad: &mut bool) { + let mut child = match 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; + } + }; + match child.wait() { + Ok(exit_status) => { + if exit_status.success() { + return; + } + eprintln!("eslint command failed"); + } + Err(error) => eprintln!("eslint command failed: {error:?}"), + } + *bad = true; +} + +fn get_eslint_version_inner(global: bool) -> Option { + 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 { + get_eslint_version_inner(false).or_else(|| get_eslint_version_inner(true)) +} + +const ESLINT_VERSION: &str = "8.6.0"; + +pub fn check(librustdoc_path: &Path, tools_path: &Path, bad: &mut bool) { + match get_eslint_version() { + Some(version) => { + if version != ESLINT_VERSION { + *bad = true; + eprintln!( + "⚠️ 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; + } + } + 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; + } + } + let mut files_to_check = 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_to_check.push(path.path().into()); + }, + ); + println!("Running eslint on rustdoc JS files"); + run_eslint(&files_to_check, librustdoc_path.join("html/static"), bad); + + 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); +} From 9e5dd511669a0ee091a1964e7939c40788211c28 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 28 May 2025 22:35:48 +0200 Subject: [PATCH 2/5] Remove checks that are run with `tidy` --- src/ci/docker/host-x86_64/mingw-check/Dockerfile | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ci/docker/host-x86_64/mingw-check/Dockerfile b/src/ci/docker/host-x86_64/mingw-check/Dockerfile index 418408e9242ae..df73c7382b519 100644 --- a/src/ci/docker/host-x86_64/mingw-check/Dockerfile +++ b/src/ci/docker/host-x86_64/mingw-check/Dockerfile @@ -65,7 +65,4 @@ ENV SCRIPT \ python3 ../x.py test collect-license-metadata && \ # Runs checks to ensure that there are no issues in our JS code. es-check es2019 ../src/librustdoc/html/static/js/*.js && \ - eslint -c ../src/librustdoc/html/static/.eslintrc.js ../src/librustdoc/html/static/js/*.js && \ - eslint -c ../src/tools/rustdoc-js/.eslintrc.js ../src/tools/rustdoc-js/tester.js && \ - eslint -c ../src/tools/rustdoc-gui/.eslintrc.js ../src/tools/rustdoc-gui/tester.js && \ tsc --project ../src/librustdoc/html/static/js/tsconfig.json From f0f661db2b4edad3aa22df32a24176407125d70a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 29 May 2025 14:50:48 +0200 Subject: [PATCH 3/5] Install eslint in host-x86_64 Dockerfile --- src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile b/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile index 9ca8cc740a5c7..2ec8eb62c7ac4 100644 --- a/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile +++ b/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile @@ -24,6 +24,13 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ mingw-w64 \ && rm -rf /var/lib/apt/lists/* +COPY scripts/nodejs.sh /scripts/ +RUN sh /scripts/nodejs.sh /node +ENV PATH="/node/bin:${PATH}" + +# Install eslint +RUN npm install eslint@8.6.0 + COPY scripts/sccache.sh /scripts/ RUN sh /scripts/sccache.sh From b1723fc83be899ba084fb31935352cf5393956b4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 29 May 2025 14:58:32 +0200 Subject: [PATCH 4/5] Centralize the eslint version between tidy and docker --- .../host-x86_64/mingw-check-tidy/Dockerfile | 3 ++- .../mingw-check-tidy/eslint.version | 1 + src/tools/tidy/src/main.rs | 2 +- src/tools/tidy/src/rustdoc_js.rs | 22 +++++++++++++------ 4 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 src/ci/docker/host-x86_64/mingw-check-tidy/eslint.version diff --git a/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile b/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile index 2ec8eb62c7ac4..903145c1bb617 100644 --- a/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile +++ b/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile @@ -29,7 +29,8 @@ RUN sh /scripts/nodejs.sh /node ENV PATH="/node/bin:${PATH}" # Install eslint -RUN npm install eslint@8.6.0 +COPY host-x86_64/mingw-check-tidy/eslint.version /tmp/ +RUN npm install eslint@$(head -n 1 /tmp/eslint.version) COPY scripts/sccache.sh /scripts/ RUN sh /scripts/sccache.sh diff --git a/src/ci/docker/host-x86_64/mingw-check-tidy/eslint.version b/src/ci/docker/host-x86_64/mingw-check-tidy/eslint.version new file mode 100644 index 0000000000000..1acea15afd690 --- /dev/null +++ b/src/ci/docker/host-x86_64/mingw-check-tidy/eslint.version @@ -0,0 +1 @@ +8.6.0 \ No newline at end of file diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 671d796dba959..776f1bde2eb71 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -109,7 +109,7 @@ 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); + check!(rustdoc_js, &librustdoc_path, &tools_path, &src_path); check!(known_bug, &crashes_path); check!(unknown_revision, &tests_path); diff --git a/src/tools/tidy/src/rustdoc_js.rs b/src/tools/tidy/src/rustdoc_js.rs index 73a654f51e6d1..2517e2de12ce5 100644 --- a/src/tools/tidy/src/rustdoc_js.rs +++ b/src/tools/tidy/src/rustdoc_js.rs @@ -51,27 +51,35 @@ fn get_eslint_version() -> Option { get_eslint_version_inner(false).or_else(|| get_eslint_version_inner(true)) } -const ESLINT_VERSION: &str = "8.6.0"; - -pub fn check(librustdoc_path: &Path, tools_path: &Path, bad: &mut bool) { +pub fn check(librustdoc_path: &Path, tools_path: &Path, src_path: &Path, bad: &mut bool) { + 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; + } + }; match get_eslint_version() { Some(version) => { - if version != ESLINT_VERSION { + if version != eslint_version { *bad = true; eprintln!( "⚠️ Installed version of eslint (`{version}`) is different than the \ - one used in the CI (`{ESLINT_VERSION}`)", + 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}`", + `npm install eslint@{eslint_version}`", ); return; } } 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}`"); + eprintln!("You can install it using `npm install eslint@{eslint_version}`"); return; } } From 8645ef7d8e4053562f621578d3fe44460ccfa8c0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 29 May 2025 15:32:52 +0200 Subject: [PATCH 5/5] Fix npm install error --- src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile b/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile index 903145c1bb617..d17f7ed7171f8 100644 --- a/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile +++ b/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile @@ -30,7 +30,6 @@ ENV PATH="/node/bin:${PATH}" # Install eslint COPY host-x86_64/mingw-check-tidy/eslint.version /tmp/ -RUN npm install eslint@$(head -n 1 /tmp/eslint.version) COPY scripts/sccache.sh /scripts/ RUN sh /scripts/sccache.sh @@ -44,5 +43,5 @@ COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/ # NOTE: intentionally uses python2 for x.py so we can test it still works. # validate-toolstate only runs in our CI, so it's ok for it to only support python3. -ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \ - --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp +ENV SCRIPT TIDY_PRINT_DIFF=1 npm install eslint@$(head -n 1 /tmp/eslint.version) && \ + python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp