Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The borderRadius property is missing from equality and hashCode in UnderlineInputBorder, equality and hashCode tests of UnderlineInputBorder and OutlineInputBorder are not exhaustive #118282

Closed
rydmike opened this issue Jan 10, 2023 · 4 comments · Fixed by #118284
Assignees
Labels
f: material design flutter/packages/flutter/material repository. found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on r: fixed Issue is closed as already fixed in a newer version

Comments

@rydmike
Copy link
Contributor

rydmike commented Jan 10, 2023

The UnderlineInputBorder has equality and hashCode gap

The borderRadius property is missing from equality and hashCode in UnderlineInputBorder. This causes update issues in ThemeData if a theme changes only the upper side border radius in an InputDecoratorTheme, the application theme does not update since the UnderlineInputBorder is seen as identical.

The UnderlineInputBorder has properties borderSide and borderRadius , but only borderSide is included in equality and hashCode.

class UnderlineInputBorder extends InputBorder {

class UnderlineInputBorder extends InputBorder {
  /// Creates an underline border for an [InputDecorator].
  ///
  /// The [borderSide] parameter defaults to [BorderSide.none] (it must not be
  /// null). Applications typically do not specify a [borderSide] parameter
  /// because the input decorator substitutes its own, using [copyWith], based
  /// on the current theme and [InputDecorator.isFocused].
  ///
  /// The [borderRadius] parameter defaults to a value where the top left
  /// and right corners have a circular radius of 4.0. The [borderRadius]
  /// parameter must not be null.
  const UnderlineInputBorder({
    BorderSide borderSide = const BorderSide(),
    this.borderRadius = const BorderRadius.only(
      topLeft: Radius.circular(4.0),
      topRight: Radius.circular(4.0),
    ),
  }) : assert(borderRadius != null),
       super(borderSide: borderSide);

However borderRadius is not included in hashCode or equality.

  @override
  bool operator ==(Object other) {
    if (identical(this, other))
      return true;
    if (other.runtimeType != runtimeType)
      return false;
    return other is InputBorder
        && other.borderSide == borderSide;
  }

  @override
  int get hashCode => borderSide.hashCode;
}

Q: Is there some reason why the UnderlineInputBorder equality is designed this way?

This gap causes certain theme updates to fail.

Will try to submit a fix PR.

Expected results

Expect these unit tests to pass:

void main() {
  group('UnderlineInputBorder test', () {
    test('UnderlineInputBorder equality', () {
      // UnderlineInputBorder's equality is defined only by the borderSide
      const UnderlineInputBorder underlineInputBorder =
          UnderlineInputBorder(borderSide: BorderSide(color: Colors.blue));
      expect(
          underlineInputBorder,
          const UnderlineInputBorder(
              borderSide: BorderSide(color: Colors.blue)));
      expect(underlineInputBorder, isNot(const UnderlineInputBorder()));
      // UnderlineInputBorder's equality is defined by the borderSide and borderRadius
      const UnderlineInputBorder underlineInputBorderRadius =
          UnderlineInputBorder(
        borderSide: BorderSide(color: Colors.red),
        borderRadius: BorderRadius.only(
          topLeft: Radius.circular(5.0),
          topRight: Radius.circular(5.0),
        ),
      );
      expect(
        underlineInputBorderRadius,
        const UnderlineInputBorder(
          borderSide: BorderSide(color: Colors.red),
          borderRadius: BorderRadius.only(
            topLeft: Radius.circular(5.0),
            topRight: Radius.circular(5.0),
          ),
        ),
      );
      expect(
        underlineInputBorderRadius,
        isNot(
          const UnderlineInputBorder(
            borderSide: BorderSide(color: Colors.red),
            borderRadius: BorderRadius.only(
              topLeft: Radius.circular(6.0),
              topRight: Radius.circular(6.0),
            ),
          ),
        ),
      );
    });

    test('UnderlineInputBorder hashCodes', () {
      // UnderlineInputBorder's hashCode is defined only by the borderSide
      const UnderlineInputBorder underlineInputBorder =
          UnderlineInputBorder(borderSide: BorderSide(color: Colors.blue));
      expect(
          underlineInputBorder.hashCode,
          const UnderlineInputBorder(borderSide: BorderSide(color: Colors.blue))
              .hashCode);
      expect(underlineInputBorder.hashCode,
          isNot(const UnderlineInputBorder().hashCode));
      // UnderlineInputBorder's hashCode is defined by the borderSide and borderRadius
      const UnderlineInputBorder underlineInputBorderRadius =
          UnderlineInputBorder(
        borderSide: BorderSide(color: Colors.red),
        borderRadius: BorderRadius.only(
          topLeft: Radius.circular(5.0),
          topRight: Radius.circular(5.0),
        ),
      );
      expect(
        underlineInputBorderRadius.hashCode,
        const UnderlineInputBorder(
          borderSide: BorderSide(color: Colors.red),
          borderRadius: BorderRadius.only(
            topLeft: Radius.circular(5.0),
            topRight: Radius.circular(5.0),
          ),
        ).hashCode,
      );
      expect(
        underlineInputBorderRadius.hashCode,
        isNot(
          const UnderlineInputBorder(
            borderSide: BorderSide(color: Colors.red),
            borderRadius: BorderRadius.only(
              topLeft: Radius.circular(6.0),
              topRight: Radius.circular(6.0),
            ),
          ).hashCode,
        ),
      );
    });
  });
}

Actual results

The tests FAIL.

package:test_api                                    expect
package:flutter_test/src/widget_tester.dart 460:16  expect
test/input_decorator_test.dart 34:7                 main.<fn>.<fn>

Expected: not UnderlineInputBorder:<UnderlineInputBorder()>
  Actual: UnderlineInputBorder:<UnderlineInputBorder()>

package:test_api                                    expect
package:flutter_test/src/widget_tester.dart 460:16  expect
test/input_decorator_test.dart 77:7                 main.<fn>.<fn>

Expected: not <307579946>
  Actual: <307579946>

The existing tests for UnderlineInputBorder and OutlineInputBorder are not exhaustive enough to capture an issue like this. Only the UnderlineInputBorder has this gap, the OutlineInputBorder does not have the same issue, but if it would have had a similar gap, its current test would not have caught the issue.

Proposal

