null safety #50
null safety #50
Conversation
|
@SteveAlexander, thanks for this conversion. We're able to use this with our null safety conversion with a few changes. To use change your We also had to change a few declarations from static DateTimeZoneProvider? _defaultProvider;In file FixedFormatInfoPatternParser<Time>? _timePatternParser;
<...snip 9 lines...>
FixedFormatInfoPatternParser<AnnualDate>? _annualDatePatternParser;We can now run :-). We do get 6 warnings at compile time like: /lib/src/skipped_time_error.dart:39:72: Warning: Operand of null-aware operation '?.' has type 'DateTimeZone' which excludes null.
- 'DateTimeZone' is from 'package:time_machine/src/datetimezone.dart'.
: message = "Local time $localDateTime is invalid in time zone ${zone?.id ?? 'null'}";@Dana-Ferguson any thoughts on the timeline for a null safe version? Thanks all!! |
|
@Dana-Ferguson any plans to move this forward? |
…in stepped pattern builder
…ute in fake_clock
… test is not relevant -- rather than using <TestData>[null]
…ind of timezones are not in the standard tzdb
|
I updated the PR description with the latest status — all tests from |
|
@SteveAlexander thanks for the migration to null safety. We are now able to run with "full null safety" - hooray. However, we're having trouble with testing, particularly mocking with Mockito. Both issues are with Mockito's overrides not being "valid" and occur on several TimeMachine objects (e.g. This is because the TimeMachine override of The second issue is similar but for This second one is also mismatched parameter definitions but the fix doesn't seem as straightforward... It would seem the cleanest fix would be to introduce another function to be the official string formatting function and change the overridden object Mockito has an old related issue dart-lang/mockito#228 that suggests overriding the definition but that is for the previous version that doesn't generate code. We see this problem on mocking objects (e.g. Person) that just return a property of type |
|
Hi @rich-j Thanks for describing these issues. The first one, about the equality operator, makes sense to me. I’m expecting it would be a straightforward change in time machine, and one we should make. The second issue, about toString, I’m not so sure about. The Dart language specification (section 11.2.2, Correct Member Overrides, and section 20.4.4 rule 15) says that |
…h change in Dart's Object class
… are highlighted by disabled lint options
…class that uses late final
|
@SteveAlexander thanks for all of the work. I agree that the Update - gave up on Mockito (too many issues generating mocks) and went with Mocktail. |
|
@SteveAlexander - Getting closer to everything running null safe. We have a set of tests that run locally but fail when we run them in GitHub actions. It seems that GitHub actions running on ubuntu doesn't have a valid locale defined so calling Lines 96-100 // Default Culture
var cultureId = io.Platform.localeName.split('.').first.replaceAll('_', '-');
Culture? culture = await Cultures.getCulture(cultureId);
ICultures.currentCulture = culture!; // <-- the non-null assertion here fails
// todo: remove Culture.currentCultureexplicitly assumes the culture is found. It also has a We need some way to get past initialization. One thought is to use the |
|
Do you know what |
var locale = io.Platform.localeName;
print("locale '$locale'");printed https://unix.stackexchange.com/questions/597962/how-widespread-is-the-c-utf-8-locale |
Hello!
Latest news on this branch, 28th March 2021:
As of this commit, there are 0 analysis / linter errors (pedantic rules), and 0 failing tests from
dart test. So, I consider this branch worth checking out in more details.I'm sure there are areas of the code I changed that could use some tidying up, changing in style, or outright fixing.
Also, I've often used the dart autoformatter, so there may be areas of code that the maintainers prefer to keep in a hand-crafted style rather than the autoformat style.
Similarly, I renamed some variables to conform to the "pedantic" analysis rules, but there might be reasons to revert these, and add a comment for the analyzer to ignore the rules.
Previous comment on this branch from December 2020:
I've done a first cut at null safety on this branch. I have no idea if it all works! This branch is not intended for merging, or even necessarily working on. It's here to show what kind of work is involved in making this package null safe.
I've basically gone through removing all of the "Problems" highlighted by the analyzer. There remain 48 lint warnings. I think it will be clear what to do about these once someone with actual understanding of the code has looked over it.
There were lots of judgement calls about whether to have something return a nullable type or not. Also, I commented out most of the "@internal" markers as they no longer seem to do what's intended with the latest Dart.