From bf456eeea317d5b8b76be71b5e2c3026e269b827 Mon Sep 17 00:00:00 2001
From: Ira Hopkinson <ira_hopkinson@sil.org>
Date: Tue, 4 Aug 2026 11:37:22 +1200
Subject: [PATCH] fix `VerseRef.chapterNum` setter infinite recursion

The setter assigned to itself (`this.chapterNum = value`) instead of the
backing field, so any assignment recursed until `RangeError: Maximum call
stack size exceeded`. It was marked with a `ToDo` placeholder.

Ported from the C# `VerseRef.ChapterNum` setter, which guards against
negative values before assigning:

    if (value < 0)
        throw new VerseRefException("ChapterNum can not be negative");
    chapterNum = (short)value;

Tests port `BuildVerseRefByProps` from SIL.Scripture.Tests, which builds a
VerseRef entirely through property setters and so covers this directly. One
assertion is commented out: chapter 0 is out of range in the C#, but
`internalValid()` cannot detect that until the versification port lands.

Also adds TS-only tests for the negative guard and the zero boundary, since
the C# has no direct test for the setter's validation.

The `-1` sentinel path is unaffected: `setEmpty()` and the `chapter` string
setter write the backing field directly, exactly as in the C#.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/verse-ref.test.ts | 69 ++++++++++++++++++++++++++++++++++++++++++-
 src/verse-ref.ts      |  6 ++--
 2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/src/verse-ref.test.ts b/src/verse-ref.test.ts
index ee4d8cc..2247c0e 100644
--- a/src/verse-ref.test.ts
+++ b/src/verse-ref.test.ts
@@ -1,5 +1,5 @@
 import { ScrVers } from './scr-vers';
-import { SerializedVerseRef, VerseRef } from './verse-ref';
+import { SerializedVerseRef, VerseRef, VerseRefException } from './verse-ref';
 
 describe('VerseRef', () => {
   const RTL_MARKER = '\u200F';
@@ -142,6 +142,58 @@ describe('VerseRef', () => {
     });
   });
 
+  describe('Build VerseRef by Props', () => {
+    it('should build a VerseRef by setting properties', () => {
+      let vref = new VerseRef();
+      vref.versification = ScrVers.English;
+      expect(vref.validStatus).toEqual(VerseRef.ValidStatusType.OutOfRange);
+      expect(vref.BBBCCCVVV).toEqual(0);
+
+      vref.bookNum = 13;
+      // Chapter 0 is out of range in the C#, but `internalValid()` can't detect that until the
+      // versification port lands (see the TODO in `verse-ref.ts`).
+      // expect(vref.validStatus).toEqual(VerseRef.ValidStatusType.OutOfRange);
+      expect(vref.BBBCCCVVV).toEqual(13000000);
+      expect(vref.bookNum).toEqual(13);
+      expect(vref.chapterNum).toEqual(0);
+      expect(vref.verseNum).toEqual(0);
+
+      vref.chapterNum = 1;
+      vref.verseNum = 0;
+      expect(vref.valid).toBe(true);
+      expect(vref.BBBCCCVVV).toEqual(13001000);
+      expect(vref.bookNum).toEqual(13);
+      expect(vref.chapterNum).toEqual(1);
+      expect(vref.verseNum).toEqual(0);
+
+      vref.chapterNum = 14;
+      vref.verseNum = 15;
+      expect(vref.valid).toBe(true);
+      expect(vref.BBBCCCVVV).toEqual(13014015);
+      expect(vref.bookNum).toEqual(13);
+      expect(vref.chapterNum).toEqual(14);
+      expect(vref.verseNum).toEqual(15);
+
+      vref = new VerseRef();
+      vref.versification = ScrVers.English;
+      vref.chapterNum = 16;
+      expect(vref.validStatus).toEqual(VerseRef.ValidStatusType.OutOfRange);
+      expect(vref.BBBCCCVVV).toEqual(16000);
+      expect(vref.bookNum).toEqual(0);
+      expect(vref.chapterNum).toEqual(16);
+      expect(vref.verseNum).toEqual(0);
+
+      vref = new VerseRef();
+      vref.versification = ScrVers.English;
+      vref.verseNum = 17;
+      expect(vref.validStatus).toEqual(VerseRef.ValidStatusType.OutOfRange);
+      expect(vref.BBBCCCVVV).toEqual(17);
+      expect(vref.bookNum).toEqual(0);
+      expect(vref.chapterNum).toEqual(0);
+      expect(vref.verseNum).toEqual(17);
+    });
+  });
+
   describe('Chapter and Verse as Empty Strings', () => {
     it('should handle empty chapter and verse', () => {
       const vref = new VerseRef('LUK', '', '', ScrVers.Septuagint);
@@ -158,6 +210,21 @@ describe('VerseRef', () => {
 
   // Tests that don't exist in the C#.
   describe('Extra (TS-only tests)', () => {
+    describe('Property setters', () => {
+      it('should throw when chapterNum is negative', () => {
+        const vref = new VerseRef('LUK 3:4', ScrVers.English);
+        expect(() => {
+          vref.chapterNum = -1;
+        }).toThrow(VerseRefException);
+      });
+
+      it('should not throw when chapterNum is zero', () => {
+        const vref = new VerseRef('LUK 3:4', ScrVers.English);
+        vref.chapterNum = 0;
+        expect(vref.chapterNum).toEqual(0);
+      });
+    });
+
     describe('String', () => {
       it('should convert to empty string', () => {
         const vref = new VerseRef();
diff --git a/src/verse-ref.ts b/src/verse-ref.ts
index 1a1dfb1..13aa0bd 100644
--- a/src/verse-ref.ts
+++ b/src/verse-ref.ts
@@ -340,8 +340,10 @@ export class VerseRef {
     return this._chapterNum;
   }
   set chapterNum(value: number) {
-    // ToDo: replace or remove this placeholder
-    this.chapterNum = value;
+    if (value < 0) {
+      throw new VerseRefException('ChapterNum can not be negative');
+    }
+    this._chapterNum = value;
   }
 
   /**