Skip to content

Tighten guidelines around dynamic #6286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/misc/lib/effective_dart/design_bad.dart
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ void miscDeclAnalyzedButNotTested() {
}

{
// #docregion prefer-dynamic
// #docregion prefer-object-question
mergeJson(original, changes) => ellipsis();
// #enddocregion prefer-dynamic
// #enddocregion prefer-object-question
}

// #docregion avoid-function
Expand Down
18 changes: 3 additions & 15 deletions examples/misc/lib/effective_dart/design_good.dart
Original file line number Diff line number Diff line change
Expand Up @@ -271,21 +271,9 @@ void miscDeclAnalyzedButNotTested() {
}

{
// #docregion prefer-dynamic
dynamic mergeJson(dynamic original, dynamic changes) => ellipsis();
// #enddocregion prefer-dynamic
}

{
// #docregion infer-dynamic
Map<String, dynamic> readJson() => ellipsis();

void printUsers() {
var json = readJson();
var users = json['users'];
print(users);
}
// #enddocregion infer-dynamic
// #docregion prefer-object-question
Object? mergeJson(Object? original, Object? changes) => ellipsis();
// #enddocregion prefer-object-question
}

// #docregion avoid-function
Expand Down
69 changes: 30 additions & 39 deletions src/content/effective-dart/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -1410,47 +1410,36 @@ var completer = Completer<Map<String, int>>();
```


### DO annotate with `dynamic` instead of letting inference fail
### DO annotate with `Object?` instead of letting inference fail

When inference doesn't fill in a type, it usually defaults to `dynamic`. If
`dynamic` is the type you want, this is technically the most terse way to get
it. However, it's not the most *clear* way. A casual reader of your code who
sees that an annotation is missing has no way of knowing if you intended it to be
`dynamic`, expected inference to fill in some other type, or simply forgot to
write the annotation.
When inference doesn't fill in a type, it usually defaults to `dynamic`, which
is rarely the best type to use. A `dynamic` reference allows for unsafe
operations that use identical syntax to operations that are statically safe when
the type is not `dynamic`. An `Object?` reference is safer. For example a
`dynamic` reference may fail a type cast that is not visible in the syntax,
while an `Object?` reference will guarantee that the `as` cast is written in
source.

When `dynamic` is the type you want, write that explicitly to make your intent
clear and highlight that this code has less static safety.
Use `Object?` to indicate in a signature that any type of object, or null, is
allowed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall metarule for the guidelines is "unless stated otherwise, express things the most terse way". If you want some return type or parameter type to have type dynamic, the most terse way is to simply write nothing.

The original intent of this guideline was to opt out of that default "use the shortest thing" rule. That's why the guideline is about "instead of letting inference fail".

The new guideline here is really about "when should you use Object? versus dynamic, which is a different thing. I think the second updated guideline below does a pretty good job of covering that.

So for this one, I think it would be better to recast it to something like `DON'T let inference fall back to dynamic".


<?code-excerpt "design_good.dart (prefer-dynamic)"?>
<?code-excerpt "design_good.dart (prefer-object-question)"?>
```dart tag=good
dynamic mergeJson(dynamic original, dynamic changes) => ...
Object? mergeJson(Object? original, Object? changes) => ...
```

<?code-excerpt "design_bad.dart (prefer-dynamic)"?>
<?code-excerpt "design_bad.dart (prefer-object-question)"?>
```dart tag=bad
mergeJson(original, changes) => ...
```

Note that it's OK to omit the type when Dart *successfully* infers `dynamic`.

<?code-excerpt "design_good.dart (infer-dynamic)"?>
```dart tag=good
Map<String, dynamic> readJson() => ...

void printUsers() {
var json = readJson();
var users = json['users'];
print(users);
}
```

Here, Dart infers `Map<String, dynamic>` for `json` and then from that infers
`dynamic` for `users`. It's fine to leave `users` without a type annotation. The
distinction is a little subtle. It's OK to allow inference to *propagate*
`dynamic` through your code from a `dynamic` type annotation somewhere else, but
you don't want it to inject a `dynamic` type annotation in a place where your
code did not specify one.
In the cases where a dynamic member will be invoked this is technically the most
terse way to get a dynamic reference. However, it's not the most *clear* way. A
casual reader of your code who sees that an annotation is missing has no way of
knowing if you intended it to be `dynamic`, expected inference to fill in some
other type, or simply forgot to write the annotation. When `dynamic` is the type
you want, write that explicitly to make your intent clear and highlight that
this code has less static safety.

:::note
With Dart's strong type system and type inference,
Expand Down Expand Up @@ -1648,16 +1637,16 @@ The new syntax is a little more verbose, but is consistent with other locations
where you must use the new syntax.


### AVOID using `dynamic` unless you want to disable static checking
### AVOID using `dynamic` unless you want to invoke dynamic members

Some operations work with any possible object. For example, a `log()` method
could take any object and call `toString()` on it. Two types in Dart permit all
values: `Object?` and `dynamic`. However, they convey different things. If you
simply want to state that you allow all objects, use `Object?`. If you want to
allow all objects *except* `null`, then use `Object`.

The type `dynamic` not only accepts all objects, but it also permits all
*operations*. Any member access on a value of type `dynamic` is allowed at
The type `dynamic` not only accepts all objects, but it also statically permits
all *operations*. Any member access on a value of type `dynamic` is allowed at
compile time, but may fail and throw an exception at runtime. If you want
exactly that risky but flexible dynamic dispatch, then `dynamic` is the right
type to use.
Expand All @@ -1678,11 +1667,13 @@ bool convertToBool(Object arg) {
}
```

The main exception to this rule is when working with existing APIs that use
`dynamic`, especially inside a generic type. For example, JSON objects have type
`Map<String, dynamic>` and your code will need to accept that same type. Even
so, when using a value from one of these APIs, it's often a good idea to cast it
to a more precise type before accessing members.
Prefer using `Object?` over `dynamic` in code that is not invoking a member
dynamically, even when working with existing APIs that use `dynamic`.
For example, JSON objects have runtime type `Map<String, dynamic>`, which is an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say "JSON objects traditionally have the static type Map<String, dynamic>"

The runtime type doesn't matter, there is no meaningful difference between dynamic and Object? in a runtime type.

Also "JSON objects" is ambiguous. It can either mean any object created by jsonDecode, which is typed as just dynamic, or it can mean a Dart representation of a "JSON Object" (based on a JavaScript Object), which is indeed traditionally represented by a map with static type Map<String, dynamic>.
The dart: convert code doesn't mandate that you use Map<String, dynamic> instead of Map<String, Object?> when you match or cast to a Map type, that's entirely user habit. It's this just saying that you should cast those to ... Object?... before using them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped the references to JSON objects and I'm referring just to an example of Map<String, dynamic> more generally.

equivalent type to `Map<String, Object?>`.
When using a value from one of these APIs, it's often a good idea to treat it as
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which APIs is this? (First use of "API" here, but the definite form suggests that it refers to something mentioned earlier.)

Is it something that expects a Map<String, dynamic>, or which produces it?
Could the previous paragraph start with
"APIs which accept or produce JSON-object maps traditionally type those as Map<String, dynamic> ..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"these APIs" refers back to "existing APIS that use dynamic", not limited to the JSON example mentioned in between.

I took another try at rewriting this paragraph, and I added an example of using the (foo as dynamic).member syntax.

`Object?` which will enforce that it is cast it to a more precise type before
accessing members.


### DO use `Future<void>` as the return type of asynchronous members that do not produce values
Expand Down
Loading