Fallback to more lax parsing of SPDX expressions

examples/04_gnu_licenses/Cargo.toml

@@ -2,7 +2,7 @@
 name = "gnu-licenses"
 version = "0.1.0"
 authors = ["Jake Shadle <jake.shadle@embark-studios.com>"]
-license = "GPL-2.0-or-later AND LGPL-3.0-only"
+license = "Apache-2.0/GPL-2.0+ AND LGPL-3.0-only"
 edition = "2018"
 
 [[bin]]

examples/04_gnu_licenses/deny.toml

@@ -1,3 +1,3 @@
 [licenses]
-allow = [ "GPL-3.0" ]
+allow = ["GPL-3.0", "LGPL-3.0"]
 copyleft = "deny"

src/licenses/gather.rs

@@ -583,10 +583,16 @@ impl Gatherer {
                 match &krate.license {
                     Some(license_field) => {
                         // Reasons this can fail:
+                        //
                         // * Empty! The rust crate used to validate this field has a bug
                         // https://github.com/rust-lang-nursery/license-exprs/issues/23
                         // * It also just does basic lexing, so parens, duplicate operators,
                         // unpaired exceptions etc can all fail validation
+                        //
+                        // Note that these only apply to _old_ versions, as `spdx`
+                        // is now used by crates.io to validate, but it uses lax
+                        // rules to allow some license identifiers that aren't
+                        // technically correct
 
                         match spdx::Expression::parse(license_field) {
                             Ok(validated) => {
@@ -612,8 +618,36 @@ impl Gatherer {
 
                                 labels.push(
                                     Label::secondary(id, lic_span)
-                                        .with_message(format!("{}", err.reason)),
+                                        .with_message(err.reason.to_string()),
                                 );
+
+                                // If we fail strict parsing, attempt to use lax parsing,
+                                // though still emitting a warning so the user is aware
+                                if let Ok(validated) = spdx::Expression::parse_mode(
+                                    license_field,
+                                    spdx::ParseMode {
+                                        allow_lower_case_operators: true,
+                                        // We already force correct this when loading crates
+                                        allow_slash_as_or_operator: false,
+                                        allow_imprecise_license_names: true,
+                                        allow_postfix_plus_on_gpl: true,
+                                    },
+                                ) {
+                                    let (id, span) = get_span("license");
+
+                                    return KrateLicense {
+                                        krate,
+                                        lic_info: LicenseInfo::SpdxExpression {
+                                            expr: validated,
+                                            nfo: LicenseExprInfo {
+                                                file_id: id,
+                                                offset: span.start,
+                                                source: LicenseExprSource::Metadata,
+                                            },
+                                        },
+                                        labels,
+                                    };
+                                }
                             }
                         }
                     }

tests/licenses.rs

@@ -138,7 +138,7 @@ fn flags_unencountered_exceptions() {
     let cfg = tu::Config::new(
         "allow = ['MIT']
     unlicensed = 'allow'
-    exceptions = [{name='bippity-boopity-bop', allow = ['Aladdin']}]",
+    exceptions = [{name='bippity-boppity-boop', allow = ['Aladdin']}]",
     );
 
     // Override the warning to be a failure
@@ -152,3 +152,56 @@ fn flags_unencountered_exceptions() {
 
     insta::assert_json_snapshot!(diags);
 }
+
+/// Ensures that invalid SPDX expressions in strict mode can be parsed when
+/// falling back to more lax rules, but still output a warning
+#[test]
+fn lax_fallback() {
+    let mut cmd = krates::Cmd::new();
+    cmd.manifest_path("examples/04_gnu_licenses/Cargo.toml");
+
+    let krates: Krates = krates::Builder::new()
+        .build(cmd, krates::NoneFilter)
+        .unwrap();
+
+    let gatherer = licenses::Gatherer::default()
+        .with_store(store())
+        .with_confidence_threshold(0.8);
+
+    let mut files = codespan::Files::new();
+
+    let cfg: tu::Config<crate::licenses::cfg::Config> = tu::Config::new(
+        "allow = ['GPL-2.0', 'LGPL-3.0']
+    unlicensed = 'deny'",
+    );
+
+    let lic_cfg = {
+        let des: licenses::cfg::Config = toml::from_str(&cfg.config).unwrap();
+        let cfg_id = files.add("config.toml", cfg.config.clone());
+
+        let mut diags = Vec::new();
+        use cargo_deny::UnvalidatedConfig;
+        des.validate(cfg_id, &mut diags)
+    };
+
+    let summary = gatherer.gather(&krates, &mut files, Some(&lic_cfg));
+
+    let diags = tu::gather_diagnostics_with_files::<crate::licenses::cfg::Config, _, _>(
+        &krates,
+        "lax_fallback",
+        cfg,
+        files,
+        |ctx, _cs, tx| {
+            crate::licenses::check(
+                ctx,
+                summary,
+                diag::ErrorSink {
+                    overrides: None,
+                    channel: tx,
+                },
+            );
+        },
+    );
+
+    insta::assert_json_snapshot!(diags);
+}

tests/snapshots/licenses__flags_unencountered_exceptions.snap

@@ -12,7 +12,7 @@ expression: diags
           "column": 25,
           "line": 3,
           "message": "unmatched license exception",
-          "span": "'bippity-boopity-bop'"
+          "span": "'bippity-boppity-boop'"
         }
       ],
       "message": "license exception was not encountered",

tests/snapshots/licenses__lax_fallback.snap

@@ -0,0 +1,54 @@
+---
+source: tests/licenses.rs
+expression: diags
+---
+[
+  {
+    "fields": {
+      "code": "accepted",
+      "graphs": [
+        {
+          "Krate": {
+            "name": "gnu-licenses",
+            "version": "0.1.0"
+          }
+        }
+      ],
+      "labels": [
+        {
+          "column": 33,
+          "line": 4,
+          "message": "a GNU license was followed by a `+`",
+          "span": "+"
+        },
+        {
+          "column": 12,
+          "line": 4,
+          "message": "license expression retrieved via Cargo.toml `license`",
+          "span": "Apache-2.0 OR GPL-2.0+ AND LGPL-3.0-only"
+        },
+        {
+          "column": 12,
+          "line": 4,
+          "message": "rejected: not explicitly allowed",
+          "span": "Apache-2.0"
+        },
+        {
+          "column": 26,
+          "line": 4,
+          "message": "accepted: license is explicitly allowed",
+          "span": "GPL-2.0"
+        },
+        {
+          "column": 39,
+          "line": 4,
+          "message": "accepted: license is explicitly allowed",
+          "span": "LGPL-3.0-only"
+        }
+      ],
+      "message": "license requirements satisfied",
+      "severity": "help"
+    },
+    "type": "diagnostic"
+  }
+]