279 lines
13 KiB
Markdown
279 lines
13 KiB
Markdown
# Contributing to the Flutter engine
|
|
|
|
_See also: [Flutter's code of conduct][code_of_conduct]_
|
|
|
|
## Welcome
|
|
|
|
For an introduction to contributing to Flutter, see [our contributor
|
|
guide][contrib_guide].
|
|
|
|
For specific instructions regarding building Flutter's engine, see [Setting up
|
|
the Engine development environment][engine_dev_setup] on our wiki. Those
|
|
instructions are part of the broader onboarding instructions described in the
|
|
contributing guide.
|
|
|
|
## Style
|
|
|
|
The Flutter engine _generally_ follows Google style for the languages it uses,
|
|
with some exceptions.
|
|
|
|
### C/C++
|
|
|
|
Follows the [Google C++ Style Guide][google_cpp_style] and is automatically
|
|
formatted with `clang-format`.
|
|
|
|
Some additional considerations that are in compliance with the style guide, but
|
|
are worth noting:
|
|
|
|
#### Judiciously use shared_ptr
|
|
|
|
The engine currently (as of 2024-05-15) uses `shared_ptr` liberally, which can
|
|
be expensive to copy, and is not always necessary.
|
|
|
|
The C++ style guide has a
|
|
[section on ownership and smart pointers][cpp_ownership] worth reading:
|
|
|
|
> Do not design your code to use shared ownership without a very good reason.
|
|
> One such reason is to avoid expensive copy operations, but you should only do
|
|
> this if the performance benefits are significant, and the underlying object is
|
|
> immutable.
|
|
|
|
Prefer using `std::unique_ptr` when possible.
|
|
|
|
#### Judiciously use `auto`
|
|
|
|
The C++ style guide has a [section on type deduction][cpp_auto] that is worth
|
|
reading:
|
|
|
|
> The fundamental rule is: use type deduction only to make the code clearer or
|
|
> safer, and do not use it merely to avoid the inconvenience of writing an
|
|
> explicit type. When judging whether the code is clearer, keep in mind that
|
|
> your readers are not necessarily on your team, or familiar with your project,
|
|
> so types that you and your reviewer experience as unnecessary clutter will
|
|
> very often provide useful information to others. For example, you can assume
|
|
> that the return type of `make_unique<Foo>()` is obvious, but the return type
|
|
> of `MyWidgetFactory()` probably isn't.
|
|
|
|
Due to our codebase's extensive use of `shared_ptr`, `auto` can have surprising
|
|
performance implications. See [#49801][pr_49801] for an example.
|
|
|
|
#### Linux Embedding
|
|
|
|
> [!NOTE]
|
|
> The Linux embedding instead follows idiomatic GObject-based C style.
|
|
|
|
Use of C++ in the [Linux embedding][] is discouraged in that embedding to avoid
|
|
creating hybrid code that feels unfamiliar to either developers used to working
|
|
with `GObject` or C++ developers.
|
|
|
|
For example, _do not_ use STL collections or `std::string`, but _do_:
|
|
|
|
- Use C++ casts (C-style casts are forbidden).
|
|
- Use `nullptr` rather than `NULL`.
|
|
- Avoid `#define`; for internal constants use `static constexpr` instead.
|
|
|
|
### Dart
|
|
|
|
The Flutter engine _intends_ to follow the [Dart style guide][dart_style] but
|
|
currently follows the [Flutter style guide][flutter_style], with the following
|
|
exceptions:
|
|
|
|
#### Use of type inference is allowed
|
|
|
|
The [Dart style guide][dart_inference] only requires explicit types when type
|
|
inference is not possible, but the Flutter style guide always requires explicit
|
|
types. The engine is moving towards the Dart style guide, but this is a gradual
|
|
process. In the meantime, follow these guidelines:
|
|
|
|
- **Always** annotate when inference is not possible.
|
|
- **Prefer** annotating when inference is possible but the type is not
|
|
obvious.
|
|
|
|
Some cases when using `var`/`final`/`const` is appropriate:
|
|
|
|
- When the type is obvious from the right-hand side of the assignment:
|
|
|
|
```dart
|
|
// Capitalized constructor name always returns a Foo.
|
|
var foo = Foo();
|
|
|
|
// Similar with factory constructors.
|
|
var bar = Bar.create();
|
|
|
|
// Literals (strings, numbers, lists, maps, etc) always return the same type.
|
|
var name = 'John Doe';
|
|
var flag = true;
|
|
var numbers = [1, 2, 3];
|
|
var map = {'one': 1, 'two': 2, 'three': 3};
|
|
```
|
|
|
|
- When the type is obvious from the method name:
|
|
|
|
```dart
|
|
// toString() always returns a String.
|
|
var string = foo().toString();
|
|
|
|
// It's reasonable to assume that length returns an int.
|
|
var length = string.length;
|
|
```
|
|
|
|
- When the type is obvious from the context:
|
|
|
|
```dart
|
|
// When variables are in the same scope, reduce() clearly returns an int.
|
|
var list = [1, 2, 3];
|
|
var sum = list.reduce((a, b) => a + b);
|
|
```
|
|
|
|
Some cases where an explicit type should be considered:
|
|
|
|
- When the type is not obvious from the right-hand side of the assignment:
|
|
|
|
```dart
|
|
// What does 'fetchLatest()' return?
|
|
ImageBuffer buffer = fetchLatest();
|
|
|
|
// What does this large chain of method calls return?
|
|
Iterable<int> numbers = foo().bar().map((b) => b.baz());
|
|
```
|
|
|
|
- When there are semantic implications to the type:
|
|
|
|
```dart
|
|
// Without 'num', the map would be inferred as 'Map<String, int>'.
|
|
const map = <String, num>{'one': 1, 'two': 2, 'three': 3};
|
|
```
|
|
|
|
- Or, **when a reviewer requests it**!
|
|
|
|
Remember that the goal is to make the code more readable and maintainable, and
|
|
explicit types _can_ help with that. Code can be changed, so it's always
|
|
possible to add or remove type annotations later as the code evolves, so avoid
|
|
bikeshedding over this.
|
|
|
|
### Java
|
|
|
|
Follows the [Google Java Style Guide][java_style] and is automatically formatted
|
|
with `google-java-format`.
|
|
|
|
### Objective-C
|
|
|
|
Follows the [Google Objective-C Style Guide][objc_style], including for
|
|
Objective-C++ and is automatically formatted with `clang-format`.
|
|
|
|
### Python
|
|
|
|
Follows the [Google Python Style Guide][google_python_style] and is
|
|
automatically formatted with `yapf`.
|
|
|
|
> [!WARNING]
|
|
> Historically, the engine grew a number of one-off Python scripts, often as
|
|
> part of the testing or build infrastructure (i.e. command-line tools). We are
|
|
> instead moving towards using Dart for these tasks, so new Python scripts
|
|
> should be avoided whenever possible.
|
|
|
|
### GN
|
|
|
|
Automatically formatted with `gn format`.
|
|
|
|
[cpp_auto]: https://google.github.io/styleguide/cppguide.html#Type_deduction
|
|
[cpp_ownership]: https://google.github.io/styleguide/cppguide.html#Ownership_and_Smart_Pointers
|
|
[dart_inference]: https://dart.dev/effective-dart/design#types
|
|
[dart_style]: https://dart.dev/effective-dart/style
|
|
[linux embedding]: shell/platform/linux
|
|
[google_cpp_style]: https://google.github.io/styleguide/cppguide.html
|
|
[pr_49801]: https://github.com/flutter/engine/pull/49801
|
|
[code_of_conduct]: https://github.com/flutter/flutter/blob/master/CODE_OF_CONDUCT.md
|
|
[contrib_guide]: https://github.com/flutter/flutter/blob/master/CONTRIBUTING.md
|
|
[engine_dev_setup]: docs/contributing/Setting-up-the-Engine-development-environment.md
|
|
[flutter_style]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
|
|
[objc_style]: https://google.github.io/styleguide/objcguide.html
|
|
[java_style]: https://google.github.io/styleguide/javaguide.html
|
|
[google_python_style]: https://google.github.io/styleguide/pyguide.html
|
|
|
|
## Testing
|
|
|
|
The testing policy for contributing to the flutter engine can be found at the
|
|
[Tree Hygiene Wiki][tree_hygiene_wiki]. The summary is that all PR's to the
|
|
engine should be tested or have an explicit test exemption.
|
|
|
|
Because the engine targets multiple platforms the testing infrastructure is
|
|
fairly complicated. Here are some more resources to help guide writing tests:
|
|
|
|
- [Testing the engine wiki][testing_the_engine_wiki] - A guide on writing tests
|
|
for the engine including an overview of the different tests and the different
|
|
technologies the engine uses.
|
|
- [`//testing`](./testing) - This is where the `run_tests.py` script is located.
|
|
All tests will have the ability to be executed with `run_tests.py`.
|
|
- [`//ci/builders`](./ci/builders) - The JSON files that determine how tests are
|
|
executed on CI.
|
|
|
|
Tests will be executed on CI, but some tests will be executed before PR's can be
|
|
merged (presubmit) and others after they have been merged (postsubmit). Ideally
|
|
everything would be presubmit but tests that take up more resources are executed
|
|
in postsubmit.
|
|
|
|
### Skia Gold
|
|
|
|
The Flutter engine uses [Skia Gold][skia_gold] for image comparison tests which
|
|
fail if the image is different from an accepted baseline image.
|
|
|
|
[skia_gold]: https://flutter-engine-gold.skia.org/
|
|
|
|
Any untriaged failures will block presubmit and postsubmit tests.
|
|
|
|
### Example run_tests.py invocation
|
|
|
|
```sh
|
|
# Configure host build for macOS arm64 debug.
|
|
$ flutter/tools/gn --runtime-mode=debug --unoptimized --no-lto --mac-cpu=arm64
|
|
# Compile default targets (should cover all applicable run_tests.py requirements).
|
|
$ ninja -j100 -C out/host_debug_unopt_arm64
|
|
# Run all cross-platform C++ tests for the debug build arm64 variant.
|
|
$ cd flutter/testing
|
|
$ ./run_tests.py --variant=host_debug_unopt_arm64 --type=engine
|
|
```
|
|
|
|
### Directory
|
|
|
|
| Name | run_tests.py type | Description |
|
|
| ---------------------------------------- | ------------------- | --------------------------------------------------------------- |
|
|
| accessibility_unittests | engine | |
|
|
| assets_unittests | engine | |
|
|
| client_wrapper_glfw_unittests | engine | |
|
|
| client_wrapper_unittests | engine | |
|
|
| common_cpp_core_unittests | engine | |
|
|
| common_cpp_unittests | engine | |
|
|
| dart_plugin_registrant_unittests | engine | |
|
|
| display_list_rendertests | engine | |
|
|
| display_list_unittests | engine | |
|
|
| embedder_a11y_unittests | engine | |
|
|
| embedder_proctable_unittests | engine | |
|
|
| embedder_unittests | engine | |
|
|
| felt | n/a | The test runner for flutter web. See //lib/web_ui |
|
|
| flow_unittests | engine | |
|
|
| flutter_tester | dart | Launcher for engine dart tests. |
|
|
| fml_unittests | engine | Unit tests for //fml |
|
|
| framework_common_unittests | engine(mac) | |
|
|
| gpu_surface_metal_unittests | engine(mac) | |
|
|
| impeller_dart_unittests | engine | |
|
|
| impeller_golden_tests | engine(mac) | Generates golden images for impeller (vulkan, metal, opengles). |
|
|
| impeller_unittests | engine | impeller unit tests and interactive tests |
|
|
| ios_scenario_app | android | Integration and golden tests for iOS. |
|
|
| ios_test_flutter | objc | dynamic library of objc tests to be run with XCTest |
|
|
| jni_unittests | engine(not windows) | |
|
|
| no_dart_plugin_registrant_unittests | engine | |
|
|
| platform_view_android_delegate_unittests | engine(not windows) | |
|
|
| runtime_unittests | engine | |
|
|
| shell_unittests | engine(not windows) | |
|
|
| tonic_unittests | engine | Unit tests for //third_party/tonic |
|
|
| txt_unittests | engine(linux) | |
|
|
| ui_unittests | engine | |
|
|
|
|
[tree_hygiene_wiki]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
|
|
[testing_the_engine_wiki]: https://github.com/flutter/flutter/blob/main/docs/engine/testing/Testing-the-engine.md
|
|
|
|
## Fuchsia Contributions from Googlers
|
|
|
|
Googlers contributing to Fuchsia should follow the additional steps at:
|
|
go/flutter-fuchsia-pr-policy.
|