Make operator == override parameters non-nullable #677

srawlins · 2023-01-17T18:26:04Z

Make operator == override parameters non-nullable. See dart-lang/linter#3441

…r#3441 Work towards dart-lang/sdk#51038.

srawlins · 2023-01-17T18:50:32Z

I'd love to see those CI failure details.

srawlins · 2023-01-17T19:47:40Z

packages/reactive_ble_platform_interface/lib/src/model/discovered_characteristic.dart

      runtimeType == other.runtimeType &&
-      characteristicId == other.characteristicId &&
-      serviceId == other.serviceId;
+      characteristicId == (other as DiscoveredCharacteristic).characteristicId &&


There is some technical debt here, as this cast could fail at runtime. But I am only here to make a no-op change, and not change behavior.

@srawlins I came to the same conclusion here.

What if you did

@override bool operator ==(Object other) { return other is DiscoveredCharacteristic && runtimeType == other.runtimeType && characteristicId == other. characteristicId && serviceId == other. serviceId; }

This is definitely the change I would want to make at some point if I owned the code.

This would be a behavior-changing change, which I am not very comfortable making. x.runtime == y.runtime is different from x == Y. I don't know what subtypes there are, or where exception catching happens and if specific exceptions are caught here or there. I don't want to land a change that is then rolled back because of the changes unrelated to correcting the static analysis issue.

All that being said, I can make your suggested change if you would really like me to.

Then again, now that I can see the static analysis issues, it looks like I must make the suggested change, to avoid test_types_in_equals.

navaronbracke · 2023-01-18T07:36:17Z

packages/reactive_ble_platform_interface/lib/src/model/qualified_characteristic.dart

@@ -31,9 +31,9 @@ class QualifiedCharacteristic {
      deviceId.hashCode;

  @override
-  bool operator ==(dynamic other) =>


Same thing here:

@override bool operator ==(Object other) { return other is QualifiedCharacteristic && runtimeType == other.runtimeType && characteristicId == other. characteristicId && serviceId == other. serviceId && deviceId == other. deviceId; }

navaronbracke · 2023-01-18T07:38:26Z

packages/reactive_ble_platform_interface/lib/src/model/uuid.dart

@@ -68,9 +68,9 @@ class Uuid {
      data.fold(17, (hash, octet) => 37 * hash + octet.hashCode);

  @override
-  bool operator ==(dynamic other) =>
+  bool operator ==(Object other) =>


Same thing here:

@override bool operator ==(Object other) { return other is Uuid && other.runtimeType == runtimeType && const DeepCollectionEquality().equals(other.data, data); }

Taym95 · 2023-01-18T10:17:58Z

@srawlins

[reactive_ble_platform_interface]: 
[reactive_ble_platform_interface]: 
[reactive_ble_platform_interface]:    info • Test type arguments in operator ==(Object other) • lib/src/model/discovered_characteristic.dart:43:28 • test_types_in_equals
[reactive_ble_platform_interface]:    info • Test type arguments in operator ==(Object other) • lib/src/model/discovered_characteristic.dart:44:21 • test_types_in_equals
[reactive_ble_platform_interface]:    info • Unnecessary cast • lib/src/model/discovered_characteristic.dart:44:21 • unnecessary_cast
[reactive_ble_platform_interface]:    info • Test type arguments in operator ==(Object other) • lib/src/model/qualified_characteristic.dart:36:28 • test_types_in_equals
[reactive_ble_platform_interface]:    info • Test type arguments in operator ==(Object other) • lib/src/model/qualified_characteristic.dart:37:21 • test_types_in_equals
[reactive_ble_platform_interface]:    info • Unnecessary cast • lib/src/model/qualified_characteristic.dart:37:21 • unnecessary_cast
[reactive_ble_platform_interface]:    info • Test type arguments in operator ==(Object other) • lib/src/model/qualified_characteristic.dart:38:20 • test_types_in_equals
[reactive_ble_platform_interface]:    info • Unnecessary cast • lib/src/model/qualified_characteristic.dart:38:20 • unnecessary_cast
[reactive_ble_platform_interface]:    info • Test type arguments in operator ==(Object other) • lib/src/model/uuid.dart:73:46 • test_types_in_equals
[reactive_ble_platform_interface]: 
[reactive_ble_platform_interface]: 9 issues found. (ran in 1.3s)

You dont need CI, you can check it locally with melos

srawlins · 2023-01-19T21:37:53Z

Ready for review

Taym95 · 2023-01-23T12:36:44Z

packages/reactive_ble_platform_interface/lib/src/model/uuid.dart

+    return other is Uuid &&
+        other.runtimeType == runtimeType &&
+        const DeepCollectionEquality().equals(other.data, data);
+  }


Still failing, please check quality locally:

$ melos exec └> flutter analyze └> FAILED (in 1 packages) └> reactive_ble_platform_interface (with exit code 1) melos run analyze └> melos exec -- flutter analyze └> FAILED ScriptException: The script analyze failed to execute

Thanks, complete.

srawlins · 2023-01-30T21:05:06Z

Ready for review

srawlins · 2023-02-06T20:32:07Z

Ready for review

srawlins · 2023-02-07T15:20:05Z

Thank you!

* Make operator == override parameters no-nullable. See dart-lang/linter#3441 Work towards dart-lang/sdk#51038.

Make operator == override parameters no-nullable. See dart-lang/linte…

7a5b262

…r#3441 Work towards dart-lang/sdk#51038.

srawlins mentioned this pull request Jan 17, 2023

Flutter customer tests override operator == with a dynamic parameter. dart-lang/sdk#51038

Closed

6 tasks

fix with casts

533af88

srawlins commented Jan 17, 2023

View reviewed changes

navaronbracke reviewed Jan 18, 2023

View reviewed changes

feedback

326679f

Taym95 reviewed Jan 23, 2023

View reviewed changes

feedback

e4d8989

Taym95 approved these changes Feb 7, 2023

View reviewed changes

Taym95 merged commit e14d8d0 into PhilipsHue:master Feb 7, 2023

srawlins deleted the nullable-equals branch February 7, 2023 15:19

Taym95 pushed a commit that referenced this pull request Jul 6, 2023

Make operator == override parameters non-nullable (#677)

4cfae9f

* Make operator == override parameters no-nullable. See dart-lang/linter#3441 Work towards dart-lang/sdk#51038.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make operator == override parameters non-nullable #677

Make operator == override parameters non-nullable #677

srawlins commented Jan 17, 2023

srawlins commented Jan 17, 2023

srawlins Jan 17, 2023

navaronbracke Jan 18, 2023

srawlins Jan 18, 2023

srawlins Jan 18, 2023

srawlins Jan 18, 2023

navaronbracke Jan 18, 2023

srawlins Jan 18, 2023

navaronbracke Jan 18, 2023

srawlins Jan 18, 2023

Taym95 commented Jan 18, 2023 •

edited

Loading

srawlins commented Jan 19, 2023

Taym95 Jan 23, 2023

srawlins Jan 23, 2023

srawlins commented Jan 30, 2023

srawlins commented Feb 6, 2023

srawlins commented Feb 7, 2023

Make operator == override parameters non-nullable #677

Make operator == override parameters non-nullable #677

Conversation

srawlins commented Jan 17, 2023

srawlins commented Jan 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Taym95 commented Jan 18, 2023 • edited Loading

srawlins commented Jan 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srawlins commented Jan 30, 2023

srawlins commented Feb 6, 2023

srawlins commented Feb 7, 2023

Taym95 commented Jan 18, 2023 •

edited

Loading