Sdk: quick fix for diagnostic_describe_all_properties

Created on 28 Sep 2019  Â·  7Comments  Â·  Source: dart-lang/sdk

Follow-up from: https://github.com/flutter/flutter/issues/41513#issuecomment-536117462.

@jacob314: feel free to chime in w/ any implementation notes here. Any heuristics for code generation would be greatly appreciated!


Property subclasses to support

  • [x] BooleanProperty -- bool (https://github.com/dart-lang/sdk/commit/23aca57eda0697e2832b11f287ff476d63df6c65)
  • [x] ColorProperty -- for Color (https://github.com/dart-lang/sdk/commit/0a63bd4b0acef0a1613ddc5f9e536fc6de052a6b)
  • [x] EnumProperty -- for any enum class (https://github.com/dart-lang/sdk/commit/512d763b9daf7e6a436055c9d763ff830d35cf65)
  • [x] IntProperty -- int (https://github.com/dart-lang/sdk/commit/512d763b9daf7e6a436055c9d763ff830d35cf65)
  • [x] DoubleProperty -- double (https://github.com/dart-lang/sdk/commit/512d763b9daf7e6a436055c9d763ff830d35cf65)
  • [x] IterableProperty -- any iterable (https://github.com/dart-lang/sdk/commit/0a63bd4b0acef0a1613ddc5f9e536fc6de052a6b)
  • [x] StringProperty -- string (https://github.com/dart-lang/sdk/commit/512d763b9daf7e6a436055c9d763ff830d35cf65)
  • [x] TransformProperty -- Matrix4 (https://github.com/dart-lang/sdk/commit/0a63bd4b0acef0a1613ddc5f9e536fc6de052a6b)
  • [x] DiagnosticsProperty for any T that doesn't match one of the other cases (https://github.com/dart-lang/sdk/commit/69b612beec0bc632564a7ea5aacd9bdf59ad5e01)
analyzer-linter analyzer-quick-fix area-analyzer customer-flutter

All 7 comments

@goderbauer Could you send us a couple of before/after pairs? That would likely be very helpful for informing what the fix should do.

I can provide all the before and after pairs.
https://api.flutter.dev/flutter/foundation/DiagnosticsProperty-class.html
gives a list of all subclasses.
ColorProperty -- for Color
EnumProperty -- for any enum class
IntProperty -- int
DoubleProperty -- double
IterableProperty -- any iterable
StringProperty -- string
TransformProperty -- Matrix4
DiagnosticsProperty for any T that doesn't match one of the other cases.

For each case the quick fix is to either add a line at the end of the existing debugFillProperties method or to override the debugFillProperties method and then add the property.
For example, for an int property you would add the line
properties.add(IntProperty('myPropertyName', myPropertyName));
method or add a new one. The big time savings for the quick fix is otherwise we were finding it was a real pain to for every single property navigate to the debugFillProperties method to add the property. Sometimes it wasn't obvious where the method was and almost all of these migrations are mechanical.

That should be fairly easy to automate. Given the amount of work there is it sounds like we should also consider enabling that via dartfix, which could apply the fix in bulk.

There would still be a bit of manual work to make sure the properties have
the right default values and determine whether each property shouldn't be
added or not so lets hold up on dartfix integration.

On Sat, Sep 28, 2019 at 9:35 PM Brian Wilkerson notifications@github.com
wrote:

That should be fairly easy to automate. Given the amount of work there is
it sounds like we should also consider enabling that via dartfix, which
could apply the fix in bulk.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dart-lang/sdk/issues/38633?email_source=notifications&email_token=AAJLQPG72DULJTHGEATY3L3QMAWCBA5CNFSM4I3OJ24KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD73H6ZQ#issuecomment-536248166,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJLQPBXVM2KNZZRA6P2OBLQMAWCBANCNFSM4I3OJ24A
.

How do you manually determine the default value for a property? Is it possible to statically determine the right default value for a property? Maybe by looking at the field's initializer and/or the constructors?

How do you manually determine which properties to add? I would have assumed that the lint was able to do this in order to not produce false positives. If that's not the case then we should at least be able to add an 'ignore' comment for properties that shouldn't be added. If you do that, then those diagnostics will be suppressed and given that fixes are only applied where a diagnostic is reported, we should still be able to use dartfix after marking the properties that shouldn't be added.

Video demonstrating the simple/common case...

diagProps1

🤘

/cc @jacob314

Was this page helpful?
0 / 5 - 0 ratings