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

♻️ Remove unnecessary null checks, which result in compiler warnings. #34

Merged
merged 1 commit into from May 16, 2022

Conversation

Phil9l
Copy link
Contributor

@Phil9l Phil9l commented Apr 13, 2022

According to the Dart flow analysis, the following null-checks are not needed. Removing them to get rid of these compiler warnings.

../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/cupertino_adaptive_theme.dart:122:20: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
 - 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/widgets/binding.dart').
    WidgetsBinding.instance?.addObserver(this);
                   ^

../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/cupertino_adaptive_theme.dart:144:43: Warning: Operand of null-aware operation '!' has type 'SchedulerBinding' which excludes null.
 - 'SchedulerBinding' is from 'package:flutter/src/scheduler/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/scheduler/binding.dart').
      final brightness = SchedulerBinding.instance!.window.platformBrightness;
                                          ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/cupertino_adaptive_theme.dart:221:43: Warning: Operand of null-aware operation '!' has type 'SchedulerBinding' which excludes null.
 - 'SchedulerBinding' is from 'package:flutter/src/scheduler/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/scheduler/binding.dart').

      final brightness = SchedulerBinding.instance!.window.platformBrightness;
                                          ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/cupertino_adaptive_theme.dart:231:20: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
 - 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/widgets/binding.dart').
    WidgetsBinding.instance?.removeObserver(this);
                   ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/adaptive_theme.dart:119:20: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
 - 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/widgets/binding.dart').
    WidgetsBinding.instance?.addObserver(this);
                   ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/adaptive_theme.dart:139:43: Warning: Operand of null-aware operation '!' has type 'SchedulerBinding' which excludes null.
 - 'SchedulerBinding' is from 'package:flutter/src/scheduler/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/scheduler/binding.dart').
      final brightness = SchedulerBinding.instance!.window.platformBrightness;

                                          ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/adaptive_theme.dart:218:20: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.

 - 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/widgets/binding.dart').
    WidgetsBinding.instance?.removeObserver(this);
                   ^

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Alternate Designs

Why Should This Be In Core?

Benefits

Possible Drawbacks

Verification Process

Applicable Issues

According to the Dart flow analysis, the following null-checks are not needed. Removing them to get rid of these compiler warnings.

```
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/cupertino_adaptive_theme.dart:122:20: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
 - 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/widgets/binding.dart').
    WidgetsBinding.instance?.addObserver(this);
                   ^

../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/cupertino_adaptive_theme.dart:144:43: Warning: Operand of null-aware operation '!' has type 'SchedulerBinding' which excludes null.
 - 'SchedulerBinding' is from 'package:flutter/src/scheduler/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/scheduler/binding.dart').
      final brightness = SchedulerBinding.instance!.window.platformBrightness;
                                          ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/cupertino_adaptive_theme.dart:221:43: Warning: Operand of null-aware operation '!' has type 'SchedulerBinding' which excludes null.
 - 'SchedulerBinding' is from 'package:flutter/src/scheduler/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/scheduler/binding.dart').

      final brightness = SchedulerBinding.instance!.window.platformBrightness;
                                          ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/cupertino_adaptive_theme.dart:231:20: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
 - 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/widgets/binding.dart').
    WidgetsBinding.instance?.removeObserver(this);
                   ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/adaptive_theme.dart:119:20: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
 - 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/widgets/binding.dart').
    WidgetsBinding.instance?.addObserver(this);
                   ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/adaptive_theme.dart:139:43: Warning: Operand of null-aware operation '!' has type 'SchedulerBinding' which excludes null.
 - 'SchedulerBinding' is from 'package:flutter/src/scheduler/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/scheduler/binding.dart').
      final brightness = SchedulerBinding.instance!.window.platformBrightness;

                                          ^
../../../Dev/flutter/.pub-cache/hosted/pub.dartlang.org/adaptive_theme-2.3.1/lib/src/adaptive_theme.dart:218:20: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.

 - 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../Dev/flutter/packages/flutter/lib/src/widgets/binding.dart').
    WidgetsBinding.instance?.removeObserver(this);
                   ^
```
@BirjuVachhani
Copy link
Owner

BirjuVachhani commented Apr 14, 2022

@Phil9l I am on Flutter stable and I am not able to see this lint warnings. WidgetsBinding.instance is null so it needs the null check operator. Also, the tests are failing for the same reason. Are you sure you're on Flutter stable?

@jeromeDms
Copy link

Hi
I'm facing the same issue using flutter beta channel.
I switched to flutter beta as it largely improve performances on Android apps displaying AdMob ads.
Moving to this beta channel results in WidgetsBinding.instance generating warnings.
Jerome

@BirjuVachhani
Copy link
Owner

I'll try this using beta channel. Maybe a solution would be to release a beta/dev version of the package for flutter beta channel.

@Livinglist
Copy link

@BirjuVachhani Just tried on Flutter 3.0 with Dart 2.17.0, warnings can be seen now.

@BirjuVachhani BirjuVachhani added the enhancement New feature or request label May 12, 2022
@BirjuVachhani
Copy link
Owner

@Phil9l Could you please check why tests failing?

@Livinglist
Copy link

@BirjuVachhani probably because the workflow is using Flutter 2.10.4 instead of 3.0

Run actions/checkout@v1
[2](https://github.com/BirjuVachhani/adaptive_theme/runs/6018676980?check_suite_focus=true#step:4:2)
  with:
[3](https://github.com/BirjuVachhani/adaptive_theme/runs/6018676980?check_suite_focus=true#step:4:3)
    clean: true
[4](https://github.com/BirjuVachhani/adaptive_theme/runs/6018676980?check_suite_focus=true#step:4:4)
  env:
[5](https://github.com/BirjuVachhani/adaptive_theme/runs/6018676980?check_suite_focus=true#step:4:5)
    FLUTTER_ROOT: /opt/hostedtoolcache/flutter/2.10.4-stable/x[6](https://github.com/BirjuVachhani/adaptive_theme/runs/6018676980?check_suite_focus=true#step:4:6)4
6
    PUB_CACHE: /opt/hostedtoolcache/flutter/2.10.4-stable/x64/.pub-cache

@codecov-commenter
Copy link

codecov-commenter commented May 16, 2022

Codecov Report

Merging #34 (30a3cce) into main (6cae29f) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #34   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          186       186           
=========================================
  Hits           186       186           
Impacted Files Coverage Δ
lib/src/adaptive_theme.dart 100.00% <100.00%> (ø)
lib/src/cupertino_adaptive_theme.dart 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b8199c...30a3cce. Read the comment docs.

@BirjuVachhani BirjuVachhani reopened this May 16, 2022
@BirjuVachhani BirjuVachhani reopened this May 16, 2022
@BirjuVachhani BirjuVachhani merged commit b76e5bd into BirjuVachhani:main May 16, 2022
@BirjuVachhani
Copy link
Owner

This has been released in v3.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants