diff options
Diffstat (limited to 'src/tools/tidy')
| -rw-r--r-- | src/tools/tidy/src/alphabetical.rs | 94 | ||||
| -rw-r--r-- | src/tools/tidy/src/alphabetical/tests.rs | 188 | ||||
| -rw-r--r-- | src/tools/tidy/src/deps.rs | 268 | ||||
| -rw-r--r-- | src/tools/tidy/src/extdeps.rs | 36 | ||||
| -rw-r--r-- | src/tools/tidy/src/features.rs | 16 | ||||
| -rw-r--r-- | src/tools/tidy/src/lib.rs | 16 | ||||
| -rw-r--r-- | src/tools/tidy/src/mir_opt_tests.rs | 3 | ||||
| -rw-r--r-- | src/tools/tidy/src/style.rs | 7 | ||||
| -rw-r--r-- | src/tools/tidy/src/ui_tests.rs | 2 |
9 files changed, 498 insertions, 132 deletions
diff --git a/src/tools/tidy/src/alphabetical.rs b/src/tools/tidy/src/alphabetical.rs index fdc411c89..150a95943 100644 --- a/src/tools/tidy/src/alphabetical.rs +++ b/src/tools/tidy/src/alphabetical.rs @@ -1,6 +1,6 @@ //! Checks that a list of items is in alphabetical order //! -//! To use, use the following annotation in the code: +//! Use the following marker in the code: //! ```rust //! // tidy-alphabetical-start //! fn aaa() {} @@ -10,17 +10,23 @@ //! ``` //! //! The following lines are ignored: +//! - Empty lines //! - Lines that are indented with more or less spaces than the first line -//! - Lines starting with `//`, `#[`, `)`, `]`, `}` if the comment has the same indentation as -//! the first line +//! - Lines starting with `//`, `#` (except those starting with `#!`), `)`, `]`, `}` if the comment +//! has the same indentation as the first line +//! - Lines starting with a closing delimiter (`)`, `[`, `}`) are ignored. //! -//! If a line ends with an opening bracket, the line is ignored and the next line will have -//! its extra indentation ignored. +//! If a line ends with an opening delimiter, we effectively join the following line to it before +//! checking it. E.g. `foo(\nbar)` is treated like `foo(bar)`. -use std::{fmt::Display, path::Path}; +use std::fmt::Display; +use std::path::Path; use crate::walk::{filter_dirs, walk}; +#[cfg(test)] +mod tests; + fn indentation(line: &str) -> usize { line.find(|c| c != ' ').unwrap_or(0) } @@ -29,28 +35,36 @@ fn is_close_bracket(c: char) -> bool { matches!(c, ')' | ']' | '}') } -// Don't let tidy check this here :D -const START_COMMENT: &str = concat!("// tidy-alphabetical", "-start"); -const END_COMMENT: &str = "// tidy-alphabetical-end"; +const START_MARKER: &str = "tidy-alphabetical-start"; +const END_MARKER: &str = "tidy-alphabetical-end"; fn check_section<'a>( file: impl Display, lines: impl Iterator<Item = (usize, &'a str)>, + err: &mut dyn FnMut(&str) -> std::io::Result<()>, bad: &mut bool, ) { - let content_lines = lines.take_while(|(_, line)| !line.contains(END_COMMENT)); - let mut prev_line = String::new(); let mut first_indent = None; let mut in_split_line = None; - for (line_idx, line) in content_lines { - if line.contains(START_COMMENT) { - tidy_error!( + for (idx, line) in lines { + if line.is_empty() { + continue; + } + + if line.contains(START_MARKER) { + tidy_error_ext!( + err, bad, - "{file}:{} found `{START_COMMENT}` expecting `{END_COMMENT}`", - line_idx - ) + "{file}:{} found `{START_MARKER}` expecting `{END_MARKER}`", + idx + 1 + ); + return; + } + + if line.contains(END_MARKER) { + return; } let indent = first_indent.unwrap_or_else(|| { @@ -60,6 +74,7 @@ fn check_section<'a>( }); let line = if let Some(prev_split_line) = in_split_line { + // Join the split lines. in_split_line = None; format!("{prev_split_line}{}", line.trim_start()) } else { @@ -73,7 +88,7 @@ fn check_section<'a>( let trimmed_line = line.trim_start_matches(' '); if trimmed_line.starts_with("//") - || trimmed_line.starts_with("#[") + || (trimmed_line.starts_with("#") && !trimmed_line.starts_with("#!")) || trimmed_line.starts_with(is_close_bracket) { continue; @@ -87,25 +102,44 @@ fn check_section<'a>( let prev_line_trimmed_lowercase = prev_line.trim_start_matches(' ').to_lowercase(); if trimmed_line.to_lowercase() < prev_line_trimmed_lowercase { - tidy_error!(bad, "{file}:{}: line not in alphabetical order", line_idx + 1,); + tidy_error_ext!(err, bad, "{file}:{}: line not in alphabetical order", idx + 1); } prev_line = line; } + + tidy_error_ext!(err, bad, "{file}: reached end of file expecting `{END_MARKER}`") } -pub fn check(path: &Path, bad: &mut bool) { - walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| { - let file = &entry.path().display(); +fn check_lines<'a>( + file: &impl Display, + mut lines: impl Iterator<Item = (usize, &'a str)>, + err: &mut dyn FnMut(&str) -> std::io::Result<()>, + bad: &mut bool, +) { + while let Some((idx, line)) = lines.next() { + if line.contains(END_MARKER) { + tidy_error_ext!( + err, + bad, + "{file}:{} found `{END_MARKER}` expecting `{START_MARKER}`", + idx + 1 + ) + } - let mut lines = contents.lines().enumerate().peekable(); - while let Some((_, line)) = lines.next() { - if line.contains(START_COMMENT) { - check_section(file, &mut lines, bad); - if lines.peek().is_none() { - tidy_error!(bad, "{file}: reached end of file expecting `{END_COMMENT}`") - } - } + if line.contains(START_MARKER) { + check_section(file, &mut lines, err, bad); } + } +} + +pub fn check(path: &Path, bad: &mut bool) { + let skip = + |path: &_, _is_dir| filter_dirs(path) || path.ends_with("tidy/src/alphabetical/tests.rs"); + + walk(path, skip, &mut |entry, contents| { + let file = &entry.path().display(); + let lines = contents.lines().enumerate(); + check_lines(file, lines, &mut crate::tidy_error, bad) }); } diff --git a/src/tools/tidy/src/alphabetical/tests.rs b/src/tools/tidy/src/alphabetical/tests.rs new file mode 100644 index 000000000..560e0284b --- /dev/null +++ b/src/tools/tidy/src/alphabetical/tests.rs @@ -0,0 +1,188 @@ +use super::*; +use std::io::Write; +use std::str::from_utf8; + +fn test(lines: &str, name: &str, expected_msg: &str, expected_bad: bool) { + let mut actual_msg = Vec::new(); + let mut actual_bad = false; + let mut err = |args: &_| { + write!(&mut actual_msg, "{args}")?; + Ok(()) + }; + check_lines(&name, lines.lines().enumerate(), &mut err, &mut actual_bad); + assert_eq!(expected_msg, from_utf8(&actual_msg).unwrap()); + assert_eq!(expected_bad, actual_bad); +} + +fn good(lines: &str) { + test(lines, "good", "", false); +} + +fn bad(lines: &str, expected_msg: &str) { + test(lines, "bad", expected_msg, true); +} + +#[test] +fn test_no_markers() { + let lines = "\ + def + abc + xyz + "; + good(lines); +} + +#[test] +fn test_rust_good() { + let lines = "\ + // tidy-alphabetical-start + abc + def + xyz + // tidy-alphabetical-end"; // important: end marker on last line + good(lines); +} + +#[test] +fn test_complex_good() { + let lines = "\ + zzz + + // tidy-alphabetical-start + abc + // Rust comments are ok + def + # TOML comments are ok + xyz + // tidy-alphabetical-end + + # tidy-alphabetical-start + foo(abc); + // blank lines are ok + + // split line gets joined + foo( + def + ); + + foo(xyz); + # tidy-alphabetical-end + + % tidy-alphabetical-start + abc + ignored_due_to_different_indent + def + % tidy-alphabetical-end + + aaa + "; + good(lines); +} + +#[test] +fn test_rust_bad() { + let lines = "\ + // tidy-alphabetical-start + abc + xyz + def + // tidy-alphabetical-end + "; + bad(lines, "bad:4: line not in alphabetical order"); +} + +#[test] +fn test_toml_bad() { + let lines = "\ + # tidy-alphabetical-start + abc + xyz + def + # tidy-alphabetical-end + "; + bad(lines, "bad:4: line not in alphabetical order"); +} + +#[test] +fn test_features_bad() { + // Even though lines starting with `#` are treated as comments, lines + // starting with `#!` are an exception. + let lines = "\ + tidy-alphabetical-start + #![feature(abc)] + #![feature(xyz)] + #![feature(def)] + tidy-alphabetical-end + "; + bad(lines, "bad:4: line not in alphabetical order"); +} + +#[test] +fn test_indent_bad() { + // All lines are indented the same amount, and so are checked. + let lines = "\ + $ tidy-alphabetical-start + abc + xyz + def + $ tidy-alphabetical-end + "; + bad(lines, "bad:4: line not in alphabetical order"); +} + +#[test] +fn test_split_bad() { + let lines = "\ + || tidy-alphabetical-start + foo(abc) + foo( + xyz + ) + foo( + def + ) + && tidy-alphabetical-end + "; + bad(lines, "bad:7: line not in alphabetical order"); +} + +#[test] +fn test_double_start() { + let lines = "\ + tidy-alphabetical-start + abc + tidy-alphabetical-start + "; + bad(lines, "bad:3 found `tidy-alphabetical-start` expecting `tidy-alphabetical-end`"); +} + +#[test] +fn test_missing_start() { + let lines = "\ + abc + tidy-alphabetical-end + abc + "; + bad(lines, "bad:2 found `tidy-alphabetical-end` expecting `tidy-alphabetical-start`"); +} + +#[test] +fn test_missing_end() { + let lines = "\ + tidy-alphabetical-start + abc + "; + bad(lines, "bad: reached end of file expecting `tidy-alphabetical-end`"); +} + +#[test] +fn test_double_end() { + let lines = "\ + tidy-alphabetical-start + abc + tidy-alphabetical-end + def + tidy-alphabetical-end + "; + bad(lines, "bad:5 found `tidy-alphabetical-end` expecting `tidy-alphabetical-start`"); +} diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 7d3ef4197..f88f91655 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -31,12 +31,49 @@ const LICENSES: &[&str] = &[ // tidy-alphabetical-end ]; +type ExceptionList = &'static [(&'static str, &'static str)]; + +/// The workspaces to check for licensing and optionally permitted dependencies. +/// +/// Each entry consists of a tuple with the following elements: +/// +/// * The path to the workspace root Cargo.toml file. +/// * The list of license exceptions. +/// * Optionally a tuple of: +/// * A list of crates for which dependencies need to be explicitly allowed. +/// * The list of allowed dependencies. +// FIXME auto detect all cargo workspaces +pub(crate) const WORKSPACES: &[(&str, ExceptionList, Option<(&[&str], &[&str])>)] = &[ + // The root workspace has to be first for check_rustfix to work. + (".", EXCEPTIONS, Some((&["rustc-main"], PERMITTED_RUSTC_DEPENDENCIES))), + // Outside of the alphabetical section because rustfmt formats it using multiple lines. + ( + "compiler/rustc_codegen_cranelift", + EXCEPTIONS_CRANELIFT, + Some((&["rustc_codegen_cranelift"], PERMITTED_CRANELIFT_DEPENDENCIES)), + ), + // tidy-alphabetical-start + //("compiler/rustc_codegen_gcc", EXCEPTIONS_GCC, None), // FIXME uncomment once all deps are vendored + //("library/backtrace", &[], None), // FIXME uncomment once rust-lang/backtrace#562 has been synced back to the rust repo + //("library/portable-simd", &[], None), // FIXME uncomment once rust-lang/portable-simd#363 has been synced back to the rust repo + //("library/stdarch", EXCEPTIONS_STDARCH, None), // FIXME uncomment once rust-lang/stdarch#1462 has been synced back to the rust repo + ("src/bootstrap", EXCEPTIONS_BOOTSTRAP, None), + ("src/ci/docker/host-x86_64/test-various/uefi_qemu_test", EXCEPTIONS_UEFI_QEMU_TEST, None), + //("src/etc/test-float-parse", &[], None), // FIXME uncomment once all deps are vendored + ("src/tools/cargo", EXCEPTIONS_CARGO, None), + //("src/tools/miri/test-cargo-miri", &[], None), // FIXME uncomment once all deps are vendored + //("src/tools/miri/test_dependencies", &[], None), // FIXME uncomment once all deps are vendored + ("src/tools/rust-analyzer", EXCEPTIONS_RUST_ANALYZER, None), + ("src/tools/x", &[], None), + // tidy-alphabetical-end +]; + /// These are exceptions to Rust's permissive licensing policy, and /// should be considered bugs. Exceptions are only allowed in Rust /// tooling. It is _crucial_ that no exception crates be dependencies /// of the Rust runtime (std/test). #[rustfmt::skip] -const EXCEPTIONS: &[(&str, &str)] = &[ +const EXCEPTIONS: ExceptionList = &[ // tidy-alphabetical-start ("ar_archive_writer", "Apache-2.0 WITH LLVM-exception"), // rustc ("colored", "MPL-2.0"), // rustfmt @@ -47,18 +84,29 @@ const EXCEPTIONS: &[(&str, &str)] = &[ ("instant", "BSD-3-Clause"), // rustc_driver/tracing-subscriber/parking_lot ("mdbook", "MPL-2.0"), // mdbook ("openssl", "Apache-2.0"), // opt-dist + ("option-ext", "MPL-2.0"), // cargo-miri (via `directories`) ("rustc_apfloat", "Apache-2.0 WITH LLVM-exception"), // rustc (license is the same as LLVM uses) - ("ryu", "Apache-2.0 OR BSL-1.0"), // cargo/... (because of serde) + ("ryu", "Apache-2.0 OR BSL-1.0"), // BSL is not acceptble, but we use it under Apache-2.0 // cargo/... (because of serde) ("self_cell", "Apache-2.0"), // rustc (fluent translations) ("snap", "BSD-3-Clause"), // rustc // tidy-alphabetical-end ]; -const EXCEPTIONS_CARGO: &[(&str, &str)] = &[ +// FIXME uncomment once rust-lang/stdarch#1462 lands +/* +const EXCEPTIONS_STDARCH: ExceptionList = &[ + // tidy-alphabetical-start + ("ryu", "Apache-2.0 OR BSL-1.0"), // BSL is not acceptble, but we use it under Apache-2.0 + ("wasmparser", "Apache-2.0 WITH LLVM-exception"), + ("wasmprinter", "Apache-2.0 WITH LLVM-exception"), + // tidy-alphabetical-end +]; +*/ + +const EXCEPTIONS_CARGO: ExceptionList = &[ // tidy-alphabetical-start ("bitmaps", "MPL-2.0+"), ("bytesize", "Apache-2.0"), - ("byteyarn", "Apache-2.0"), ("ciborium", "Apache-2.0"), ("ciborium-io", "Apache-2.0"), ("ciborium-ll", "Apache-2.0"), @@ -68,16 +116,30 @@ const EXCEPTIONS_CARGO: &[(&str, &str)] = &[ ("im-rc", "MPL-2.0+"), ("normalize-line-endings", "Apache-2.0"), ("openssl", "Apache-2.0"), - ("ryu", "Apache-2.0 OR BSL-1.0"), + ("ryu", "Apache-2.0 OR BSL-1.0"), // BSL is not acceptble, but we use it under Apache-2.0 ("sha1_smol", "BSD-3-Clause"), ("similar", "Apache-2.0"), ("sized-chunks", "MPL-2.0+"), ("subtle", "BSD-3-Clause"), + ("supports-hyperlinks", "Apache-2.0"), ("unicode-bom", "Apache-2.0"), // tidy-alphabetical-end ]; -const EXCEPTIONS_CRANELIFT: &[(&str, &str)] = &[ +const EXCEPTIONS_RUST_ANALYZER: ExceptionList = &[ + // tidy-alphabetical-start + ("anymap", "BlueOak-1.0.0 OR MIT OR Apache-2.0"), // BlueOak is not acceptable, but we use it under MIT OR Apache-2 .0 + ("dissimilar", "Apache-2.0"), + ("instant", "BSD-3-Clause"), + ("notify", "CC0-1.0"), + ("pulldown-cmark-to-cmark", "Apache-2.0"), + ("ryu", "Apache-2.0 OR BSL-1.0"), // BSL is not acceptble, but we use it under Apache-2.0 + ("scip", "Apache-2.0"), + ("snap", "BSD-3-Clause"), + // tidy-alphabetical-end +]; + +const EXCEPTIONS_CRANELIFT: ExceptionList = &[ // tidy-alphabetical-start ("cranelift-bforest", "Apache-2.0 WITH LLVM-exception"), ("cranelift-codegen", "Apache-2.0 WITH LLVM-exception"), @@ -98,8 +160,22 @@ const EXCEPTIONS_CRANELIFT: &[(&str, &str)] = &[ // tidy-alphabetical-end ]; -const EXCEPTIONS_BOOTSTRAP: &[(&str, &str)] = &[ - ("ryu", "Apache-2.0 OR BSL-1.0"), // through serde +// FIXME uncomment once all deps are vendored +/* +const EXCEPTIONS_GCC: ExceptionList = &[ + // tidy-alphabetical-start + ("gccjit", "GPL-3.0"), + ("gccjit_sys", "GPL-3.0"), + // tidy-alphabetical-end +]; +*/ + +const EXCEPTIONS_BOOTSTRAP: ExceptionList = &[ + ("ryu", "Apache-2.0 OR BSL-1.0"), // through serde. BSL is not acceptble, but we use it under Apache-2.0 +]; + +const EXCEPTIONS_UEFI_QEMU_TEST: ExceptionList = &[ + ("r-efi", "MIT OR Apache-2.0 OR LGPL-2.1-or-later"), // LGPL is not acceptible, but we use it under MIT OR Apache-2.0 ]; /// These are the root crates that are part of the runtime. The licenses for @@ -141,6 +217,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "darling_core", "darling_macro", "datafrog", + "derivative", "derive_more", "derive_setters", "digest", @@ -152,7 +229,6 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "ena", "equivalent", "errno", - "errno-dragonfly", "expect-test", "fallible-iterator", // dependency of `thorin` "fastrand", @@ -171,7 +247,10 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "hashbrown", "hermit-abi", "icu_list", + "icu_list_data", "icu_locid", + "icu_locid_transform", + "icu_locid_transform_data", "icu_provider", "icu_provider_adapters", "icu_provider_macros", @@ -183,6 +262,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "is-terminal", "itertools", "itoa", + "jemalloc-sys", "jobserver", "lazy_static", "libc", @@ -210,6 +290,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "perf-event-open-sys", "pin-project-lite", "polonius-engine", + "portable-atomic", // dependency for platforms doesn't support `AtomicU64` in std "ppv-lite86", "proc-macro-hack", "proc-macro2", @@ -320,12 +401,37 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ // tidy-alphabetical-end ]; +// These crates come from ICU4X and are licensed under the unicode license. +// It currently doesn't have an SPDX identifier, so they cannot put one there. +// See https://github.com/unicode-org/icu4x/pull/3875 +// FIXME: This should be removed once ICU4X crates update. +const ICU4X_UNICODE_LICENSE_DEPENDENCIES: &[&str] = &[ + // tidy-alphabetical-start + "icu_list", + "icu_list_data", + "icu_locid", + "icu_locid_transform", + "icu_locid_transform_data", + "icu_provider", + "icu_provider_adapters", + "icu_provider_macros", + "litemap", + "tinystr", + "writeable", + "yoke", + "yoke-derive", + "zerofrom", + "zerofrom-derive", + "zerovec", + "zerovec-derive", + // tidy-alphabetical-end +]; + const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[ // tidy-alphabetical-start "ahash", "anyhow", "arbitrary", - "autocfg", "bitflags", "bumpalo", "cfg-if", @@ -382,65 +488,90 @@ 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, bad: &mut bool) { - let mut cmd = cargo_metadata::MetadataCommand::new(); - cmd.cargo_path(cargo) - .manifest_path(root.join("Cargo.toml")) - .features(cargo_metadata::CargoOpt::AllFeatures); - let metadata = t!(cmd.exec()); - let runtime_ids = compute_runtime_crates(&metadata); - check_license_exceptions(&metadata, EXCEPTIONS, runtime_ids, bad); - check_permitted_dependencies( - &metadata, - "rustc", - PERMITTED_RUSTC_DEPENDENCIES, - &["rustc_driver", "rustc_codegen_llvm"], - bad, - ); - - // Check cargo independently as it has it's own workspace. - let mut cmd = cargo_metadata::MetadataCommand::new(); - cmd.cargo_path(cargo) - .manifest_path(root.join("src/tools/cargo/Cargo.toml")) - .features(cargo_metadata::CargoOpt::AllFeatures); - let cargo_metadata = t!(cmd.exec()); - let runtime_ids = HashSet::new(); - check_license_exceptions(&cargo_metadata, EXCEPTIONS_CARGO, runtime_ids, bad); - check_rustfix(&metadata, &cargo_metadata, bad); - - // Check rustc_codegen_cranelift independently as it has it's own workspace. - let mut cmd = cargo_metadata::MetadataCommand::new(); - cmd.cargo_path(cargo) - .manifest_path(root.join("compiler/rustc_codegen_cranelift/Cargo.toml")) - .features(cargo_metadata::CargoOpt::AllFeatures); - let metadata = t!(cmd.exec()); - let runtime_ids = HashSet::new(); - check_license_exceptions(&metadata, EXCEPTIONS_CRANELIFT, runtime_ids, bad); - check_permitted_dependencies( - &metadata, - "cranelift", - PERMITTED_CRANELIFT_DEPENDENCIES, - &["rustc_codegen_cranelift"], - bad, - ); - - let mut cmd = cargo_metadata::MetadataCommand::new(); - cmd.cargo_path(cargo) - .manifest_path(root.join("src/bootstrap/Cargo.toml")) - .features(cargo_metadata::CargoOpt::AllFeatures); - let metadata = t!(cmd.exec()); - let runtime_ids = HashSet::new(); - check_license_exceptions(&metadata, EXCEPTIONS_BOOTSTRAP, runtime_ids, bad); + let mut checked_runtime_licenses = false; + let mut rust_metadata = None; + + for &(workspace, exceptions, permitted_deps) in WORKSPACES { + if !root.join(workspace).join("Cargo.lock").exists() { + tidy_error!(bad, "the `{workspace}` workspace doesn't have a Cargo.lock"); + continue; + } + + let mut cmd = cargo_metadata::MetadataCommand::new(); + cmd.cargo_path(cargo) + .manifest_path(root.join(workspace).join("Cargo.toml")) + .features(cargo_metadata::CargoOpt::AllFeatures) + .other_options(vec!["--locked".to_owned()]); + let metadata = t!(cmd.exec()); + + check_license_exceptions(&metadata, exceptions, bad); + if let Some((crates, permitted_deps)) = permitted_deps { + check_permitted_dependencies(&metadata, workspace, permitted_deps, crates, bad); + } + + if workspace == "." { + let runtime_ids = compute_runtime_crates(&metadata); + check_runtime_license_exceptions(&metadata, runtime_ids, bad); + checked_runtime_licenses = true; + rust_metadata = Some(metadata); + } else if workspace == "src/tools/cargo" { + check_rustfix( + rust_metadata + .as_ref() + .expect("The root workspace should be the first to be checked"), + &metadata, + bad, + ); + } + } + + // Sanity check to ensure we don't accidentally remove the workspace containing the runtime + // crates. + assert!(checked_runtime_licenses); } -/// Check that all licenses are in the valid list in `LICENSES`. +/// Check that all licenses of runtime dependencies are in the valid list in `LICENSES`. /// -/// Packages listed in `exceptions` are allowed for tools. -fn check_license_exceptions( +/// Unlike for tools we don't allow exceptions to the `LICENSES` list for the runtime with the sole +/// exception of `fortanix-sgx-abi` which is only used on x86_64-fortanix-unknown-sgx. +fn check_runtime_license_exceptions( metadata: &Metadata, - exceptions: &[(&str, &str)], runtime_ids: HashSet<&PackageId>, bad: &mut bool, ) { + for pkg in &metadata.packages { + if !runtime_ids.contains(&pkg.id) { + // Only checking dependencies of runtime libraries here. + continue; + } + if pkg.source.is_none() { + // No need to check local packages. + continue; + } + let license = match &pkg.license { + Some(license) => license, + None => { + tidy_error!(bad, "dependency `{}` does not define a license expression", pkg.id); + continue; + } + }; + if !LICENSES.contains(&license.as_str()) { + // This is a specific exception because SGX is considered "third party". + // See https://github.com/rust-lang/rust/issues/62620 for more. + // In general, these should never be added and this exception + // should not be taken as precedent for any new target. + if pkg.name == "fortanix-sgx-abi" && pkg.license.as_deref() == Some("MPL-2.0") { + continue; + } + tidy_error!(bad, "invalid license `{}` in `{}`", license, pkg.id); + } + } +} + +/// Check that all licenses of tool dependencies are in the valid list in `LICENSES`. +/// +/// Packages listed in `exceptions` are allowed for tools. +fn check_license_exceptions(metadata: &Metadata, exceptions: &[(&str, &str)], bad: &mut bool) { // Validate the EXCEPTIONS list hasn't changed. for (name, license) in exceptions { // Check that the package actually exists. @@ -482,24 +613,21 @@ fn check_license_exceptions( // No need to check local packages. continue; } - if !runtime_ids.contains(&pkg.id) && exception_names.contains(&pkg.name.as_str()) { + if exception_names.contains(&pkg.name.as_str()) { continue; } let license = match &pkg.license { Some(license) => license, None => { + if ICU4X_UNICODE_LICENSE_DEPENDENCIES.contains(&pkg.name.as_str()) { + // See the comment on ICU4X_UNICODE_LICENSE_DEPENDENCIES. + continue; + } tidy_error!(bad, "dependency `{}` does not define a license expression", pkg.id); continue; } }; if !LICENSES.contains(&license.as_str()) { - if pkg.name == "fortanix-sgx-abi" { - // This is a specific exception because SGX is considered - // "third party". See - // https://github.com/rust-lang/rust/issues/62620 for more. In - // general, these should never be added. - continue; - } tidy_error!(bad, "invalid license `{}` in `{}`", license, pkg.id); } } diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index aad57cacb..ff71ca537 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -9,25 +9,33 @@ const ALLOWED_SOURCES: &[&str] = &["\"registry+https://github.com/rust-lang/crat /// Checks for external package sources. `root` is the path to the directory that contains the /// workspace `Cargo.toml`. pub fn check(root: &Path, bad: &mut bool) { - // `Cargo.lock` of rust. - let path = root.join("Cargo.lock"); + for &(workspace, _, _) in crate::deps::WORKSPACES { + // FIXME check other workspaces too + // `Cargo.lock` of rust. + let path = root.join(workspace).join("Cargo.lock"); - // Open and read the whole file. - let cargo_lock = t!(fs::read_to_string(&path)); - - // Process each line. - for line in cargo_lock.lines() { - // Consider only source entries. - if !line.starts_with("source = ") { + if !path.exists() { + tidy_error!(bad, "the `{workspace}` workspace doesn't have a Cargo.lock"); continue; } - // Extract source value. - let source = line.split_once('=').unwrap().1.trim(); + // Open and read the whole file. + let cargo_lock = t!(fs::read_to_string(&path)); + + // Process each line. + for line in cargo_lock.lines() { + // Consider only source entries. + if !line.starts_with("source = ") { + continue; + } + + // Extract source value. + let source = line.split_once('=').unwrap().1.trim(); - // Ensure source is allowed. - if !ALLOWED_SOURCES.contains(&&*source) { - tidy_error!(bad, "invalid source: {}", source); + // Ensure source is allowed. + if !ALLOWED_SOURCES.contains(&&*source) { + tidy_error!(bad, "invalid source: {}", source); + } } } } diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index d900c04c1..8e791a7dc 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -30,7 +30,7 @@ const FEATURE_GROUP_END_PREFIX: &str = "// feature-group-end"; #[derive(Debug, PartialEq, Clone)] pub enum Status { - Stable, + Accepted, Removed, Unstable, } @@ -38,7 +38,7 @@ pub enum Status { impl fmt::Display for Status { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let as_str = match *self { - Status::Stable => "stable", + Status::Accepted => "accepted", Status::Unstable => "unstable", Status::Removed => "removed", }; @@ -279,9 +279,9 @@ fn test_filen_gate(filen_underscore: &str, features: &mut Features) -> bool { pub fn collect_lang_features(base_compiler_path: &Path, bad: &mut bool) -> Features { let mut features = Features::new(); - collect_lang_features_in(&mut features, base_compiler_path, "active.rs", bad); collect_lang_features_in(&mut features, base_compiler_path, "accepted.rs", bad); collect_lang_features_in(&mut features, base_compiler_path, "removed.rs", bad); + collect_lang_features_in(&mut features, base_compiler_path, "unstable.rs", bad); features } @@ -336,11 +336,11 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba let mut parts = line.split(','); let level = match parts.next().map(|l| l.trim().trim_start_matches('(')) { - Some("active") => Status::Unstable, + Some("unstable") => Status::Unstable, Some("incomplete") => Status::Unstable, Some("internal") => Status::Unstable, Some("removed") => Status::Removed, - Some("accepted") => Status::Stable, + Some("accepted") => Status::Accepted, _ => continue, }; let name = parts.next().unwrap().trim(); @@ -449,7 +449,7 @@ fn get_and_check_lib_features( Ok((name, f)) => { let mut check_features = |f: &Feature, list: &Features, display: &str| { if let Some(ref s) = list.get(name) { - if f.tracking_issue != s.tracking_issue && f.level != Status::Stable { + if f.tracking_issue != s.tracking_issue && f.level != Status::Accepted { tidy_error!( bad, "{}:{}: `issue` \"{}\" mismatches the {} `issue` of \"{}\"", @@ -566,7 +566,7 @@ fn map_lib_features( let level = if line.contains("[unstable(") { Status::Unstable } else if line.contains("[stable(") { - Status::Stable + Status::Accepted } else { continue; }; @@ -581,7 +581,7 @@ fn map_lib_features( Some(Err(_err)) => { err!("malformed stability attribute: can't parse `since` key"); } - None if level == Status::Stable => { + None if level == Status::Accepted => { err!("malformed stability attribute: missing the `since` key"); } None => None, diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index fc69c1432..eb0a2fda2 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -3,8 +3,6 @@ //! This library contains the tidy lints and exposes it //! to be used by tools. -use std::fmt::Display; - use termcolor::WriteColor; /// A helper macro to `unwrap` a result except also print out details like: @@ -31,16 +29,22 @@ macro_rules! t { macro_rules! tidy_error { ($bad:expr, $($fmt:tt)*) => ({ - $crate::tidy_error($bad, format_args!($($fmt)*)).expect("failed to output error"); + $crate::tidy_error(&format_args!($($fmt)*).to_string()).expect("failed to output error"); + *$bad = true; }); } -fn tidy_error(bad: &mut bool, args: impl Display) -> std::io::Result<()> { +macro_rules! tidy_error_ext { + ($tidy_error:path, $bad:expr, $($fmt:tt)*) => ({ + $tidy_error(&format_args!($($fmt)*).to_string()).expect("failed to output error"); + *$bad = true; + }); +} + +fn tidy_error(args: &str) -> std::io::Result<()> { use std::io::Write; use termcolor::{Color, ColorChoice, ColorSpec, StandardStream}; - *bad = true; - let mut stderr = StandardStream::stdout(ColorChoice::Auto); stderr.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; diff --git a/src/tools/tidy/src/mir_opt_tests.rs b/src/tools/tidy/src/mir_opt_tests.rs index c307bcb93..76feeb343 100644 --- a/src/tools/tidy/src/mir_opt_tests.rs +++ b/src/tools/tidy/src/mir_opt_tests.rs @@ -26,7 +26,8 @@ fn check_unused_files(path: &Path, bless: bool, bad: &mut bool) { for file in rs_files { for bw in [32, 64] { for ps in [PanicStrategy::Unwind, PanicStrategy::Abort] { - for output_file in miropt_test_tools::files_for_miropt_test(&file, bw, ps) { + let mir_opt_test = miropt_test_tools::files_for_miropt_test(&file, bw, ps); + for output_file in mir_opt_test.files { output_files.remove(&output_file.expected_file); } } diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 11480e2be..cb40c6e3a 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -427,9 +427,12 @@ pub fn check(path: &Path, bad: &mut bool) { "copyright notices attributed to the Rust Project Developers are deprecated" ); } - if is_unexplained_ignore(&extension, line) { - err(UNEXPLAINED_IGNORE_DOCTEST_INFO); + if !file.components().any(|c| c.as_os_str() == "rustc_baked_icu_data") { + if is_unexplained_ignore(&extension, line) { + err(UNEXPLAINED_IGNORE_DOCTEST_INFO); + } } + if filename.ends_with(".cpp") && line.contains("llvm_unreachable") { err(LLVM_UNREACHABLE_INFO); } diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 2b828e58d..7e24793ad 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -11,7 +11,7 @@ use std::path::{Path, PathBuf}; const ENTRY_LIMIT: usize = 900; // FIXME: The following limits should be reduced eventually. const ISSUES_ENTRY_LIMIT: usize = 1854; -const ROOT_ENTRY_LIMIT: usize = 865; +const ROOT_ENTRY_LIMIT: usize = 867; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files |