Rollup merge of #141705 - GuillaumeGomez:eslint-tidy, r=Kobzol

Add eslint as part of `tidy` run

Rustdoc uses `eslint` to run lints on the JS files. Currently you need to run it by hand since it's not part of any `x.py` command. This PR makes it part of `test tidy`. However, to prevent having all rust developers to install `npm` and `eslint`, I made it optional: if `eslint` is not installed, then the check is simply skipped (but will tell that it is being skipped).

The second commit removes the manual checks from the docker file since `eslint` is run as part of tidy.

cc `@lolbinarycat,` [#t-rustdoc > eslint seems to only be run in CI](https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/eslint.20seems.20to.20only.20be.20run.20in.20CI/with/520761477)
This commit is contained in:
Guillaume Gomez
2025-05-29 17:03:00 +02:00
committed by GitHub
6 changed files with 112 additions and 5 deletions
@@ -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
COPY host-x86_64/mingw-check-tidy/eslint.version /tmp/
COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh
@@ -36,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
@@ -0,0 +1 @@
8.6.0
@@ -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
+1
View File
@@ -82,6 +82,7 @@ fn tidy_error(args: &str) -> std::io::Result<()> {
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;
+2
View File
@@ -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<String> = env::args().skip(1).collect();
@@ -108,6 +109,7 @@ macro_rules! check {
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!(known_bug, &crashes_path);
check!(unknown_revision, &tests_path);
+99
View File
@@ -0,0 +1,99 @@
//! 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<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 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 {
*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);
}