npm / node-semver

The semver parser for node (the one npm uses)
ISC License
5.09k stars 492 forks source link

[BUG] BNF does not match the implementation #665

Open ShuiRuTian opened 11 months ago

ShuiRuTian commented 11 months ago

Is there an existing issue for this?

Current Behavior

semver.valid('1.2.3-00') // 'null'
semver.valid('1.2.3+00') // '1.2.3'

However, according to the BNF

qualifier  ::= ( '-' pre )? ( '+' build )?
pre        ::= parts
build      ::= parts
parts      ::= part ( '.' part ) *
part       ::= nr | [-0-9A-Za-z]+

pre and build are just the same.

Expected Behavior

The behavior of program is correct, but BNF is not.

We need to update the BNF form.

Steps To Reproduce

No response

Environment

mbtools commented 9 months ago

Agree. Numeric parts of a pre-release must not have leading zero. Also the part definition isn't BNF but a regex.

How about this?

alphanum   ::= ( ['0'-'9'] | ['A'-'Z'] | ['a'-'z'] | '-' ) +
pre        ::= ( nr | alphanum ) ( '.' ( nr | alphanum ) ) *
build      ::= alphanum ( '.' alphanum ) *

PS: This isn't perfect since alphanum could be a just numbers starting with a zero but the definition makes the difference between pre-release and build much clearer.

ShuiRuTian commented 8 months ago

Sorry for the late response. I was on Chinese Spring Festival.

It looks good to me!

And just out of curious, why we did not keep it same with(or close to) Semver?

mbtools commented 8 months ago

Because it was invented 10 years ago. I'm working on a update to a formal EBNF that will settle several issues (PR after my vacation)