Skip to content

Commit

Permalink
Style Guide updates (flutter#152525)
Browse files Browse the repository at this point in the history
This pull request updates the style guide for improved internal consistency and to match current linter rules.

I'll make comments for each change here!
  • Loading branch information
nate-thegrate authored Aug 16, 2024
1 parent febf4cd commit a0c0453
Showing 1 changed file with 106 additions and 103 deletions.
209 changes: 106 additions & 103 deletions docs/contributing/Style-guide-for-Flutter-repo.md
Original file line number Diff line number Diff line change
Expand Up @@ -752,13 +752,13 @@ abstract class RenderBox extends RenderObject {
// more complicated asserts:
assert(() {
final RenderObject parent = this.parent;
if (owner.debugDoingLayout)
return (RenderObject.debugActiveLayout == parent) &&
parent.debugDoingThisLayout;
if (owner.debugDoingPaint)
return ((RenderObject.debugActivePaint == parent) &&
parent.debugDoingThisPaint) ||
((RenderObject.debugActivePaint == this) && debugDoingThisPaint);
if (owner.debugDoingLayout) {
return (RenderObject.debugActiveLayout == parent) && parent.debugDoingThisLayout;
}
if (owner.debugDoingPaint) {
return ((RenderObject.debugActivePaint == parent) && parent.debugDoingThisPaint)
|| ((RenderObject.debugActivePaint == this) && debugDoingThisPaint);
}
assert(parent == this.parent);
return false;
});
Expand Down Expand Up @@ -928,8 +928,9 @@ TheType get theProperty => _theProperty;
TheType _theProperty;
void set theProperty(TheType value) {
assert(value != null);
if (_theProperty == value)
if (_theProperty == value) {
return;
}
_theProperty = value;
markNeedsWhatever(); // the method to mark the object dirty
}
Expand Down Expand Up @@ -1073,7 +1074,7 @@ const double kParagraphSpacing = 1.5;
const String kSaveButtonTitle = 'Save';
```

However, where possible avoid global constants. Rather than `kDefaultButtonColor`, consider `Button.defaultColor`. If necessary, consider creating a class with a private constructor to hold relevant constants.
However, where possible avoid global constants. Rather than `kDefaultButtonColor`, consider `Button.defaultColor`. If necessary, consider creating an `abstract final class` to hold relevant constants.


### Avoid abbreviations
Expand Down Expand Up @@ -1249,7 +1250,9 @@ include a link to that issue in the code.
Generally the closure passed to `setState` should include all the code that changes the state. Sometimes this is not possible because the state changed elsewhere and the `setState` is called in response. In those cases, include a comment in the `setState` closure that explains what the state is that changed.

```dart
setState(() { /* The animation ticked. We use the animation's value in the build method. */ });
setState(() {
// The animation ticked. We use the animation's value in the build method.
});
```


Expand Down Expand Up @@ -1328,25 +1331,21 @@ to the superclass.

```dart
// one-line constructor example
abstract class Foo extends StatelessWidget {
Foo(this.bar, { Key key, this.child }) : super(key: key);
final int bar;
final Widget child;
class ConstantTween<T> extends Tween<T> {
ConstantTween(T value) : super(begin: value, end: value);
// ...
}
// fully expanded constructor example
abstract class Foo extends StatelessWidget {
Foo(
this.bar, {
Key key,
Widget childWidget,
}) : child = childWidget,
super(
key: key,
);
final int bar;
final Widget child;
class ConstantTween<T> extends Tween<T> {
ConstantTween(
T value,
) : super(
begin: value,
end: value,
);
// ...
}
```
Expand Down Expand Up @@ -1432,8 +1431,6 @@ Example:
bar,
baz,
);
foo(bar,
baz);
```

### Use a trailing comma for arguments, parameters, and list items, but only if they each have their own line.
Expand All @@ -1453,6 +1450,42 @@ foo1(
foo2(bar, baz);
```

If one of the items is a multi-line callback, collection literal,
or switch expression, it can be added without a trailing comma.

```dart
// GOOD:
foo(
bar,
baz,
switch (value) {
true => ScrollDirection.forward,
false => ScrollDirection.reverse,
null => ScrollDirection.idle,
},
);
// also GOOD:
foo(bar, baz, switch (value) {
true => ScrollDirection.forward,
false => ScrollDirection.reverse,
null => ScrollDirection.idle,
});
// The same applies to collection literals and callbacks:
foo(<String>[
'list item 1',
'list item 2',
'list item 3',
]);
Future.delayed(Durations.short1, () {
if (mounted && _shouldOpenDrawer) {
_drawerController.forward();
}
});
```

Whether to put things all on one line or whether to have one line per item is an aesthetic choice. We prefer whatever ends up being most readable. Typically this means that when everything would fit on one line, put it all on one line, otherwise, split it one item to a line.

However, there are exceptions. For example, if there are six back-to-back lists and all but one of them need multiple lines, then one would not want to have the single case that does fit on one line use a different style than the others.
Expand Down Expand Up @@ -1488,17 +1521,6 @@ However, there are exceptions. For example, if there are six back-to-back lists
];
```

### Prefer single quotes for strings

Use double quotes for nested strings or (optionally) for strings that contain single quotes.
For all other strings, use single quotes.

Example:

```dart
print('Hello ${name.split(" ")[0]}');
```


### Consider using `=>` for short functions and methods

Expand All @@ -1520,14 +1542,49 @@ String capitalize(String s) {
}
```

### Use `=>` for inline callbacks that just return literals or switch expressions
### Use `=>` for getters and callbacks that just return literals or switch expressions

```dart
// GOOD:
List<Color> get favorites => <Color>[
const Color(0xFF80FFFF),
const Color(0xFF00FFF0),
const Color(0xFF4000FF),
_mysteryColor(),
];
// GOOD:
bool get isForwardOrCompleted => switch (status) {
AnimationStatus.forward || AnimationStatus.completed => true,
AnimationStatus.reverse || AnimationStatus.dismissed => false,
};
```

It's important to use discretion, since there are cases where a function body
is easier to visually parse:

```dart
// OKAY, but the code is more dense than it could be:
String? get validated => switch(input[_inputIndex]?.trim()) {
final String value when value.isNotEmpty => value,
_ => null,
}
If your code is passing an inline closure that merely returns a list or
map literal, or a switch expression, or is merely calling another function,
then if the argument is on its own line, then rather than using braces and a
`return` statement, you can instead use the `=>` form. When doing this, the
closing `]`, `}`, or `)` bracket will line up with the argument name, for
named arguments, or the `(` of the argument list, for positional arguments.
// BETTER (more verbose, but also more readable):
String? get validated {
final String? value = input[_inputIndex]?.trim();
if (value != null && value.isNotEmpty) {
return value;
}
return null;
}
```

If your code is passing an inline closure containing only a `return` statement,
you can instead use the `=>` form.\
When doing this, the closing `]`, `}`, or `)` bracket will have the same
indentation as the line where the callback starts.

For example:

Expand All @@ -1541,11 +1598,11 @@ For example:
return <PopupMenuItem<String>>[
PopupMenuItem<String>(
value: 'Friends',
child: MenuItemWithIcon(Icons.people, 'Friends', '5 new')
child: MenuItemWithIcon(Icons.people, 'Friends', '5 new'),
),
PopupMenuItem<String>(
value: 'Events',
child: MenuItemWithIcon(Icons.event, 'Events', '12 upcoming')
child: MenuItemWithIcon(Icons.event, 'Events', '12 upcoming'),
),
];
}
Expand All @@ -1560,11 +1617,11 @@ For example:
itemBuilder: (BuildContext context) => <PopupMenuItem<String>>[
PopupMenuItem<String>(
value: 'Friends',
child: MenuItemWithIcon(Icons.people, 'Friends', '5 new')
child: MenuItemWithIcon(Icons.people, 'Friends', '5 new'),
),
PopupMenuItem<String>(
value: 'Events',
child: MenuItemWithIcon(Icons.event, 'Events', '12 upcoming')
child: MenuItemWithIcon(Icons.event, 'Events', '12 upcoming'),
),
]
);
Expand Down Expand Up @@ -1666,60 +1723,6 @@ final List<String> args = <String>[
Use a block (with braces) when a body would wrap onto more than one line (as opposed to using `=>`; the cases where you can use `=>` are discussed in the previous two guidelines).


### Separate the 'if' expression from its statement

(This is enforced by the `always_put_control_body_on_new_line` and `curly_braces_in_flow_control_structures` lints.)

Don't put the statement part of an 'if' statement on the same line as
the expression, even if it is short. (Doing so makes it unobvious that
there is relevant code there. This is especially important for early
returns.)

Example:

```dart
// BAD:
if (notReady) return;
// GOOD:
// Use this style for code that is expected to be publicly read by developers
if (notReady) {
return;
}
```

If the body is more than one line, or if there is an `else` clause, wrap the body in braces:

```dart
// BAD:
if (foo)
bar(
'baz',
);
// BAD:
if (foo)
bar();
else
baz();
// GOOD:
if (foo) {
bar(
'baz',
);
}
// GOOD:
if (foo) {
bar();
} else {
baz();
}
```

We require bodies to make it very clear where the bodies belong.

### Align expressions

Where possible, subexpressions on different lines should be aligned, to make the structure of the expression easier. When doing this with a `return` statement chaining `||` or `&&` operators, consider putting the operators on the left hand side instead of the right hand side.
Expand Down

0 comments on commit a0c0453

Please sign in to comment.