new lints #8849
new lints #8849
Conversation
| } | ||
| if (name != null) { | ||
| arguments.add(new NamedExpression('name', new StringLiteral(name))); | ||
| arguments.add( NamedExpression('name', StringLiteral(name))); |
There was a problem hiding this comment.
remove space after parenthesis (here and at several place in the diff)
lib/stub_ui/lib/geometry.dart
Outdated
| @@ -194,7 +194,7 @@ class Offset extends OffsetBase { | |||
| // This is included for completeness, because [Size.infinite] exists. | |||
| static const Offset infinite = const Offset(double.infinity, double.infinity); | |||
|
|
|||
| /// Returns a new offset with the x component scaled by `scaleX` and the y | |||
| /// Returns a offset with the x component scaled by `scaleX` and the y | |||
There was a problem hiding this comment.
new should not be removed inside comments :)
There was a problem hiding this comment.
Ah thanks. There were a few places where it made sense in the comments for sample code, guess I was overzealous
There was a problem hiding this comment.
Leaving stub_ui as is, since there's another PR to remove all those comments anyway
|
To support analysis/testing with set literals, this involved updating the pre-built Dart SDK we use. I've dumped the python hook for a cipd package and bumped the version. |
|
For a similar reason as I posted on the other review: please add exclude to stub_ui for this analysis code, as it will make syncing + unforking more difficult |
| @@ -159,7 +159,7 @@ void main() { | |||
| }; | |||
| }); | |||
|
|
|||
| final ByteData testData = new ByteData.view(new Uint8List(0).buffer); | |||
| final ByteData testData = ByteData.view(new Uint8List(0).buffer); | |||
There was a problem hiding this comment.
I wonder why the analyzer misses that.
|
@jonahwilliams I think I don't understand what stub_ui is then. Is it really better to let it drift further out of sync with ui? |
|
Only the API conformance matters. Everything else will be owned by the web team and I leave it up to them how to lint their codebase. stub_ui is going to be replaced wholesale within a short period of time with the real implementation. The replacement may not conform to all of these lints, and asking for a sync and fix in the same commit is too much I think. |
|
I've reverted stub_ui portions of this PR. I suspect what we really want here though is a set of interface/abstract classes that both implementations have to conform to... |
|
Ideally yes, but that is not currently possible with dart:* libraries. We could declare a set of interfaces and compile them out, though it would complicate the native ui implementation. The way the SDK handles different implementations is via patch files (https://github.com/dart-lang/sdk/blob/master/sdk/lib/_internal/js_runtime/lib/io_patch.dart). The problems are that you can no longer read the code to figure out what it does, and it requires designing the interface to be patched. In our case it would be breaking. |
|
Also reverted |
|
If it helps I could try splitting some of these out - it's a little tricky but can be done. The |
flutter/engine@149b7b8...0fc6efb git log 149b7b8..0fc6efb --no-merges --oneline 0fc6efb Roll src/third_party/dart a8b6e18b62..8143450941 (11 commits) 93d1c37 Remove absolute path in new Fuchsia SDK based runner target dependency. (flutter/engine#8888) 0b406c3 Copy the Flutter Runner from //topaz into the engine. (flutter/engine#8886) 2b1f992 new lints (flutter/engine#8849) 0c7a3f2 Copy //dart-pkg/zircon|fuchsia from Topaz into the engine. (flutter/engine#8884) ba82ea2 Roll src/third_party/skia 056944747072..51e15a69ce5b (8 commits) (flutter/engine#8885) d717044 Copy //runtime/dart/utils from Topaz into the engine. (flutter/engine#8871) 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 (chinmaygarde@google.com), and stop the roller if necessary.
Dart lints added: * Avoid optional new * Avoid optional const * Prefer single quotes * Prefer default assignment `=`
Adds lints for:
Updates a bunch of files that were not conforming to other lints as well.
Updates some miscellaneous formatting issues.
Updates a test that wasn't using the variable it declared.
@jonahwilliams can you validate the changes in web_sdk for me? I can pull those out if we're worried about them. Some of those files seem completely wrong right now.