diff --git a/.github/bors.toml b/.github/bors.toml index 4714de6e..b40fa934 100644 --- a/.github/bors.toml +++ b/.github/bors.toml @@ -3,7 +3,9 @@ delete_merged_branches = true required_approvals = 1 timeout_sec = 14400 status = [ - "build (1.36.0)", + "build (1.40.0)", "build (stable)", - "test" + "test", + "test-strict", + "clippy_check", ] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ce708e3..98b4336c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: continue-on-error: ${{ matrix.experimental || false }} strategy: matrix: - rust: [ 1.36.0, stable ] + rust: [ 1.40.0, stable ] include: # Test nightly but don't fail the build. - rust: nightly @@ -44,3 +44,19 @@ jobs: - uses: actions-rs/cargo@v1 with: command: test + + test-strict: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v2 + - name: Install Rust + uses: actions-rs/toolchain@v1 + with: + toolchain: stable + profile: minimal + override: true + - uses: actions-rs/cargo@v1 + with: + command: test + args: --all-features diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 787f369e..adc3a6ed 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -12,7 +12,7 @@ jobs: - uses: actions-rs/toolchain@v1 with: profile: minimal - toolchain: 1.36.0 + toolchain: stable override: true components: clippy - uses: actions-rs/clippy-check@v1 diff --git a/README.md b/README.md index 9542e815..2298fcac 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ This project is developed and maintained by the [Tools team][team]. ## Minimum Supported Rust Version (MSRV) -This crate is guaranteed to compile on stable Rust 1.36.0 and up. It *might* +This crate is guaranteed to compile on stable Rust 1.40.0 and up. It *might* compile with older versions but that may change in any new patch release. ## License diff --git a/src/elementext.rs b/src/elementext.rs index 41d2c8cd..4ba76e91 100644 --- a/src/elementext.rs +++ b/src/elementext.rs @@ -40,9 +40,9 @@ impl ElementExt for Element { Err(e) => match e.downcast_ref() { // if tag is empty just ignore it Some(SVDError::EmptyTag(_, _)) => Ok(None), - _ => return Err(e), + _ => Err(e), }, - Ok(s) => Ok(Some(s.to_owned())), + Ok(s) => Ok(Some(s)), } } else { Ok(None) diff --git a/src/error.rs b/src/error.rs index 82295f61..07de6ab1 100644 --- a/src/error.rs +++ b/src/error.rs @@ -3,11 +3,13 @@ pub use anyhow::{Context, Result}; use core::u64; +#[cfg(feature = "strict")] use once_cell::sync::Lazy; +#[cfg(feature = "strict")] use regex::Regex; use xmltree::Element; -#[allow(clippy::large_enum_variant)] +#[allow(clippy::large_enum_variant, clippy::upper_case_acronyms)] #[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)] pub enum SVDError { #[error("Unknown endianness `{0}`")] @@ -87,6 +89,7 @@ pub enum ResetValueError { MaskTooLarge(u64, u32), } +#[cfg(feature = "strict")] pub(crate) fn check_name(name: &str, tag: &str) -> Result<()> { static PATTERN: Lazy = Lazy::new(|| Regex::new("^[_A-Za-z0-9]*$").unwrap()); if PATTERN.is_match(name) { @@ -98,6 +101,7 @@ pub(crate) fn check_name(name: &str, tag: &str) -> Result<()> { } } +#[cfg(feature = "strict")] pub(crate) fn check_dimable_name(name: &str, tag: &str) -> Result<()> { static PATTERN: Lazy = Lazy::new(|| { Regex::new("^(((%s)|(%s)[_A-Za-z]{1}[_A-Za-z0-9]*)|([_A-Za-z]{1}[_A-Za-z0-9]*(\\[%s\\])?)|([_A-Za-z]{1}[_A-Za-z0-9]*(%s)?[_A-Za-z0-9]*))$").unwrap() @@ -117,6 +121,7 @@ pub(crate) fn check_has_placeholder(name: &str, tag: &str) -> Result<()> { } } +#[cfg(feature = "strict")] pub(crate) fn check_derived_name(name: &str, tag: &str) -> Result<()> { for x in name.split('.') { check_dimable_name(x, tag)? @@ -127,7 +132,7 @@ pub(crate) fn check_derived_name(name: &str, tag: &str) -> Result<()> { pub(crate) fn check_reset_value( size: Option, value: Option, - mask: Option, + _mask: Option, ) -> Result<()> { const MAX_BITS: u32 = u64::MAX.count_ones(); @@ -136,20 +141,24 @@ pub(crate) fn check_reset_value( return Err(ResetValueError::ValueTooLarge(value, size).into()); } } - if let (Some(size), Some(mask)) = (size, mask) { - if MAX_BITS - mask.leading_zeros() > size { - return Err(ResetValueError::MaskTooLarge(mask, size).into()); + #[cfg(feature = "strict")] + { + if let (Some(size), Some(mask)) = (size, _mask) { + if MAX_BITS - mask.leading_zeros() > size { + return Err(ResetValueError::MaskTooLarge(mask, size).into()); + } } - } - if let (Some(value), Some(mask)) = (value, mask) { - if value & mask != value { - return Err(ResetValueError::MaskConflict(value, mask).into()); + if let (Some(value), Some(mask)) = (value, _mask) { + if value & mask != value { + return Err(ResetValueError::MaskConflict(value, mask).into()); + } } } Ok(()) } +#[cfg(feature = "strict")] #[cfg(test)] mod tests { use crate::error::check_reset_value; diff --git a/src/lib.rs b/src/lib.rs index 545a1785..d4b3f8cb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,12 +8,10 @@ //! use std::fs::File; //! use std::io::Read; //! -//! fn main() { -//! let xml = &mut String::new(); -//! File::open("STM32F30x.svd").unwrap().read_to_string(xml); +//! let xml = &mut String::new(); +//! File::open("STM32F30x.svd").unwrap().read_to_string(xml); //! -//! println!("{:?}", svd::parse(xml)); -//! } +//! println!("{:?}", svd::parse(xml)); //! ``` //! //! # References diff --git a/src/parse.rs b/src/parse.rs index 7a20a593..8e433fab 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -16,7 +16,7 @@ pub trait Parse { /// Parses an optional child element with the provided name and Parse function /// Returns an none if the child doesn't exist, Ok(Some(e)) if parsing succeeds, /// and Err() if parsing fails. -pub fn optional<'a, T>(n: &str, e: &'a Element) -> anyhow::Result> +pub fn optional(n: &str, e: &Element) -> anyhow::Result> where T: Parse, { diff --git a/src/svd/clusterinfo.rs b/src/svd/clusterinfo.rs index 04ffa9f1..a4855461 100644 --- a/src/svd/clusterinfo.rs +++ b/src/svd/clusterinfo.rs @@ -11,6 +11,7 @@ use crate::svd::{registercluster::RegisterCluster, registerproperties::RegisterP #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct ClusterInfo { /// String to identify the cluster. /// Cluster names are required to be unique within the scope of a peripheral @@ -38,10 +39,6 @@ pub struct ClusterInfo { pub default_register_properties: RegisterProperties, pub children: Vec, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, Default, PartialEq)] @@ -99,17 +96,19 @@ impl ClusterInfoBuilder { children: self .children .ok_or_else(|| BuildError::Uninitialized("children".to_string()))?, - _extensible: (), }) .validate() } } impl ClusterInfo { + #[allow(clippy::unnecessary_wraps)] fn validate(self) -> Result { + #[cfg(feature = "strict")] check_dimable_name(&self.name, "name")?; - if let Some(name) = self.derived_from.as_ref() { - check_derived_name(name, "derivedFrom")?; + if let Some(_name) = self.derived_from.as_ref() { + #[cfg(feature = "strict")] + check_derived_name(_name, "derivedFrom")?; } else if self.children.is_empty() { #[cfg(feature = "strict")] return Err(SVDError::EmptyCluster)?; diff --git a/src/svd/cpu.rs b/src/svd/cpu.rs index fc0698a0..9775dd39 100644 --- a/src/svd/cpu.rs +++ b/src/svd/cpu.rs @@ -12,6 +12,7 @@ use crate::types::Parse; #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct Cpu { pub name: String, @@ -32,10 +33,6 @@ pub struct Cpu { /// Indicate whether the processor implements a vendor-specific System Tick Timer pub has_vendor_systick: bool, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, Default, PartialEq)] @@ -101,13 +98,13 @@ impl CpuBuilder { has_vendor_systick: self .has_vendor_systick .ok_or_else(|| BuildError::Uninitialized("has_vendor_systick".to_string()))?, - _extensible: (), }) .validate() } } impl Cpu { + #[allow(clippy::unnecessary_wraps)] fn validate(self) -> Result { // TODO Ok(self) diff --git a/src/svd/device.rs b/src/svd/device.rs index 219ff2f7..0bc73c77 100644 --- a/src/svd/device.rs +++ b/src/svd/device.rs @@ -15,6 +15,7 @@ use crate::svd::{cpu::Cpu, peripheral::Peripheral, registerproperties::RegisterP #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct Device { /// The string identifies the device or device series. Device names are required to be unique pub name: String, @@ -53,10 +54,6 @@ pub struct Device { pub peripherals: Vec, pub default_register_properties: RegisterProperties, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, Default)] @@ -124,7 +121,6 @@ impl DeviceBuilder { .peripherals .ok_or_else(|| BuildError::Uninitialized("peripherals".to_string()))?, default_register_properties: self.default_register_properties, - _extensible: (), }) .validate() } @@ -134,7 +130,7 @@ impl Device { fn validate(self) -> Result { // TODO if self.peripherals.is_empty() { - return Err(SVDError::EmptyDevice)?; + return Err(SVDError::EmptyDevice.into()); } Ok(self) } diff --git a/src/svd/dimelement.rs b/src/svd/dimelement.rs index 265d0f04..f4ea373f 100644 --- a/src/svd/dimelement.rs +++ b/src/svd/dimelement.rs @@ -10,6 +10,7 @@ use crate::error::*; #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct DimElement { /// Defines the number of elements in an array or list pub dim: u32, @@ -22,10 +23,6 @@ pub struct DimElement { #[cfg_attr(feature = "serde", serde(default))] #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))] pub dim_index: Option>, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, Default, PartialEq)] @@ -57,7 +54,6 @@ impl DimElementBuilder { .dim_increment .ok_or_else(|| BuildError::Uninitialized("dim_increment".to_string()))?, dim_index: self.dim_index, - _extensible: (), }) } } diff --git a/src/svd/enumeratedvalue.rs b/src/svd/enumeratedvalue.rs index 71bbc1c7..6ab1c7a6 100644 --- a/src/svd/enumeratedvalue.rs +++ b/src/svd/enumeratedvalue.rs @@ -11,6 +11,7 @@ use crate::types::Parse; #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct EnumeratedValue { /// String describing the semantics of the value. Can be displayed instead of the value pub name: String, @@ -29,10 +30,6 @@ pub struct EnumeratedValue { #[cfg_attr(feature = "serde", serde(default))] #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))] pub is_default: Option, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)] @@ -77,7 +74,6 @@ impl EnumeratedValueBuilder { description: self.description, value: self.value, is_default: self.is_default, - _extensible: (), }) .validate() } @@ -85,6 +81,7 @@ impl EnumeratedValueBuilder { impl EnumeratedValue { fn validate(self) -> Result { + #[cfg(feature = "strict")] check_name(&self.name, "name")?; match (&self.value, &self.is_default) { (Some(_), None) | (None, Some(_)) => Ok(self), diff --git a/src/svd/enumeratedvalues.rs b/src/svd/enumeratedvalues.rs index ebd166e0..3513214c 100644 --- a/src/svd/enumeratedvalues.rs +++ b/src/svd/enumeratedvalues.rs @@ -13,6 +13,7 @@ use crate::types::Parse; #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct EnumeratedValues { /// Identifier for the whole enumeration section #[cfg_attr(feature = "serde", serde(default))] @@ -30,10 +31,6 @@ pub struct EnumeratedValues { pub derived_from: Option, pub values: Vec, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)] @@ -73,7 +70,6 @@ impl EnumeratedValuesBuilder { usage: self.usage, derived_from: self.derived_from, values: self.values.unwrap_or_default(), - _extensible: (), }) .validate() } @@ -81,11 +77,15 @@ impl EnumeratedValuesBuilder { impl EnumeratedValues { fn validate(self) -> Result { - if let Some(name) = self.name.as_ref() { - check_name(name, "name")?; + #[cfg(feature = "strict")] + { + if let Some(name) = self.name.as_ref() { + check_name(name, "name")?; + } } - if let Some(dname) = self.derived_from.as_ref() { - check_derived_name(dname, "derivedFrom")?; + if let Some(_dname) = self.derived_from.as_ref() { + #[cfg(feature = "strict")] + check_derived_name(_dname, "derivedFrom")?; Ok(self) } else if self.values.is_empty() { Err(EnumeratedValuesError::Empty.into()) diff --git a/src/svd/fieldinfo.rs b/src/svd/fieldinfo.rs index eef87331..2187911b 100644 --- a/src/svd/fieldinfo.rs +++ b/src/svd/fieldinfo.rs @@ -17,6 +17,7 @@ use crate::svd::{ #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct FieldInfo { /// Name string used to identify the field. /// Field names must be unique within a register @@ -52,10 +53,6 @@ pub struct FieldInfo { #[cfg_attr(feature = "serde", serde(default))] #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))] pub modified_write_values: Option, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, Default, PartialEq)] @@ -117,7 +114,6 @@ impl FieldInfoBuilder { enumerated_values: self.enumerated_values.unwrap_or_default(), write_constraint: self.write_constraint, modified_write_values: self.modified_write_values, - _extensible: (), }) .validate() } @@ -125,9 +121,13 @@ impl FieldInfoBuilder { impl FieldInfo { fn validate(self) -> Result { + #[cfg(feature = "strict")] check_dimable_name(&self.name, "name")?; - if let Some(name) = self.derived_from.as_ref() { - check_derived_name(name, "derivedFrom")?; + #[cfg(feature = "strict")] + { + if let Some(name) = self.derived_from.as_ref() { + check_derived_name(name, "derivedFrom")?; + } } if self.bit_range.width == 0 { diff --git a/src/svd/peripheral.rs b/src/svd/peripheral.rs index c59b513f..2addf046 100644 --- a/src/svd/peripheral.rs +++ b/src/svd/peripheral.rs @@ -18,6 +18,7 @@ use crate::svd::{ #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct Peripheral { /// The string identifies the peripheral. Peripheral names are required to be unique for a device pub name: String, @@ -65,10 +66,6 @@ pub struct Peripheral { #[cfg_attr(feature = "serde", serde(default))] #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))] pub derived_from: Option, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, Default, PartialEq)] @@ -148,18 +145,20 @@ impl PeripheralBuilder { default_register_properties: self.default_register_properties, registers: self.registers, derived_from: self.derived_from, - _extensible: (), }) .validate() } } impl Peripheral { + #[allow(clippy::unnecessary_wraps)] fn validate(self) -> Result { // TODO + #[cfg(feature = "strict")] check_dimable_name(&self.name, "name")?; - if let Some(name) = self.derived_from.as_ref() { - check_dimable_name(name, "derivedFrom")?; + if let Some(_name) = self.derived_from.as_ref() { + #[cfg(feature = "strict")] + check_dimable_name(_name, "derivedFrom")?; } else if let Some(registers) = self.registers.as_ref() { if registers.is_empty() { #[cfg(feature = "strict")] diff --git a/src/svd/registerinfo.rs b/src/svd/registerinfo.rs index 0e8e3ce9..dc5dc674 100644 --- a/src/svd/registerinfo.rs +++ b/src/svd/registerinfo.rs @@ -17,6 +17,7 @@ use crate::svd::{ #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct RegisterInfo { /// String to identify the register. /// Register names are required to be unique within the scope of a peripheral @@ -76,10 +77,6 @@ pub struct RegisterInfo { #[cfg_attr(feature = "serde", serde(default))] #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))] pub modified_write_values: Option, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } #[derive(Clone, Debug, Default, PartialEq)] @@ -172,23 +169,28 @@ impl RegisterInfoBuilder { fields: self.fields, write_constraint: self.write_constraint, modified_write_values: self.modified_write_values, - _extensible: (), }) .validate() } } impl RegisterInfo { + #[allow(clippy::unnecessary_wraps)] fn validate(self) -> Result { + #[cfg(feature = "strict")] check_dimable_name(&self.name, "name")?; - if let Some(name) = self.alternate_group.as_ref() { - check_name(name, "alternateGroup")?; - } - if let Some(name) = self.alternate_register.as_ref() { - check_dimable_name(name, "alternateRegister")?; + #[cfg(feature = "strict")] + { + if let Some(name) = self.alternate_group.as_ref() { + check_name(name, "alternateGroup")?; + } + if let Some(name) = self.alternate_register.as_ref() { + check_dimable_name(name, "alternateRegister")?; + } } - if let Some(name) = self.derived_from.as_ref() { - check_derived_name(name, "derivedFrom")?; + if let Some(_name) = self.derived_from.as_ref() { + #[cfg(feature = "strict")] + check_derived_name(_name, "derivedFrom")?; } else if let Some(fields) = self.fields.as_ref() { if fields.is_empty() { #[cfg(feature = "strict")] diff --git a/src/svd/registerproperties.rs b/src/svd/registerproperties.rs index b446813e..731b8959 100644 --- a/src/svd/registerproperties.rs +++ b/src/svd/registerproperties.rs @@ -13,6 +13,7 @@ use crate::svd::access::Access; /// Register default properties #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[derive(Clone, Copy, Debug, Default, PartialEq)] +#[non_exhaustive] pub struct RegisterProperties { /// Default bit-width of any register #[cfg_attr(feature = "serde", serde(default))] @@ -33,10 +34,6 @@ pub struct RegisterProperties { #[cfg_attr(feature = "serde", serde(default))] #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))] pub access: Option, - - // Reserve the right to add more fields to this struct - #[cfg_attr(feature = "serde", serde(skip))] - _extensible: (), } impl Parse for RegisterProperties { @@ -44,11 +41,12 @@ impl Parse for RegisterProperties { type Error = anyhow::Error; fn parse(tree: &Element) -> Result { - let mut p = RegisterProperties::default(); - p.size = parse::optional::("size", tree)?; - p.reset_value = parse::optional::("resetValue", tree)?; - p.reset_mask = parse::optional::("resetMask", tree)?; - p.access = parse::optional::("access", tree)?; + let p = RegisterProperties { + size: parse::optional::("size", tree)?, + reset_value: parse::optional::("resetValue", tree)?, + reset_mask: parse::optional::("resetMask", tree)?, + access: parse::optional::("access", tree)?, + }; check_reset_value(p.size, p.reset_value, p.reset_mask)?; Ok(p) } diff --git a/src/types.rs b/src/types.rs index 57127315..355bccb3 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,4 +1,5 @@ //! Shared primitive types for use in SVD objects. +#![allow(clippy::manual_strip)] use xmltree::Element;