Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve NpmSpec parsing #116

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
4 changes: 3 additions & 1 deletion ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@ ChangeLog

* Add support for Django 3.1
* Add support for Python 3.7 / 3.8 / 3.9
* `#115 <https://github.com/rbarrois/python-semanticversion/issues/98>`_:
Improve ``NpmSpec`` compatibility with official ``node-semver`` implementation by allowing some whitespace before the version (e.g. ``> 1.2.3``, ``1.2.3 - 4.5.6``) and adding ``~>`` as an alias for ``~``.


2.8.5 (2020-04-29)
------------------

*Bugfix:*

* `98 <https://github.com/rbarrois/python-semanticversion/issues/98>`_:
* `#98 <https://github.com/rbarrois/python-semanticversion/issues/98>`_:
Properly handle wildcards in ``SimpleSpec`` (e.g. ``==1.2.*``).


Expand Down
25 changes: 19 additions & 6 deletions semantic_version/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1230,15 +1230,21 @@ class Parser:

NUMBER = r'x|X|\*|0|[1-9][0-9]*'
PART = r'[a-zA-Z0-9.-]*'
OP = r'~>|<=|>=|>|<|\^|~|='
NPM_SPEC_BLOCK = re.compile(r"""
^(?:v)? # Strip optional initial v
(?P<op><|<=|>=|>|=|\^|~|) # Operator, can be empty
(?P<op>{op}|) # Operator, can be empty
(?P<major>{nb})(?:\.(?P<minor>{nb})(?:\.(?P<patch>{nb}))?)?
(?:-(?P<prerel>{part}))? # Optional re-release
(?:\+(?P<build>{part}))? # Optional build
$""".format(nb=NUMBER, part=PART),
$""".format(nb=NUMBER, part=PART, op=OP),
re.VERBOSE,
)
OP_RE = re.compile(r"""
^(?:{op})$ # A standalone operator, cannot be empty
""".format(op=OP),
re.VERBOSE,
)

@classmethod
def range(cls, operator, target):
Expand All @@ -1256,15 +1262,21 @@ def parse(cls, expression):
subclauses = []
if cls.HYPHEN in group:
low, high = group.split(cls.HYPHEN, 2)
subclauses = cls.parse_simple('>=' + low) + cls.parse_simple('<=' + high)
subclauses = cls.parse_simple('>=' + low.strip()) + cls.parse_simple('<=' + high.strip())

else:
blocks = group.split(' ')
blocks = group.split()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now splits across multiple whitespace, but also allows for newlines (so it would allow >0.1.2 \n<3.4.5)
Would it be worth raising an error at the start if the expression contains newlines?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this is not worth

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you're referring to "not worth raising an error for newlines", as opposed to allowing multiple spaces between the subclauses?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you're referring to "not worth raising an error for newlines"

Correct. How many times are there new lines across all the package.json anyway?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I wouldn't expect it as an issue, just a slight nitpick of my own code where it's different to the current npm implementation.

maybe_prepend_op = None
for block in blocks:
if not cls.NPM_SPEC_BLOCK.match(block):
block = maybe_prepend_op + block if maybe_prepend_op else block
if cls.NPM_SPEC_BLOCK.match(block):
subclauses.extend(cls.parse_simple(block))
maybe_prepend_op = None
elif maybe_prepend_op is None and cls.OP_RE.match(block):
maybe_prepend_op = block
else:
raise ValueError("Invalid NPM block in %r: %r" % (expression, block))

subclauses.extend(cls.parse_simple(block))

prerelease_clauses = []
non_prerel_clauses = []
Expand Down Expand Up @@ -1314,6 +1326,7 @@ def parse(cls, expression):

PREFIX_ALIASES = {
'': PREFIX_EQ,
'~>': PREFIX_TILDE,
}

PREFIX_TO_OPERATOR = {
Expand Down
70 changes: 70 additions & 0 deletions tests/test_npm.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,35 @@ def test_spec(self):
self.assertNotIn(base.Version(version), base.NpmSpec(spec))

expansions = {
# Basic
'>1.2.3': '>1.2.3',
'<1.2.3': '<1.2.3',
'<=1.2.3': '<=1.2.3',

# Hyphen ranges
'1.2.3 - 2.3.4': '>=1.2.3 <=2.3.4',
'1.2 - 2.3.4': '>=1.2.0 <=2.3.4',
'1.2.3 - 2.3': '>=1.2.3 <2.4.0',
'1.2.3 - 2': '>=1.2.3 <3',
'1.2.3 - 2': '>=1.2.3 <3',

# X-Ranges
'*': '>=0.0.0',
'>=*': '>=0.0.0',
'1.x': '>=1.0.0 <2.0.0',
'1.2.x': '>=1.2.0 <1.3.0',
'': '*',
'x': '*',
'1': '1.x.x',
'1.x.x': '>=1.0.0 <2.0.0',
'1.2': '1.2.x',

# Partial GT LT Ranges
'>=1': '>=1.0.0',
'>1': '>=2.0.0',
'>1.2': '>=1.3.0',
'<1': '<1.0.0',

# Tilde ranges
'~1.2.3': '>=1.2.3 <1.3.0',
'~1.2': '>=1.2.0 <1.3.0',
Expand All @@ -89,6 +103,10 @@ def test_spec(self):
'~0.2': '>=0.2.0 <0.3.0',
'~0': '>=0.0.0 <1.0.0',
'~1.2.3-beta.2': '>=1.2.3-beta.2 <1.3.0',
'~ 1.2.3': '>=1.2.3 <1.3.0',
'~ 1.2.3': '>=1.2.3 <1.3.0',
'~>1.2.3': '>=1.2.3 <1.3.0',
'~> 1.2.3': '>=1.2.3 <1.3.0',

# Caret ranges
'^1.2.3': '>=1.2.3 <2.0.0',
Expand All @@ -101,6 +119,23 @@ def test_spec(self):
'^0.0': '>=0.0.0 <0.1.0',
'^1.x': '>=1.0.0 <2.0.0',
'^0.x': '>=0.0.0 <1.0.0',
'^0': '>=0.0.0 <1.0.0',
'^ 1.2.3': '>=1.2.3 <2.0.0',
'^ 1.2.3': '>=1.2.3 <2.0.0',

# Weird whitespace ranges
'>= 1.2.3': '>=1.2.3',
'>=\t1.2.3': '>=1.2.3',
'>= 1.2.3': '>=1.2.3',
'>=1.2.3 <2.0.0': '>=1.2.3 <2.0.0',
' >=1.2.3 <2.0.0 ': '>=1.2.3 <2.0.0',
'>= 1.2.3 < 2.0.0': '>=1.2.3 <2.0.0',
'>=1.2.3 < 2.0.0': '>=1.2.3 <2.0.0',
'>= 1.2.3 < 2.0.0': '>=1.2.3 <2.0.0',
'>= 1.2.3 < 2.0.0': '>=1.2.3 <2.0.0',
' >=1.2.3 <2.0.0 ': '>=1.2.3 <2.0.0',
'1.2.7 || >=1.2.9 <2.0.0': '1.2.7 || >=1.2.9 <2.0.0',
'1.2.7 || >= 1.2.9 < 2.0.0': '1.2.7 || >=1.2.9 <2.0.0',
}

def test_expand(self):
Expand All @@ -110,3 +145,38 @@ def test_expand(self):
base.NpmSpec(source).clause,
base.NpmSpec(expanded).clause,
)

equivalent_npmspecs = {
'||': '*',
#'>1.2.3 || *': '*',
#'>=1.2.1 <=2.3.4': '>1.2.0 <2.3.5',
}

def test_equivalent(self):
# like expand, but we also simplify both sides
# NOTE: some specs can be equivalent but don't
# currently simplify to the same clauses
for left, right in self.equivalent_npmspecs.items():
with self.subTest(l=left, r=right):
self.assertEqual(
base.NpmSpec(left).clause.simplify(),
base.NpmSpec(right).clause.simplify(),
)

invalid_npmspecs = [
'==0.1.2',
'>>0.1.2',
'> = 0.1.2',
'<=>0.1.2',
'~1.2.3beta',
'~=1.2.3',
'>01.02.03',
'!0.1.2',
'!=0.1.2',
]

def test_invalid(self):
for invalid in self.invalid_npmspecs:
with self.subTest(spec=invalid):
with self.assertRaises(ValueError, msg="NpmSpec(%r) should be invalid" % invalid):
base.NpmSpec(invalid)