Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
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);
check!(known_bug, &crashes_path);
check!(unknown_revision, &tests_path);

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

const ESLINT_VERSION: &str = "8.6.0";

pub fn check(librustdoc_path: &Path, tools_path: &Path, bad: &mut bool) {
Copy link
Member

Choose a reason for hiding this comment

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

It really feels like we should add a npm lockfile to tidy, same as we have requirements.txt. But that can be done later.

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