From 9b3f4a2c0f10f2d950319da14f5f87adaeee62cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Fri, 10 May 2024 17:41:44 +0200 Subject: [PATCH] ci: Add lint to check if all sub-crates features can be enabled from ruma crate --- xtask/src/cargo.rs | 50 ++++++++++++++++++++++++++-- xtask/src/ci.rs | 29 +++++++++++------ xtask/src/ci/reexport_features.rs | 54 +++++++++++++++++++++++++++++++ xtask/src/main.rs | 10 ++++-- 4 files changed, 129 insertions(+), 14 deletions(-) create mode 100644 xtask/src/ci/reexport_features.rs diff --git a/xtask/src/cargo.rs b/xtask/src/cargo.rs index ca9ca62b..8fd21c75 100644 --- a/xtask/src/cargo.rs +++ b/xtask/src/cargo.rs @@ -1,9 +1,14 @@ -use std::path::PathBuf; +#![allow(clippy::disallowed_types)] +use std::{collections::HashMap, path::PathBuf}; + +#[cfg(feature = "default")] use reqwest::blocking::Client; use semver::Version; use serde::{de::IgnoredAny, Deserialize}; +#[cfg(feature = "default")] use toml_edit::{value, Document}; +#[cfg(feature = "default")] use xshell::{cmd, pushd, read_file, write_file}; use crate::{util::ask_yes_no, Metadata, Result}; @@ -22,11 +27,51 @@ pub struct Package { /// The package's manifest path. pub manifest_path: PathBuf, - /// A map of the package dependencies. + /// A list of the package dependencies. #[serde(default)] pub dependencies: Vec, + + /// A map of the package features. + #[serde(default)] + pub features: HashMap>, } +impl Package { + /// Whether this package has a way to enable the given feature from the given package. + pub fn can_enable_feature(&self, package_name: &str, feature_name: &str) -> bool { + for activated_feature in self.features.values().flatten() { + // Remove optional `dep:` at the start. + let remaining = activated_feature.trim_start_matches("dep:"); + + // Check that we have the package name. + let Some(remaining) = remaining.strip_prefix(package_name) else { + continue; + }; + + if remaining.is_empty() { + // The feature only enables the dependency. + continue; + } + + // Remove optional `?`. + let remaining = remaining.trim_start_matches('?'); + + let Some(remaining) = remaining.strip_prefix('/') else { + // This is another package name starting with the same string. + continue; + }; + + // Finally, only the feature name is remaining. + if remaining == feature_name { + return true; + } + } + + false + } +} + +#[cfg(feature = "default")] impl Package { /// Update the version of this crate. pub fn update_version(&mut self, version: &Version, dry_run: bool) -> Result<()> { @@ -203,6 +248,7 @@ pub enum DependencyKind { Build, } +#[cfg(feature = "default")] /// A crate from the `GET /crates/{crate}` endpoint of crates.io. #[derive(Deserialize)] struct CratesIoCrate { diff --git a/xtask/src/ci.rs b/xtask/src/ci.rs index 52df38bf..37db97a7 100644 --- a/xtask/src/ci.rs +++ b/xtask/src/ci.rs @@ -1,15 +1,17 @@ // Triggers at the `#[clap(subcommand)]` line, but not easily reproducible outside this crate. #![allow(unused_qualifications)] -use std::path::PathBuf; +use std::path::Path; use clap::{Args, Subcommand}; use xshell::pushd; use crate::{cmd, Metadata, Result, NIGHTLY}; +mod reexport_features; mod spec_links; +use reexport_features::check_reexport_features; use spec_links::check_spec_links; const MSRV: &str = "1.75"; @@ -66,6 +68,8 @@ pub enum CiCmd { Dependencies, /// Check spec links point to a recent version (lint) SpecLinks, + /// Check all cargo features of sub-crates can be enabled from ruma (lint) + ReexportFeatures, /// Check typos Typos, } @@ -75,18 +79,22 @@ pub struct CiTask { /// Which command to run. cmd: Option, - /// The root of the workspace. - project_root: PathBuf, + /// The metadata of the workspace. + project_metadata: Metadata, } impl CiTask { pub(crate) fn new(cmd: Option) -> Result { - let project_root = Metadata::load()?.workspace_root; - Ok(Self { cmd, project_root }) + let project_metadata = Metadata::load()?; + Ok(Self { cmd, project_metadata }) + } + + fn project_root(&self) -> &Path { + &self.project_metadata.workspace_root } pub(crate) fn run(self) -> Result<()> { - let _p = pushd(&self.project_root)?; + let _p = pushd(self.project_root())?; match self.cmd { Some(CiCmd::Msrv) => self.msrv()?, @@ -110,7 +118,8 @@ impl CiTask { Some(CiCmd::ClippyAll) => self.clippy_all()?, Some(CiCmd::Lint) => self.lint()?, Some(CiCmd::Dependencies) => self.dependencies()?, - Some(CiCmd::SpecLinks) => check_spec_links(&self.project_root.join("crates"))?, + Some(CiCmd::SpecLinks) => check_spec_links(&self.project_root().join("crates"))?, + Some(CiCmd::ReexportFeatures) => check_reexport_features(&self.project_metadata)?, Some(CiCmd::Typos) => self.typos()?, None => { self.msrv() @@ -301,9 +310,11 @@ impl CiTask { // Check dependencies being sorted let dependencies_res = self.dependencies(); // Check that all links point to the same version of the spec - let spec_links_res = check_spec_links(&self.project_root.join("crates")); + let spec_links_res = check_spec_links(&self.project_root().join("crates")); + // Check that all cargo features of sub-crates can be enabled from ruma. + let reexport_features_res = check_reexport_features(&self.project_metadata); - dependencies_res.and(spec_links_res) + dependencies_res.and(spec_links_res).and(reexport_features_res) } /// Check the sorting of dependencies with the nightly version. diff --git a/xtask/src/ci/reexport_features.rs b/xtask/src/ci/reexport_features.rs new file mode 100644 index 00000000..596d3711 --- /dev/null +++ b/xtask/src/ci/reexport_features.rs @@ -0,0 +1,54 @@ +use crate::{Metadata, Result}; + +/// Check that the ruma crate allows to enable all the features of the other ruma-* crates. +/// +/// For simplicity, this function assumes that: +/// +/// - Those dependencies are not renamed. +/// - ruma does not use `default-features = false` on those dependencies. +/// +/// This does not check if all features are re-exported individually, as that is not always wanted. +pub(crate) fn check_reexport_features(metadata: &Metadata) -> Result<()> { + println!("Checking all features can be enabled from ruma…"); + let mut n_errors = 0; + + let Some(ruma) = metadata.find_package("ruma") else { + return Err("ruma package not found in workspace".into()); + }; + + for package in ruma.dependencies.iter().filter_map(|dep| metadata.find_package(&dep.name)) { + println!("Checking features of {}…", package.name); + + // Exclude ruma and xtask. + if !package.name.starts_with("ruma-") { + continue; + } + + // Filter features that are enabled by other features of the same package. + let features = package.features.keys().filter(|feature_name| { + !package.features.values().flatten().any(|activated_feature| { + activated_feature.trim_start_matches("dep:") == *feature_name + }) + }); + + for feature_name in features { + // Let's assume that ruma never has `default-features = false`. + if feature_name == "default" { + continue; + } + + if !ruma.can_enable_feature(&package.name, feature_name) { + println!(r#" Missing feature "{}/{feature_name}""#, package.name); + n_errors += 1; + } + } + } + + if n_errors > 0 { + // Visual aid to separate the end error message. + println!(); + return Err(format!("Found {n_errors} missing features").into()); + } + + Ok(()) +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 5fd239ea..f62f7444 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -17,7 +17,6 @@ use serde_json::from_str as from_json_str; // Keep in sync with version in `rust-toolchain.toml` and `.github/workflows/ci.yml` const NIGHTLY: &str = "nightly-2024-02-14"; -#[cfg(feature = "default")] mod cargo; mod ci; mod doc; @@ -26,6 +25,7 @@ mod release; #[cfg(feature = "default")] mod util; +use cargo::Package; use ci::{CiArgs, CiTask}; use doc::DocTask; #[cfg(feature = "default")] @@ -70,8 +70,7 @@ fn main() -> Result<()> { #[derive(Clone, Debug, Deserialize)] struct Metadata { pub workspace_root: PathBuf, - #[cfg(feature = "default")] - pub packages: Vec, + pub packages: Vec, } impl Metadata { @@ -80,6 +79,11 @@ impl Metadata { let metadata_json = cmd!("cargo metadata --no-deps --format-version 1").read()?; Ok(from_json_str(&metadata_json)?) } + + /// Find the package with the given name. + pub fn find_package(&self, name: &str) -> Option<&Package> { + self.packages.iter().find(|p| p.name == name) + } } #[cfg(feature = "default")]