Skip to content

run spellcheck as a tidy extra check in ci #145025

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion compiler/rustc_next_trait_solver/src/canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ enum CanonicalizeInputKind {
ParamEnv,
/// When canonicalizing predicates, we don't keep `'static`. If we're
/// currently outside of the trait solver and canonicalize the root goal
/// during HIR typeck, we replace each occurance of a region with a
/// during HIR typeck, we replace each occurrence of a region with a
/// unique region variable. See the comment on `InferCtxt::in_hir_typeck`
/// for more details.
Predicate { is_hir_typeck_root_goal: bool },
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
ImportKind::Glob { .. } => {
// FIXME: Use mutable resolver directly as a hack, this should be an output of
// specualtive resolution.
// speculative resolution.
self.get_mut_unchecked().resolve_glob_import(import);
return 0;
}
Expand Down Expand Up @@ -907,7 +907,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// We need the `target`, `source` can be extracted.
let imported_binding = this.import(binding, import);
// FIXME: Use mutable resolver directly as a hack, this should be an output of
// specualtive resolution.
// speculative resolution.
this.get_mut_unchecked().define_binding_local(
parent,
target,
Expand All @@ -921,7 +921,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if target.name != kw::Underscore {
let key = BindingKey::new(target, ns);
// FIXME: Use mutable resolver directly as a hack, this should be an output of
// specualtive resolution.
// speculative resolution.
this.get_mut_unchecked().update_local_resolution(
parent,
key,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4276,7 +4276,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
if path.len() == 2
&& let [segment] = prefix_path
{
// Delay to check whether methond name is an associated function or not
// Delay to check whether method name is an associated function or not
// ```
// let foo = Foo {};
// foo::bar(); // possibly suggest to foo.bar();
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sync/nonpoison/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ pub struct MutexGuard<'a, T: ?Sized + 'a> {
lock: &'a Mutex<T>,
}

/// A [`MutexGuard`] is not `Send` to maximize platform portablity.
/// A [`MutexGuard`] is not `Send` to maximize platform portability.
///
/// On platforms that use POSIX threads (commonly referred to as pthreads) there is a requirement to
/// release mutex locks on the same thread they were acquired.
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sync/poison/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ pub struct MutexGuard<'a, T: ?Sized + 'a> {
poison: poison::Guard,
}

/// A [`MutexGuard`] is not `Send` to maximize platform portablity.
/// A [`MutexGuard`] is not `Send` to maximize platform portability.
///
/// On platforms that use POSIX threads (commonly referred to as pthreads) there is a requirement to
/// release mutex locks on the same thread they were acquired.
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![allow(unsafe_op_in_unsafe_fn)]

/// The configure builtins provides runtime support compiler-builtin features
/// which require dynamic intialization to work as expected, e.g. aarch64
/// which require dynamic initialization to work as expected, e.g. aarch64
/// outline-atomics.
mod configure_builtins;

Expand Down
2 changes: 1 addition & 1 deletion src/ci/docker/host-x86_64/tidy/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@ RUN bash -c 'npm install -g eslint@$(cat /tmp/eslint.version)'
# 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,js
src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck
2 changes: 1 addition & 1 deletion src/librustdoc/html/static/js/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -1529,7 +1529,7 @@ function preLoadCss(cssUrl) {
["&#9166;", "Go to active search result"],
["+", "Expand all sections"],
["-", "Collapse all sections"],
// for the sake of brevity, we don't say "inherint impl blocks",
// for the sake of brevity, we don't say "inherit impl blocks",
// although that would be more correct,
// since trait impl blocks are collapsed by -
["_", "Collapse all sections, including impl blocks"],
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/static/js/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -3333,7 +3333,7 @@ class DocSearch {
}

// sort unstable items later
// FIXME: there is some doubt if this is the most effecient way to implement this.
// FIXME: there is some doubt if this is the most efficient way to implement this.
// alternative options include:
// * put is_unstable on each item when the index is built.
// increases memory usage but avoids a hashmap lookup.
Expand Down
47 changes: 20 additions & 27 deletions src/tools/tidy/src/extra_checks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ const RUFF_CONFIG_PATH: &[&str] = &["src", "tools", "tidy", "config", "ruff.toml
const RUFF_CACHE_PATH: &[&str] = &["cache", "ruff_cache"];
const PIP_REQ_PATH: &[&str] = &["src", "tools", "tidy", "config", "requirements.txt"];

// this must be kept in sync with with .github/workflows/spellcheck.yml
const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", "src/librustdoc"];

pub fn check(
Expand All @@ -51,6 +50,7 @@ pub fn check(
librustdoc_path: &Path,
tools_path: &Path,
npm: &Path,
cargo: &Path,
bless: bool,
extra_checks: Option<&str>,
pos_args: &[String],
Expand All @@ -63,6 +63,7 @@ pub fn check(
librustdoc_path,
tools_path,
npm,
cargo,
bless,
extra_checks,
pos_args,
Expand All @@ -78,6 +79,7 @@ fn check_impl(
librustdoc_path: &Path,
tools_path: &Path,
npm: &Path,
cargo: &Path,
bless: bool,
extra_checks: Option<&str>,
pos_args: &[String],
Expand Down Expand Up @@ -293,7 +295,7 @@ fn check_impl(
} else {
eprintln!("spellcheck files");
}
spellcheck_runner(&args)?;
spellcheck_runner(root_path, &outdir, &cargo, &args)?;
}

if js_lint || js_typecheck {
Expand Down Expand Up @@ -576,34 +578,25 @@ fn shellcheck_runner(args: &[&OsStr]) -> Result<(), Error> {
if status.success() { Ok(()) } else { Err(Error::FailedCheck("shellcheck")) }
}

/// Check that spellchecker is installed then run it at the given path
fn spellcheck_runner(args: &[&str]) -> Result<(), Error> {
// sync version with .github/workflows/spellcheck.yml
let expected_version = "typos-cli 1.34.0";
match Command::new("typos").arg("--version").output() {
Ok(o) => {
let stdout = String::from_utf8_lossy(&o.stdout);
if stdout.trim() != expected_version {
return Err(Error::Version {
program: "typos",
required: expected_version,
installed: stdout.trim().to_string(),
});
/// Ensure that spellchecker is installed then run it at the given path
fn spellcheck_runner(
src_root: &Path,
outdir: &Path,
cargo: &Path,
args: &[&str],
) -> Result<(), Error> {
let bin_path =
crate::ensure_version_or_cargo_install(outdir, cargo, "typos-cli", "typos", "1.34.0")?;
match Command::new(bin_path).current_dir(src_root).args(args).status() {
Ok(status) => {
if status.success() {
Ok(())
} else {
Err(Error::FailedCheck("typos"))
}
}
Err(e) if e.kind() == io::ErrorKind::NotFound => {
return Err(Error::MissingReq(
"typos",
"spellcheck file checks",
// sync version with .github/workflows/spellcheck.yml
Some("install tool via `cargo install [email protected]`".to_owned()),
));
}
Err(e) => return Err(e.into()),
Err(err) => Err(Error::Generic(format!("failed to run typos tool: {err:?}"))),
}

let status = Command::new("typos").args(args).status()?;
if status.success() { Ok(()) } else { Err(Error::FailedCheck("typos")) }
}

/// Check git for tracked files matching an extension
Expand Down
66 changes: 66 additions & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
//! to be used by tools.

use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::{env, io};

use build_helper::ci::CiEnv;
use build_helper::git::{GitConfig, get_closest_upstream_commit};
Expand Down Expand Up @@ -180,6 +182,70 @@ pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool {
!v.is_empty()
}

/// If the given executable is installed with the given version, use that,
/// otherwise install via cargo.
pub fn ensure_version_or_cargo_install(
build_dir: &Path,
cargo: &Path,
pkg_name: &str,
bin_name: &str,
version: &str,
) -> io::Result<PathBuf> {
// ignore the process exit code here and instead just let the version number check fail.
// we also importantly don't return if the program wasn't installed,
// instead we want to continue to the fallback.
'ck: {
// FIXME: rewrite as if-let chain once this crate is 2024 edition.
let Ok(output) = Command::new(bin_name).arg("--version").output() else {
break 'ck;
};
let Ok(s) = str::from_utf8(&output.stdout) else {
break 'ck;
};
let Some(v) = s.trim().split_whitespace().last() else {
break 'ck;
};
if v == version {
return Ok(PathBuf::from(bin_name));
}
}

let tool_root_dir = build_dir.join("misc-tools");
let tool_bin_dir = tool_root_dir.join("bin");
eprintln!("building external tool {bin_name} from package {pkg_name}@{version}");
// use --force to ensure that if the required version is bumped, we update it.
// use --target-dir to ensure we have a build cache so repeated invocations aren't slow.
// modify PATH so that cargo doesn't print a warning telling the user to modify the path.
let cargo_exit_code = Command::new(cargo)
.args(["install", "--locked", "--force", "--quiet"])
.arg("--root")
.arg(&tool_root_dir)
.arg("--target-dir")
.arg(tool_root_dir.join("target"))
.arg(format!("{pkg_name}@{version}"))
.env(
"PATH",
env::join_paths(
env::split_paths(&env::var("PATH").unwrap())
.chain(std::iter::once(tool_bin_dir.clone())),
)
.expect("build dir contains invalid char"),
)
.env("RUSTFLAGS", "-Copt-level=0")
.spawn()?
.wait()?;
if !cargo_exit_code.success() {
return Err(io::Error::other("cargo install failed"));
}
let bin_path = tool_bin_dir.join(bin_name);
assert!(
matches!(bin_path.try_exists(), Ok(true)),
"cargo install did not produce the expected binary"
);
eprintln!("finished building tool {bin_name}");
Ok(bin_path)
}

pub mod alphabetical;
pub mod bins;
pub mod debug_artifacts;
Expand Down
1 change: 1 addition & 0 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ fn main() {
&librustdoc_path,
&tools_path,
&npm,
&cargo,
bless,
extra_checks,
pos_args
Expand Down
Loading