diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index b733a1e490..e55dc1d1f0 100644 --- a/lib/api/model/model.dart +++ b/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, diff --git a/lib/api/model/narrow.dart b/lib/api/model/narrow.dart index 72a4d7ff77..8082ac64ff 100644 --- a/lib/api/model/narrow.dart +++ b/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});