From 03101cb30e8a606123c87a7904b09d1090ff02a9 Mon Sep 17 00:00:00 2001
From: Ira Hopkinson <ira_hopkinson@sil.org>
Date: Thu, 6 Aug 2026 16:11:52 +1200
Subject: [PATCH] fix `VerseRef.verseNum` setter mis-port
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The setter assigned the backing field and nothing else, marked with a `ToDo`
placeholder. It was missing both of the other things the C# `VerseRef.VerseNum`
setter does:

    if (value < 0)
        throw new VerseRefException("VerseNum can not be negative");
    verseNum = (short)value;
    verse = null;

So negative verse numbers were accepted, and assigning `verseNum` left the
range/segment string in place. Setting `verseNum = 9` on `LUK 3:4b-5a` left a
stale `4b-5a` in the `verse` getter and `hasMultiple` still `true`.

Uses `this._verse = undefined` rather than `null`, per the repo convention of
preferring `undefined` for missing values. The C# `(short)` cast is not
replicated.

The C# test that covers the range-clearing is `CopyVerseFrom`, which sets
`VerseNum = 9` on a `LUK 3:4b-6a` source and then asserts the copied `Verse` is
`"9"`. That test cannot be ported yet — `copyVerseFrom` is not implemented in
this port — so the cases are added as TS-only tests instead: the negative
guard, the zero boundary, and the clearing of both a range and a segment.

`BuildVerseRefByProps` already exercised `verseNum = 0/15/17` and still passes;
those are plain numbers with no verse string to clear.

Behaviour change for consumers: unlike the sibling `chapterNum` setter fixed in
#58, this setter worked before, so code reading `verse`/`hasMultiple` after
assigning `verseNum` will see different results.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 CLAUDE.md             |  3 ---
 src/verse-ref.test.ts | 34 ++++++++++++++++++++++++++++++++++
 src/verse-ref.ts      |  5 ++++-
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index 2952a3e..3fae34a 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -138,9 +138,6 @@ porting a test — several `VerseRefTests` cases can't pass yet, and it's not ob
   needs `Versification.getLastBook/getLastChapter/getLastVerse`, which needs the `.vrs`
   versification data ported — see the stalled `improve-verseref` branch.
 - **`VerseRef.isExcluded`** is hardcoded `return false` with a `TODO`.
-- **`VerseRef.set verseNum`** is a mis-port still carrying a `ToDo`: it assigns the backing field
-  but omits the C# negative guard _and_ the `verse = null` that clears a range string. So setting
-  `verseNum` on `'LUK 3:4b-5a'` leaves a stale `'4b-5a'` in the `verse` getter.
 - **`BBBCCCVVVS`** is declared but not implemented.
 - **The numeric constructor bypasses the setters**, assigning `_bookNum`/`_chapterNum`/`_verseNum`
   directly. So the C# `Invalid` test's constructor-throws cases (e.g. `new VerseRef(2, -42, 1)`)
diff --git a/src/verse-ref.test.ts b/src/verse-ref.test.ts
index 2247c0e..29687c0 100644
--- a/src/verse-ref.test.ts
+++ b/src/verse-ref.test.ts
@@ -223,6 +223,40 @@ describe('VerseRef', () => {
         vref.chapterNum = 0;
         expect(vref.chapterNum).toEqual(0);
       });
+
+      it('should throw when verseNum is negative', () => {
+        const vref = new VerseRef('LUK 3:4', ScrVers.English);
+        expect(() => {
+          vref.verseNum = -1;
+        }).toThrow(VerseRefException);
+      });
+
+      it('should not throw when verseNum is zero', () => {
+        const vref = new VerseRef('LUK 3:4', ScrVers.English);
+        vref.verseNum = 0;
+        expect(vref.verseNum).toEqual(0);
+      });
+
+      it('should clear a verse range when verseNum is set', () => {
+        const vref = new VerseRef('LUK', '3', '4b-5a', ScrVers.English);
+        expect(vref.verse).toEqual('4b-5a');
+        expect(vref.hasMultiple).toBe(true);
+
+        vref.verseNum = 9;
+
+        expect(vref.verseNum).toEqual(9);
+        expect(vref.verse).toEqual('9');
+        expect(vref.hasMultiple).toBe(false);
+      });
+
+      it('should clear a verse segment when verseNum is set', () => {
+        const vref = new VerseRef('LUK', '3', '4b', ScrVers.English);
+        expect(vref.verse).toEqual('4b');
+
+        vref.verseNum = 9;
+
+        expect(vref.verse).toEqual('9');
+      });
     });
 
     describe('String', () => {
diff --git a/src/verse-ref.ts b/src/verse-ref.ts
index 13aa0bd..dea454d 100644
--- a/src/verse-ref.ts
+++ b/src/verse-ref.ts
@@ -353,8 +353,11 @@ export class VerseRef {
     return this._verseNum;
   }
   set verseNum(value: number) {
-    // ToDo: replace or remove this placeholder
+    if (value < 0) {
+      throw new VerseRefException('VerseNum can not be negative');
+    }
     this._verseNum = value;
+    this._verse = undefined;
   }
 
   /**