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