  • Add borderRadius to equality and hashCode of UnderlineInputBorder.
  • Improve tests of UnderlineInputBorder and OutlineInputBorder to capture a gap like this.

Used Flutter Version

Tested on channel master, 3.7.0-15.0.pre.16

This issue exists on all channels.

flutter doctor -v

[!] Flutter (Channel master, 3.7.0-15.0.pre.16, on macOS 13.0.1 22A400 darwin-arm64, locale en-US)
    • Flutter version 3.7.0-15.0.pre.16 on channel master at /Users/rydmike/fvm/versions/master
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 7ddf42eae5 (2 days ago), 2023-01-06 17:44:44 -0500
    • Engine revision 33d7f8a1b3
    • Dart version 3.0.0 (build 3.0.0-85.0.dev)
    • DevTools version 2.20.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/rydmike/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14C18
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[✓] IntelliJ IDEA Community Edition (version 2022.3.1)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin version 71.2.6
    • Dart plugin version 223.8214.16

[✓] VS Code (version 1.73.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.54.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 13.0.1 22A400 darwin-arm64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 108.0.5359.124

[✓] HTTP Host Availability
    • All required HTTP hosts are available

@danagbemava-nc danagbemava-nc added the in triage Presently being triaged by the triage team label Jan 11, 2023
@danagbemava-nc
Copy link
Member

Reproducible using the test cases provided above.

logs
00:01 +0 -1: UnderlineInputBorder test UnderlineInputBorder equality [E]                                                                            
  Expected: not UnderlineInputBorder:<UnderlineInputBorder()>
    Actual: UnderlineInputBorder:<UnderlineInputBorder()>
  
  package:test_api                                    expect
  package:flutter_test/src/widget_tester.dart 460:16  expect
  test/widget_test.dart 34:7                          main.<fn>.<fn>
  

To run this test again: /Users/nexus/dev/sdks/flutters/bin/cache/dart-sdk/bin/dart test /Users/nexus/projects/nevercode/te/test/widget_test.dart -p vm --plain-name 'UnderlineInputBorder test UnderlineInputBorder equality'
00:01 +0 -2: UnderlineInputBorder test UnderlineInputBorder hashCodes [E]                                                                           
  Expected: not <409750690>
    Actual: <409750690>
  
  package:test_api                                    expect
  package:flutter_test/src/widget_tester.dart 460:16  expect
  test/widget_test.dart 77:7                          main.<fn>.<fn>
  

To run this test again: /Users/nexus/dev/sdks/flutters/bin/cache/dart-sdk/bin/dart test /Users/nexus/projects/nevercode/te/test/widget_test.dart -p vm --plain-name 'UnderlineInputBorder test UnderlineInputBorder hashCodes'
00:01 +0 -2: Some tests failed.
flutter doctor -v
[✓] Flutter (Channel stable, 3.3.10, on macOS 13.1 22C65 darwin-arm, locale en-GB)
    • Flutter version 3.3.10 on channel stable at /Users/nexus/dev/sdks/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 135454af32 (4 weeks ago), 2022-12-15 07:36:55 -0800
    • Engine revision 3316dd8728
    • Dart version 2.18.6
    • DevTools version 2.15.0

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14C18
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[✓] VS Code (version 1.74.3)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.56.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 13.1 22C65 darwin-arm
    • Chrome (web)    • chrome • web-javascript • Google Chrome 108.0.5359.124

[✓] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!
[!] Flutter (Channel master, 3.7.0-17.0.pre.21, on macOS 13.1 22C65 darwin-arm64, locale en-GB)
    • Flutter version 3.7.0-17.0.pre.21 on channel master at /Users/nexus/dev/sdks/flutters
    ! Warning: `flutter` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 55e3894a8c (4 hours ago), 2023-01-10 21:37:34 -0500
    • Engine revision 53bd4bbf96
    • Dart version 3.0.0 (build 3.0.0-108.0.dev)
    • DevTools version 2.20.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14C18
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[✓] VS Code (version 1.74.3)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.56.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 13.1 22C65 darwin-arm64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 108.0.5359.124

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

@danagbemava-nc danagbemava-nc added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. has reproducible steps The issue has been confirmed reproducible and is ready to work on found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 and removed in triage Presently being triaged by the triage team labels Jan 11, 2023
@rydmike
Copy link
Contributor Author

rydmike commented Jan 11, 2023

@TahaTesser just as a curiosity, the reason why I found this issue was because if you in FlexColorScheme's ThemesPlayground https://rydmike.com/flexcolorscheme/themesplayground-v6 go to TextField interactive theming panel, select underline input border and change its border radius it does not update, like when using the outline input border.

The update does not happen until you also change something else in ThemeData that makes it "unequal" and triggers the rebuild. So toggling from underline to outline, after a change in border radius, will show it on the outline and then when toggling back to underline, it will be shown on underline input border as well.

Demoed with a GIF recording here.

underlineinputborder_issue

So I wondered why it happens and found this issue as the cause.

Actually known it for a while, I was just lazy reporting it, see this log in docs about it:
https://docs.flexcolorscheme.com/known_issues#flutter-sdk-underlineinputborder

Maybe there is some reason why the quality check is the way it is? I can't see one though. Probably it was just forgotten to be added when underline input border at some later point received the border radius prop on top edges. That's my guess anyway.

@TahaTesser
Copy link
Member

TahaTesser commented Jan 11, 2023

@rydmike

I can see UnderlineInputBorder was added in #13734 (a long time ago) and It didn't have borderRadius in the original PR. The borderRadius was introduced in #16272, it doesn't update the hash code.

I myself sometimes forget to update the hash code. I agree with your assessment that it must've been forgotten.

@rydmike rydmike changed the title The borderRadius property is missing from equality and hashCode in UnderlineInputBorder The borderRadius property is missing from equality and hashCode in UnderlineInputBorder, equality and hashCode tests of UnderlineInputBorder and OutlineInputBorder are not exhaustive Jan 11, 2023
@TahaTesser TahaTesser added the r: fixed Issue is closed as already fixed in a newer version label Jan 11, 2023
@github-actions
Copy link

github-actions bot commented Mar 4, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f: material design flutter/packages/flutter/material repository. found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on r: fixed Issue is closed as already fixed in a newer version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants