api [nfc]: Seal the TopicName migration by making non-transparent

We're now explicitly using apiName, canonicalize, or displayName in
each of the places where we had been implicitly treating a topic name
as a String.  Let the type-checker catch any future regressions on
that front, by removing "implements String".

More precisely, I *think* we've covered each such place.  The
loophole is that Object methods can still be called, including
toString, so an interpolation like "${channelName} > ${topic}" won't
be flagged by the analyzer.  I found some such interpolations with a
grep, but that's necessarily heuristic and there could be more.
So the analyzer won't give us quite as much help here as we'd like;
but it'll give quite a bit, and this is the most I see how to do.

Without the "implements String", a TopicName value can no longer be
implicitly converted to String or have String methods called on it.
Instead the type-checker will require any code that has such a value
to call one of the members we've declared on TopicName (or a member
of Object) in order to do anything with it.  That way the code will
be explicit about whether it needs the API name, or the display
name, or the canonicalized form for making equality comparisons.

And that in turn will enable us to make "display name" behave
differently from "API name", for zulip#1250 the "general chat" feature,
with a reliable way of tracking down which sites need which version.

Author: gnprice

lib/api/model/model.dart

@@ -656,8 +656,14 @@ enum MessageFlag {
 }
 
 /// The name of a Zulip topic.
-// TODO(#1250): Migrate all implicit uses as String; remove "implements String".
-extension type const TopicName(String _value) implements String {
+// 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,

lib/api/model/narrow.dart

@@ -28,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});