Make sonar happier about our code & tests (#3388)

spec/integ/crypto.spec.ts

@@ -1111,7 +1111,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
         } catch (e) {
             expect((e as any).name).toEqual("UnknownDeviceError");
             expect([...(e as any).devices.keys()]).toEqual([aliceClient.getUserId()!]);
-            expect((e as any).devices.get(aliceClient.getUserId()!).has("DEVICE_ID"));
+            expect((e as any).devices.get(aliceClient.getUserId()!).has("DEVICE_ID")).toBeTruthy();
         }
 
         // mark the device as known, and resend.

spec/olm-loader.ts

@@ -23,5 +23,5 @@ try {
     global.Olm = require("@matrix-org/olm");
     logger.log("loaded libolm");
 } catch (e) {
-    logger.warn("unable to run crypto tests: libolm not available");
+    logger.warn("unable to run crypto tests: libolm not available", e);
 }

spec/unit/crypto.spec.ts

@@ -381,12 +381,7 @@ describe("Crypto", function () {
                     event.senderCurve25519Key = null;
                     // @ts-ignore private properties
                     event.claimedEd25519Key = null;
-                    try {
+                    await expect(bobClient.crypto!.decryptEvent(event)).rejects.toBeTruthy();
-                        await bobClient.crypto!.decryptEvent(event);
-                    } catch (e) {
-                        // we expect this to fail because we don't have the
-                        // decryption keys yet
-                    }
                 }),
             );
 
@@ -617,12 +612,7 @@ describe("Crypto", function () {
                     event.senderCurve25519Key = null;
                     // @ts-ignore private properties
                     event.claimedEd25519Key = null;
-                    try {
+                    await expect(secondAliceClient.crypto!.decryptEvent(event)).rejects.toBeTruthy();
-                        await secondAliceClient.crypto!.decryptEvent(event);
-                    } catch (e) {
-                        // we expect this to fail because we don't have the
-                        // decryption keys yet
-                    }
                 }),
             );
 
@@ -725,12 +715,7 @@ describe("Crypto", function () {
                     event.senderCurve25519Key = null;
                     // @ts-ignore private properties
                     event.claimedEd25519Key = null;
-                    try {
+                    await expect(bobClient.crypto!.decryptEvent(event)).rejects.toBeTruthy();
-                        await bobClient.crypto!.decryptEvent(event);
-                    } catch (e) {
-                        // we expect this to fail because we don't have the
-                        // decryption keys yet
-                    }
                 }),
             );
 
@@ -805,12 +790,7 @@ describe("Crypto", function () {
                     event.senderCurve25519Key = null;
                     // @ts-ignore private properties
                     event.claimedEd25519Key = null;
-                    try {
+                    await expect(bobClient.crypto!.decryptEvent(event)).rejects.toBeTruthy();
-                        await bobClient.crypto!.decryptEvent(event);
-                    } catch (e) {
-                        // we expect this to fail because we don't have the
-                        // decryption keys yet
-                    }
                 }),
             );
 
@@ -897,12 +877,7 @@ describe("Crypto", function () {
                     event.senderCurve25519Key = null;
                     // @ts-ignore private properties
                     event.claimedEd25519Key = null;
-                    try {
+                    await expect(bobClient.crypto!.decryptEvent(event)).rejects.toBeTruthy();
-                        await bobClient.crypto!.decryptEvent(event);
-                    } catch (e) {
-                        // we expect this to fail because we don't have the
-                        // decryption keys yet
-                    }
                 }),
             );
 

spec/unit/crypto/secrets.spec.ts

@@ -148,22 +148,14 @@ describe("Secrets", function () {
     it("should throw if given a key that doesn't exist", async function () {
         const alice = await makeTestClient({ userId: "@alice:example.com", deviceId: "Osborne2" });
 
-        try {
+        await expect(alice.storeSecret("foo", "bar", ["this secret does not exist"])).rejects.toBeTruthy();
-            await alice.storeSecret("foo", "bar", ["this secret does not exist"]);
-            // should be able to use expect(...).toThrow() but mocha still fails
-            // the test even when it throws for reasons I have no inclination to debug
-            expect(true).toBeFalsy();
-        } catch (e) {}
         alice.stopClient();
     });
 
     it("should refuse to encrypt with zero keys", async function () {
         const alice = await makeTestClient({ userId: "@alice:example.com", deviceId: "Osborne2" });
 
-        try {
+        await expect(alice.storeSecret("foo", "bar", [])).rejects.toBeTruthy();
-            await alice.storeSecret("foo", "bar", []);
-            expect(true).toBeFalsy();
-        } catch (e) {}
         alice.stopClient();
     });
 
@@ -214,10 +206,7 @@ describe("Secrets", function () {
     it("should refuse to encrypt if no keys given and no default key", async function () {
         const alice = await makeTestClient({ userId: "@alice:example.com", deviceId: "Osborne2" });
 
-        try {
+        await expect(alice.storeSecret("foo", "bar")).rejects.toBeTruthy();
-            await alice.storeSecret("foo", "bar");
-            expect(true).toBeFalsy();
-        } catch (e) {}
         alice.stopClient();
     });
 

spec/unit/crypto/verification/sas.spec.ts

@@ -144,7 +144,7 @@ describe("SAS verification", function () {
                                 expect(e.sas).toEqual(aliceSasEvent.sas);
                                 e.confirm();
                                 aliceSasEvent.confirm();
-                            } catch (error) {
+                            } catch {
                                 e.mismatch();
                                 aliceSasEvent.mismatch();
                             }
@@ -169,7 +169,7 @@ describe("SAS verification", function () {
                         expect(e.sas).toEqual(bobSasEvent.sas);
                         e.confirm();
                         bobSasEvent.confirm();
-                    } catch (error) {
+                    } catch {
                         e.mismatch();
                         bobSasEvent.mismatch();
                     }
@@ -519,7 +519,7 @@ describe("SAS verification", function () {
                                 expect(e.sas).toEqual(aliceSasEvent.sas);
                                 e.confirm();
                                 aliceSasEvent.confirm();
-                            } catch (error) {
+                            } catch {
                                 e.mismatch();
                                 aliceSasEvent.mismatch();
                             }
@@ -543,7 +543,7 @@ describe("SAS verification", function () {
                         expect(e.sas).toEqual(bobSasEvent.sas);
                         e.confirm();
                         bobSasEvent.confirm();
-                    } catch (error) {
+                    } catch {
                         e.mismatch();
                         bobSasEvent.mismatch();
                     }

spec/unit/room.spec.ts

@@ -1932,13 +1932,7 @@ describe("Room", function () {
         it("should allow retry on error", async function () {
             const client = createClientMock(new Error("server says no"));
             const room = new Room(roomId, client as any, null!, { lazyLoadMembers: true });
-            let hasThrown = false;
+            await expect(room.loadMembersIfNeeded()).rejects.toBeTruthy();
-            try {
-                await room.loadMembersIfNeeded();
-            } catch (err) {
-                hasThrown = true;
-            }
-            expect(hasThrown).toEqual(true);
 
             client.members.mockReturnValue({ chunk: [memberEvent] });
             await room.loadMembersIfNeeded();

spec/unit/webrtc/call.spec.ts

@@ -1054,14 +1054,7 @@ describe("Call", function () {
 
             mockSendEvent.mockReset();
 
-            let caught = false;
+            expect(() => call.reject()).toThrow();
-            try {
-                call.reject();
-            } catch (e) {
-                caught = true;
-            }
-
-            expect(caught).toEqual(true);
             expect(client.client.sendEvent).not.toHaveBeenCalled();
 
             call.hangup(CallErrorCode.UserHangup, true);

spec/unit/webrtc/groupCall.spec.ts

@@ -186,10 +186,7 @@ describe("Group Call", function () {
         it("sets state to local call feed uninitialized when getUserMedia() fails", async () => {
             jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockRejectedValue("Error");
 
-            try {
+            await expect(groupCall.initLocalCallFeed()).rejects.toBeTruthy();
-                await groupCall.initLocalCallFeed();
-            } catch (e) {}
-
             expect(groupCall.state).toBe(GroupCallState.LocalCallFeedUninitialized);
         });
 

src/autodiscovery.ts

@@ -142,7 +142,7 @@ export class AutoDiscovery {
             },
         };
 
-        if (!wellknown || !wellknown["m.homeserver"]) {
+        if (!wellknown?.["m.homeserver"]) {
             logger.error("No m.homeserver key in config");
 
             clientConfig["m.homeserver"].state = AutoDiscovery.FAIL_PROMPT;
@@ -171,7 +171,7 @@ export class AutoDiscovery {
 
         // Step 3: Make sure the homeserver URL points to a homeserver.
         const hsVersions = await this.fetchWellKnownObject(`${hsUrl}/_matrix/client/versions`);
-        if (!hsVersions || !hsVersions.raw?.["versions"]) {
+        if (!hsVersions?.raw?.["versions"]) {
             logger.error("Invalid /versions response");
             clientConfig["m.homeserver"].error = AutoDiscovery.ERROR_INVALID_HOMESERVER;
 
@@ -345,7 +345,7 @@ export class AutoDiscovery {
 
         const response = await this.fetchWellKnownObject(`https://${domain}/.well-known/matrix/client`);
         if (!response) return {};
-        return response.raw || {};
+        return response.raw ?? {};
     }
 
     /**

src/client.ts

@@ -36,7 +36,11 @@ import { StubStore } from "./store/stub";
 import { CallEvent, CallEventHandlerMap, createNewMatrixCall, MatrixCall, supportsMatrixCall } from "./webrtc/call";
 import { Filter, IFilterDefinition, IRoomEventFilter } from "./filter";
 import { CallEventHandlerEvent, CallEventHandler, CallEventHandlerEventHandlerMap } from "./webrtc/callEventHandler";
-import { GroupCallEventHandlerEvent, GroupCallEventHandlerEventHandlerMap } from "./webrtc/groupCallEventHandler";
+import {
+    GroupCallEventHandler,
+    GroupCallEventHandlerEvent,
+    GroupCallEventHandlerEventHandlerMap,
+} from "./webrtc/groupCallEventHandler";
 import * as utils from "./utils";
 import { replaceParam, QueryDict, sleep, noUnsafeEventProps, safeSet } from "./utils";
 import { Direction, EventTimeline } from "./models/event-timeline";
@@ -180,7 +184,6 @@ import { IThreepid } from "./@types/threepids";
 import { CryptoStore, OutgoingRoomKeyRequest } from "./crypto/store/base";
 import { GroupCall, IGroupCallDataChannelOptions, GroupCallIntent, GroupCallType } from "./webrtc/groupCall";
 import { MediaHandler } from "./webrtc/mediaHandler";
-import { GroupCallEventHandler } from "./webrtc/groupCallEventHandler";
 import { LoginTokenPostResponse, ILoginFlowsResponse, IRefreshTokenResponse, SSOAction } from "./@types/auth";
 import { TypedEventEmitter } from "./models/typed-event-emitter";
 import { MAIN_ROOM_TIMELINE, ReceiptType } from "./@types/read_receipts";
@@ -4087,27 +4090,23 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             queryString["server_name"] = opts.viaServers;
         }
 
-        try {
+        const data: IJoinRequestBody = {};
-            const data: IJoinRequestBody = {};
+        const signedInviteObj = await signPromise;
-            const signedInviteObj = await signPromise;
+        if (signedInviteObj) {
-            if (signedInviteObj) {
+            data.third_party_signed = signedInviteObj;
-                data.third_party_signed = signedInviteObj;
-            }
-
-            const path = utils.encodeUri("/join/$roomid", { $roomid: roomIdOrAlias });
-            const res = await this.http.authedRequest<{ room_id: string }>(Method.Post, path, queryString, data);
-
-            const roomId = res.room_id;
-            const syncApi = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions());
-            const room = syncApi.createRoom(roomId);
-            if (opts.syncRoom) {
-                // v2 will do this for us
-                // return syncApi.syncRoom(room);
-            }
-            return room;
-        } catch (e) {
-            throw e; // rethrow for reject
         }
+
+        const path = utils.encodeUri("/join/$roomid", { $roomid: roomIdOrAlias });
+        const res = await this.http.authedRequest<{ room_id: string }>(Method.Post, path, queryString, data);
+
+        const roomId = res.room_id;
+        const syncApi = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions());
+        const syncRoom = syncApi.createRoom(roomId);
+        if (opts.syncRoom) {
+            // v2 will do this for us
+            // return syncApi.syncRoom(room);
+        }
+        return syncRoom;
     }
 
     /**
@@ -4689,7 +4688,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         const eventType: string = EventType.RoomMessage;
         const sendContent: IContent = content as IContent;
 
-        return this.sendEvent(roomId, threadId as string | null, eventType, sendContent, txnId);
+        return this.sendEvent(roomId, threadId, eventType, sendContent, txnId);
     }
 
     /**
@@ -5005,7 +5004,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         rpEvent?: MatrixEvent,
     ): Promise<{}> {
         const room = this.getRoom(roomId);
-        if (room && room.hasPendingEvent(rmEventId)) {
+        if (room?.hasPendingEvent(rmEventId)) {
             throw new Error(`Cannot set read marker to a pending event (${rmEventId})`);
         }
 
@@ -5058,9 +5057,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         const key = ts + "_" + url;
 
         // If there's already a request in flight (or we've handled it), return that instead.
-        const cachedPreview = this.urlPreviewCache[key];
+        if (key in this.urlPreviewCache) {
-        if (cachedPreview) {
+            return this.urlPreviewCache[key];
-            return cachedPreview;
         }
 
         const resp = this.http.authedRequest<IPreviewUrlResponse>(

src/crypto/CrossSigning.ts

@@ -688,7 +688,7 @@ export function createCryptoStoreCacheCallbacks(store: CryptoStore, olmDevice: O
             _expectedPublicKey: string,
         ): Promise<Uint8Array> {
             const key = await new Promise<any>((resolve) => {
-                return store.doTxn("readonly", [IndexedDBCryptoStore.STORE_ACCOUNT], (txn) => {
+                store.doTxn("readonly", [IndexedDBCryptoStore.STORE_ACCOUNT], (txn) => {
                     store.getSecretStorePrivateKey(txn, resolve, type);
                 });
             });
@@ -790,7 +790,7 @@ export async function requestKeysDuringVerification(
         })();
 
         // We call getCrossSigningKey() for its side-effects
-        return Promise.race<KeysDuringVerification | void>([
+        Promise.race<KeysDuringVerification | void>([
             Promise.all([
                 crossSigning.getCrossSigningKey("master"),
                 crossSigning.getCrossSigningKey("self_signing"),

src/crypto/DeviceList.ts

@@ -116,7 +116,7 @@ export class DeviceList extends TypedEventEmitter<EmittedEvents, CryptoEventHand
     public async load(): Promise<void> {
         await this.cryptoStore.doTxn("readonly", [IndexedDBCryptoStore.STORE_DEVICE_DATA], (txn) => {
             this.cryptoStore.getEndToEndDeviceData(txn, (deviceData) => {
-                this.hasFetched = Boolean(deviceData && deviceData.devices);
+                this.hasFetched = Boolean(deviceData?.devices);
                 this.devices = deviceData ? deviceData.devices : {};
                 this.crossSigningInfo = deviceData ? deviceData.crossSigningInfo || {} : {};
                 this.deviceTrackingStatus = deviceData ? deviceData.trackingStatus : {};

src/crypto/store/memory-crypto-store.ts

@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import { logger } from "../../logger";
-import { deepCompare, promiseTry } from "../../utils";
+import { safeSet, deepCompare, promiseTry } from "../../utils";
 import {
     CryptoStore,
     IDeviceData,
@@ -33,7 +33,6 @@ import { ICrossSigningKey } from "../../client";
 import { IOlmDevice } from "../algorithms/megolm";
 import { IRoomEncryption } from "../RoomList";
 import { InboundGroupSessionData } from "../OlmDevice";
-import { safeSet } from "../../utils";
 
 /**
  * Internal module. in-memory storage for e2e.

src/crypto/verification/request/InRoomChannel.ts

@@ -125,7 +125,7 @@ export class InRoomChannel implements IVerificationChannel {
         // part of a verification request, so be noisy when rejecting something
         if (type === REQUEST_TYPE) {
             if (!content || typeof content.to !== "string" || !content.to.length) {
-                logger.log("InRoomChannel: validateEvent: " + "no valid to " + (content && content.to));
+                logger.log("InRoomChannel: validateEvent: " + "no valid to " + content.to);
                 return false;
             }
 
@@ -134,7 +134,7 @@ export class InRoomChannel implements IVerificationChannel {
                 logger.log(
                     "InRoomChannel: validateEvent: " +
                         `not directed to or sent by me: ${event.getSender()}` +
-                        `, ${content && content.to}`,
+                        `, ${content.to}`,
                 );
                 return false;
             }

src/embedded.ts

@@ -25,14 +25,13 @@ import {
     ISendEventFromWidgetResponseData,
 } from "matrix-widget-api";
 
-import { IEvent, IContent, EventStatus } from "./models/event";
+import { MatrixEvent, IEvent, IContent, EventStatus } from "./models/event";
 import { ISendEventResponse } from "./@types/requests";
 import { EventType } from "./@types/event";
 import { logger } from "./logger";
 import { MatrixClient, ClientEvent, IMatrixClientCreateOpts, IStartClientOpts, SendToDeviceContentMap } from "./client";
 import { SyncApi, SyncState } from "./sync";
 import { SlidingSyncSdk } from "./sliding-sync-sdk";
-import { MatrixEvent } from "./models/event";
 import { User } from "./models/user";
 import { Room } from "./models/room";
 import { ToDeviceBatch, ToDevicePayload } from "./models/ToDeviceMessage";