fix up analysis for Dart in Engine #7404
Conversation
|
/cc @zanderso - I added some ignores to Fuchsia specific files, not sure if there's a better way to handle that though (and I'm assuming it won't break anything, but it'd be nice to get the OK from you). |
| - prefer_bool_in_asserts | ||
| - prefer_collection_literals | ||
| - prefer_conditional_assignment | ||
| - prefer_const_constructors |
There was a problem hiding this comment.
ideally we'd find a way to make this file always exactly the same as the version in the flutter/flutter repo
There was a problem hiding this comment.
I imagine a lot of projects would want that functionality -- "start with this canonical analysis options file, then override it with the following options"
Perhaps the analyzer could beef up its include directive to support not only relative paths, but also an enumerated set of SDKs? In this world, we'd move the canonical analysis options that currently live in the flutter repo into the Dart SDK as the "flutter analysis options", then the flutter repo would just say "I use Flutter's options"...
/cc @pq
There was a problem hiding this comment.
Somewhat like clang-format use chromium style?
There was a problem hiding this comment.
I would like that too - I usually start a new Flutter project by copying the Flutter repo's analysis_options
There was a problem hiding this comment.
There was a problem hiding this comment.
That has landed - pulling it in to this will require a bunch more changes in the implementation files (get rid of optional new, use = instead of : for parameters, etc.) that would be better to do in a separate PR.
lib/snapshot/snapshot.dart
Outdated
| // found in the LICENSE file. | ||
|
|
||
| // ignore_for_file: unused_import,uri_does_not_exist | ||
|
|
There was a problem hiding this comment.
I'd rather we used explicit // ignore on each line rather than ignore_for_file. ignore_for_file makes it very easy to add new problems. If we really want to just turn it off, we can do so in the analysis_options file or using grep -v or some such.
There was a problem hiding this comment.
I'd like to add native_function_body_in_non_sdk_code: ignore to analysis options then - There are a lot of places it gets used, and it would have to be remembered by anyone who adds a native binding method to lib/ui. Thoughts on that?
There was a problem hiding this comment.
For instance, painting.dart has something like 100 lines that use the native keyword.
| sky_engine: | ||
| path: ../../../out/host_debug_unopt/gen/dart-pkg/sky_engine | ||
| sky_services: | ||
| path: ../../../out/host_debug_unopt/gen/dart-pkg/sky_services |
lib/ui/ui.dart
Outdated
| /// This library exposes the lowest-level services that Flutter frameworks use | ||
| /// to bootstrap applications, such as classes for driving the input, graphics | ||
| /// text, layout, and rendering subsystems. | ||
There was a problem hiding this comment.
(the dartdocs here apply to the library keyword)
flutter/engine@b7f6bf0...35daf14 git log b7f6bf0..35daf14 --no-merges --oneline 35daf14 Roll src/third_party/skia d2fa7e33798c..c334df71b8f9 (45 commits) (flutter/engine#7448) 6179ac6 fix up analysis for Dart in Engine (flutter/engine#7404) bec12d8 Reland Dart SDK rolls made since 2019/01/08 (flutter/engine#7446) 3f99878 Match the ios number input type behavior to what is said in the docs (flutter/engine#7281) 358a24c Make SetLocales more consistent with other RuntimeController methods (flutter/engine#7447) 0c11836 Use anti-aliasing when drawing text in the performance overlay (flutter/engine#7445) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
flutter/engine@b7f6bf0...35daf14 git log b7f6bf0..35daf14 --no-merges --oneline 35daf14 Roll src/third_party/skia d2fa7e33798c..c334df71b8f9 (45 commits) (flutter/engine#7448) 6179ac6 fix up analysis for Dart in Engine (flutter/engine#7404) bec12d8 Reland Dart SDK rolls made since 2019/01/08 (flutter/engine#7446) 3f99878 Match the ios number input type behavior to what is said in the docs (flutter/engine#7281) 358a24c Make SetLocales more consistent with other RuntimeController methods (flutter/engine#7447) 0c11836 Use anti-aliasing when drawing text in the performance overlay (flutter/engine#7445) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
7 participants
This PR does a few things:
dart:uistufffinalandconst)newfrom test files - I didn't touch this in the acutal Dart implementation filesnativekeyword usage outside the SDK, and in a few places ignoring unused elements (until Analysis should not mark a member decorated with@pragma('vm:entry-point')as unused dart-lang/sdk#35164 is resolved)Locally, this resolves all dart analysis issues in VSCode when working with the engine repo, and should resolve the issues reported in Dart-Code/Dart-Code#692!