Skip to content

Add eslint as part of tidy run #141705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
1 change: 1 addition & 0 deletions src/ci/docker/host-x86_64/mingw-check-tidy/eslint.version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8.6.0
3 changes: 0 additions & 3 deletions src/ci/docker/host-x86_64/mingw-check/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 && \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels weird to run eslint locally, but not tsc/es-check. Is there a reason for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is kinda odd, and i wouldn't mind fixing it, but we are at least fully compatible with the latest version of tsc, which is why i only complained about eslint.

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 change: 1 addition & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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, &src_path);
check!(known_bug, &crashes_path);
check!(unknown_revision, &tests_path);

Expand Down
99 changes: 99 additions & 0 deletions src/tools/tidy/src/rustdoc_js.rs
Original file line number Diff line number Diff line change
@@ -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);
}
Loading