diff --git a/Cargo.lock b/Cargo.lock index 973214a802e6e..96106f59efb53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5514,6 +5514,7 @@ dependencies = [ "semver", "serde", "similar", + "tempfile", "termcolor", "toml 0.7.8", "walkdir", diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 154b209b20252..daf8e6dc27938 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1235,6 +1235,12 @@ impl Step for Tidy { if builder.config.cmd.bless() { cmd.arg("--bless"); } + if builder.config.cmd.pre_push() { + cmd.arg("--pre-push"); + } + if builder.config.cmd.include_untracked() { + cmd.arg("--include-untracked"); + } if let Some(s) = builder.config.cmd.extra_checks().or(builder.config.tidy_extra_checks.as_deref()) { diff --git a/src/bootstrap/src/core/config/flags.rs b/src/bootstrap/src/core/config/flags.rs index c01b71b926068..c83d6c241d5ac 100644 --- a/src/bootstrap/src/core/config/flags.rs +++ b/src/bootstrap/src/core/config/flags.rs @@ -391,6 +391,12 @@ pub enum Subcommand { /// whether to automatically update stderr/stdout files bless: bool, #[arg(long)] + /// Whether to run Tidy on the most recent commit, used in the pre-push git-hook. + pre_push: bool, + #[arg(long)] + /// Whether to include files untracked by git when running Tidy. + include_untracked: bool, + #[arg(long)] /// comma-separated list of other files types to check (accepts py, py:lint, /// py:fmt, shell, cpp, cpp:fmt, js, js:lint, js:typecheck, spellcheck) /// @@ -571,6 +577,20 @@ impl Subcommand { } } + pub fn pre_push(&self) -> bool { + match *self { + Subcommand::Test { pre_push, .. } => pre_push, + _ => false, + } + } + + pub fn include_untracked(&self) -> bool { + match *self { + Subcommand::Test { include_untracked, .. } => include_untracked, + _ => false, + } + } + pub fn extra_checks(&self) -> Option<&str> { match *self { Subcommand::Test { ref extra_checks, .. } => extra_checks.as_ref().map(String::as_str), diff --git a/src/etc/completions/x.fish b/src/etc/completions/x.fish index 544f9b97237cb..747627e6b2a1c 100644 --- a/src/etc/completions/x.fish +++ b/src/etc/completions/x.fish @@ -332,6 +332,8 @@ complete -c x -n "__fish_x_using_subcommand test" -l no-fail-fast -d 'run all te complete -c x -n "__fish_x_using_subcommand test" -l no-doc -d 'do not run doc tests' complete -c x -n "__fish_x_using_subcommand test" -l doc -d 'only run doc tests' complete -c x -n "__fish_x_using_subcommand test" -l bless -d 'whether to automatically update stderr/stdout files' +complete -c x -n "__fish_x_using_subcommand test" -l pre-push -d 'Whether to run Tidy on the most recent commit, used in the pre-push git-hook' +complete -c x -n "__fish_x_using_subcommand test" -l include-untracked -d 'Whether to include files untracked by git when running Tidy' complete -c x -n "__fish_x_using_subcommand test" -l force-rerun -d 'rerun tests even if the inputs are unchanged' complete -c x -n "__fish_x_using_subcommand test" -l only-modified -d 'only run tests that result has been changed' complete -c x -n "__fish_x_using_subcommand test" -l rustfix-coverage -d 'enable this to generate a Rustfix coverage file, which is saved in `//rustfix_missing_coverage.txt`' diff --git a/src/etc/completions/x.ps1 b/src/etc/completions/x.ps1 index b03acf930f70d..0b6839bac4d38 100644 --- a/src/etc/completions/x.ps1 +++ b/src/etc/completions/x.ps1 @@ -379,6 +379,8 @@ Register-ArgumentCompleter -Native -CommandName 'x' -ScriptBlock { [CompletionResult]::new('--no-doc', '--no-doc', [CompletionResultType]::ParameterName, 'do not run doc tests') [CompletionResult]::new('--doc', '--doc', [CompletionResultType]::ParameterName, 'only run doc tests') [CompletionResult]::new('--bless', '--bless', [CompletionResultType]::ParameterName, 'whether to automatically update stderr/stdout files') + [CompletionResult]::new('--pre-push', '--pre-push', [CompletionResultType]::ParameterName, 'Whether to run Tidy on the most recent commit, used in the pre-push git-hook') + [CompletionResult]::new('--include-untracked', '--include-untracked', [CompletionResultType]::ParameterName, 'Whether to include files untracked by git when running Tidy') [CompletionResult]::new('--force-rerun', '--force-rerun', [CompletionResultType]::ParameterName, 'rerun tests even if the inputs are unchanged') [CompletionResult]::new('--only-modified', '--only-modified', [CompletionResultType]::ParameterName, 'only run tests that result has been changed') [CompletionResult]::new('--rustfix-coverage', '--rustfix-coverage', [CompletionResultType]::ParameterName, 'enable this to generate a Rustfix coverage file, which is saved in `//rustfix_missing_coverage.txt`') diff --git a/src/etc/completions/x.py.fish b/src/etc/completions/x.py.fish index 08e4cd26ce887..b14f2de926306 100644 --- a/src/etc/completions/x.py.fish +++ b/src/etc/completions/x.py.fish @@ -332,6 +332,8 @@ complete -c x.py -n "__fish_x.py_using_subcommand test" -l no-fail-fast -d 'run complete -c x.py -n "__fish_x.py_using_subcommand test" -l no-doc -d 'do not run doc tests' complete -c x.py -n "__fish_x.py_using_subcommand test" -l doc -d 'only run doc tests' complete -c x.py -n "__fish_x.py_using_subcommand test" -l bless -d 'whether to automatically update stderr/stdout files' +complete -c x.py -n "__fish_x.py_using_subcommand test" -l pre-push -d 'Whether to run Tidy on the most recent commit, used in the pre-push git-hook' +complete -c x.py -n "__fish_x.py_using_subcommand test" -l include-untracked -d 'Whether to include files untracked by git when running Tidy' complete -c x.py -n "__fish_x.py_using_subcommand test" -l force-rerun -d 'rerun tests even if the inputs are unchanged' complete -c x.py -n "__fish_x.py_using_subcommand test" -l only-modified -d 'only run tests that result has been changed' complete -c x.py -n "__fish_x.py_using_subcommand test" -l rustfix-coverage -d 'enable this to generate a Rustfix coverage file, which is saved in `//rustfix_missing_coverage.txt`' diff --git a/src/etc/completions/x.py.ps1 b/src/etc/completions/x.py.ps1 index 3d95d88af4955..553cc1e46c8eb 100644 --- a/src/etc/completions/x.py.ps1 +++ b/src/etc/completions/x.py.ps1 @@ -379,6 +379,8 @@ Register-ArgumentCompleter -Native -CommandName 'x.py' -ScriptBlock { [CompletionResult]::new('--no-doc', '--no-doc', [CompletionResultType]::ParameterName, 'do not run doc tests') [CompletionResult]::new('--doc', '--doc', [CompletionResultType]::ParameterName, 'only run doc tests') [CompletionResult]::new('--bless', '--bless', [CompletionResultType]::ParameterName, 'whether to automatically update stderr/stdout files') + [CompletionResult]::new('--pre-push', '--pre-push', [CompletionResultType]::ParameterName, 'Whether to run Tidy on the most recent commit, used in the pre-push git-hook') + [CompletionResult]::new('--include-untracked', '--include-untracked', [CompletionResultType]::ParameterName, 'Whether to include files untracked by git when running Tidy') [CompletionResult]::new('--force-rerun', '--force-rerun', [CompletionResultType]::ParameterName, 'rerun tests even if the inputs are unchanged') [CompletionResult]::new('--only-modified', '--only-modified', [CompletionResultType]::ParameterName, 'only run tests that result has been changed') [CompletionResult]::new('--rustfix-coverage', '--rustfix-coverage', [CompletionResultType]::ParameterName, 'enable this to generate a Rustfix coverage file, which is saved in `//rustfix_missing_coverage.txt`') diff --git a/src/etc/completions/x.py.sh b/src/etc/completions/x.py.sh index 8ff0eaf35c89a..bd505ec08d141 100644 --- a/src/etc/completions/x.py.sh +++ b/src/etc/completions/x.py.sh @@ -3875,7 +3875,7 @@ _x.py() { return 0 ;; x.py__test) - opts="-v -i -j -h --no-fail-fast --test-args --compiletest-rustc-args --no-doc --doc --bless --extra-checks --force-rerun --only-modified --compare-mode --pass --run --rustfix-coverage --no-capture --test-codegen-backend --verbose --incremental --config --build-dir --build --host --target --exclude --skip --include-default-paths --rustc-error-format --on-fail --dry-run --dump-bootstrap-shims --stage --keep-stage --keep-stage-std --src --jobs --warnings --json-output --compile-time-deps --color --bypass-bootstrap-lock --rust-profile-generate --rust-profile-use --llvm-profile-use --llvm-profile-generate --enable-bolt-settings --skip-stage0-validation --reproducible-artifact --set --ci --skip-std-check-if-no-download-rustc --help [PATHS]... [ARGS]..." + opts="-v -i -j -h --no-fail-fast --test-args --compiletest-rustc-args --no-doc --doc --bless --pre-push --include-untracked --extra-checks --force-rerun --only-modified --compare-mode --pass --run --rustfix-coverage --no-capture --test-codegen-backend --verbose --incremental --config --build-dir --build --host --target --exclude --skip --include-default-paths --rustc-error-format --on-fail --dry-run --dump-bootstrap-shims --stage --keep-stage --keep-stage-std --src --jobs --warnings --json-output --compile-time-deps --color --bypass-bootstrap-lock --rust-profile-generate --rust-profile-use --llvm-profile-use --llvm-profile-generate --enable-bolt-settings --skip-stage0-validation --reproducible-artifact --set --ci --skip-std-check-if-no-download-rustc --help [PATHS]... [ARGS]..." if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 diff --git a/src/etc/completions/x.py.zsh b/src/etc/completions/x.py.zsh index 9d2d73e582ec6..bd6f7d0b5ce50 100644 --- a/src/etc/completions/x.py.zsh +++ b/src/etc/completions/x.py.zsh @@ -379,6 +379,8 @@ _arguments "${_arguments_options[@]}" : \ '--no-doc[do not run doc tests]' \ '--doc[only run doc tests]' \ '--bless[whether to automatically update stderr/stdout files]' \ +'--pre-push[Whether to run Tidy on the most recent commit, used in the pre-push git-hook]' \ +'--include-untracked[Whether to include files untracked by git when running Tidy]' \ '--force-rerun[rerun tests even if the inputs are unchanged]' \ '--only-modified[only run tests that result has been changed]' \ '--rustfix-coverage[enable this to generate a Rustfix coverage file, which is saved in \`//rustfix_missing_coverage.txt\`]' \ diff --git a/src/etc/completions/x.sh b/src/etc/completions/x.sh index c1b73fb7c9e34..aa868a2e83b82 100644 --- a/src/etc/completions/x.sh +++ b/src/etc/completions/x.sh @@ -3875,7 +3875,7 @@ _x() { return 0 ;; x__test) - opts="-v -i -j -h --no-fail-fast --test-args --compiletest-rustc-args --no-doc --doc --bless --extra-checks --force-rerun --only-modified --compare-mode --pass --run --rustfix-coverage --no-capture --test-codegen-backend --verbose --incremental --config --build-dir --build --host --target --exclude --skip --include-default-paths --rustc-error-format --on-fail --dry-run --dump-bootstrap-shims --stage --keep-stage --keep-stage-std --src --jobs --warnings --json-output --compile-time-deps --color --bypass-bootstrap-lock --rust-profile-generate --rust-profile-use --llvm-profile-use --llvm-profile-generate --enable-bolt-settings --skip-stage0-validation --reproducible-artifact --set --ci --skip-std-check-if-no-download-rustc --help [PATHS]... [ARGS]..." + opts="-v -i -j -h --no-fail-fast --test-args --compiletest-rustc-args --no-doc --doc --bless --pre-push --include-untracked --extra-checks --force-rerun --only-modified --compare-mode --pass --run --rustfix-coverage --no-capture --test-codegen-backend --verbose --incremental --config --build-dir --build --host --target --exclude --skip --include-default-paths --rustc-error-format --on-fail --dry-run --dump-bootstrap-shims --stage --keep-stage --keep-stage-std --src --jobs --warnings --json-output --compile-time-deps --color --bypass-bootstrap-lock --rust-profile-generate --rust-profile-use --llvm-profile-use --llvm-profile-generate --enable-bolt-settings --skip-stage0-validation --reproducible-artifact --set --ci --skip-std-check-if-no-download-rustc --help [PATHS]... [ARGS]..." if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 diff --git a/src/etc/completions/x.zsh b/src/etc/completions/x.zsh index 29237ef9bf8fd..f54f39f5de8d9 100644 --- a/src/etc/completions/x.zsh +++ b/src/etc/completions/x.zsh @@ -379,6 +379,8 @@ _arguments "${_arguments_options[@]}" : \ '--no-doc[do not run doc tests]' \ '--doc[only run doc tests]' \ '--bless[whether to automatically update stderr/stdout files]' \ +'--pre-push[Whether to run Tidy on the most recent commit, used in the pre-push git-hook]' \ +'--include-untracked[Whether to include files untracked by git when running Tidy]' \ '--force-rerun[rerun tests even if the inputs are unchanged]' \ '--only-modified[only run tests that result has been changed]' \ '--rustfix-coverage[enable this to generate a Rustfix coverage file, which is saved in \`//rustfix_missing_coverage.txt\`]' \ diff --git a/src/etc/pre-push.sh b/src/etc/pre-push.sh index 33ed2f0e406b1..a044d338e694f 100755 --- a/src/etc/pre-push.sh +++ b/src/etc/pre-push.sh @@ -29,7 +29,9 @@ cd "$ROOT_DIR" # The env var is necessary for printing diffs in py (fmt/lint) and cpp. TIDY_PRINT_DIFF=1 ./x test tidy \ --set build.locked-deps=true \ + --pre-push \ --extra-checks auto:py,auto:cpp,auto:js + if [ $? -ne 0 ]; then echo "You may use \`git push --no-verify\` to skip this check." exit 1 diff --git a/src/tools/features-status-dump/src/main.rs b/src/tools/features-status-dump/src/main.rs index a4f88362ab816..430e6581f1055 100644 --- a/src/tools/features-status-dump/src/main.rs +++ b/src/tools/features-status-dump/src/main.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; use anyhow::{Context, Result}; use clap::Parser; -use tidy::diagnostics::RunningCheck; +use tidy::diagnostics::{RunningCheck, TidyFlags}; use tidy::features::{Feature, collect_lang_features, collect_lib_features}; #[derive(Debug, Parser)] @@ -31,7 +31,7 @@ fn main() -> Result<()> { let Cli { compiler_path, library_path, output_path } = Cli::parse(); let lang_features_status = collect_lang_features(&compiler_path, &mut RunningCheck::new_noop()); - let lib_features_status = collect_lib_features(&library_path) + let lib_features_status = collect_lib_features(&library_path, &TidyFlags::default()) .into_iter() .filter(|&(ref name, _)| !lang_features_status.contains_key(name)) .collect(); diff --git a/src/tools/replace-version-placeholder/src/main.rs b/src/tools/replace-version-placeholder/src/main.rs index fb2838a4ea03b..e17c2ba1d864b 100644 --- a/src/tools/replace-version-placeholder/src/main.rs +++ b/src/tools/replace-version-placeholder/src/main.rs @@ -1,5 +1,6 @@ use std::path::PathBuf; +use tidy::diagnostics::TidyFlags; use tidy::{t, walk}; pub const VERSION_PLACEHOLDER: &str = "CURRENT_RUSTC_VERSION"; @@ -16,6 +17,7 @@ fn main() { &root_path.join("src/doc/rustc"), &root_path.join("src/doc/rustdoc"), ], + &TidyFlags::default(), |path, _is_dir| walk::filter_dirs(path), &mut |entry, contents| { if !contents.contains(VERSION_PLACEHOLDER) { diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml index c1f27de7ed4a5..7375b57f9e917 100644 --- a/src/tools/tidy/Cargo.toml +++ b/src/tools/tidy/Cargo.toml @@ -19,6 +19,9 @@ fluent-syntax = "0.12" similar = "2.5.0" toml = "0.7.8" +[dev-dependencies] +tempfile = "3" + [features] build-metrics = ["dep:serde"] diff --git a/src/tools/tidy/Readme.md b/src/tools/tidy/Readme.md index fc9dc6350e92d..14918c1f98eb7 100644 --- a/src/tools/tidy/Readme.md +++ b/src/tools/tidy/Readme.md @@ -45,9 +45,20 @@ You can run tidy manually with: `./x test tidy` +### Tidy Flags + To first run the relevant formatter and then run tidy you can add `--bless`. `./x test tidy --bless` + +To include files untracked by git you can add `--include-untracked`. This does not currently change the behavior of `--extra-checks`. To include untracked `js`, `py`, or `cpp` files you'll need to manually add them with git. + +`./x test tidy --include-untracked` \ + +To run tidy on the `HEAD` commit (the most recent commit of your current working branch) you can add `--pre-push`. This flag is added automatically when used with the pre-push git hook. + +`./x test tidy --pre-push` + ### Extra Checks [`extra_checks`](https://doc.rust-lang.org/nightly/nightly-rustc/tidy/extra_checks/index.html) are optional checks primarily focused on other file types and programming languages. diff --git a/src/tools/tidy/src/alphabetical.rs b/src/tools/tidy/src/alphabetical.rs index f93d3b5611309..6b5df2a75f6f7 100644 --- a/src/tools/tidy/src/alphabetical.rs +++ b/src/tools/tidy/src/alphabetical.rs @@ -24,7 +24,7 @@ use std::fmt::Display; use std::iter::Peekable; use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx}; use crate::walk::{filter_dirs, walk}; #[cfg(test)] @@ -130,13 +130,13 @@ fn check_lines<'a>( } } -pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("alphabetical").path(path)); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("alphabetical").path(path)); let skip = |path: &_, _is_dir| filter_dirs(path) || path.ends_with("tidy/src/alphabetical/tests.rs"); - walk(path, skip, &mut |entry, contents| { + walk(path, &tidy_ctx.tidy_flags, skip, &mut |entry, contents| { let file = &entry.path().display(); let lines = contents.lines().enumerate(); check_lines(file, lines, &mut check) diff --git a/src/tools/tidy/src/alphabetical/tests.rs b/src/tools/tidy/src/alphabetical/tests.rs index b181ab8f744f9..331bba1ddfb8e 100644 --- a/src/tools/tidy/src/alphabetical/tests.rs +++ b/src/tools/tidy/src/alphabetical/tests.rs @@ -1,12 +1,12 @@ use std::path::Path; use crate::alphabetical::check_lines; -use crate::diagnostics::DiagCtx; +use crate::diagnostics::{TidyCtx, TidyFlags}; #[track_caller] fn test(lines: &str, name: &str, expected_msg: &str, expected_bad: bool) { - let diag_ctx = DiagCtx::new(Path::new("/"), false); - let mut check = diag_ctx.start_check("alphabetical-test"); + let tidy_ctx = TidyCtx::new(Path::new("/"), TidyFlags::default(), false); + let mut check = tidy_ctx.start_check("alphabetical-test"); check_lines(&name, lines.lines().enumerate(), &mut check); assert_eq!(expected_bad, check.is_bad()); diff --git a/src/tools/tidy/src/bins.rs b/src/tools/tidy/src/bins.rs index 10b6186997167..67f20992acb4d 100644 --- a/src/tools/tidy/src/bins.rs +++ b/src/tools/tidy/src/bins.rs @@ -12,13 +12,13 @@ pub use os_impl::*; mod os_impl { use std::path::Path; - use crate::diagnostics::DiagCtx; + use crate::diagnostics::TidyCtx; pub fn check_filesystem_support(_sources: &[&Path], _output: &Path) -> bool { return false; } - pub fn check(_path: &Path, _diag_ctx: DiagCtx) {} + pub fn check(_path: &Path, _tidy_ctx: TidyCtx) {} } #[cfg(unix)] @@ -28,6 +28,7 @@ mod os_impl { use std::path::Path; use std::process::{Command, Stdio}; + #[cfg(unix)] use crate::walk::{filter_dirs, walk_no_read}; enum FilesystemSupport { @@ -38,7 +39,7 @@ mod os_impl { use FilesystemSupport::*; - use crate::diagnostics::DiagCtx; + use crate::diagnostics::TidyCtx; fn is_executable(path: &Path) -> std::io::Result { Ok(path.metadata()?.mode() & 0o111 != 0) @@ -110,8 +111,8 @@ mod os_impl { } #[cfg(unix)] - pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check("bins"); + pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("bins"); use std::ffi::OsStr; @@ -127,6 +128,7 @@ mod os_impl { // (e.g. using `git ls-files`). walk_no_read( &[path], + &tidy_ctx.tidy_flags, |path, _is_dir| { filter_dirs(path) || path.ends_with("src/etc") diff --git a/src/tools/tidy/src/debug_artifacts.rs b/src/tools/tidy/src/debug_artifacts.rs index 19effaede817b..5e232a55f8bf0 100644 --- a/src/tools/tidy/src/debug_artifacts.rs +++ b/src/tools/tidy/src/debug_artifacts.rs @@ -2,16 +2,17 @@ use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::walk::{filter_dirs, filter_not_rust, walk}; const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in test"; -pub fn check(test_dir: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("debug_artifacts").path(test_dir)); +pub fn check(test_dir: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("debug_artifacts").path(test_dir)); walk( test_dir, + &tidy_ctx.tidy_flags, |path, _is_dir| filter_dirs(path) || filter_not_rust(path), &mut |entry, contents| { for (i, line) in contents.lines().enumerate() { diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 3f40bef821c9d..26d84a7597874 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -9,7 +9,7 @@ use build_helper::ci::CiEnv; use cargo_metadata::semver::Version; use cargo_metadata::{Metadata, Package, PackageId}; -use crate::diagnostics::{DiagCtx, RunningCheck}; +use crate::diagnostics::{RunningCheck, TidyCtx}; #[path = "../../../bootstrap/src/utils/proc_macro_deps.rs"] mod proc_macro_deps; @@ -666,8 +666,9 @@ const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[ /// /// `root` is path to the directory with the root `Cargo.toml` (for the workspace). `cargo` is path /// to the cargo executable. -pub fn check(root: &Path, cargo: &Path, bless: bool, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check("deps"); +pub fn check(root: &Path, cargo: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("deps"); + let bless = tidy_ctx.tidy_flags.bless; let mut checked_runtime_licenses = false; diff --git a/src/tools/tidy/src/diagnostics.rs b/src/tools/tidy/src/diagnostics.rs index 6e95f97d0104f..921ba148b686d 100644 --- a/src/tools/tidy/src/diagnostics.rs +++ b/src/tools/tidy/src/diagnostics.rs @@ -1,32 +1,82 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fmt::{Display, Formatter}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; +use build_helper::git::{get_git_untracked_files, output_result}; use termcolor::{Color, WriteColor}; +#[derive(Clone, Default, Debug)] +pub struct TidyFlags { + pub bless: bool, + pub pre_push: bool, + pub include_untracked: bool, + pub pre_push_content: HashMap, + pub untracked_files: HashSet, +} + +impl TidyFlags { + pub fn new(root_path: &Path, cfg_args: &[String]) -> Self { + let mut flags = Self::default(); + + for arg in cfg_args { + match arg.as_str() { + "--bless" => flags.bless = true, + "--include-untracked" => flags.include_untracked = true, + "--pre-push" => flags.pre_push = true, + _ => continue, + } + } + + //Get a map of file names and file content from last commit for `--pre-push`. + let pre_push_content = match flags.pre_push { + true => get_git_last_commit_content(root_path), + false => HashMap::new(), + }; + + //Get all of the untracked files, used by default to exclude untracked from tidy. + let untracked_files = match get_git_untracked_files(Some(root_path)) { + Ok(Some(untracked_paths)) => { + untracked_paths.into_iter().map(|s| PathBuf::from(root_path).join(s)).collect() + } + _ => HashSet::new(), + }; + + flags.pre_push_content = pre_push_content; + flags.untracked_files = untracked_files; + + flags + } +} + /// Collects diagnostics from all tidy steps, and contains shared information /// that determines how should message and logs be presented. /// /// Since checks are executed in parallel, the context is internally synchronized, to avoid /// all checks to lock it explicitly. #[derive(Clone)] -pub struct DiagCtx(Arc>); +pub struct TidyCtx { + diag_ctx: Arc>, + pub tidy_flags: TidyFlags, +} -impl DiagCtx { - pub fn new(root_path: &Path, verbose: bool) -> Self { - Self(Arc::new(Mutex::new(DiagCtxInner { - running_checks: Default::default(), - finished_checks: Default::default(), - root_path: root_path.to_path_buf(), - verbose, - }))) +impl TidyCtx { + pub fn new(root_path: &Path, tidy_flags: TidyFlags, verbose: bool) -> Self { + Self { + diag_ctx: Arc::new(Mutex::new(DiagCtx { + running_checks: Default::default(), + finished_checks: Default::default(), + root_path: root_path.to_path_buf(), + verbose, + })), + tidy_flags, + } } pub fn start_check>(&self, id: Id) -> RunningCheck { let mut id = id.into(); - let mut ctx = self.0.lock().unwrap(); + let mut ctx = self.diag_ctx.lock().unwrap(); // Shorten path for shorter diagnostics id.path = match id.path { @@ -38,27 +88,27 @@ impl DiagCtx { RunningCheck { id, bad: false, - ctx: self.0.clone(), + ctx: self.diag_ctx.clone(), #[cfg(test)] errors: vec![], } } pub fn into_failed_checks(self) -> Vec { - let ctx = Arc::into_inner(self.0).unwrap().into_inner().unwrap(); + let ctx = Arc::into_inner(self.diag_ctx).unwrap().into_inner().unwrap(); assert!(ctx.running_checks.is_empty(), "Some checks are still running"); ctx.finished_checks.into_iter().filter(|c| c.bad).collect() } } -struct DiagCtxInner { +struct DiagCtx { running_checks: HashSet, finished_checks: HashSet, verbose: bool, root_path: PathBuf, } -impl DiagCtxInner { +impl DiagCtx { fn start_check(&mut self, id: CheckId) { if self.has_check_id(&id) { panic!("Starting a check named `{id:?}` for the second time"); @@ -140,7 +190,7 @@ impl FinishedCheck { pub struct RunningCheck { id: CheckId, bad: bool, - ctx: Arc>, + ctx: Arc>, #[cfg(test)] errors: Vec, } @@ -151,7 +201,7 @@ impl RunningCheck { /// Useful if you want to run some functions from tidy without configuring /// diagnostics. pub fn new_noop() -> Self { - let ctx = DiagCtx::new(Path::new(""), false); + let ctx = TidyCtx::new(Path::new(""), TidyFlags::default(), false); ctx.start_check("noop") } @@ -241,3 +291,21 @@ pub fn output_message(msg: &str, id: Option<&CheckId>, color: Option) { writeln!(&mut stderr, ": {msg}").unwrap(); } + +fn get_git_last_commit_content(git_root: &Path) -> HashMap { + let mut content_map = HashMap::new(); + // Get all of the file names that have been modified in the working dir. + let file_names = + t!(output_result(std::process::Command::new("git").args(["diff", "--name-only", "HEAD"]))) + .lines() + .map(|s| s.trim().to_owned()) + .collect::>(); + for file in file_names { + let content = t!(output_result( + // Get the content of the files from the last commit. Used for '--pre-push' tidy flag. + std::process::Command::new("git").arg("show").arg(format!("HEAD:{}", &file)) + )); + content_map.insert(PathBuf::from(&git_root).join(file), content); + } + content_map +} diff --git a/src/tools/tidy/src/edition.rs b/src/tools/tidy/src/edition.rs index 448e0b0e0a8c6..23296c328a384 100644 --- a/src/tools/tidy/src/edition.rs +++ b/src/tools/tidy/src/edition.rs @@ -2,12 +2,12 @@ use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::walk::{filter_dirs, walk}; -pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("edition").path(path)); - walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| { +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("edition").path(path)); + walk(path, &tidy_ctx.tidy_flags, |path, _is_dir| filter_dirs(path), &mut |entry, contents| { let file = entry.path(); let filename = file.file_name().unwrap(); if filename != "Cargo.toml" { diff --git a/src/tools/tidy/src/error_codes.rs b/src/tools/tidy/src/error_codes.rs index 83fbefa43d975..bc97cc5ab8273 100644 --- a/src/tools/tidy/src/error_codes.rs +++ b/src/tools/tidy/src/error_codes.rs @@ -22,7 +22,7 @@ use std::path::Path; use regex::Regex; -use crate::diagnostics::{DiagCtx, RunningCheck}; +use crate::diagnostics::{RunningCheck, TidyCtx, TidyFlags}; use crate::walk::{filter_dirs, walk, walk_many}; const ERROR_CODES_PATH: &str = "compiler/rustc_error_codes/src/lib.rs"; @@ -36,8 +36,8 @@ const IGNORE_DOCTEST_CHECK: &[&str] = &["E0464", "E0570", "E0601", "E0602", "E07 const IGNORE_UI_TEST_CHECK: &[&str] = &["E0461", "E0465", "E0514", "E0554", "E0640", "E0717", "E0729"]; -pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check("error_codes"); +pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("error_codes"); // Check that no error code explanation was removed. check_removed_error_code_explanation(ci_info, &mut check); @@ -48,13 +48,20 @@ pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo, check.verbose_msg(format!("Highest error code: `{}`", error_codes.iter().max().unwrap())); // Stage 2: check list has docs - let no_longer_emitted = check_error_codes_docs(root_path, &error_codes, &mut check); + let no_longer_emitted = + check_error_codes_docs(root_path, &error_codes, &mut check, &tidy_ctx.tidy_flags); // Stage 3: check list has UI tests check_error_codes_tests(root_path, &error_codes, &mut check, &no_longer_emitted); // Stage 4: check list is emitted by compiler - check_error_codes_used(search_paths, &error_codes, &mut check, &no_longer_emitted); + check_error_codes_used( + search_paths, + &error_codes, + &mut check, + &no_longer_emitted, + &tidy_ctx.tidy_flags, + ); } fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, check: &mut RunningCheck) { @@ -154,12 +161,13 @@ fn check_error_codes_docs( root_path: &Path, error_codes: &[String], check: &mut RunningCheck, + tidy_flags: &TidyFlags, ) -> Vec { let docs_path = root_path.join(Path::new(ERROR_DOCS_PATH)); let mut no_longer_emitted_codes = Vec::new(); - walk(&docs_path, |_, _| false, &mut |entry, contents| { + walk(&docs_path, &tidy_flags, |_, _| false, &mut |entry, contents| { let path = entry.path(); // Error if the file isn't markdown. @@ -331,42 +339,48 @@ fn check_error_codes_used( error_codes: &[String], check: &mut RunningCheck, no_longer_emitted: &[String], + tidy_flags: &TidyFlags, ) { // Search for error codes in the form `E0123`. let regex = Regex::new(r#"\bE\d{4}\b"#).unwrap(); let mut found_codes = Vec::new(); - walk_many(search_paths, |path, _is_dir| filter_dirs(path), &mut |entry, contents| { - let path = entry.path(); - - // Return early if we aren't looking at a source file. - if path.extension() != Some(OsStr::new("rs")) { - return; - } + walk_many( + search_paths, + tidy_flags, + |path, _is_dir| filter_dirs(path), + &mut |entry, contents| { + let path = entry.path(); - for line in contents.lines() { - // We want to avoid parsing error codes in comments. - if line.trim_start().starts_with("//") { - continue; + // Return early if we aren't looking at a source file. + if path.extension() != Some(OsStr::new("rs")) { + return; } - for cap in regex.captures_iter(line) { - if let Some(error_code) = cap.get(0) { - let error_code = error_code.as_str().to_owned(); + for line in contents.lines() { + // We want to avoid parsing error codes in comments. + if line.trim_start().starts_with("//") { + continue; + } - if !error_codes.contains(&error_code) { - // This error code isn't properly defined, we must error. - check.error(format!("Error code `{error_code}` is used in the compiler but not defined and documented in `compiler/rustc_error_codes/src/lib.rs`.")); - continue; - } + for cap in regex.captures_iter(line) { + if let Some(error_code) = cap.get(0) { + let error_code = error_code.as_str().to_owned(); + + if !error_codes.contains(&error_code) { + // This error code isn't properly defined, we must error. + check.error(format!("Error code `{error_code}` is used in the compiler but not defined and documented in `compiler/rustc_error_codes/src/lib.rs`.")); + continue; + } - // This error code can now be marked as used. - found_codes.push(error_code); + // This error code can now be marked as used. + found_codes.push(error_code); + } } } - } - }); + }, + ); for code in error_codes { if !found_codes.contains(code) && !no_longer_emitted.contains(code) { diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index f75de13b45ceb..220664c38a225 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -4,7 +4,7 @@ use std::fs; use std::path::Path; use crate::deps::WorkspaceInfo; -use crate::diagnostics::DiagCtx; +use crate::diagnostics::TidyCtx; /// List of allowed sources for packages. const ALLOWED_SOURCES: &[&str] = &[ @@ -15,7 +15,7 @@ const ALLOWED_SOURCES: &[&str] = &[ /// Checks for external package sources. `root` is the path to the directory that contains the /// workspace `Cargo.toml`. -pub fn check(root: &Path, diag_ctx: DiagCtx) { +pub fn check(root: &Path, diag_ctx: TidyCtx) { let mut check = diag_ctx.start_check("extdeps"); for &WorkspaceInfo { path, submodules, .. } in crate::deps::WORKSPACES { diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index 9f2f2da2e86d7..61517da9c2a11 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -23,8 +23,10 @@ use std::process::Command; use std::str::FromStr; use std::{fmt, fs, io}; +use build_helper::git::get_git_untracked_files; + use crate::CiInfo; -use crate::diagnostics::DiagCtx; +use crate::diagnostics::{TidyCtx, TidyFlags}; mod rustdoc_js; @@ -52,12 +54,11 @@ pub fn check( tools_path: &Path, npm: &Path, cargo: &Path, - bless: bool, extra_checks: Option<&str>, pos_args: &[String], - diag_ctx: DiagCtx, + tidy_ctx: TidyCtx, ) { - let mut check = diag_ctx.start_check("extra_checks"); + let mut check = tidy_ctx.start_check("extra_checks"); if let Err(e) = check_impl( root_path, @@ -67,9 +68,9 @@ pub fn check( tools_path, npm, cargo, - bless, extra_checks, pos_args, + &tidy_ctx.tidy_flags, ) { check.error(e); } @@ -83,12 +84,13 @@ fn check_impl( tools_path: &Path, npm: &Path, cargo: &Path, - bless: bool, extra_checks: Option<&str>, pos_args: &[String], + tidy_flags: &TidyFlags, ) -> Result<(), Error> { let show_diff = std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("true") || v == "1"); + let bless = tidy_flags.bless; // Split comma-separated args up let mut lint_args = match extra_checks { @@ -333,12 +335,12 @@ fn check_impl( } else { eprintln!("linting javascript files and applying suggestions"); } - let res = rustdoc_js::lint(outdir, librustdoc_path, tools_path, bless); + let res = rustdoc_js::lint(outdir, librustdoc_path, tools_path, tidy_flags); if res.is_err() { rerun_with_bless("js:lint", "apply eslint suggestions"); } res?; - rustdoc_js::es_check(outdir, librustdoc_path)?; + rustdoc_js::es_check(outdir, librustdoc_path, tidy_flags)?; } if js_typecheck { @@ -372,12 +374,19 @@ fn run_ruff( cache_dir.as_os_str(), ]); + let untracked; + if let Ok(Some(untracked_files)) = get_git_untracked_files(Some(root_path)) { + untracked = format!("--exclude={}", untracked_files.join(",")); + cfg_args_ruff.push(untracked.as_ref()); + } + if file_args_ruff.is_empty() { file_args_ruff.push(root_path.as_os_str()); } let mut args: Vec<&OsStr> = ruff_args.to_vec(); args.extend(merge_args(&cfg_args_ruff, &file_args_ruff)); + py_runner(py_path, true, None, "ruff", &args) } diff --git a/src/tools/tidy/src/extra_checks/rustdoc_js.rs b/src/tools/tidy/src/extra_checks/rustdoc_js.rs index 5137e61836728..3d3c630d1d0c4 100644 --- a/src/tools/tidy/src/extra_checks/rustdoc_js.rs +++ b/src/tools/tidy/src/extra_checks/rustdoc_js.rs @@ -9,6 +9,7 @@ use std::process::{Child, Command}; use build_helper::npm; use ignore::DirEntry; +use crate::diagnostics::TidyFlags; use crate::walk::walk_no_read; fn node_module_bin(outdir: &Path, name: &str) -> PathBuf { @@ -28,10 +29,11 @@ pub(super) fn npm_install(root_path: &Path, outdir: &Path, npm: &Path) -> Result Ok(()) } -fn rustdoc_js_files(librustdoc_path: &Path) -> Vec { +fn rustdoc_js_files(librustdoc_path: &Path, tidy_flags: &TidyFlags) -> Vec { let mut files = Vec::new(); walk_no_read( &[&librustdoc_path.join("html/static/js")], + tidy_flags, |path, is_dir| is_dir || path.extension().is_none_or(|ext| ext != OsStr::new("js")), &mut |path: &DirEntry| { files.push(path.path().into()); @@ -67,9 +69,10 @@ pub(super) fn lint( outdir: &Path, librustdoc_path: &Path, tools_path: &Path, - bless: bool, + tidy_flags: &TidyFlags, ) -> Result<(), super::Error> { - let files_to_check = rustdoc_js_files(librustdoc_path); + let files_to_check = rustdoc_js_files(librustdoc_path, tidy_flags); + let bless = tidy_flags.bless; println!("Running eslint on rustdoc JS files"); run_eslint(outdir, &files_to_check, librustdoc_path.join("html/static"), bless)?; @@ -100,8 +103,12 @@ pub(super) fn typecheck(outdir: &Path, librustdoc_path: &Path) -> Result<(), sup } } -pub(super) fn es_check(outdir: &Path, librustdoc_path: &Path) -> Result<(), super::Error> { - let files_to_check = rustdoc_js_files(librustdoc_path); +pub(super) fn es_check( + outdir: &Path, + librustdoc_path: &Path, + tidy_flags: &TidyFlags, +) -> Result<(), super::Error> { + let files_to_check = rustdoc_js_files(librustdoc_path, tidy_flags); let mut cmd = Command::new(node_module_bin(outdir, "es-check")); cmd.arg("es2019"); for f in files_to_check { diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index 0a0ba217c6338..05fe9fa597199 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -16,7 +16,7 @@ use std::num::NonZeroU32; use std::path::{Path, PathBuf}; use std::{fmt, fs}; -use crate::diagnostics::{DiagCtx, RunningCheck}; +use crate::diagnostics::{RunningCheck, TidyCtx, TidyFlags}; use crate::walk::{filter_dirs, filter_not_rust, walk, walk_many}; #[cfg(test)] @@ -76,10 +76,10 @@ pub struct CollectedFeatures { } // Currently only used for unstable book generation -pub fn collect_lib_features(base_src_path: &Path) -> Features { +pub fn collect_lib_features(base_src_path: &Path, tidy_flags: &TidyFlags) -> Features { let mut lib_features = Features::new(); - map_lib_features(base_src_path, &mut |res, _, _| { + map_lib_features(base_src_path, tidy_flags, &mut |res, _, _| { if let Ok((name, feature)) = res { lib_features.insert(name.to_owned(), feature); } @@ -92,14 +92,15 @@ pub fn check( tests_path: &Path, compiler_path: &Path, lib_path: &Path, - diag_ctx: DiagCtx, + tidy_ctx: TidyCtx, ) -> CollectedFeatures { - let mut check = diag_ctx.start_check("features"); + let mut check = tidy_ctx.start_check("features"); let mut features = collect_lang_features(compiler_path, &mut check); assert!(!features.is_empty()); - let lib_features = get_and_check_lib_features(lib_path, &mut check, &features); + let lib_features = + get_and_check_lib_features(lib_path, &tidy_ctx.tidy_flags, &mut check, &features); assert!(!lib_features.is_empty()); walk_many( @@ -109,6 +110,7 @@ pub fn check( &tests_path.join("rustdoc-ui"), &tests_path.join("rustdoc"), ], + &tidy_ctx.tidy_flags, |path, _is_dir| { filter_dirs(path) || filter_not_rust(path) @@ -440,11 +442,12 @@ fn collect_lang_features_in( fn get_and_check_lib_features( base_src_path: &Path, + tidy_flags: &TidyFlags, check: &mut RunningCheck, lang_features: &Features, ) -> Features { let mut lib_features = Features::new(); - map_lib_features(base_src_path, &mut |res, file, line| match res { + map_lib_features(base_src_path, tidy_flags, &mut |res, file, line| match res { Ok((name, f)) => { let mut check_features = |f: &Feature, list: &Features, display: &str| { if let Some(s) = list.get(name) @@ -472,10 +475,12 @@ fn get_and_check_lib_features( fn map_lib_features( base_src_path: &Path, + tidy_flags: &TidyFlags, mf: &mut (dyn Send + Sync + FnMut(Result<(&str, Feature), &str>, &Path, usize)), ) { walk( base_src_path, + tidy_flags, |path, _is_dir| filter_dirs(path) || path.ends_with("tests"), &mut |entry, contents| { let file = entry.path(); @@ -612,12 +617,13 @@ fn should_document(var: &str) -> bool { ["SDKROOT", "QNX_TARGET", "COLORTERM", "TERM"].contains(&var) } -pub fn collect_env_vars(compiler: &Path) -> BTreeSet { +pub fn collect_env_vars(compiler: &Path, tidy_flags: &TidyFlags) -> BTreeSet { let env_var_regex: Regex = Regex::new(r#"env::var(_os)?\("([^"]+)"#).unwrap(); let mut vars = BTreeSet::new(); walk( compiler, + tidy_flags, // skip build scripts, tests, and non-rust files |path, _is_dir| { filter_dirs(path) diff --git a/src/tools/tidy/src/filenames.rs b/src/tools/tidy/src/filenames.rs index 835cbefbf6910..a355588fd99ca 100644 --- a/src/tools/tidy/src/filenames.rs +++ b/src/tools/tidy/src/filenames.rs @@ -10,10 +10,10 @@ use std::path::Path; use std::process::Command; -use crate::diagnostics::DiagCtx; +use crate::diagnostics::TidyCtx; -pub fn check(root_path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check("filenames"); +pub fn check(root_path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("filenames"); let stat_output = Command::new("git") .arg("-C") .arg(root_path) diff --git a/src/tools/tidy/src/fluent_alphabetical.rs b/src/tools/tidy/src/fluent_alphabetical.rs index 769f92d04f0af..d311ecc5e8d3d 100644 --- a/src/tools/tidy/src/fluent_alphabetical.rs +++ b/src/tools/tidy/src/fluent_alphabetical.rs @@ -9,7 +9,7 @@ use fluent_syntax::ast::Entry; use fluent_syntax::parser; use regex::Regex; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx}; use crate::walk::{filter_dirs, walk}; fn message() -> &'static Regex { @@ -87,12 +87,14 @@ fn sort_messages( out } -pub fn check(path: &Path, bless: bool, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("fluent_alphabetical").path(path)); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("fluent_alphabetical").path(path)); + let bless = tidy_ctx.tidy_flags.bless; let mut all_defined_msgs = HashMap::new(); walk( path, + &tidy_ctx.tidy_flags, |path, is_dir| filter_dirs(path) || (!is_dir && !is_fluent(path)), &mut |ent, contents| { if bless { @@ -120,5 +122,5 @@ pub fn check(path: &Path, bless: bool, diag_ctx: DiagCtx) { assert!(!all_defined_msgs.is_empty()); - crate::fluent_used::check(path, all_defined_msgs, diag_ctx); + crate::fluent_used::check(path, all_defined_msgs, tidy_ctx); } diff --git a/src/tools/tidy/src/fluent_lowercase.rs b/src/tools/tidy/src/fluent_lowercase.rs index 1d80fda8f3b86..25e2731fcefe3 100644 --- a/src/tools/tidy/src/fluent_lowercase.rs +++ b/src/tools/tidy/src/fluent_lowercase.rs @@ -4,7 +4,7 @@ use std::path::Path; use fluent_syntax::ast::{Entry, Message, PatternElement}; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx}; use crate::walk::{filter_dirs, walk}; #[rustfmt::skip] @@ -53,10 +53,11 @@ fn check_lowercase(filename: &str, contents: &str, check: &mut RunningCheck) { } } -pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("fluent_lowercase").path(path)); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("fluent_lowercase").path(path)); walk( path, + &tidy_ctx.tidy_flags, |path, is_dir| filter_dirs(path) || (!is_dir && filter_fluent(path)), &mut |ent, contents| { check_lowercase(ent.path().to_str().unwrap(), contents, &mut check); diff --git a/src/tools/tidy/src/fluent_period.rs b/src/tools/tidy/src/fluent_period.rs index c7c760b8d54fd..385152f31d8e7 100644 --- a/src/tools/tidy/src/fluent_period.rs +++ b/src/tools/tidy/src/fluent_period.rs @@ -4,7 +4,7 @@ use std::path::Path; use fluent_syntax::ast::{Entry, PatternElement}; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx}; use crate::walk::{filter_dirs, walk}; fn filter_fluent(path: &Path) -> bool { @@ -75,11 +75,12 @@ fn find_line(haystack: &str, needle: &str) -> usize { 1 } -pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("fluent_period").path(path)); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("fluent_period").path(path)); walk( path, + &tidy_ctx.tidy_flags, |path, is_dir| filter_dirs(path) || (!is_dir && filter_fluent(path)), &mut |ent, contents| { check_period(ent.path().to_str().unwrap(), contents, &mut check); diff --git a/src/tools/tidy/src/fluent_used.rs b/src/tools/tidy/src/fluent_used.rs index 2047089631b38..7b46c2828ad75 100644 --- a/src/tools/tidy/src/fluent_used.rs +++ b/src/tools/tidy/src/fluent_used.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::walk::{filter_dirs, walk}; fn filter_used_messages( @@ -28,11 +28,11 @@ fn filter_used_messages( } } -pub fn check(path: &Path, mut all_defined_msgs: HashMap, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("fluent_used").path(path)); +pub fn check(path: &Path, mut all_defined_msgs: HashMap, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("fluent_used").path(path)); let mut msgs_appear_only_once = HashMap::new(); - walk(path, |path, _| filter_dirs(path), &mut |_, contents| { + walk(path, &tidy_ctx.tidy_flags, |path, _| filter_dirs(path), &mut |_, contents| { filter_used_messages(contents, &mut all_defined_msgs, &mut msgs_appear_only_once); }); diff --git a/src/tools/tidy/src/gcc_submodule.rs b/src/tools/tidy/src/gcc_submodule.rs index 3a6e3247de606..95c5248951957 100644 --- a/src/tools/tidy/src/gcc_submodule.rs +++ b/src/tools/tidy/src/gcc_submodule.rs @@ -4,9 +4,9 @@ use std::path::Path; use std::process::Command; -use crate::diagnostics::DiagCtx; +use crate::diagnostics::TidyCtx; -pub fn check(root_path: &Path, compiler_path: &Path, diag_ctx: DiagCtx) { +pub fn check(root_path: &Path, compiler_path: &Path, diag_ctx: TidyCtx) { let mut check = diag_ctx.start_check("gcc_submodule"); let cg_gcc_version_path = compiler_path.join("rustc_codegen_gcc/libgccjit.version"); diff --git a/src/tools/tidy/src/known_bug.rs b/src/tools/tidy/src/known_bug.rs index d3b75e0cf5b8c..a02395042a6e5 100644 --- a/src/tools/tidy/src/known_bug.rs +++ b/src/tools/tidy/src/known_bug.rs @@ -2,29 +2,34 @@ use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::walk::*; -pub fn check(filepath: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("known_bug").path(filepath)); - walk(filepath, |path, _is_dir| filter_not_rust(path), &mut |entry, contents| { - let file: &Path = entry.path(); +pub fn check(filepath: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("known_bug").path(filepath)); + walk( + filepath, + &tidy_ctx.tidy_flags, + |path, _is_dir| filter_not_rust(path), + &mut |entry, contents| { + let file: &Path = entry.path(); - // files in "auxiliary" do not need to crash by themselves - let test_path_segments = - file.iter().map(|s| s.to_string_lossy().into()).collect::>(); - let test_path_segments_str = - test_path_segments.iter().map(|s| s.as_str()).collect::>(); + // files in "auxiliary" do not need to crash by themselves + let test_path_segments = + file.iter().map(|s| s.to_string_lossy().into()).collect::>(); + let test_path_segments_str = + test_path_segments.iter().map(|s| s.as_str()).collect::>(); - if !matches!( - test_path_segments_str[..], - [.., "tests", "crashes", "auxiliary", _aux_file_rs] - ) && !contents.lines().any(|line| line.starts_with("//@ known-bug: ")) - { - check.error(format!( - "{} crash/ice test does not have a \"//@ known-bug: \" directive", - file.display() - )); - } - }); + if !matches!( + test_path_segments_str[..], + [.., "tests", "crashes", "auxiliary", _aux_file_rs] + ) && !contents.lines().any(|line| line.starts_with("//@ known-bug: ")) + { + check.error(format!( + "{} crash/ice test does not have a \"//@ known-bug: \" directive", + file.display() + )); + } + }, + ); } diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 0acbcd64f067c..efeacb0dbd800 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -12,7 +12,7 @@ use build_helper::ci::CiEnv; use build_helper::git::{GitConfig, get_closest_upstream_commit}; use build_helper::stage0_parser::{Stage0Config, parse_stage0_file}; -use crate::diagnostics::{DiagCtx, RunningCheck}; +use crate::diagnostics::{RunningCheck, TidyCtx}; macro_rules! static_regex { ($re:literal) => {{ @@ -52,8 +52,8 @@ pub struct CiInfo { } impl CiInfo { - pub fn new(diag_ctx: DiagCtx) -> Self { - let mut check = diag_ctx.start_check("CI history"); + pub fn new(tidy_ctx: TidyCtx) -> Self { + let mut check = tidy_ctx.start_check("CI history"); let stage0 = parse_stage0_file(); let Stage0Config { nightly_branch, git_merge_commit_email, .. } = stage0.config; @@ -258,6 +258,7 @@ pub mod target_policy; pub mod target_specific_tests; pub mod tests_placement; pub mod tests_revision_unpaired_stdout_stderr; +mod tidy_flags; pub mod triagebot; pub mod ui_tests; pub mod unit_tests; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 93bc16111992a..763f78a78453f 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -11,7 +11,7 @@ use std::str::FromStr; use std::thread::{self, ScopedJoinHandle, scope}; use std::{env, process}; -use tidy::diagnostics::{COLOR_ERROR, COLOR_SUCCESS, DiagCtx, output_message}; +use tidy::diagnostics::{COLOR_ERROR, COLOR_SUCCESS, TidyCtx, TidyFlags, output_message}; use tidy::*; fn main() { @@ -46,12 +46,20 @@ fn main() { None => (&args[..], [].as_slice()), }; let verbose = cfg_args.iter().any(|s| *s == "--verbose"); - let bless = cfg_args.iter().any(|s| *s == "--bless"); + let extra_checks = cfg_args.iter().find(|s| s.starts_with("--extra-checks=")).map(String::as_str); - let diag_ctx = DiagCtx::new(&root_path, verbose); - let ci_info = CiInfo::new(diag_ctx.clone()); + let tidy_flags = TidyFlags::new(&root_path, cfg_args); + + if !tidy_flags.include_untracked { + let num_untracked = tidy_flags.untracked_files.len(); + println!("tidy: skipped {num_untracked} untracked files"); + println!("tidy: to include untracked files use `--include-untracked`"); + } + + let tidy_ctx = TidyCtx::new(&root_path, tidy_flags, verbose); + let ci_info = CiInfo::new(tidy_ctx.clone()); let drain_handles = |handles: &mut VecDeque>| { // poll all threads for completion before awaiting the oldest one @@ -86,9 +94,9 @@ fn main() { (@ $p:ident, name=$name:expr $(, $args:expr)* ) => { drain_handles(&mut handles); - let diag_ctx = diag_ctx.clone(); + let tidy_ctx = tidy_ctx.clone(); let handle = thread::Builder::new().name($name).spawn_scoped(s, || { - $p::check($($args, )* diag_ctx); + $p::check($($args, )* tidy_ctx); }).unwrap(); handles.push_back(handle); } @@ -97,15 +105,15 @@ fn main() { check!(target_specific_tests, &tests_path); // Checks that are done on the cargo workspace. - check!(deps, &root_path, &cargo, bless); + check!(deps, &root_path, &cargo); check!(extdeps, &root_path); // Checks over tests. check!(tests_placement, &root_path); check!(tests_revision_unpaired_stdout_stderr, &tests_path); check!(debug_artifacts, &tests_path); - check!(ui_tests, &root_path, bless); - check!(mir_opt_tests, &tests_path, bless); + check!(ui_tests, &root_path); + check!(mir_opt_tests, &tests_path); check!(rustdoc_gui_tests, &tests_path); check!(rustdoc_css_themes, &librustdoc_path); check!(rustdoc_templates, &librustdoc_path); @@ -115,7 +123,7 @@ fn main() { // Checks that only make sense for the compiler. check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], &ci_info); - check!(fluent_alphabetical, &compiler_path, bless); + check!(fluent_alphabetical, &compiler_path); check!(fluent_period, &compiler_path); check!(fluent_lowercase, &compiler_path); check!(target_policy, &root_path); @@ -156,7 +164,7 @@ fn main() { let collected = { drain_handles(&mut handles); - features::check(&src_path, &tests_path, &compiler_path, &library_path, diag_ctx.clone()) + features::check(&src_path, &tests_path, &compiler_path, &library_path, tidy_ctx.clone()) }; check!(unstable_book, &src_path, collected); @@ -169,13 +177,12 @@ fn main() { &tools_path, &npm, &cargo, - bless, extra_checks, pos_args ); }); - let failed_checks = diag_ctx.into_failed_checks(); + let failed_checks = tidy_ctx.into_failed_checks(); if !failed_checks.is_empty() { let mut failed: Vec = failed_checks.into_iter().map(|c| c.id().to_string()).collect(); diff --git a/src/tools/tidy/src/mir_opt_tests.rs b/src/tools/tidy/src/mir_opt_tests.rs index 0f9fab51d096f..36e78711b4b36 100644 --- a/src/tools/tidy/src/mir_opt_tests.rs +++ b/src/tools/tidy/src/mir_opt_tests.rs @@ -5,15 +5,17 @@ use std::path::{Path, PathBuf}; use miropt_test_tools::PanicStrategy; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx, TidyFlags}; use crate::walk::walk_no_read; -fn check_unused_files(path: &Path, bless: bool, check: &mut RunningCheck) { +fn check_unused_files(path: &Path, tidy_flags: &TidyFlags, check: &mut RunningCheck) { let mut rs_files = Vec::::new(); let mut output_files = HashSet::::new(); + let bless = tidy_flags.bless; walk_no_read( &[&path.join("mir-opt")], + &tidy_flags, |path, _is_dir| path.file_name() == Some("README.md".as_ref()), &mut |file| { let filepath = file.path(); @@ -74,9 +76,10 @@ fn check_dash_files(path: &Path, bless: bool, check: &mut RunningCheck) { } } -pub fn check(path: &Path, bless: bool, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("mir_opt_tests").path(path)); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("mir_opt_tests").path(path)); + let bless = tidy_ctx.tidy_flags.bless; - check_unused_files(path, bless, &mut check); + check_unused_files(path, &tidy_ctx.tidy_flags, &mut check); check_dash_files(path, bless, &mut check); } diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index cefad7d9596a6..012c988a2dd91 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -32,7 +32,7 @@ use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx}; use crate::walk::{filter_dirs, walk}; // Paths that may contain platform-specific code. @@ -68,13 +68,13 @@ const EXCEPTION_PATHS: &[&str] = &[ "library/std/src/io/error.rs", // Repr unpacked needed for UEFI ]; -pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("pal").path(path)); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("pal").path(path)); // Sanity check that the complex parsing here works. let mut saw_target_arch = false; let mut saw_cfg_bang = false; - walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| { + walk(path, &tidy_ctx.tidy_flags, |path, _is_dir| filter_dirs(path), &mut |entry, contents| { let file = entry.path(); let filestr = file.to_string_lossy().replace("\\", "/"); if !filestr.ends_with(".rs") { diff --git a/src/tools/tidy/src/rustdoc_css_themes.rs b/src/tools/tidy/src/rustdoc_css_themes.rs index 8d4af7a3bd564..e3932bd92205a 100644 --- a/src/tools/tidy/src/rustdoc_css_themes.rs +++ b/src/tools/tidy/src/rustdoc_css_themes.rs @@ -3,9 +3,9 @@ use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx}; -pub fn check(librustdoc_path: &Path, diag_ctx: DiagCtx) { +pub fn check(librustdoc_path: &Path, diag_ctx: TidyCtx) { let mut check = diag_ctx.start_check(CheckId::new("rustdoc_css_themes").path(librustdoc_path)); let rustdoc_css = "html/static/css/rustdoc.css"; diff --git a/src/tools/tidy/src/rustdoc_gui_tests.rs b/src/tools/tidy/src/rustdoc_gui_tests.rs index 8ec300c42ce9f..c9979693391a3 100644 --- a/src/tools/tidy/src/rustdoc_gui_tests.rs +++ b/src/tools/tidy/src/rustdoc_gui_tests.rs @@ -2,13 +2,14 @@ use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; -pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("rustdoc_gui_tests").path(path)); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("rustdoc_gui_tests").path(path)); crate::walk::walk( &path.join("rustdoc-gui"), + &tidy_ctx.tidy_flags, |p, is_dir| !is_dir && p.extension().is_none_or(|e| e != "goml"), &mut |entry, content| { for line in content.lines() { diff --git a/src/tools/tidy/src/rustdoc_json.rs b/src/tools/tidy/src/rustdoc_json.rs index ade774616c71b..59d569c8a293c 100644 --- a/src/tools/tidy/src/rustdoc_json.rs +++ b/src/tools/tidy/src/rustdoc_json.rs @@ -4,11 +4,11 @@ use std::path::Path; use std::str::FromStr; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; const RUSTDOC_JSON_TYPES: &str = "src/rustdoc-json-types"; -pub fn check(src_path: &Path, ci_info: &crate::CiInfo, diag_ctx: DiagCtx) { +pub fn check(src_path: &Path, ci_info: &crate::CiInfo, diag_ctx: TidyCtx) { let mut check = diag_ctx.start_check(CheckId::new("rustdoc_json").path(src_path)); let Some(base_commit) = &ci_info.base_commit else { diff --git a/src/tools/tidy/src/rustdoc_templates.rs b/src/tools/tidy/src/rustdoc_templates.rs index 4e5b9988d5391..27f026378ddfe 100644 --- a/src/tools/tidy/src/rustdoc_templates.rs +++ b/src/tools/tidy/src/rustdoc_templates.rs @@ -6,17 +6,18 @@ use std::path::Path; use ignore::DirEntry; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::walk::walk; // Array containing `("beginning of tag", "end of tag")`. const TAGS: &[(&str, &str)] = &[("{#", "#}"), ("{%", "%}"), ("{{", "}}")]; -pub fn check(librustdoc_path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("rustdoc_templates").path(librustdoc_path)); +pub fn check(librustdoc_path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("rustdoc_templates").path(librustdoc_path)); walk( &librustdoc_path.join("html/templates"), + &tidy_ctx.tidy_flags, |path, is_dir| is_dir || path.extension().is_none_or(|ext| ext != OsStr::new("html")), &mut |path: &DirEntry, file_content: &str| { let mut lines = file_content.lines().enumerate().peekable(); diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index d17278edc849a..8d6c2358f5626 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -24,7 +24,7 @@ use std::sync::LazyLock; use regex::RegexSetBuilder; use rustc_hash::FxHashMap; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::walk::{filter_dirs, walk}; #[cfg(test)] @@ -339,8 +339,8 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool { true } -pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("style").path(path)); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("style").path(path)); fn skip(path: &Path, is_dir: bool) -> bool { if path.file_name().is_some_and(|name| name.to_string_lossy().starts_with(".#")) { @@ -379,7 +379,7 @@ pub fn check(path: &Path, diag_ctx: DiagCtx) { // or comments. A simple workaround is to just allowlist this file. let this_file = Path::new(file!()); - walk(path, skip, &mut |entry, contents| { + walk(path, &tidy_ctx.tidy_flags, skip, &mut |entry, contents| { let file = entry.path(); let path_str = file.to_string_lossy(); let filename = file.file_name().unwrap().to_string_lossy(); diff --git a/src/tools/tidy/src/target_policy.rs b/src/tools/tidy/src/target_policy.rs index cfcfcaf2435b3..40d6e24f24a0a 100644 --- a/src/tools/tidy/src/target_policy.rs +++ b/src/tools/tidy/src/target_policy.rs @@ -5,7 +5,7 @@ use std::collections::HashSet; use std::path::Path; -use crate::diagnostics::DiagCtx; +use crate::diagnostics::TidyCtx; use crate::walk::{filter_not_rust, walk}; const TARGET_DEFINITIONS_PATH: &str = "compiler/rustc_target/src/spec/targets/"; @@ -24,8 +24,8 @@ const EXCEPTIONS: &[&str] = &[ "xtensa_esp32s3_espidf", ]; -pub fn check(root_path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check("target_policy"); +pub fn check(root_path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("target_policy"); let mut targets_to_find = HashSet::new(); @@ -46,15 +46,20 @@ pub fn check(root_path: &Path, diag_ctx: DiagCtx) { let _ = targets_to_find.insert(target_name); } - walk(&root_path.join(ASSEMBLY_LLVM_TEST_PATH), |_, _| false, &mut |_, contents| { - for line in contents.lines() { - let Some(_) = line.find(REVISION_LINE_START) else { - continue; - }; - let (_, target_name) = line.split_at(REVISION_LINE_START.len()); - targets_to_find.remove(target_name); - } - }); + walk( + &root_path.join(ASSEMBLY_LLVM_TEST_PATH), + &tidy_ctx.tidy_flags, + |_, _| false, + &mut |_, contents| { + for line in contents.lines() { + let Some(_) = line.find(REVISION_LINE_START) else { + continue; + }; + let (_, target_name) = line.split_at(REVISION_LINE_START.len()); + targets_to_find.remove(target_name); + } + }, + ); for target in targets_to_find { if !EXCEPTIONS.contains(&target.as_str()) { diff --git a/src/tools/tidy/src/target_specific_tests.rs b/src/tools/tidy/src/target_specific_tests.rs index c1db3874ad547..0102ace1041d1 100644 --- a/src/tools/tidy/src/target_specific_tests.rs +++ b/src/tools/tidy/src/target_specific_tests.rs @@ -4,7 +4,7 @@ use std::collections::BTreeMap; use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::iter_header::{HeaderLine, iter_header}; use crate::walk::filter_not_rust; @@ -17,75 +17,81 @@ struct RevisionInfo<'a> { llvm_components: Option>, } -pub fn check(tests_path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("target-specific-tests").path(tests_path)); +pub fn check(tests_path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("target-specific-tests").path(tests_path)); - crate::walk::walk(tests_path, |path, _is_dir| filter_not_rust(path), &mut |entry, content| { - if content.contains("// ignore-tidy-target-specific-tests") { - return; - } + crate::walk::walk( + tests_path, + &tidy_ctx.tidy_flags, + |path, _is_dir| filter_not_rust(path), + &mut |entry, content| { + if content.contains("// ignore-tidy-target-specific-tests") { + return; + } - let file = entry.path().display(); - let mut header_map = BTreeMap::new(); - iter_header(content, &mut |HeaderLine { revision, directive, .. }| { - if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) { - let info = header_map.entry(revision).or_insert(RevisionInfo::default()); - let comp_vec = info.llvm_components.get_or_insert(Vec::new()); - for component in value.split(' ') { - let component = component.trim(); - if !component.is_empty() { - comp_vec.push(component); + let file = entry.path().display(); + let mut header_map = BTreeMap::new(); + iter_header(content, &mut |HeaderLine { revision, directive, .. }| { + if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) { + let info = header_map.entry(revision).or_insert(RevisionInfo::default()); + let comp_vec = info.llvm_components.get_or_insert(Vec::new()); + for component in value.split(' ') { + let component = component.trim(); + if !component.is_empty() { + comp_vec.push(component); + } + } + } else if let Some(compile_flags) = directive.strip_prefix(COMPILE_FLAGS_HEADER) + && let Some((_, v)) = compile_flags.split_once("--target") + { + let v = v.trim_start_matches([' ', '=']); + let info = header_map.entry(revision).or_insert(RevisionInfo::default()); + if v.starts_with("{{") { + info.target_arch.replace(None); + } else if let Some((arch, _)) = v.split_once("-") { + info.target_arch.replace(Some(arch)); + } else { + check.error(format!("{file}: seems to have a malformed --target value")); } } - } else if let Some(compile_flags) = directive.strip_prefix(COMPILE_FLAGS_HEADER) - && let Some((_, v)) = compile_flags.split_once("--target") + }); + + // Skip run-make tests as revisions are not supported. + if entry.path().strip_prefix(tests_path).is_ok_and(|rest| rest.starts_with("run-make")) { - let v = v.trim_start_matches([' ', '=']); - let info = header_map.entry(revision).or_insert(RevisionInfo::default()); - if v.starts_with("{{") { - info.target_arch.replace(None); - } else if let Some((arch, _)) = v.split_once("-") { - info.target_arch.replace(Some(arch)); - } else { - check.error(format!("{file}: seems to have a malformed --target value")); - } + return; } - }); - // Skip run-make tests as revisions are not supported. - if entry.path().strip_prefix(tests_path).is_ok_and(|rest| rest.starts_with("run-make")) { - return; - } - - for (rev, RevisionInfo { target_arch, llvm_components }) in &header_map { - let rev = rev.unwrap_or("[unspecified]"); - match (target_arch, llvm_components) { - (None, None) => {} - (Some(target_arch), None) => { - let llvm_component = - target_arch.map_or_else(|| "".to_string(), arch_to_llvm_component); - check.error(format!( + for (rev, RevisionInfo { target_arch, llvm_components }) in &header_map { + let rev = rev.unwrap_or("[unspecified]"); + match (target_arch, llvm_components) { + (None, None) => {} + (Some(target_arch), None) => { + let llvm_component = target_arch + .map_or_else(|| "".to_string(), arch_to_llvm_component); + check.error(format!( "{file}: revision {rev} should specify `{LLVM_COMPONENTS_HEADER} {llvm_component}` as it has `--target` set" )); - } - (None, Some(_)) => { - check.error(format!( + } + (None, Some(_)) => { + check.error(format!( "{file}: revision {rev} should not specify `{LLVM_COMPONENTS_HEADER}` as it doesn't need `--target`" )); - } - (Some(target_arch), Some(llvm_components)) => { - if let Some(target_arch) = target_arch { - let llvm_component = arch_to_llvm_component(target_arch); - if !llvm_components.contains(&llvm_component.as_str()) { - check.error(format!( + } + (Some(target_arch), Some(llvm_components)) => { + if let Some(target_arch) = target_arch { + let llvm_component = arch_to_llvm_component(target_arch); + if !llvm_components.contains(&llvm_component.as_str()) { + check.error(format!( "{file}: revision {rev} should specify `{LLVM_COMPONENTS_HEADER} {llvm_component}` as it has `--target` set" )); + } } } } } - } - }); + }, + ); } fn arch_to_llvm_component(arch: &str) -> String { diff --git a/src/tools/tidy/src/tests_placement.rs b/src/tools/tidy/src/tests_placement.rs index 8ba8cf552bd7f..18b00608fdce5 100644 --- a/src/tools/tidy/src/tests_placement.rs +++ b/src/tools/tidy/src/tests_placement.rs @@ -1,12 +1,12 @@ use std::path::Path; -use crate::diagnostics::DiagCtx; +use crate::diagnostics::TidyCtx; const FORBIDDEN_PATH: &str = "src/test"; const ALLOWED_PATH: &str = "tests"; -pub fn check(root_path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check("tests_placement"); +pub fn check(root_path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("tests_placement"); if root_path.join(FORBIDDEN_PATH).exists() { check.error(format!( diff --git a/src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs b/src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs index 1738088a3a0c0..23b7eb5ef2020 100644 --- a/src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs +++ b/src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::ffi::OsStr; use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::iter_header::*; use crate::walk::*; @@ -22,12 +22,12 @@ const IGNORES: &[&str] = &[ const EXTENSIONS: &[&str] = &["stdout", "stderr"]; const SPECIAL_TEST: &str = "tests/ui/command/need-crate-arg-ignore-tidy.x.rs"; -pub fn check(tests_path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx +pub fn check(tests_path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx .start_check(CheckId::new("tests_revision_unpaired_stdout_stderr").path(tests_path)); // Recurse over subdirectories under `tests/` - walk_dir(tests_path.as_ref(), filter, &mut |entry| { + walk_dir(tests_path.as_ref(), &tidy_ctx.tidy_flags, filter, &mut |entry| { // We are inspecting a folder. Collect the paths to interesting files `.rs`, `.stderr`, // `.stdout` under the current folder (shallow). let mut files_under_inspection = BTreeSet::new(); diff --git a/src/tools/tidy/src/tidy_flags/mod.rs b/src/tools/tidy/src/tidy_flags/mod.rs new file mode 100644 index 0000000000000..87c2771955a9d --- /dev/null +++ b/src/tools/tidy/src/tidy_flags/mod.rs @@ -0,0 +1,2 @@ +#[cfg(test)] +mod tests; diff --git a/src/tools/tidy/src/tidy_flags/tests.rs b/src/tools/tidy/src/tidy_flags/tests.rs new file mode 100644 index 0000000000000..f23f99ce63e91 --- /dev/null +++ b/src/tools/tidy/src/tidy_flags/tests.rs @@ -0,0 +1,105 @@ +//use std::collections::HashSet; +use std::path::PathBuf; + +use build_helper::git::output_result; + +//use tempfile::{NamedTempFile, tempdir_in}; +use crate::diagnostics::TidyFlags; +use crate::walk::walk; + +fn get_test_dir() -> PathBuf { + let root_path = PathBuf::from( + t!(output_result(std::process::Command::new("git").args(["rev-parse", "--show-toplevel"]))) + .trim(), + ); + let test_dir = root_path.join("src/tools/tidy/src/tidy_flags/"); + test_dir +} + +//fn get_untracked(root_path: &PathBuf) -> HashSet { +//let untracked_files = match get_git_untracked_files(Some(&root_path)) { +//Ok(Some(untracked_paths)) => { +//untracked_paths.into_iter().map(|s| root_path.join(s)).collect() +//} +//_ => HashSet::new(), +//}; +//untracked_files +//} + +//#[test] +////Creates a temp untracked file and checks that there are more untracked files than +////previously. +//fn test_get_untracked() { +//let temp_dir = tempdir_in(get_test_dir()).unwrap(); +//let temp_dir_path = temp_dir.path().to_path_buf(); + +//let num_untracked = get_untracked(&temp_dir_path).len(); + +//assert!(num_untracked == 0); + +//let _new_file = tempfile::NamedTempFile::new_in(&temp_dir_path).unwrap(); + +//let new_num_untracked = get_untracked(&temp_dir_path).len(); + +//assert!(new_num_untracked == 1); +//} + +//#[test] +////Various checks for the walk function and interactions with `TidyFlags`. +//fn test_tidy_walk() { +//let temp_dir = tempdir_in(get_test_dir()).unwrap(); +//let temp_dir_path = temp_dir.path().to_path_buf(); + +//let _file_guards: Vec = (0..5) +//.map(|_| tempfile::Builder::new().prefix("temp_file").tempfile_in(&temp_dir_path).unwrap()) +//.collect(); + +////Checks that untracked files are included. +//let mut tidy_flags = TidyFlags::new(&temp_dir_path, &[]); +//tidy_flags.include_untracked = true; + +//let mut file_count = 0; +//walk(&temp_dir_path, &tidy_flags, |_, _| false, &mut |_, _| { +//file_count += 1; +//}); + +//assert!(file_count == 5); + +////Checks that untracked files are excluded. +//tidy_flags.include_untracked = false; + +//let mut file_count = 0; +//walk(&temp_dir_path, &tidy_flags, |_, _| false, &mut |_, _| { +//file_count += 1; +//}); + +//assert!(file_count == 0); + +////Default behavior include ALL files, including untracked. This is the previous behavior, so any +////downstream functions using `tidy::walk::walk` and now passing in `TidyFlags::default()` should +////be the same as before. +//let tidy_flags = TidyFlags::default(); + +//let mut file_count = 0; +//walk(&temp_dir_path, &tidy_flags, |_, _| false, &mut |_, _| { +//file_count += 1; +//}); + +//assert!(file_count == 5); +//} + +#[test] +//Checks that the tidy walk function will include files in `tidy/tidy_flags/`. +fn test_tidy_walk_real_files() { + let test_dir = get_test_dir(); + let tidy_flags = TidyFlags::new(&test_dir, &[]); + + //These files should be tracked and included in `walk`. + let mut file_count = 0; + walk(&test_dir, &tidy_flags, |_, _| false, &mut |_, _| { + file_count += 1; + }); + + //This number could change, but if it does you're already here. + assert!(file_count == 2); +} diff --git a/src/tools/tidy/src/triagebot.rs b/src/tools/tidy/src/triagebot.rs index 41d61dcd14118..01401c94d7309 100644 --- a/src/tools/tidy/src/triagebot.rs +++ b/src/tools/tidy/src/triagebot.rs @@ -4,10 +4,10 @@ use std::path::Path; use toml::Value; -use crate::diagnostics::DiagCtx; +use crate::diagnostics::TidyCtx; -pub fn check(path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check("triagebot"); +pub fn check(path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("triagebot"); let triagebot_path = path.join("triagebot.toml"); // This check is mostly to catch broken path filters *within* `triagebot.toml`, and not enforce diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index c74ecf3d43f46..8f2835e34788c 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -7,16 +7,17 @@ use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx, TidyFlags}; const ISSUES_TXT_HEADER: &str = r#"============================================================ ⚠️⚠️⚠️NOTHING SHOULD EVER BE ADDED TO THIS LIST⚠️⚠️⚠️ ============================================================ "#; -pub fn check(root_path: &Path, bless: bool, diag_ctx: DiagCtx) { +pub fn check(root_path: &Path, tidy_ctx: TidyCtx) { let path = &root_path.join("tests"); - let mut check = diag_ctx.start_check(CheckId::new("ui_tests").path(path)); + let mut check = tidy_ctx.start_check(CheckId::new("ui_tests").path(path)); + let bless = tidy_ctx.tidy_flags.bless; // the list of files in ui tests that are allowed to start with `issue-XXXX` // BTreeSet because we would like a stable ordering so --bless works @@ -44,7 +45,8 @@ pub fn check(root_path: &Path, bless: bool, diag_ctx: DiagCtx) { deny_new_top_level_ui_tests(&mut check, &path.join("ui")); - let remaining_issue_names = recursively_check_ui_tests(&mut check, path, &allowed_issue_names); + let remaining_issue_names = + recursively_check_ui_tests(&mut check, path, &allowed_issue_names, &tidy_ctx.tidy_flags); // if there are any file names remaining, they were moved on the fs. // our data must remain up to date, so it must be removed from issues.txt @@ -104,12 +106,13 @@ fn recursively_check_ui_tests<'issues>( check: &mut RunningCheck, path: &Path, allowed_issue_names: &'issues BTreeSet<&'issues str>, + tidy_flags: &TidyFlags, ) -> BTreeSet<&'issues str> { let mut remaining_issue_names: BTreeSet<&str> = allowed_issue_names.clone(); let (ui, ui_fulldeps) = (path.join("ui"), path.join("ui-fulldeps")); let paths = [ui.as_path(), ui_fulldeps.as_path()]; - crate::walk::walk_no_read(&paths, |_, _| false, &mut |entry| { + crate::walk::walk_no_read(&paths, tidy_flags, |_, _| false, &mut |entry| { let file_path = entry.path(); if let Some(ext) = file_path.extension().and_then(OsStr::to_str) { check_unexpected_extension(check, file_path, ext); diff --git a/src/tools/tidy/src/unit_tests.rs b/src/tools/tidy/src/unit_tests.rs index cab445ac63a1b..fa14211655367 100644 --- a/src/tools/tidy/src/unit_tests.rs +++ b/src/tools/tidy/src/unit_tests.rs @@ -11,11 +11,11 @@ use std::path::Path; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; use crate::walk::{filter_dirs, walk}; -pub fn check(root_path: &Path, stdlib: bool, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("unit_tests").path(root_path)); +pub fn check(root_path: &Path, stdlib: bool, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("unit_tests").path(root_path)); let skip = move |path: &Path, is_dir| { let file_name = path.file_name().unwrap_or_default(); @@ -68,7 +68,7 @@ pub fn check(root_path: &Path, stdlib: bool, diag_ctx: DiagCtx) { } }; - walk(root_path, skip, &mut |entry, contents| { + walk(root_path, &tidy_ctx.tidy_flags, skip, &mut |entry, contents| { let path = entry.path(); let package = path .strip_prefix(root_path) diff --git a/src/tools/tidy/src/unknown_revision.rs b/src/tools/tidy/src/unknown_revision.rs index 776d45e25de0d..ba60faf8947d2 100644 --- a/src/tools/tidy/src/unknown_revision.rs +++ b/src/tools/tidy/src/unknown_revision.rs @@ -12,14 +12,15 @@ use std::sync::OnceLock; use ignore::DirEntry; use regex::Regex; -use crate::diagnostics::{CheckId, DiagCtx, RunningCheck}; +use crate::diagnostics::{CheckId, RunningCheck, TidyCtx}; use crate::iter_header::{HeaderLine, iter_header}; use crate::walk::{filter_dirs, filter_not_rust, walk}; -pub fn check(tests_path: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("unknown_revision").path(tests_path)); +pub fn check(tests_path: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("unknown_revision").path(tests_path)); walk( tests_path, + &tidy_ctx.tidy_flags, |path, is_dir| { filter_dirs(path) || filter_not_rust(path) || { // Auxiliary source files for incremental tests can refer to revisions diff --git a/src/tools/tidy/src/unstable_book.rs b/src/tools/tidy/src/unstable_book.rs index bab294abee02d..cfaf56ae7b641 100644 --- a/src/tools/tidy/src/unstable_book.rs +++ b/src/tools/tidy/src/unstable_book.rs @@ -2,7 +2,7 @@ use std::collections::BTreeSet; use std::fs; use std::path::{Path, PathBuf}; -use crate::diagnostics::{DiagCtx, RunningCheck}; +use crate::diagnostics::{RunningCheck, TidyCtx}; use crate::features::{CollectedFeatures, Features, Status}; pub const PATH_STR: &str = "doc/unstable-book"; @@ -85,8 +85,8 @@ fn maybe_suggest_dashes(names: &BTreeSet, feature_name: &str, check: &mu } } -pub fn check(path: &Path, features: CollectedFeatures, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check("unstable_book"); +pub fn check(path: &Path, features: CollectedFeatures, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("unstable_book"); let lang_features = features.lang; let lib_features = features diff --git a/src/tools/tidy/src/walk.rs b/src/tools/tidy/src/walk.rs index 7825ebeb5baa6..5137c7c890fac 100644 --- a/src/tools/tidy/src/walk.rs +++ b/src/tools/tidy/src/walk.rs @@ -1,3 +1,4 @@ +use std::collections::{HashMap, HashSet}; use std::ffi::OsStr; use std::fs::File; use std::io::Read; @@ -5,6 +6,8 @@ use std::path::Path; use ignore::DirEntry; +use crate::diagnostics::TidyFlags; + /// The default directory filter. pub fn filter_dirs(path: &Path) -> bool { // bootstrap/etc @@ -46,19 +49,34 @@ pub fn filter_not_rust(path: &Path) -> bool { pub fn walk( path: &Path, + tidy_flags: &TidyFlags, skip: impl Send + Sync + 'static + Fn(&Path, bool) -> bool, f: &mut dyn FnMut(&DirEntry, &str), ) { - walk_many(&[path], skip, f); + walk_many(&[path], tidy_flags, skip, f); } pub fn walk_many( paths: &[&Path], + tidy_flags: &TidyFlags, skip: impl Send + Sync + 'static + Fn(&Path, bool) -> bool, f: &mut dyn FnMut(&DirEntry, &str), ) { let mut contents = Vec::new(); - walk_no_read(paths, skip, &mut |entry| { + + let pre_push = tidy_flags.pre_push; + let pre_push_content = match pre_push { + true => &tidy_flags.pre_push_content, + false => &HashMap::new(), + }; + + walk_no_read(paths, tidy_flags, skip, &mut |entry| { + if tidy_flags.pre_push && tidy_flags.pre_push_content.keys().any(|k| k == entry.path()) { + if let Some(content) = pre_push_content.get(entry.path().into()) { + f(entry, content); + } + return; + } contents.clear(); let mut file = t!(File::open(entry.path()), entry.path()); t!(file.read_to_end(&mut contents), entry.path()); @@ -72,15 +90,22 @@ pub fn walk_many( pub(crate) fn walk_no_read( paths: &[&Path], + tidy_flags: &TidyFlags, skip: impl Send + Sync + 'static + Fn(&Path, bool) -> bool, f: &mut dyn FnMut(&DirEntry), ) { + let untracked_files = match tidy_flags.include_untracked { + true => HashSet::new(), + false => tidy_flags.untracked_files.clone(), + }; + let mut walker = ignore::WalkBuilder::new(paths[0]); for path in &paths[1..] { walker.add(path); } let walker = walker.filter_entry(move |e| { !skip(e.path(), e.file_type().map(|ft| ft.is_dir()).unwrap_or(false)) + && !untracked_files.contains(e.path()) }); for entry in walker.build().flatten() { if entry.file_type().is_none_or(|kind| kind.is_dir() || kind.is_symlink()) { @@ -93,11 +118,18 @@ pub(crate) fn walk_no_read( // Walk through directories and skip symlinks. pub(crate) fn walk_dir( path: &Path, + tidy_flags: &TidyFlags, skip: impl Send + Sync + 'static + Fn(&Path) -> bool, f: &mut dyn FnMut(&DirEntry), ) { + let untracked_files = match tidy_flags.include_untracked { + true => HashSet::new(), + false => tidy_flags.untracked_files.clone(), + }; + let mut walker = ignore::WalkBuilder::new(path); - let walker = walker.filter_entry(move |e| !skip(e.path())); + let walker = + walker.filter_entry(move |e| !skip(e.path()) && !untracked_files.contains(e.path())); for entry in walker.build().flatten() { if entry.path().is_dir() { f(&entry); diff --git a/src/tools/tidy/src/x_version.rs b/src/tools/tidy/src/x_version.rs index b3e322d9403f4..28fd725cd3e23 100644 --- a/src/tools/tidy/src/x_version.rs +++ b/src/tools/tidy/src/x_version.rs @@ -3,10 +3,10 @@ use std::process::{Command, Stdio}; use semver::Version; -use crate::diagnostics::{CheckId, DiagCtx}; +use crate::diagnostics::{CheckId, TidyCtx}; -pub fn check(root: &Path, cargo: &Path, diag_ctx: DiagCtx) { - let mut check = diag_ctx.start_check(CheckId::new("x_version").path(root)); +pub fn check(root: &Path, cargo: &Path, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check(CheckId::new("x_version").path(root)); let cargo_list = Command::new(cargo).args(["install", "--list"]).stdout(Stdio::piped()).spawn(); let child = match cargo_list { diff --git a/src/tools/unstable-book-gen/src/main.rs b/src/tools/unstable-book-gen/src/main.rs index 16550f83003dc..bed7b3726e017 100644 --- a/src/tools/unstable-book-gen/src/main.rs +++ b/src/tools/unstable-book-gen/src/main.rs @@ -5,7 +5,7 @@ use std::env; use std::fs::{self, write}; use std::path::Path; -use tidy::diagnostics::RunningCheck; +use tidy::diagnostics::{RunningCheck, TidyFlags}; use tidy::features::{Features, collect_env_vars, collect_lang_features, collect_lib_features}; use tidy::t; use tidy::unstable_book::{ @@ -124,11 +124,11 @@ fn main() { let dest_path = Path::new(&dest_path_str); let lang_features = collect_lang_features(compiler_path, &mut RunningCheck::new_noop()); - let lib_features = collect_lib_features(library_path) + let lib_features = collect_lib_features(library_path, &TidyFlags::default()) .into_iter() .filter(|&(ref name, _)| !lang_features.contains_key(name)) .collect(); - let env_vars = collect_env_vars(compiler_path); + let env_vars = collect_env_vars(compiler_path, &TidyFlags::default()); let doc_src_path = src_path.join(PATH_STR);