Presubmit error for bad delimiters

This change introduces a new error for license fields that use any of
the following `["/", ";", " and ", " or "]`.

I chose to include the offending character/s in the error message
because I find it easier to parse error messages that tell me exactly
which character is the bad one. Similarly I've included conditions in
the reason to handle the plural cases correctly, generating either:

`License contains a bad delimiter character ...`, or
`License contains bad delimiter characters ...`

I realise this means that any downstream rules looking to detect this
error will need to check for a common subset, e.g 'bad delimiter
character', however I think it's worth it for the improved user
experience of receiving the error.

I've also anticipated that most of these errors will be due to
situations where multiple licenses are offered, and included additional
text explaining that only the most permissive of the choices should be
included.

This will affect 9 dependencies and they need to choose between multiple licenses anyway so it's okay to generate an error and have partybug file bugs.

Bug: http://b/374850412
Change-Id: I6eb53a8a3bd541a1801dff133884b719dcdfe04d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6181848
Reviewed-by: Jiewei Qian <qjw@chromium.org>
Reviewed-by: Rachael Newitt <renewitt@google.com>
Commit-Queue: Jordan Brown <rop@google.com>

metadata/fields/custom/license.py

@@ -11,6 +11,9 @@ from typing import List, Tuple, Optional
 _THIS_DIR = os.path.abspath(os.path.dirname(__file__))
 # The repo's root directory.
 _ROOT_DIR = os.path.abspath(os.path.join(_THIS_DIR, "..", "..", ".."))
+# Bad delimiter characters.
+BAD_DELIMITERS = ["/", ";", " and ", " or "]
+BAD_DELIMITERS_REGEX = re.compile("|".join(re.escape(delimiter) for delimiter in BAD_DELIMITERS))
 
 # Add the repo's root directory for clearer imports.
 sys.path.insert(0, _ROOT_DIR)
@@ -92,6 +95,15 @@ class LicenseField(field_types.SingleLineTextField):
       if util.is_empty(license):
         return vr.ValidationError(
                     reason=f"{self._name} has an empty value.")
+      if BAD_DELIMITERS_REGEX.search(license):
+        return vr.ValidationError(
+                reason=f"{self._name} contains a bad license separator. "
+                "Separate licenses by commas only.",
+                # Try and preemptively address the root cause of this behaviour,
+                # which is having multiple choices for a license.
+                additional=[f"When given a choice of licenses, chose the most "
+                            "permissive one, do not list all options."]
+        )
       if not allowed:
         not_allowlisted.append(license)
 

metadata/tests/fields_test.py

@@ -123,10 +123,17 @@ class FieldValidationTest(unittest.TestCase):
                 "APSL-2.0, MIT",
                 "APSL-2.0 ,MIT",
             ],
-            error_values=["", "\n", ",", "Apache 2.0 ,"],
+            error_values=[
+                "",
+                "\n",
+                ",",
+                "Apache 2.0 ,",
+                "Custom / MIT",
+                "Apache-2.0 and MIT",
+                "Apache-2.0; MIT; BSD-2-Clause",
+            ],
             warning_values=[
                 "Custom license",
-                "Custom / MIT",
                 "Custom, MIT",
             ],
         )