From 190c23fa418fc525d6ae3b02525a3d0b6c8eec68 Mon Sep 17 00:00:00 2001
From: Ee Durbin <ewdurbin@gmail.com>
Date: Fri, 13 Sep 2024 10:22:14 -0400
Subject: [PATCH] Apply suggestions from code review

Co-authored-by: Brett Cannon <brett@python.org>
---
 src/packaging/licenses/__init__.py | 15 ++++++---------
 tests/test_metadata.py             |  8 ++++++--
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/packaging/licenses/__init__.py b/src/packaging/licenses/__init__.py
index df07b0c5..9c65c818 100644
--- a/src/packaging/licenses/__init__.py
+++ b/src/packaging/licenses/__init__.py
@@ -49,18 +49,17 @@ def normalize_license_expression(raw_license_expression: str) -> str | None:
         if ref.lower().startswith("licenseref-")
     }
 
-    # First normalize to lower case so we can look up licenses/exceptions
-    # and so boolean operators are Python-compatible
+    # First, normalize to lower case so we can look up licenses/exceptions
+    # and so boolean operators are Python-compatible.
     license_expression = raw_license_expression.lower()
 
-    # Then pad parentheses so tokenization can be achieved by merely splitting on
-    # white space
+    # Pad any parentheses so tokenization can be achieved by merely splitting on
+    # white space.
     license_expression = license_expression.replace("(", " ( ").replace(")", " ) ")
 
-    # Now we begin parsing
     tokens = license_expression.split()
 
-    # Rather than implementing boolean logic we create an expression that Python can
+    # Rather than implementing boolean logic, we create an expression that Python can
     # parse. Everything that is not involved with the grammar itself is treated as
     # `False` and the expression should evaluate as such.
     python_tokens = []
@@ -85,7 +84,7 @@ def normalize_license_expression(raw_license_expression: str) -> str | None:
         message = f"invalid license expression: {raw_license_expression}"
         raise ValueError(message) from None
 
-    # Take a final pass to check for unknown licenses/exceptions
+    # Take a final pass to check for unknown licenses/exceptions.
     normalized_tokens = []
     for token in tokens:
         if token in {"or", "and", "with", "(", ")"}:
@@ -119,8 +118,6 @@ def normalize_license_expression(raw_license_expression: str) -> str | None:
                     cast(str, LICENSES[final_token]["id"]) + suffix
                 )
 
-    # Construct the normalized expression
     normalized_expression = " ".join(normalized_tokens)
 
-    # Fix internal padding for parentheses
     return normalized_expression.replace("( ", "(").replace(" )", ")")
diff --git a/tests/test_metadata.py b/tests/test_metadata.py
index 322eaf74..6421cdd8 100644
--- a/tests/test_metadata.py
+++ b/tests/test_metadata.py
@@ -685,7 +685,7 @@ def test_valid_license_expression(self, license_expression, expected):
         "license_expression",
         [
             "Use-it-after-midnight",
-            "Apache-2.0 OR 2-BSD-Clause",
+            "Apache-2.0 OR 2-BSD-Clause",  # "2-BSD-Clause is not a valid license.
             "LicenseRef-License with spaces",
             "LicenseRef-License_with_underscores",
             "or",
@@ -700,7 +700,7 @@ def test_valid_license_expression(self, license_expression, expected):
             "(mit",
             "mit)",
             "mit or or apache-2.0",
-            "mit or apache-2.0 (bsd-3-clause and MPL-2.0)",
+            "mit or apache-2.0 (bsd-3-clause and MPL-2.0)",  # Missing an operator before `(`.
         ],
     )
     def test_invalid_license_expression(self, license_expression):
@@ -728,10 +728,14 @@ def test_valid_license_files(self, license_files):
     @pytest.mark.parametrize(
         "license_files",
         [
+            # Can't escape out of the project's directory.
             ["../LICENSE"],
             ["./../LICENSE"],
+            # Paths should be resolved.
             ["licenses/../LICENSE"],
+            # Absolute paths are not allowed.
             ["/licenses/LICENSE"],
+            # Paths must be valid (i.e. glob pattern didn't escape out of pyproject.toml.
             ["licenses/*"],
         ],
     )