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.

Sign up for GitHub

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
Open

Improve NpmSpec parsing #116

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f4ce03c
Add tests to handle weird whitespace
gnattishness Jul 4, 2021
7b00926
Test for spaces between simple spec clauses.
gnattishness Jul 10, 2021
77ef19a
NpmSpec parsing to allow spaces between simple clauses.
gnattishness Jul 10, 2021
2fc126d
NpmSpec to allow whitespace after operator.
gnattishness Jul 10, 2021
4f72d2f
Add more tests for invalid, weird npm expansions.
gnattishness Jul 10, 2021
3afef0d
Fix mistake with op matching part of the string.
gnattishness Jul 10, 2021
912cc8a
Add more tests, separate runner for ones requiring simplify
gnattishness Jul 10, 2021
2e21f5a
Add '~>' as an alias for '~'.
gnattishness Jul 10, 2021
12d67c8
Update changelog.
gnattishness Jul 10, 2021
34320c7
Update semantic_version/base.py
gnattishness May 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ChangeLog
View file
Open in desktop
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
View file
Open in desktop
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'<|<=|=|>=|>|\^|~|~>'
gnattishness marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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)