Update (#38)

* Fix 1.53 lints

This bumps the MSRV to 1.53, but this crate doesn't have a ton of
dependencies, and most of them are using 0.3.x in any case.

* Fixup changelog

* Replace difference with similar-asserts

* Add MSRV to README + CI

* Update CHANGELOG

* Remove unneeded ignore

Author: Jake-Shadle

.github/workflows/ci.yaml

@@ -34,6 +34,19 @@ jobs:
       - uses: actions/checkout@v2
       - uses: EmbarkStudios/cargo-deny-action@v1
 
+  msrv-check:
+    name: Minimum Stable Rust Version Check
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: "1.53.0"
+          override: true
+      - run: cargo fetch
+      - name: cargo check
+        run: cargo check --all-targets
+
   test:
     name: Test
     strategy:

CHANGELOG.md

@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
+### Changed
+- [PR#38](https://github.com/EmbarkStudios/spdx/pull/38) fixed various clippy lints which also bumps the MSRV to [1.53.0](https://blog.rust-lang.org/2021/06/17/Rust-1.53.0.html). Previously, PR#37 had bumped the MSRV to 1.52 so now this crate will check the MSRV so changes are intentional.
+- [PR#38](https://github.com/EmbarkStudios/spdx/pull/38) replaced the unmaintained `difference` crate with `similar-asserts`.
+
 ## [0.4.1] - 2021-06-14
 ### Changed
 - [PR#37](https://github.com/EmbarkStudios/spdx/pull/37) removed the dependencies on regex and lazy_static used for parsing some license expression parts, which gives a nice compile speed up with no behavior changes. Thanks [@Swagadon](https://github.com/Swagadon)!
@@ -51,9 +55,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - The `GFDL*` licenses are now marked as copyleft
 
 ### Fixed
-- When creating a `LicenseReq` from a GNU license, the license identifier is converted into its base form,
-eg. `GPL-2.0-or-later` becomes `GPL-2.0+` so that the GNU style license identifiers can be used just the same
-as all of the other ones. See [this issue](https://github.com/EmbarkStudios/cargo-deny/issues/55)
+- When creating a `LicenseReq` from a GNU license, the license identifier is converted into its base form, eg. `GPL-2.0-or-later` becomes `GPL-2.0+` so that the GNU style license identifiers can be used just the same as all of the other ones. See [this issue](https://github.com/EmbarkStudios/cargo-deny/issues/55)
 
 ## [0.2.4] - 2019-11-25
 ### Added

Cargo.toml

@@ -19,7 +19,9 @@ include = [
 ]
 
 [dependencies]
+# In most cases expressions are quite small so we can avoid heap allocations
 smallvec = "1.6"
 
 [dev-dependencies]
-difference = "2.0"
+# Used to print colored diffs in case of test failures
+similar-asserts = "1.1"

README.md

@@ -4,6 +4,7 @@
 [![Embark](https://img.shields.io/badge/discord-ark-%237289da.svg?logo=discord)](https://discord.gg/dAuKfZS)
 [![Crates.io](https://img.shields.io/crates/v/spdx.svg)](https://crates.io/crates/spdx)
 [![Docs](https://docs.rs/spdx/badge.svg)](https://docs.rs/spdx)
+[![Minimum Stable Rust Version](https://img.shields.io/badge/Rust-1.53.0-blue?color=fc8d62&logo=rust)](https://blog.rust-lang.org/2021/06/17/Rust-1.53.0.html)
 [![SPDX Version](https://img.shields.io/badge/SPDX%20Version-3.11-blue.svg)](https://spdx.org/licenses/)
 [![dependency status](https://deps.rs/repo/github/EmbarkStudios/spdx/status.svg)](https://deps.rs/repo/github/EmbarkStudios/spdx)
 [![Build Status](https://github.com/EmbarkStudios/spdx/workflows/CI/badge.svg)](https://github.com/EmbarkStudios/spdx/actions?workflow=CI)

deny.toml

@@ -4,8 +4,6 @@ vulnerability = "deny"
 unsound = "deny"
 yanked = "deny"
 ignore = [
-    # difference is unmaintained but it works and is only a dev dependency
-    "RUSTSEC-2020-0095",
 ]
 
 [bans]

src/expression/mod.rs

@@ -87,7 +87,7 @@ impl Expression {
     pub fn requirements(&self) -> impl Iterator<Item = &ExpressionReq> {
         self.expr.iter().filter_map(|item| match item {
             ExprNode::Req(req) => Some(req),
-            _ => None,
+            ExprNode::Op(_op) => None,
         })
     }
 

src/expression/parser.rs

@@ -76,13 +76,11 @@ impl Expression {
 
         let make_err_for_token = |last_token: Option<Token<'_>>, span: std::ops::Range<usize>| {
             let expected: &[&str] = match last_token {
-                None | Some(Token::And) | Some(Token::Or) | Some(Token::OpenParen) => {
-                    &["<license>", "("]
-                }
+                None | Some(Token::And | Token::Or | Token::OpenParen) => &["<license>", "("],
                 Some(Token::CloseParen) => &["AND", "OR"],
                 Some(Token::Exception(_)) => &["AND", "OR", ")"],
                 Some(Token::Spdx(_)) => &["AND", "OR", "WITH", ")", "+"],
-                Some(Token::LicenseRef { .. }) | Some(Token::Plus) => &["AND", "OR", "WITH", ")"],
+                Some(Token::LicenseRef { .. } | Token::Plus) => &["AND", "OR", "WITH", ")"],
                 Some(Token::With) => &["<exception>"],
             };
 
@@ -98,7 +96,7 @@ impl Expression {
             let lt = tok?;
             match &lt.token {
                 Token::Spdx(id) => match last_token {
-                    None | Some(Token::And) | Some(Token::Or) | Some(Token::OpenParen) => {
+                    None | Some(Token::And | Token::Or | Token::OpenParen) => {
                         expr_queue.push(ExprNode::Req(ExpressionReq {
                             req: LicenseReq::from(*id),
                             span: lt.span.start as u32..lt.span.end as u32,
@@ -107,7 +105,7 @@ impl Expression {
                     _ => return make_err_for_token(last_token, lt.span),
                 },
                 Token::LicenseRef { doc_ref, lic_ref } => match last_token {
-                    None | Some(Token::And) | Some(Token::Or) | Some(Token::OpenParen) => {
+                    None | Some(Token::And | Token::Or | Token::OpenParen) => {
                         expr_queue.push(ExprNode::Req(ExpressionReq {
                             req: LicenseReq {
                                 license: LicenseItem::Other {
@@ -147,15 +145,17 @@ impl Expression {
                     _ => return make_err_for_token(last_token, lt.span),
                 },
                 Token::With => match last_token {
-                    Some(Token::Spdx(_)) | Some(Token::LicenseRef { .. }) | Some(Token::Plus) => {}
+                    Some(Token::Spdx(_) | Token::LicenseRef { .. } | Token::Plus) => {}
                     _ => return make_err_for_token(last_token, lt.span),
                 },
                 Token::Or | Token::And => match last_token {
-                    Some(Token::Spdx(_))
-                    | Some(Token::LicenseRef { .. })
-                    | Some(Token::CloseParen)
-                    | Some(Token::Exception(_))
-                    | Some(Token::Plus) => {
+                    Some(
+                        Token::Spdx(_)
+                        | Token::LicenseRef { .. }
+                        | Token::CloseParen
+                        | Token::Exception(_)
+                        | Token::Plus,
+                    ) => {
                         let new_op = match lt.token {
                             Token::Or => Op::Or,
                             Token::And => Op::And,
@@ -171,7 +171,7 @@ impl Expression {
 
                                         match top.op {
                                             Op::And | Op::Or => apply_op(top, &mut expr_queue)?,
-                                            _ => unreachable!(),
+                                            Op::Open => unreachable!(),
                                         }
                                     } else {
                                         break;
@@ -188,7 +188,7 @@ impl Expression {
                     _ => return make_err_for_token(last_token, lt.span),
                 },
                 Token::OpenParen => match last_token {
-                    None | Some(Token::And) | Some(Token::Or) | Some(Token::OpenParen) => {
+                    None | Some(Token::And | Token::Or | Token::OpenParen) => {
                         op_stack.push(OpAndSpan {
                             op: Op::Open,
                             span: lt.span,
@@ -198,11 +198,13 @@ impl Expression {
                 },
                 Token::CloseParen => {
                     match last_token {
-                        Some(Token::Spdx(_))
-                        | Some(Token::LicenseRef { .. })
-                        | Some(Token::Plus)
-                        | Some(Token::Exception(_))
-                        | Some(Token::CloseParen) => {
+                        Some(
+                            Token::Spdx(_)
+                            | Token::LicenseRef { .. }
+                            | Token::Plus
+                            | Token::Exception(_)
+                            | Token::CloseParen,
+                        ) => {
                             while let Some(top) = op_stack.pop() {
                                 match top.op {
                                     Op::And | Op::Or => apply_op(top, &mut expr_queue)?,
@@ -241,11 +243,13 @@ impl Expression {
 
         // Validate that the terminating token is valid
         match last_token {
-            Some(Token::Spdx(_))
-            | Some(Token::LicenseRef { .. })
-            | Some(Token::Exception(_))
-            | Some(Token::CloseParen)
-            | Some(Token::Plus) => {}
+            Some(
+                Token::Spdx(_)
+                | Token::LicenseRef { .. }
+                | Token::Exception(_)
+                | Token::CloseParen
+                | Token::Plus,
+            ) => {}
             // We have to have at least one valid license requirement
             None => {
                 return Err(ParseError {

src/lexer.rs

@@ -112,6 +112,7 @@ impl<'a> Lexer<'a> {
         }
     }
 
+    #[inline]
     fn is_ref_char(c: &char) -> bool {
         c.is_ascii_alphanumeric() || *c == '-' || *c == '.'
     }
@@ -134,6 +135,7 @@ impl<'a> Lexer<'a> {
     }
 
     /// Return a license ref if found - equivalent to the regex `^LicenseRef-([-a-zA-Z0-9.]+)`
+    #[inline]
     fn find_license_ref(text: &'a str) -> Option<&'a str> {
         Self::find_ref("LicenseRef-", text)
     }

src/lib.rs

@@ -309,7 +309,7 @@ impl LicenseItem {
     pub fn id(&self) -> Option<LicenseId> {
         match self {
             Self::Spdx { id, .. } => Some(*id),
-            _ => None,
+            Self::Other { .. } => None,
         }
     }
 }

src/licensee.rs

@@ -272,7 +272,7 @@ mod test {
         .license
         {
             LicenseItem::Spdx { id, .. } => assert_eq!(*id, mpl_id),
-            o => panic!("unexepcted {:?}", o),
+            o @ LicenseItem::Other { .. } => panic!("unexpected {:?}", o),
         }
     }
 
@@ -356,7 +356,7 @@ mod test {
             .license
             {
                 LicenseItem::Spdx { id, .. } => assert_eq!(*id, lic_id),
-                o => panic!("unexepcted {:?}", o),
+                o @ LicenseItem::Other { .. } => panic!("unexpected {:?}", o),
             }
         }
     }

tests/validation.rs

@@ -8,19 +8,10 @@ macro_rules! test_validate {
 
             $(
                 let actual = reqs.next().unwrap();
-                println!("{:?}", actual);
-
                 let actual_str = format!("{}", actual.1.req);
                 let expected_str = $expected;
 
-                if actual_str != expected_str {
-                    assert!(
-                        false,
-                        "failed @ index {} - {}",
-                        actual.0,
-                        difference::Changeset::new(expected_str, &actual_str, " ")
-                    );
-                }
+                similar_asserts::assert_eq!(actual_str, expected_str, "failed @ index {}", actual.0);
             )+
 
             if let Some((_, additional)) = reqs.next() {
@@ -40,15 +31,7 @@ macro_rules! err {
             reason: spdx::error::Reason::$reason,
         };
 
-        if act_err != expected {
-            let act_text = format!("{:?}", act_err);
-            let exp_text = format!("{:?}", expected);
-            assert!(
-                false,
-                "{}",
-                difference::Changeset::new(&exp_text, &act_text, "")
-            );
-        }
+        similar_asserts::assert_eq!(act_err, expected);
     };
 
     ($text:expr => $unexpected:expr; $range:expr) => {
@@ -60,15 +43,7 @@ macro_rules! err {
             reason: spdx::error::Reason::Unexpected($unexpected),
         };
 
-        if act_err != expected {
-            let act_text = format!("{:?}", act_err);
-            let exp_text = format!("{:?}", expected);
-            assert!(
-                false,
-                "{}",
-                difference::Changeset::new(&exp_text, &act_text, "")
-            );
-        }
+        similar_asserts::assert_eq!(act_err, expected);
     };
 }