fix(parser): keep the pre-release label when normalizing versions

[pucedoteth]

Follow-up to #588, as invited there. That PR fixed the digit-splicing case and deliberately left the bare suffix out of scope; this handles it.

Stacked on #588. It branches from that PR's head, so it currently shows both commits. Once #588 merges this reduces to the single commit fix(parser): keep the pre-release label when normalizing versions. Both PRs touch the same two files. Please merge #588 first.

Addresses the issue behind #588.

The bug

versionCheck strips every letter from a version string. #588 stopped 3.11.0rc2 from collapsing into 3.11.02 (= 3.11.2) by rewriting a letter run between two digits as a pre-release separator. A label with no trailing digit never matches ([0-9])[A-Za-z]+([0-9]), so it still falls through to the letter-stripping step:

versionCheck("3.11.0rc")    => "3.11.0"
versionCheck("3.11.0alpha") => "3.11.0"
versionCheck("3.11.0beta")  => "3.11.0"

A release candidate becomes indistinguishable from its own release. An advisory rule of version < "3.11.0" therefore reports a target running 3.11.0rc as unaffected — a false negative on exactly the pre-release builds most likely to still carry the bug.

The fix

As you noted, widening the match to make the trailing digits optional would emit a dangling 3.11.0-. Instead the label is lifted out before the letters are stripped, then re-appended:

var preReleaseSuffixRe = regexp.MustCompile(`([0-9])-?([A-Za-z]+)([0-9]*)$`)

pre := ""
if m := preReleaseSuffixRe.FindStringSubmatch(version); m != nil {
    pre = m[2] + m[3]
    version = preReleaseSuffixRe.ReplaceAllString(version, "$1")
}
// ... existing normalization ...
if pre != "" {
    newVersion += "-" + pre
}

Keeping the label itself, rather than only its number, also fixes ordering between labels — something the previous form could not express, since 3.11.0a1 and 3.11.0rc1 both normalized to 3.11.0-1 and compared equal:

3.11.0-alpha < 3.11.0-beta < 3.11.0-rc < 3.11.0 < 3.11.1
3.11.0-a1 < 3.11.0-rc1

Two deliberate limits on the match:

• The separator may be - but not ., so 1.2.3.RELEASE keeps normalizing to 1.2.3.0. That spelling denotes a final release; treating it as a pre-release would wrongly sort it before 1.2.3.
• The match is anchored at the end, so only a trailing label is treated as a pre-release.

One behaviour change to flag

This changes output for versions #588 already handled:

input	#588	this PR
3.11.0rc2	3.11.0-2	3.11.0-rc2
1.2.3b1	1.2.3-1	1.2.3-b1
1.2.3-rc1	1.2.3-1	1.2.3-rc1

Ordering relative to the release is unchanged in every case — the label is simply preserved rather than discarded. The existing expectations are updated and kept as regression guards, per your request. This also fixes 1.2.3-rc, which previously normalized to the degenerate 1.2.3-.

Tests

As requested, the bare-suffix cases (rc, alpha, beta) plus the existing digit-suffixed cases are all present as regression guards, with ordering assertions against 3.11.0 and 3.11.1:

• TestVersionCheckPreRelease — value table, extended with bare-suffix labels, the already-hyphenated spellings, and the .RELEASE guard.
• TestVersionCheckPreReleaseOrdering — new. Asserts every normalized pre-release parses under hashicorp/go-version and sorts before 3.11.0 and 3.11.1, including alpha < beta < rc.
• TestAdvisoryEvalPreRelease — unchanged from fix(parser): keep pre-release versions below their release in versionCheck #588, still passing.

All pass, along with go vet and gofmt.

One note on how I verified: the full module's dependency download kept timing out on this machine, so I ran synax.go and synax_test.go in an isolated module with only hashicorp/go-version and a stubbed gologger (which versionCheck does not touch). Every test in the file passes there, and I separately cross-checked the value and ordering tables against the real hashicorp/go-version. CI will be the first run against the full tree.

[pucedoteth]

Verification update, closing the caveat in the PR description.

I said there that I could only run these in an isolated module, because the full dependency download kept timing out on my machine, and that CI would be the first run against the real tree. That has now completed locally. The full package builds and passes:

$ go test ./common/fingerprints/parser/ -v
--- PASS: TestVersionCheckPreRelease
--- PASS: TestVersionCheckPreReleaseOrdering
--- PASS: TestAdvisoryEvalPreRelease
...
PASS
ok   github.com/Tencent/AI-Infra-Guard/common/fingerprints/parser  16.683s

