BigInt, rollover, and developer lint

spec/unit/utils.spec.js

@@ -289,30 +289,30 @@ describe("utils", function() {
 
     describe('baseToString', () => {
         it('should calculate the appropriate string from numbers', () => {
-            expect(baseToString(10)).toEqual(DEFAULT_ALPHABET[10]);
-            expect(baseToString(10, "abcdefghijklmnopqrstuvwxyz")).toEqual('k');
-            expect(baseToString(6241)).toEqual("ab");
-            expect(baseToString(53, "abcdefghijklmnopqrstuvwxyz")).toEqual('cb');
+            expect(baseToString(BigInt(10))).toEqual(DEFAULT_ALPHABET[10]);
+            expect(baseToString(BigInt(10), "abcdefghijklmnopqrstuvwxyz")).toEqual('k');
+            expect(baseToString(BigInt(6241))).toEqual("ab");
+            expect(baseToString(BigInt(53), "abcdefghijklmnopqrstuvwxyz")).toEqual('cb');
         });
     });
 
     describe('stringToBase', () => {
         it('should calculate the appropriate number for a string', () => {
-            expect(stringToBase(" ")).toEqual(0);
-            expect(stringToBase("a", "abcdefghijklmnopqrstuvwxyz")).toEqual(0);
-            expect(stringToBase("a")).toEqual(65);
-            expect(stringToBase("c", "abcdefghijklmnopqrstuvwxyz")).toEqual(2);
-            expect(stringToBase("ab")).toEqual(6241);
-            expect(stringToBase("cb", "abcdefghijklmnopqrstuvwxyz")).toEqual(53);
+            expect(stringToBase(" ")).toEqual(BigInt(0));
+            expect(stringToBase("a", "abcdefghijklmnopqrstuvwxyz")).toEqual(BigInt(0));
+            expect(stringToBase("a")).toEqual(BigInt(65));
+            expect(stringToBase("c", "abcdefghijklmnopqrstuvwxyz")).toEqual(BigInt(2));
+            expect(stringToBase("ab")).toEqual(BigInt(6241));
+            expect(stringToBase("cb", "abcdefghijklmnopqrstuvwxyz")).toEqual(BigInt(53));
         });
     });
 
     describe('averageBetweenStrings', () => {
         it('should average appropriately', () => {
-            expect(averageBetweenStrings('A', 'z')).toEqual('^');
-            expect(averageBetweenStrings('a', 'z', "abcdefghijklmnopqrstuvwxyz")).toEqual('n');
+            expect(averageBetweenStrings('A', 'z')).toEqual(']');
+            expect(averageBetweenStrings('a', 'z', "abcdefghijklmnopqrstuvwxyz")).toEqual('m');
             expect(averageBetweenStrings('AA', 'zz')).toEqual('^.');
-            expect(averageBetweenStrings('aa', 'zz', "abcdefghijklmnopqrstuvwxyz")).toEqual('na');
+            expect(averageBetweenStrings('aa', 'zz', "abcdefghijklmnopqrstuvwxyz")).toEqual('mz');
             expect(averageBetweenStrings('cat', 'doggo')).toEqual("d9>Cw");
             expect(averageBetweenStrings('cat', 'doggo', "abcdefghijklmnopqrstuvwxyz")).toEqual("cumqh");
         });
@@ -354,6 +354,53 @@ describe("utils", function() {
                 midpoint = next;
             }
         });
+
+        it('should roll over', () => {
+            const lastAlpha = DEFAULT_ALPHABET[DEFAULT_ALPHABET.length - 1];
+            const firstAlpha = DEFAULT_ALPHABET[0];
+            const secondAlpha = DEFAULT_ALPHABET[1];
+
+            // You may be looking at this and wondering how on this planet we end up with
+            // the next string being +2 on the ASCII table, and this comment is here to tell
+            // you that you're not insane. Due to a property of the baseN conversion, our
+            // +1 (and -1 for prevString) turns into +2 because the first character in the
+            // alphabet is equivalent to zero rather than one. Thus, we're actually adding
+            // the second character of the alphabet (due to adding a numeric 1) to the
+            // input string, thus resulting in a human-understandable +2 jump rather than
+            // a +1 one.
+
+            // Let's validate that +1 behaviour with math
+            expect(stringToBase(DEFAULT_ALPHABET[0])).toEqual(BigInt(0));
+            expect(stringToBase(DEFAULT_ALPHABET[1])).toEqual(BigInt(1));
+
+            const highRoll = secondAlpha + firstAlpha;
+            const lowRoll = lastAlpha;
+
+            expect(nextString(lowRoll)).toEqual(highRoll);
+            expect(prevString(highRoll)).toEqual(lowRoll);
+        });
+
+        it('should be reversible on small strings', () => {
+            // Large scale reversibility is tested for max space order value
+            const input = "cats";
+            expect(prevString(nextString(input))).toEqual(input);
+        });
+
+        // We want to explicitly make sure that Space order values are supported and roll appropriately
+        it('should properly handle rolling over at 50 characters', () => {
+            // Note: we also test reversibility of large strings here.
+
+            // See rollover test for why we use weird parts of the alphabet
+            const maxSpaceValue = DEFAULT_ALPHABET[DEFAULT_ALPHABET.length - 1].repeat(50);
+            const fiftyFirstChar = DEFAULT_ALPHABET[1] + DEFAULT_ALPHABET[0].repeat(50);
+
+            expect(nextString(maxSpaceValue)).toBe(fiftyFirstChar);
+            expect(prevString(fiftyFirstChar)).toBe(maxSpaceValue);
+
+            // We're testing that the rollover happened, which means that the next string come before
+            // the maximum space order value lexicographically.
+            expect(lexicographicCompare(maxSpaceValue, fiftyFirstChar) > 0).toBe(true);
+        });
     });
 
     describe('lexicographicCompare', () => {

src/@types/requests.ts

@@ -68,18 +68,21 @@ export interface IEventSearchOpts {
     term: string;
 }
 
+// allow camelcase as these are things go onto the wire
+/* eslint-disable camelcase */
 export interface ICreateRoomOpts {
-    room_alias_name?: string; // eslint-disable-line camelcase
+    room_alias_name?: string;
     visibility?: "public" | "private";
     name?: string;
     topic?: string;
     preset?: string;
-    power_level_content_override?: any;// eslint-disable-line camelcase
-    creation_content?: any;// eslint-disable-line camelcase
-    initial_state?: {type: string, state_key: string, content: any}[]; // eslint-disable-line camelcase
+    power_level_content_override?: any;
+    creation_content?: any;
+    initial_state?: {type: string, state_key: string, content: any}[];
     // TODO: Types (next line)
-    invite_3pid?: any[]; // eslint-disable-line camelcase
+    invite_3pid?: any[];
 }
+/* eslint-enable camelcase */
 
 export interface IRoomDirectoryOptions {
     server?: string;

src/NamespacedValue.ts

@@ -19,16 +19,14 @@ limitations under the License.
  * is provided that the stable prefix should be used when representing the identifier.
  */
 export class NamespacedValue<S extends string, U extends string> {
-    public constructor(public readonly stable: S, public readonly unstable?: U) {
+    // Stable is optional, but one of the two parameters is required, hence the weird-looking types.
+    // Goal is to to have developers explicitly say there is no stable value (if applicable).
+    public constructor(public readonly stable: S | null | undefined, public readonly unstable?: U) {
         if (!this.unstable && !this.stable) {
             throw new Error("One of stable or unstable values must be supplied");
         }
     }
 
-    public get tsType(): U | S {
-        return null; // irrelevant return
-    }
-
     public get name(): U | S {
         if (this.stable) {
             return this.stable;

src/client.ts

@@ -7777,9 +7777,8 @@ export class MatrixClient extends EventEmitter {
             UNSTABLE_MSC3089_TREE_SUBTYPE.name);
 
         if (!createEvent) throw new Error("Expected single room create event");
-        if (!purposeEvent) return null;
 
-        if (!purposeEvent.getContent()?.[UNSTABLE_MSC3088_ENABLED.name]) return null;
+        if (!purposeEvent?.getContent()?.[UNSTABLE_MSC3088_ENABLED.name]) return null;
         if (createEvent.getContent()?.[RoomCreateTypeField] !== RoomType.Space) return null;
 
         return new MSC3089TreeSpace(this, roomId);

src/models/MSC3089TreeSpace.ts

@@ -90,6 +90,8 @@ export class MSC3089TreeSpace {
      * Whether or not this is a top level space.
      */
     public get isTopLevel(): boolean {
+        // XXX: This is absolutely not how you find out if the space is top level
+        // but is safe for a managed usecase like we offer in the SDK.
         const parentEvents = this.room.currentState.getStateEvents(EventType.SpaceParent);
         if (!parentEvents?.length) return true;
         return parentEvents.every(e => !e.getContent()?.['via']);
@@ -377,6 +379,8 @@ export class MSC3089TreeSpace {
         const content = currentChild?.getContent() ?? { via: [this.client.getDomain()] };
         await this.client.sendStateEvent(parentRoom.roomId, EventType.SpaceChild, {
             ...content,
+
+            // TODO: Safely constrain to 50 character limit required by spaces.
             order: newOrder,
         }, this.roomId);
     }

src/utils.ts

@@ -457,7 +457,7 @@ export function getCrypto(): Object {
     return crypto;
 }
 
-// String averaging based upon https://stackoverflow.com/a/2510816
+// String averaging inspired by https://stackoverflow.com/a/2510816
 // Dev note: We make the alphabet a string because it's easier to write syntactically
 // than arrays. Thankfully, strings implement the useful parts of the Array interface
 // anyhow.
@@ -492,16 +492,17 @@ export function alphabetPad(s: string, n: number, alphabet = DEFAULT_ALPHABET):
  * Converts a baseN number to a string, where N is the alphabet's length.
  *
  * This is intended for use with string averaging.
- * @param {number} n The baseN number.
+ * @param {bigint} n The baseN number.
  * @param {string} alphabet The alphabet to use as a single string.
  * @returns {string} The baseN number encoded as a string from the alphabet.
  */
-export function baseToString(n: number, alphabet = DEFAULT_ALPHABET): string {
-    const len = alphabet.length;
+export function baseToString(n: bigint, alphabet = DEFAULT_ALPHABET): string {
+    const len = BigInt(alphabet.length);
     if (n < len) {
-        return alphabet[n];
+        return alphabet[Number(n)];
     }
-    return baseToString(Math.floor(n / len), alphabet) + alphabet[n % len];
+
+    return baseToString(n / len, alphabet) + alphabet[Number(n % len)];
 }
 
 /**
@@ -510,15 +511,25 @@ export function baseToString(n: number, alphabet = DEFAULT_ALPHABET): string {
  * This is intended for use with string averaging.
  * @param {string} s The string to convert to a number.
  * @param {string} alphabet The alphabet to use as a single string.
- * @returns {number} The baseN number.
+ * @returns {bigint} The baseN number.
  */
-export function stringToBase(s: string, alphabet = DEFAULT_ALPHABET): number {
-    const len = alphabet.length;
-    const reversedStr = Array.from(s).reverse().join(""); // keep as string
-    let result = 0;
-    for (let i = 0; i < reversedStr.length; i++) {
-        // Cost effective version of `result += alphabet.indexOf(reversedStr[i]) * (len ** i);`
-        result += (reversedStr.charCodeAt(i) - alphabet.charCodeAt(0)) * (len ** i);
+export function stringToBase(s: string, alphabet = DEFAULT_ALPHABET): bigint {
+    const len = BigInt(alphabet.length);
+
+    // In our conversion to baseN we do a couple performance optimizations to avoid using
+    // excess CPU and such. To create baseN numbers, the input string needs to be reversed
+    // so the exponents stack up appropriately, as the last character in the unreversed
+    // string has less impact than the first character (in "abc" the A is a lot more important
+    // for lexicographic sorts). We also do a trick with the character codes to optimize the
+    // alphabet lookup, avoiding an index scan of `alphabet.indexOf(reversedStr[i])` - we know
+    // that the alphabet and (theoretically) the input string are constrained on character sets
+    // and thus can do simple subtraction to end up with the same result.
+
+    // Developer caution: we carefully cast to BigInt here to avoid losing precision. We cannot
+    // rely on Math.pow() (for example) to be capable of handling our insane numbers.
+    let result = BigInt(0);
+    for (let i = s.length - 1, j = BigInt(0); i >= 0; i--, j++) {
+        result += BigInt(s.charCodeAt(i) - alphabet.charCodeAt(0)) * (len ** j);
     }
     return result;
 }
@@ -536,7 +547,7 @@ export function averageBetweenStrings(a: string, b: string, alphabet = DEFAULT_A
     const padN = Math.max(a.length, b.length);
     const baseA = stringToBase(alphabetPad(a, padN, alphabet), alphabet);
     const baseB = stringToBase(alphabetPad(b, padN, alphabet), alphabet);
-    return baseToString(Math.round((baseA + baseB) / 2), alphabet);
+    return baseToString((baseA + baseB) / BigInt(2), alphabet);
 }
 
 /**
@@ -548,7 +559,7 @@ export function averageBetweenStrings(a: string, b: string, alphabet = DEFAULT_A
  * @returns {string} The string which follows the input string.
  */
 export function nextString(s: string, alphabet = DEFAULT_ALPHABET): string {
-    return baseToString(stringToBase(s, alphabet) + 1, alphabet);
+    return baseToString(stringToBase(s, alphabet) + BigInt(1), alphabet);
 }
 
 /**
@@ -560,7 +571,7 @@ export function nextString(s: string, alphabet = DEFAULT_ALPHABET): string {
  * @returns {string} The string which precedes the input string.
  */
 export function prevString(s: string, alphabet = DEFAULT_ALPHABET): string {
-    return baseToString(stringToBase(s, alphabet) - 1, alphabet);
+    return baseToString(stringToBase(s, alphabet) - BigInt(1), alphabet);
 }
 
 /**