Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add workspace key to [cargo-semver-checks.lints] table #811

Merged
merged 3 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ note: skipped the following crates since they have no library target: {skipped}"
let workspace_overrides =
manifest::deserialize_lint_table(&metadata.workspace_metadata)
.context("[workspace.metadata.cargo-semver-checks] table is invalid")?
.map(Arc::new);
.map(|table| Arc::new(table.inner));

selected
.iter()
Expand Down Expand Up @@ -528,7 +528,7 @@ note: skipped the following crates since they have no library target: {skipped}"
overrides.push(Arc::clone(workspace));
}
if let Some(package) = package_overrides {
overrides.push(Arc::new(package));
overrides.push(Arc::new(package.inner));
}

let start = std::time::Instant::now();
Expand Down
101 changes: 76 additions & 25 deletions src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,31 @@ pub(crate) struct SemverChecksTable {
}

#[derive(Debug, Clone, Deserialize)]
pub(crate) struct LintTable(BTreeMap<String, OverrideConfig>);
pub(crate) struct LintTable {
/// Optional key to indicate whether to opt-in to reading
/// workspace lint configuration. If not set in the TOML as
/// `package.metadata.cargo-semver-checks.lints.workspace = true`,
/// this field is set to `false. (note that setting the key in the
/// TOML to false explicitly is invalid behavior and will be interpreted
/// as just a missing field)
///
/// Currently, we also read `lints.workspace`, but having this key
/// in a Cargo.toml manifest is invalid if there is no `[workspace.lints]
/// table in the workspace manifest. Since we are storing our lint config in
/// `[workspace.metadata.*]` for now, this could be the case. If either this
/// field is true or `lints.workspace` is set, we should read the workspace
/// lint config.
#[allow(dead_code)]
#[serde(default)]
pub(crate) workspace: bool,
/// individual `lint_name = ...` entries
#[serde(flatten, deserialize_with = "deserialize_into_overridemap")]
pub(crate) inner: OverrideMap,
}

impl From<LintTable> for OverrideMap {
fn from(value: LintTable) -> OverrideMap {
value.0.into_iter().map(|(k, v)| (k, v.into())).collect()
value.inner
}
}

Expand Down Expand Up @@ -124,37 +144,44 @@ impl From<OverrideConfig> for QueryOverride {
}
}

/// Lets serde deserialize a `BTreeMap<String, OverrideConfig>` into an [`OverrideMap`]
fn deserialize_into_overridemap<'de, D>(de: D) -> Result<OverrideMap, D::Error>
where
D: serde::de::Deserializer<'de>,
{
BTreeMap::<String, OverrideConfig>::deserialize(de)
.map(|x| x.into_iter().map(|(k, v)| (k, v.into())).collect())
}

/// Helper function to deserialize an optional lint table from a [`serde_json::Value`]
/// into a [`OverrideMap`]. Returns an `Err` if the `cargo-semver-checks` table is present
/// holding a `[package/workspace.metadata]` table holding a `cargo-semver-checks.lints` table
///
/// Returns an `Err` if the `cargo-semver-checks` table is present
/// but invalid. Returns `Ok(None)` if the table is not present.
pub(crate) fn deserialize_lint_table(
metadata: &serde_json::Value,
) -> anyhow::Result<Option<OverrideMap>> {
) -> anyhow::Result<Option<LintTable>> {
let table = Option::<MetadataTable>::deserialize(metadata)?;
Ok(table.and_then(|table| {
table
.config
.and_then(|config| config.lints.map(OverrideMap::from))
}))
Ok(table.and_then(|table| table.config.and_then(|config| config.lints)))
}

#[cfg(test)]
mod tests {
use crate::{manifest::OverrideConfig, QueryOverride};

use super::MetadataTable;
use crate::QueryOverride;

#[test]
fn test_deserialize_config() {
use crate::LintLevel::*;
use crate::RequiredSemverUpdate::*;
use OverrideConfig::*;

let manifest = r#"[package]
name = "cargo-semver-checks"
version = "1.2.3"
edition = "2021"

[package.metadata.cargo-semver-checks.lints]
workspace = true
one = "major"
two = "deny"
three = { lint-level = "warn" }
Expand All @@ -179,37 +206,55 @@ mod tests {
.metadata
.expect("Workspace metadata should be present");

let pkg = package_metadata
let pkg_table = package_metadata
.config
.expect("Semver checks table should be present")
.lints
.expect("Lint table should be present")
.0;
.expect("Lint table should be present");

assert!(
pkg_table.workspace,
"Package lints table should contain `workspace = true`"
);
let pkg = pkg_table.inner;

let wks = workspace_metadata
.config
.expect("Semver checks table should be present")
.lints
.expect("Lint table should be present")
.0;
.inner;
assert!(
matches!(pkg.get("one"), Some(&RequiredUpdate(Major))),
matches!(
pkg.get("one"),
Some(&QueryOverride {
required_update: Some(Major),
lint_level: None
})
),
"got {:?}",
pkg.get("one")
);

assert!(
matches!(pkg.get("two"), Some(&LintLevel(Deny))),
matches!(
pkg.get("two"),
Some(&QueryOverride {
lint_level: Some(Deny),
required_update: None,
})
),
"got {:?}",
pkg.get("two")
);

assert!(
matches!(
pkg.get("three"),
Some(&Structure(QueryOverride {
Some(&QueryOverride {
required_update: None,
lint_level: Some(Warn)
}))
})
),
"got {:?}",
pkg.get("three")
Expand All @@ -218,10 +263,10 @@ mod tests {
assert!(
matches!(
pkg.get("four"),
Some(&Structure(QueryOverride {
Some(&QueryOverride {
required_update: Some(Major),
lint_level: None,
}))
})
),
"got {:?}",
pkg.get("four")
Expand All @@ -231,17 +276,23 @@ mod tests {
assert!(
matches!(
pkg.get("five"),
Some(&Structure(QueryOverride {
Some(&QueryOverride {
required_update: Some(Minor),
lint_level: Some(Allow)
}))
})
),
"got {:?}",
pkg.get("five")
);

assert!(
matches!(wks.get("six"), Some(&LintLevel(Allow))),
matches!(
wks.get("six"),
Some(&QueryOverride {
lint_level: Some(Allow),
required_update: None
})
),
"got {:?}",
wks.get("six")
);
Expand Down
Loading