Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 3cce88b

Browse files
author
auto-submit[bot]
committed
Revert "Revamp the engine style guide, remove always_specify_types. (#52859)"
This reverts commit e6e37b4.
1 parent 942d7c3 commit 3cce88b

File tree

3 files changed

+19
-174
lines changed

3 files changed

+19
-174
lines changed

CONTRIBUTING.md

Lines changed: 17 additions & 170 deletions
Original file line numberDiff line numberDiff line change
@@ -14,182 +14,29 @@ contributing guide.
1414

1515
## Style
1616

17-
The Flutter engine _generally_ follows Google style for the languages it uses,
18-
with some exceptions.
17+
The Flutter engine follows Google style for the languages it uses:
18+
19+
- [C++](https://google.github.io/styleguide/cppguide.html)
20+
- **Note**: The Linux embedding generally follows idiomatic GObject-based C
21+
style. Use of C++ is discouraged in that embedding to avoid creating hybrid
22+
code that feels unfamiliar to either developers used to working with
23+
`GObject` or C++ developers. For example, do not use STL collections or
24+
`std::string`. Exceptions:
25+
- C-style casts are forbidden; use C++ casts.
26+
- Use `nullptr` rather than `NULL`.
27+
- Avoid `#define`; for internal constants use `static constexpr` instead.
28+
- [Objective-C][objc_style] (including [Objective-C++][objcc_style])
29+
- [Java][java_style]
30+
31+
C/C++ and Objective-C/C++ files are formatted with `clang-format`, and GN files
32+
with `gn format`.
1933

20-
### C/C++
21-
22-
Follows the [Google C++ Style Guide][google_cpp_style] and is automatically
23-
formatted with `clang-format`.
24-
25-
Some additional considerations that are in compliance with the style guide, but
26-
are worth noting:
27-
28-
#### Judiciously use shared_ptr
29-
30-
The engine currently (as of 2024-05-15) uses `shared_ptr` liberally, which can
31-
be expensive to copy, and is not always necessary.
32-
33-
The C++ style guide has a
34-
[section on ownership and smart pointers][cpp_ownership] worth reading:
35-
36-
> Do not design your code to use shared ownership without a very good reason.
37-
> One such reason is to avoid expensive copy operations, but you should only do
38-
> this if the performance benefits are significant, and the underlying object is
39-
> immutable.
40-
41-
Prefer using `std::unique_ptr` when possible.
42-
43-
#### Judiciously use `auto`
44-
45-
The C++ style guide has a [section on type deduction][cpp_auto] that is worth
46-
reading:
47-
48-
> The fundamental rule is: use type deduction only to make the code clearer or
49-
> safer, and do not use it merely to avoid the inconvenience of writing an
50-
> explicit type. When judging whether the code is clearer, keep in mind that
51-
> your readers are not necessarily on your team, or familiar with your project,
52-
> so types that you and your reviewer experience as unnecessary clutter will
53-
> very often provide useful information to others. For example, you can assume
54-
> that the return type of `make_unique<Foo>()` is obvious, but the return type
55-
> of `MyWidgetFactory()` probably isn't.
56-
57-
Due to our codebase's extensive use of `shared_ptr`, `auto` can have surprising
58-
performance implications. See [#49801][pr_49801] for an example.
59-
60-
#### Linux Embedding
61-
62-
> [!NOTE]
63-
> The Linux embedding instead follows idiomatic GObject-based C style.
64-
65-
Use of C++ in the [Linux embedding][] is discouraged in that embedding to avoid
66-
creating hybrid code that feels unfamiliar to either developers used to working
67-
with `GObject` or C++ developers.
68-
69-
For example, _do not_ use STL collections or `std::string`, but _do_:
70-
71-
- Use C++ casts (C-style casts are forbidden).
72-
- Use `nullptr` rather than `NULL`.
73-
- Avoid `#define`; for internal constants use `static constexpr` instead.
74-
75-
### Dart
76-
77-
The Flutter engine _intends_ to follow the [Dart style guide][dart_style] but
78-
currently follows the [Flutter style guide][flutter_style], with the following
79-
exceptions:
80-
81-
#### Use of type inference is allowed
82-
83-
The [Dart style guide][dart_inference] only requires explicit types when type
84-
inference is not possible, but the Flutter style guide always requires explicit
85-
types. The engine is moving towards the Dart style guide, but this is a gradual
86-
process. In the meantime, follow these guidelines:
87-
88-
- **Always** annotate when inference is not possible.
89-
- **Prefer** annotating when inference is possible but the type is not
90-
obvious.
91-
92-
Some cases when using `var`/`final`/`const` is appropriate:
93-
94-
- When the type is obvious from the right-hand side of the assignment:
95-
96-
```dart
97-
// Capitalized constructor name always returns a Foo.
98-
var foo = Foo();
99-
100-
// Similar with factory constructors.
101-
var bar = Bar.create();
102-
103-
// Literals (strings, numbers, lists, maps, etc) always return the same type.
104-
var name = 'John Doe';
105-
var flag = true;
106-
var numbers = [1, 2, 3];
107-
var map = {'one': 1, 'two': 2, 'three': 3};
108-
```
109-
110-
- When the type is obvious from the method name:
111-
112-
```dart
113-
// toString() always returns a String.
114-
var string = foo().toString();
115-
116-
// It's reasonable to assume that length returns an int.
117-
var length = string.length;
118-
```
119-
120-
- When the type is obvious from the context:
121-
122-
```dart
123-
// When variables are in the same scope, reduce() clearly returns an int.
124-
var list = [1, 2, 3];
125-
var sum = list.reduce((a, b) => a + b);
126-
```
127-
128-
Some cases where an explicit type should be considered:
129-
130-
- When the type is not obvious from the right-hand side of the assignment:
131-
132-
```dart
133-
// What does 'fetchLatest()' return?
134-
ImageBuffer buffer = fetchLatest();
135-
136-
// What does this large chain of method calls return?
137-
Iterable<int> numbers = foo().bar().map((b) => b.baz());
138-
```
139-
140-
- When there are semantic implications to the type:
141-
142-
```dart
143-
// Without 'num', the map would be inferred as 'Map<String, int>'.
144-
const Map<String, num> map = {'one': 1, 'two': 2, 'three': 3};
145-
```
146-
147-
- Or, **when a reviewer requests it**!
148-
149-
Remember that the goal is to make the code more readable and maintainable, and
150-
explicit types _can_ help with that. Code can be changed, so it's always
151-
possible to add or remove type annotations later as the code evolves, so avoid
152-
bikeshedding over this.
153-
154-
### Java
155-
156-
Follows the [Google Java Style Guide][java_style] and is automatically formatted
157-
with `google-java-format`.
158-
159-
### Objective-C
160-
161-
Follows the [Google Objective-C Style Guide][objc_style], including for
162-
Objective-C++ and is automatically formatted with `clang-format`.
163-
164-
### Python
165-
166-
Follows the [Google Python Style Guide][google_python_style] and is
167-
automatically formatted with `yapf`.
168-
169-
> [!WARNING]
170-
> Historically, the engine grew a number of one-off Python scripts, often as
171-
> part of the testing or build infrastructure (i.e. command-line tools). We are
172-
> instead moving towards using Dart for these tasks, so new Python scripts
173-
> should be avoided whenever possible.
174-
175-
### GN
176-
177-
Automatically formatted with `gn format`.
178-
179-
[cpp_auto]: https://google.github.io/styleguide/cppguide.html#Type_deduction
180-
[cpp_ownership]: https://google.github.io/styleguide/cppguide.html#Ownership_and_Smart_Pointers
181-
[dart_inference]: https://dart.dev/effective-dart/design#types
182-
[dart_style]: https://dart.dev/effective-dart/style
183-
[linux embedding]: shell/platform/linux
184-
[google_cpp_style]: https://google.github.io/styleguide/cppguide.html
185-
[pr_49801]: https://github.com/flutter/engine/pull/49801
18634
[code_of_conduct]: https://github.com/flutter/flutter/blob/master/CODE_OF_CONDUCT.md
18735
[contrib_guide]: https://github.com/flutter/flutter/blob/master/CONTRIBUTING.md
18836
[engine_dev_setup]: https://github.com/flutter/flutter/wiki/Setting-up-the-Engine-development-environment
189-
[flutter_style]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
19037
[objc_style]: https://google.github.io/styleguide/objcguide.html
38+
[objcc_style]: https://google.github.io/styleguide/objcguide.html#objective-c
19139
[java_style]: https://google.github.io/styleguide/javaguide.html
192-
[google_python_style]: https://google.github.io/styleguide/pyguide.html
19340

19441
## Testing
19542

analysis_options.yaml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
#
33
# This file is a copy of analysis_options.yaml from flutter repo
44
# as of 2023-12-18, but with some modifications marked with
5-
# "DIFFERENT FROM FLUTTER/FLUTTER" below:
6-
# - Rem
5+
# "DIFFERENT FROM FLUTTER/FLUTTER" below.
76

87
analyzer:
98
language:
@@ -30,7 +29,7 @@ linter:
3029
- always_declare_return_types
3130
- always_put_control_body_on_new_line
3231
# - always_put_required_named_parameters_first # we prefer having parameters in the same order as fields https://github.com/flutter/flutter/issues/10219
33-
# - always_specify_types # DIFFERENT FROM FLUTTER/FLUTTER; see https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#dart
32+
- always_specify_types
3433
# - always_use_package_imports # we do this commonly
3534
- annotate_overrides
3635
# - avoid_annotating_with_dynamic # conflicts with always_specify_types

lib/ui/analysis_options.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,4 @@ analyzer:
77

88
linter:
99
rules:
10-
always_specify_types: true # dart:ui is shipped as part of flutter/flutter, let's keep them consistent
1110
unreachable_from_main: false # lint not compatible with how dart:ui is structured

0 commit comments

Comments
 (0)