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

Migrate to null safety #139

Merged
merged 26 commits into from Jan 25, 2021
Merged

Migrate to null safety #139

merged 26 commits into from Jan 25, 2021

Conversation

natebosch
Copy link
Member

  • Append -nullsafety to version.
  • Set min SDK to 2.11.0-0, add publish_to:none since this package is
    not in the allow list.
  • Use late field initialization over constructor assignment to clean up
    the handling of backreferences in the different "phases".
  • Remove some conditional branches that are impossible to hit.

- Append `-nullsafety` to version.
- Set min SDK to `2.11.0-0`, add `publish_to:none` since this package is
  not in the allow list.
- Use late field initialization over constructor assignment to clean up
  the handling of backreferences in the different "phases".
- Remove some conditional branches that are impossible to hit.
@google-cla google-cla bot added the cla: yes Google CLA signed label Oct 14, 2020
@natebosch natebosch changed the base branch from master to null_safety October 14, 2020 22:38
@natebosch natebosch requested a review from lrhn October 16, 2020 16:23
lib/dom.dart Outdated Show resolved Hide resolved
lib/dom.dart Outdated Show resolved Hide resolved
lib/dom.dart Outdated Show resolved Hide resolved
lib/dom.dart Show resolved Hide resolved
lib/dom.dart Outdated Show resolved Hide resolved
lib/src/tokenizer.dart Outdated Show resolved Hide resolved
lib/src/tokenizer.dart Outdated Show resolved Hide resolved
lib/src/tokenizer.dart Outdated Show resolved Hide resolved
lib/src/tokenizer.dart Outdated Show resolved Hide resolved
lib/src/utils.dart Outdated Show resolved Hide resolved
- Make Text.data consistently non nullable for getter and setter.
- Add a local variable to avoid double null check for a field.
- Make the `container` argument non-nullable in multiple places.
- Remove unused `container` field.
- Use `?.` over null check then `!`.
lib/dom.dart Outdated Show resolved Hide resolved
lib/dom.dart Outdated Show resolved Hide resolved
lib/dom.dart Outdated Show resolved Hide resolved
lib/dom.dart Outdated Show resolved Hide resolved
lib/dom.dart Outdated Show resolved Hide resolved
lib/parser.dart Outdated Show resolved Hide resolved
@natebosch
Copy link
Member Author

@lhrn can you check out my responses to your earlier comments?

@scheglov
Copy link
Contributor

Will this PR be landed?

We transitively use package:html in package:analyzer a few tests, so we will have to be kept them at 2.9 for some time. Not critical, just wonder why this stuck for almost 2 months :-)

@natebosch
Copy link
Member Author

I'll revisit this PR. I think it's currently waiting for me to respond to Lasse's most recent comments.

@natebosch natebosch assigned natebosch and unassigned lrhn Dec 31, 2020
@natebosch
Copy link
Member Author

Ok, I think the last change needed was the switch to use covariant. This should be good to go PTAL @lrhn

lib/dom.dart Outdated Show resolved Hide resolved
@@ -156,28 +157,26 @@ abstract class Node {

/// A list of child nodes of the current node. This must
/// include all elements but not necessarily other node types.
final NodeList nodes = NodeList._();
late final nodes = NodeList._(this);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to avoid exposing the setter here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has an initializing expression, so ti doesn't expose the setter.

lib/dom.dart Outdated Show resolved Hide resolved
lib/dom.dart Show resolved Hide resolved
lib/dom.dart Show resolved Hide resolved
lib/parser.dart Outdated Show resolved Hide resolved
lib/src/token.dart Show resolved Hide resolved
lib/src/treebuilder.dart Outdated Show resolved Hide resolved
@natebosch natebosch changed the base branch from null_safety to master January 25, 2021 22:10
@natebosch natebosch merged commit 00cd3c2 into master Jan 25, 2021
@natebosch natebosch deleted the null_safety-migration branch January 25, 2021 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants