search

BirjuVachhani / adaptive_theme

Easiest way to add support for light and dark theme in your flutter app.
https://pub.dev/packages/adaptive_theme
Apache License 2.0
464 stars 37 forks source link

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

Closed Phil9l closed 2 years ago

Phil9l commented 2 years ago

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

Description of the Change

Alternate Designs

Why Should This Be In Core?

Benefits

Possible Drawbacks

Verification Process

Applicable Issues

BirjuVachhani commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

BirjuVachhani commented 2 years ago

@Phil9l Could you please check why tests failing?

Livinglist commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

This has been released in v3.0.0