types: Use a distinct UserId type for user IDs.

src/__tests__/lib/exampleData.js

@@ -12,7 +12,9 @@ import type {
   Subscription,
   User,
   UserGroup,
+  UserId,
 } from '../../api/modelTypes';
+import { makeUserId } from '../../api/idTypes';
 import type { Action, GlobalState, MessagesState, RealmState } from '../../reduxTypes';
 import type { Auth, Account, Outbox } from '../../types';
 import { UploadedAvatarURL } from '../../utils/avatar';
@@ -104,10 +106,12 @@ export const diverseCharacters =
 
 type UserOrBotPropertiesArgs = {|
   name?: string,
-  user_id?: number,
+  user_id?: number, // accept a plain number, for convenience in tests
 |};
 
-const randUserId: () => number = makeUniqueRandInt('user IDs', 10000);
+const randUserId: () => UserId = (mk => () => makeUserId(mk()))(
+  makeUniqueRandInt('user IDs', 10000),
+);
 const userOrBotProperties = ({ name: _name, user_id }: UserOrBotPropertiesArgs) => {
   const name = _name ?? randString();
   const capsName = name.substring(0, 1).toUpperCase() + name.substring(1);
@@ -125,7 +129,7 @@ const userOrBotProperties = ({ name: _name, user_id }: UserOrBotPropertiesArgs)
     full_name: `${capsName} User`,
     is_admin: false,
     timezone: 'UTC',
-    user_id: user_id ?? randUserId(),
+    user_id: user_id != null ? makeUserId(user_id) : randUserId(),
   });
 };
 
@@ -321,11 +325,16 @@ export const pmMessage = (args?: {|
   ...$Rest<Message, {}>,
   sender?: User,
   recipients?: User[],
+  sender_id?: number, // accept a plain number, for convenience in tests
 |}): Message => {
   // The `Object.freeze` is to work around a Flow issue:
   //   https://github.com/facebook/flow/issues/2386#issuecomment-695064325
-  const { sender = otherUser, recipients = [otherUser, selfUser], ...extra } =
-    args ?? Object.freeze({});
+  const {
+    sender = otherUser,
+    recipients = [otherUser, selfUser],
+    sender_id = undefined,
+    ...extra
+  } = args ?? Object.freeze({});
 
   const baseMessage: Message = {
     ...messagePropertiesBase,
@@ -344,7 +353,11 @@ export const pmMessage = (args?: {|
     type: 'private',
   };
 
-  return deepFreeze({ ...baseMessage, ...extra });
+  return deepFreeze({
+    ...baseMessage,
+    ...(sender_id != null && { sender_id: makeUserId(sender_id) }),
+    ...extra,
+  });
 };
 
 export const pmMessageFromTo = (from: User, to: User[], extra?: $Rest<Message, {}>): Message =>
@@ -537,7 +550,7 @@ export const action = deepFreeze({
       is_admin: false,
       realm_non_active_users: [],
       realm_users: [],
-      user_id: 4,
+      user_id: makeUserId(4),
       realm_user_groups: [],
       recent_private_conversations: [],
       streams: [],

src/actionTypes.js

@@ -80,6 +80,7 @@ import type {
   CaughtUpState,
   MuteState,
   AlertWordsState,
+  UserId,
   UserStatusEvent,
 } from './types';
 import type { ZulipVersion } from './utils/zulipVersion';
@@ -218,7 +219,7 @@ type MessageFetchCompleteAction = {|
   numAfter: number,
   foundNewest: boolean | void,
   foundOldest: boolean | void,
-  ownUserId: number,
+  ownUserId: UserId,
 |};
 
 type InitialFetchStartAction = {|
@@ -285,15 +286,15 @@ type EventSubscriptionPeerAddAction = {|
   type: typeof EVENT_SUBSCRIPTION,
   op: 'peer_add',
   subscriptions: string[],
-  user_id: number,
+  user_id: UserId,
 |};
 
 type EventSubscriptionPeerRemoveAction = {|
   ...ServerEvent,
   type: typeof EVENT_SUBSCRIPTION,
   op: 'peer_remove',
   subscriptions: string[],
-  user_id: number,
+  user_id: UserId,
 |};
 
 type GenericEventAction = {|
@@ -345,7 +346,7 @@ type EventUpdateMessageAction = {|
   rendered_content: string,
   subject_links: string[],
   subject: string,
-  user_id: number,
+  user_id: UserId,
 |};
 
 type EventReactionCommon = {|
@@ -373,13 +374,13 @@ type EventPresenceAction = {|
 
 type EventTypingCommon = {|
   ...ServerEvent,
-  ownUserId: number,
-  recipients: Array<{
-    user_id: number,
+  ownUserId: UserId,
+  recipients: $ReadOnlyArray<{
+    user_id: UserId,
     email: string,
   }>,
   sender: {
-    user_id: number,
+    user_id: UserId,
     email: string,
   },
   time: number,
@@ -422,7 +423,7 @@ type EventUserRemoveAction = {|
 type EventUserUpdateAction = {|
   ...ServerEvent,
   type: typeof EVENT_USER_UPDATE,
-  userId: number,
+  userId: UserId,
   // Include only the fields that should be overwritten.
   person: $Shape<User>,
 |};
@@ -460,15 +461,15 @@ type EventUserGroupAddMembersAction = {|
   type: typeof EVENT_USER_GROUP_ADD_MEMBERS,
   op: 'add_members',
   group_id: number,
-  user_ids: number[],
+  user_ids: UserId[],
 |};
 
 type EventUserGroupRemoveMembersAction = {|
   ...ServerEvent,
   type: typeof EVENT_USER_GROUP_REMOVE_MEMBERS,
   op: 'remove_members',
   group_id: number,
-  user_ids: number[],
+  user_ids: UserId[],
 |};
 
 type EventRealmEmojiUpdateAction = {|

src/api/eventTypes.js

@@ -10,7 +10,7 @@
  * @flow strict-local
  */
 
-import type { Message, Stream, UserPresence } from './modelTypes';
+import type { Message, Stream, UserId, UserPresence } from './modelTypes';
 
 export class EventTypes {
   static alert_words: 'alert_words' = 'alert_words';
@@ -75,7 +75,7 @@ export type SubmessageEvent = {|
   type: typeof EventTypes.submessage,
   submessage_id: number,
   message_id: number,
-  sender_id: number,
+  sender_id: UserId,
   msg_type: 'widget',
   content: string,
 |};
@@ -103,7 +103,7 @@ export type PresenceEvent = {|
 export type UserStatusEvent = {|
   ...EventCommon,
   type: typeof EventTypes.user_status,
-  user_id: number,
+  user_id: UserId,
   away?: boolean,
   status_text?: string,
 |};

src/api/idTypes.js

@@ -0,0 +1,54 @@
+/* @flow strict-local */
+
+/**
+ * A user ID.
+ *
+ * This is a number that identifies a particular Zulip user.  Different
+ * users on the same Zulip server will have different user IDs.  On the
+ * other hand, between different Zulip servers the same user ID may be used
+ * to refer to completely unrelated users.
+ *
+ * In general, if something calls for a value of this type, then you should
+ * be getting it from something that already has this type: like the
+ * `user_id` property of a `User` object, or a data structure that stores
+ * user IDs.
+ *
+ * The only other way to create a `UserId` is to call the `makeUserId`
+ * function provided by this module.
+ *
+ * See also type `User`, for the thing that one of these identifies.
+ */
+// How this type works: it's an "opaque type alias" for simply a number.
+// See Flow docs: https://flow.org/en/docs/types/opaque-types/
+//
+// This means that:
+//  * At runtime, a `UserId` value is just a number.  The use of `UserId`
+//    involves zero runtime overhead compared with simply `number`.
+//  * Because we've written a "bound" of `: number`, all code that has one
+//    of these values can freely use it as if it were simply `number`.
+//  * On the other hand, the only way to *create* such a value is to invoke
+//    something from this module to do it for you.
+//
+// For more background discussion of opaque types, see `PmKeyRecipients`.
+export opaque type UserId: number = number;
+
+/**
+ * Take a number, and declare that it truly is a user ID.
+ *
+ * This does nothing at all at runtime, just returning the value it's
+ * passed.  Its only effect is to inform the type-checker that it's OK to
+ * use this value where a user ID is required.
+ *
+ * In general the only legitimate use case for this function, outside of
+ * tests, is when parsing a user ID from a string.  When getting a user ID
+ * from any other source, if the values really are user IDs then the type of
+ * that source should be adjusted to say so.
+ */
+export const makeUserId = (id: number): UserId => id;
+
+/* Possible future work:
+    export opaque type StreamId: number = number;
+    export opaque type MessageId: number = number;
+    export const makeStreamId = (id: number): StreamId => id;
+    export const makeMessageId = (id: number): MessageId => id;
+*/

src/api/index.js

@@ -45,12 +45,6 @@ import toggleStreamNotifications from './subscriptions/toggleStreamNotifications
 import getSubscriptionToStream from './subscriptions/getSubscriptionToStream';
 import unmuteTopic from './subscriptions/unmuteTopic';
 import tryGetFileTemporaryUrl from './tryGetFileTemporaryUrl';
-import createUserGroup from './user_groups/createUserGroup';
-import deleteUserGroup from './user_groups/deleteUserGroup';
-import editUserGroup from './user_groups/editUserGroup';
-import editUserGroupMembers from './user_groups/editUserGroupMembers';
-import getUserGroupById from './user_groups/getUserGroupById';
-import getUserGroups from './user_groups/getUserGroups';
 import getUsers from './users/getUsers';
 import createUser from './users/createUser';
 import getUserProfile from './users/getUserProfile';
@@ -104,12 +98,6 @@ export {
   toggleStreamNotifications,
   unmuteTopic,
   tryGetFileTemporaryUrl,
-  createUserGroup,
-  deleteUserGroup,
-  editUserGroup,
-  editUserGroupMembers,
-  getUserGroupById,
-  getUserGroups,
   getUsers,
   createUser,
   getUserProfile,

src/api/initialDataTypes.js

@@ -9,6 +9,7 @@ import type {
   Subscription,
   User,
   UserGroup,
+  UserId,
   UserPresence,
   UserStatusMapObject,
 } from './apiTypes';
@@ -113,7 +114,7 @@ export type RawInitialDataRealmUser = {|
   is_admin: boolean,
   realm_users: Array<{| ...User, avatar_url?: string | null |}>,
   realm_non_active_users: Array<{| ...User, avatar_url?: string | null |}>,
-  user_id: number,
+  user_id: UserId,
 |};
 
 export type InitialDataRealmUser = {|
@@ -204,7 +205,7 @@ export type StreamUnreadItem = {|
   unread_message_ids: number[],
 
   /** All distinct senders of these messages; sorted. */
-  // sender_ids: number[],
+  // sender_ids: UserId[],
 |};
 
 export type HuddlesUnreadItem = {|
@@ -226,7 +227,7 @@ export type PmsUnreadItem = {|
    * the normal thing even then would be to make a bot user to send the
    * messages as.)  See server commit ca74cd6e3.
    */
-  sender_id: number,
+  sender_id: UserId,
 
   // Sorted.
   unread_message_ids: number[],

src/api/messages/getMessages.js

@@ -2,7 +2,7 @@
 import type { Auth, ApiResponseSuccess } from '../transportTypes';
 import type { Identity } from '../../types';
 import type { Message, ApiNarrow } from '../apiTypes';
-import type { Reaction } from '../modelTypes';
+import type { Reaction, UserId } from '../modelTypes';
 import { apiGet } from '../apiFetch';
 import { identityOfAuth } from '../../account/accountMisc';
 import { AvatarURL } from '../../utils/avatar';
@@ -27,7 +27,7 @@ export type ServerReaction = $ReadOnly<{|
   user: $ReadOnly<{|
     email: string,
     full_name: string,
-    id: number,
+    id: UserId,
   |}>,
 |}>;