[BUG] range.bnf incomplete regarding whitespace and "spermies"

[gnattishness]

What / Why

There are some discrepancies between what ranges are valid according to the BNF definition and the implementation.
As far as I'm aware, this implementation (or the BNF) is the "canonical" definition for NPM version ranges.

Every range abiding by the BNF is valid according to the implementation, but the implementation also accepts ranges as valid that are not in the BNF.

Where

• Current implementation (master branch - e79ac3a)
• BNF definition in docs: https://docs.npmjs.com/cli/v6/using-npm/semver#range-grammar
• BNF definition in repo: https://github.com/npm/node-semver/blob/master/range.bnf

How

Current Behavior

Implementation allows for whitespace within a "simple" range expression at the start of a "partial", but this does not abide by the BNF grammar.

Steps to Reproduce

Using latest release:

const semver = require('semver')

console.log(semver.validRange('>= 1.2.3 < 1.6.0', {loose: false} )) // >=1.2.3 <1.6.0
console.log(semver.validRange('^ 1.2.3 || ^ 2.4.6', {loose: false})) // >=1.2.3 <1.6.0

And tests involving

node-semver/test/fixtures/range-parse.js

Lines 28 to 34 in e79ac3a

	['> 1.0.0', '>1.0.0'],
	['> 1.0.0', '>1.0.0'],
	['<= 2.0.0', '<=2.0.0'],
	['<= 2.0.0', '<=2.0.0'],
	['<= 2.0.0', '<=2.0.0'],
	['< 2.0.0', '<2.0.0'],
	['<\t2.0.0', '<2.0.0'],

and

node-semver/test/fixtures/range-parse.js

Lines 57 to 60 in e79ac3a

	['~>1', '>=1.0.0 <2.0.0-0'],
	['~> 1', '>=1.0.0 <2.0.0-0'],
	['~1.0', '>=1.0.0 <1.1.0-0'],
	['~ 1.0', '>=1.0.0 <1.1.0-0'],

pass, but are not valid according to the BNF.

In particular, there is no whitespace defined between comparison operators and the number:

node-semver/range.bnf

Lines 6 to 9 in e79ac3a

	primitive ::= ( '<' | '>' | '>=' | '<=' | '=' ) partial
	partial ::= xr ( '.' xr ( '.' xr qualifier ? )? )?
	xr ::= 'x' | 'X' | '*' | nr
	nr ::= '0' | [1-9] ( [0-9] ) *

and the tilde definition does not allow for >:

node-semver/range.bnf

Line 10 in e79ac3a

tilde ::= '~' partial

Expected Behavior

Either:

1. The BNF definition is expanded to allow whitespace between

2. The implementation is restricted to match the BNF (e.g. only accept the ranges as valid when "loose"?)

   (this obviously has backwards compatibility issues)

3. Document clearly that the implementation can parse and accept ranges outside of those defined in the BNF. (Though, perhaps will only ever return canonical ranges that comply with the BNF?)

If updating the grammar, the following change to be sufficient:

partial    ::= ( ' ' )* xr ( '.' xr ( '.' xr qualifier ? )? )?
...
tilde      ::= '~' ('>')? partial

Though the range definition could probably also be updated to reflect how multiple spaces are allowed:

range      ::= hyphen | simple ( ( ' ' )+ simple ) * | ''

[gnattishness]

How would you like to proceed?
I'm happy to submit a PR to change the BNF if that's the way to go.
I'm not particularly familiar with semver ranges, and am basing the issue on this repo alone.
(I'm not aware of any other spec floating around that is the "true" definition for semver ranges)

[gnattishness]

Just bumping here.

This is relevant to other projects that make use of NPM-style version ranges or must parse package.json files.
Currently, a parser implemented according to the range.bnf would fail to parse ranges that are accepted by node-semver (so it would error reading supposedly "valid" package.json files)

Shouldn't we want a parser that implements the range.bnf to be compatible with node-semver?

If one is to interpret the range.bnf as the specification for what node-semver sees as "valid" ranges, the specification is clearly incomplete/inaccurate.
As it is, the bnf is only useful for those wanting to encode ranges such that node-semver can read them, not the other way around.

[pombredanne]

@gnattishness I guess one way to interpret this is that the grammar is not a spec but only a nice but incomplete documentation artifact and rather that only the actual implemented JS code here is the "spec"?

[pombredanne]

@isaacs you wrote this grammar, so what's your take on this?

[gnattishness]

@pombredanne Agreed, the code appears more a "reference implementation" that the BNF was made to reflect.

Though it could also be that (as in 3) the BNF defines a "canonical representation" that node-semver will output, rather than all the possibilities it would accept.
In that case, I'd prefer that expected behavior be well defined and documented - perhaps with separate BNF grammars describing the values treated as valid, and their respective "canonical" forms.