31 tests pass, 0 fail, against the real parser package with gologger and pkg/httpx in place rather than stubbed. go vet ./common/fingerprints/parser/ is clean.

Nothing about the change moved; this only replaces the isolated-module result with a real one.

[boy-hack]

@pucedoteth Thanks for the clean follow-up. The approach is correct and an improvement over #588:

• Label carried through (3.11.0rc -> 3.11.0-rc, 1.2.3-rc -> 1.2.3-rc fixing the degenerate 1.2.3-) is the right call, and keeping the label itself also fixes inter-label ordering (3.11.0a1 < 3.11.0rc1), which the digit-only form couldn't express.
• The two limits are well chosen: anchoring at $ means only a trailing label is treated as pre-release, and excluding . as the separator keeps 1.2.3.RELEASE -> 1.2.3.0 (a final release, not a pre-release). Both are covered by the test table.
• Test coverage is strong: TestVersionCheckPreRelease (value table, extended with bare suffixes + hyphenated spellings + the .RELEASE guard), TestVersionCheckPreReleaseOrdering (real hashicorp/go-version parse + alpha < beta < rc ordering), and TestAdvisoryEvalPreRelease (end-to-end).

Behavior change to confirm: inputs #588 already handled now normalize differently — 3.11.0rc2 -> 3.11.0-rc2 (was 3.11.0-2), 1.2.3b1 -> 1.2.3-b1. Ordering relative to the release is identical in every case, so this is strictly better (it preserves the label). The updated expectations as regression guards capture it correctly.

One ask: this PR is currently based on main and shows two commits (it includes #588's commit, since #588 isn't merged yet). After #588 merges, please rebase #626 onto main so it reduces to the single follow-up commit and we don't carry a duplicate of #588's change. The regex is redefined rather than duplicated, so there's no compile conflict — it's just commit hygiene.

Looks good otherwise. Ready to merge after the rebase.

(Review comment only — not merging.)

[pucedoteth]

Rebased onto main and force-pushed — head is now 633e186.

It reduces to the single follow-up commit you asked for; abdba59 is gone now that #588 is in as b437e46.

git range-diff:
  -: -------  > 1: abdba59  fix(parser): keep pre-release versions below their release
  1: 633e186  = 2: bda9ed5  fix(parser): keep the pre-release label when normalizing versions

The = is the part worth pointing at: the follow-up commit is unchanged from what you reviewed, so nothing needs a second look.

One note, since the file diff moved even though my commit did not. main gained error handling in AdvisoryEval while this sat waiting — vv.Must(vv.NewVersion(text)) became a checked v2, err := vv.NewVersion(text). The rebase carries that in from upstream rather than reverting it, which is what you want; it is just why the diff against the old tip is not empty.

go test ./common/fingerprints/parser/... on the rebased head:

--- PASS: TestVersionCheckPreRelease
--- PASS: TestVersionCheckPreReleaseOrdering
--- PASS: TestAdvisoryEvalPreRelease
ok  github.com/Tencent/AI-Infra-Guard/common/fingerprints/parser

[boy-hack]

Rebase confirmed, thanks. Head 633e186 now reduces to the single follow-up commit (the old #588 commit abdba59 is gone), and the rebase correctly carried in the upstream AdvisoryEval error-handling change (v2, err := vv.NewVersion(text)) rather than reverting it — exactly what we wanted.

The diff itself is unchanged from what I reviewed, so nothing needs a second look:

• Bare-suffix labels are now preserved (3.11.0rc → 3.11.0-rc, 1.2.3-rc → 1.2.3-rc, fixing the degenerate 1.2.3-).
• Inter-label ordering (alpha < beta < rc, 3.11.0a1 < 3.11.0rc1) is now expressible — strictly better than the digit-only form.
• The two anchoring limits (trailing $, no . separator so 1.2.3.RELEASE → 1.2.3.0) are intact and guarded by the test table.
• TestVersionCheckPreReleaseOrdering (real hashicorp/go-version parse + ordering) plus the value and advisory-eval guards all pass on the rebased head.

Behavior change to note for the changelog: versions #588 already handled now normalize with the label kept (3.11.0rc2 → 3.11.0-rc2 instead of 3.11.0-2; 1.2.3b1 → 1.2.3-b1). Ordering relative to the release is identical in every case, so this is a strict improvement and the updated expectations capture it as regression guards.

LGTM — ready to merge after #588. (Review comment only, not merging.)