fix: parsing local version with digit followed by non-digits (#3032)
When parsing the local identifier segment of `<digit><letter>` the
parser would
give an error saying the letter was unexpected.
What was happening was `accept_digits()` consumed up to the first
non-digit, and
considered this success, which prevented calling `accept_alnum()` to
finish
the parsing.
To fix, only call `accept_alnum()`, then post-process the value to
normalize
an all-digit segment.
I'm guessing `accept_digits()` stopping at the first non-digit is WAI
because
it expects to parse e.g. "3.14b", where the caller handles subsequent
characters.
Along the way, some minor doc improvements to the parser code.
Fixes https://github.com/bazel-contrib/rules_python/issues/3030
diff --git a/python/private/version.bzl b/python/private/version.bzl
index f98165d..8b5fef7 100644
--- a/python/private/version.bzl
+++ b/python/private/version.bzl
@@ -44,7 +44,13 @@
return lambda token: token in reference
def _ctx(start):
- return {"norm": "", "start": start}
+ """Creates a context, which is state for parsing (or sub-parsing)."""
+ return {
+ # The result value from parsing
+ "norm": "",
+ # Where in the parser's input string this context starts.
+ "start": start,
+ }
def _open_context(self):
"""Open an new parsing ctx.
@@ -60,7 +66,16 @@
return self.contexts[-1]
def _accept(self, key = None):
- """Close the current ctx successfully and merge the results."""
+ """Close the current ctx successfully and merge the results.
+
+ Args:
+ self: {type}`Parser}
+ key: {type}`str | None` the key to store the result in
+ the most recent context. If not set, the key is "norm".
+
+ Returns:
+ {type}`bool` always True
+ """
finished = self.contexts.pop()
self.contexts[-1]["norm"] += finished["norm"]
if key:
@@ -79,7 +94,14 @@
return False
def _new(input):
- """Create a new normalizer"""
+ """Create a new parser
+
+ Args:
+ input: {type}`str` input to parse
+
+ Returns:
+ {type}`Parser` a struct for a parser object.
+ """
self = struct(
input = input,
contexts = [_ctx(0)],
@@ -167,7 +189,7 @@
return parser.accept()
def accept_digits(parser):
- """Accept multiple digits (or placeholders).
+ """Accept multiple digits (or placeholders), up to a non-digit/placeholder.
Args:
parser: The normalizer.
@@ -275,13 +297,20 @@
Returns:
whether a separator and an alphanumeric string were accepted.
"""
- parser.open_context()
+ ctx = parser.open_context()
# PEP 440: Local version segments
- if (
- accept(parser, _in([".", "-", "_"]), ".") and
- (accept_digits(parser) or accept_alnum(parser))
- ):
+ if not accept(parser, _in([".", "-", "_"]), "."):
+ return parser.discard()
+
+ if accept_alnum(parser):
+ # First character is separator; skip it.
+ value = ctx["norm"][1:]
+
+ # PEP 440: Integer Normalization
+ if value.isdigit():
+ value = str(int(value))
+ ctx["norm"] = ctx["norm"][0] + value
return parser.accept()
return parser.discard()
diff --git a/tests/version/version_test.bzl b/tests/version/version_test.bzl
index 589f9ac..7ddb6cc 100644
--- a/tests/version/version_test.bzl
+++ b/tests/version/version_test.bzl
@@ -105,6 +105,14 @@
_tests.append(_test_normalization)
+def _test_normalize_local(env):
+ # Verify a local with a [digit][non-digit] sequence parses ok
+ in_str = "0.1.0+brt.9e"
+ actual = version.normalize(in_str)
+ env.expect.that_str(actual).equals(in_str)
+
+_tests.append(_test_normalize_local)
+
def _test_ordering(env):
want = [
# Taken from https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering