Distinguish a TopicName type from plain String

lib/api/model/events.dart

@@ -645,7 +645,7 @@ class UserTopicEvent extends Event {
   String get type => 'user_topic';
 
   final int streamId;
-  final String topicName;
+  final TopicName topicName;
   final int lastUpdated;
   final UserTopicVisibilityPolicy visibilityPolicy;
 
@@ -725,9 +725,9 @@ class UpdateMessageEvent extends Event {
   final PropagateMode? propagateMode;
 
   @JsonKey(name: 'orig_subject')
-  final String? origTopic;
+  final TopicName? origTopic;
   @JsonKey(name: 'subject')
-  final String? newTopic;
+  final TopicName? newTopic;
 
   // final List<TopicLink> topicLinks; // TODO handle
 
@@ -788,7 +788,7 @@ class DeleteMessageEvent extends Event {
   @MessageTypeConverter()
   final MessageType messageType;
   final int? streamId;
-  final String? topic;
+  final TopicName? topic;
 
   DeleteMessageEvent({
     required super.id,
@@ -924,7 +924,7 @@ class UpdateMessageFlagsMessageDetail {
   final bool? mentioned;
   final List<int>? userIds;
   final int? streamId;
-  final String? topic;
+  final TopicName? topic;
 
   UpdateMessageFlagsMessageDetail({
     required this.type,
@@ -1002,7 +1002,7 @@ class TypingEvent extends Event {
   @JsonKey(name: 'recipients', fromJson: _recipientIdsFromJson)
   final List<int>? recipientIds;
   final int? streamId;
-  final String? topic;
+  final TopicName? topic;
 
   TypingEvent({
     required super.id,

lib/api/model/initial_snapshot.dart

@@ -231,7 +231,7 @@ class UserSettings {
 @JsonSerializable(fieldRename: FieldRename.snake)
 class UserTopicItem {
   final int streamId;
-  final String topicName;
+  final TopicName topicName;
   final int lastUpdated;
   @JsonKey(unknownEnumValue: UserTopicVisibilityPolicy.unknown)
   final UserTopicVisibilityPolicy visibilityPolicy;
@@ -310,7 +310,7 @@ class UnreadDmSnapshot {
 /// An item in [UnreadMessagesSnapshot.channels].
 @JsonSerializable(fieldRename: FieldRename.snake)
 class UnreadChannelSnapshot {
-  final String topic;
+  final TopicName topic;
   final int streamId;
   final List<int> unreadMessageIds;
 

lib/api/model/model.dart

@@ -556,11 +556,11 @@ sealed class Message {
   final String senderFullName;
   final int senderId;
   final String senderRealmStr;
-  @JsonKey(name: 'subject')
-  String topic;
+
   /// Poll data if "submessages" describe a poll, `null` otherwise.
   @JsonKey(name: 'submessages', readValue: _readPoll, fromJson: Poll.fromJson, toJson: Poll.toJson)
   Poll? poll;
+
   final int timestamp;
   String get type;
 
@@ -613,7 +613,6 @@ sealed class Message {
     required this.senderFullName,
     required this.senderId,
     required this.senderRealmStr,
-    required this.topic,
     required this.timestamp,
     required this.flags,
     required this.matchContent,
@@ -656,6 +655,38 @@ enum MessageFlag {
   String toJson() => _$MessageFlagEnumMap[this]!;
 }
 
+/// The name of a Zulip topic.
+// TODO(dart): Can we forbid calling Object members on this extension type?
+//   (The lack of "implements Object" ought to do that, but doesn't.)
+//   In particular an interpolation "foo > $topic" is a bug we'd like to catch.
+// TODO(dart): Can we forbid using this extension type as a key in a Map?
+//   (The lack of "implements Object" arguably should do that, but doesn't.)
+//   Using as a Map key is almost certainly a bug because it won't case-fold;
+//   see for example #739, #980, #1205.
+extension type const TopicName(String _value) {
+  /// The string this topic is identified by in the Zulip API.
+  ///
+  /// This should be used in constructing HTTP requests to the server,
+  /// but rarely for other purposes.  See [displayName] and [canonicalize].
+  String get apiName => _value;
+
+  /// The string this topic is displayed as to the user in our UI.
+  ///
+  /// At the moment this always equals [apiName].
+  /// In the future this will become null for the "general chat" topic (#1250),
+  /// so that UI code can identify when it needs to represent the topic
+  /// specially in the way prescribed for "general chat".
+  // TODO(#1250) carry out that plan
+  String get displayName => _value;
+
+  /// The key to use for "same topic as" comparisons.
+  String canonicalize() => apiName.toLowerCase();
+
+  TopicName.fromJson(this._value);
+
+  String toJson() => apiName;
+}
+
 @JsonSerializable(fieldRename: FieldRename.snake)
 class StreamMessage extends Message {
   @override
@@ -667,8 +698,16 @@ class StreamMessage extends Message {
   // invalidated.
   @JsonKey(required: true, disallowNullValue: true)
   String? displayRecipient;
+
   int streamId;
 
+  // The topic/subject is documented to be present on DMs too, just empty.
+  // We ignore it on DMs; if a future server introduces distinct topics in DMs,
+  // that will need new UI that we'll design then as part of that feature,
+  // and ignoring the topics seems as good a fallback behavior as any.
+  @JsonKey(name: 'subject')
+  TopicName topic;
+
   StreamMessage({
     required super.client,
     required super.content,
@@ -683,13 +722,13 @@ class StreamMessage extends Message {
     required super.senderFullName,
     required super.senderId,
     required super.senderRealmStr,
-    required super.topic,
     required super.timestamp,
     required super.flags,
     required super.matchContent,
     required super.matchTopic,
     required this.displayRecipient,
     required this.streamId,
+    required this.topic,
   });
 
   factory StreamMessage.fromJson(Map<String, dynamic> json) =>
@@ -786,7 +825,6 @@ class DmMessage extends Message {
     required super.senderFullName,
     required super.senderId,
     required super.senderRealmStr,
-    required super.topic,
     required super.timestamp,
     required super.flags,
     required super.matchContent,
@@ -817,14 +855,14 @@ enum MessageEditState {
   /// The Zulip "resolved topics" feature is implemented by renaming the topic;
   /// but for purposes of [Message.editState], we want to ignore such renames.
   /// This method identifies topic moves that should be ignored in that context.
-  static bool topicMoveWasResolveOrUnresolve(String topic, String prevTopic) {
-    if (topic.startsWith(_resolvedTopicPrefix)
-        && topic.substring(_resolvedTopicPrefix.length) == prevTopic) {
+  static bool topicMoveWasResolveOrUnresolve(TopicName topic, TopicName prevTopic) {
+    if (topic.apiName.startsWith(_resolvedTopicPrefix)
+        && topic.apiName.substring(_resolvedTopicPrefix.length) == prevTopic.apiName) {
       return true;
     }
 
-    if (prevTopic.startsWith(_resolvedTopicPrefix)
-        && prevTopic.substring(_resolvedTopicPrefix.length) == topic) {
+    if (prevTopic.apiName.startsWith(_resolvedTopicPrefix)
+        && prevTopic.apiName.substring(_resolvedTopicPrefix.length) == topic.apiName) {
       return true;
     }
 
@@ -857,8 +895,10 @@ enum MessageEditState {
       }
 
       // TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers
-      final prevTopic = (entry['prev_topic'] ?? entry['prev_subject']) as String?;
-      final topic = entry['topic'] as String?;
+      final prevTopicStr = (entry['prev_topic'] ?? entry['prev_subject']) as String?;
+      final prevTopic = prevTopicStr == null ? null : TopicName.fromJson(prevTopicStr);
+      final topicStr = entry['topic'] as String?;
+      final topic = topicStr == null ? null : TopicName.fromJson(topicStr);
       if (prevTopic != null) {
         // TODO(server-5) pre-5.0 servers do not have the 'topic' field
         if (topic == null) {

lib/api/model/narrow.dart

@@ -1,5 +1,7 @@
 import 'package:json_annotation/json_annotation.dart';
 
+import 'model.dart';
+
 part 'narrow.g.dart';
 
 typedef ApiNarrow = List<ApiNarrowElement>;
@@ -26,7 +28,37 @@ ApiNarrow resolveDmElements(ApiNarrow narrow, int zulipFeatureLevel) {
 /// please add more as needed.
 sealed class ApiNarrowElement {
   String get operator;
-  Object get operand;
+
+  /// The operand of this narrow filter.
+  ///
+  /// The base-class getter [ApiNarrowElement.operand] returns `dynamic`,
+  /// and its value should only be used for encoding as JSON, for use in a
+  /// request to the Zulip server.
+  ///
+  /// For any operations that depend more specifically on the operand's type,
+  /// do not use run-time type checks on the value of [operand]; instead, make
+  /// a run-time type check (e.g. with `switch`) on the [ApiNarrowElement]
+  /// itself, and use the [operand] getter of the specific subtype.
+  ///
+  /// That makes a difference because [ApiNarrowTopic.operand] has type
+  /// [TopicName]; at runtime a [TopicName] is indistinguishable from [String],
+  /// but an [ApiNarrowTopic] can still be distinguished from other subclasses.
+  //
+  // We can't just write [Object] here; if we do, the compiler rejects the
+  // override in ApiNarrowTopic because TopicName can't be assigned to Object.
+  // The reason that could be bad is that a caller of [ApiNarrowElement.operand]
+  // could take the result and call Object members on it, like toString, even
+  // though TopicName doesn't declare those members.
+  //
+  // In this case that's fine because the only plausible thing to do with
+  // a generic [ApiNarrowElement.operand] is to encode it as JSON anyway,
+  // which behaves just fine on TopicName.
+  //
+  // ... Even if it weren't fine, in the case of Object this protection is
+  // thoroughly undermined already: code that has a TopicName can call Object
+  // members on it directly.  See comments at [TopicName].
+  dynamic get operand; // see justification for `dynamic` above
+
   final bool negated;
 
   ApiNarrowElement({this.negated = false});
@@ -54,12 +86,12 @@ class ApiNarrowStream extends ApiNarrowElement {
 class ApiNarrowTopic extends ApiNarrowElement {
   @override String get operator => 'topic';
 
-  @override final String operand;
+  @override final TopicName operand;
 
   ApiNarrowTopic(this.operand, {super.negated});
 
   factory ApiNarrowTopic.fromJson(Map<String, dynamic> json) => ApiNarrowTopic(
-    json['operand'] as String,
+    TopicName.fromJson(json['operand'] as String),
     negated: json['negated'] as bool? ?? false,
   );
 }