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

Enable strict null checks #1283

Merged
merged 19 commits into from Apr 15, 2021
Merged

Enable strict null checks #1283

merged 19 commits into from Apr 15, 2021

Conversation

nex3
Copy link
Contributor

@nex3 nex3 commented Apr 12, 2021

@nex3 nex3 requested a review from jathak April 12, 2021 22:03
jathak and others added 4 commits April 12, 2021 17:31
This also replaces package_resolver with package_config, since
package_resolver is archived and is incompatible with null-safe Dart
packages.
This allows us to be statically explicit about when the expression
does or doesn't exist.
This is more friendly to null-safe code, since we no longer rely on
`_inStyleRule` implying that `_styleRule != null`.
@nex3 nex3 force-pushed the nnbd branch 7 times, most recently from 2a3a87f to 481c96b Compare April 15, 2021 00:14
nex3 and others added 13 commits April 14, 2021 17:15
This doesn't explicitly help null-safety, but it makes the
relationship between `Declaration.value` and `Declaration.children`
more obvious.
It was always valid to pass in `null`, this just makes it more clear.
This gets the project analyzer-clean, but the tests are still failing.
"Extender" is also commonly used to refer to the parent selector of an
`@extend` rule, so this helps disambiguate.
This gets rid of the weird subset of "one-off" extensions which didn't
have target information available. Now instead, each method explicitly
declares whether it takes/returns extensions (which do have target
info) or extenders (which do not).
Copy link
Member

@jathak jathak left a comment

Choose a reason for hiding this comment

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

Overall LGTM, except for one return true that I think should be false.

We should convert any andGet calls to ?[] at some point, but we could probably save that for a separate PR.

@@ -2487,8 +2494,8 @@ class _EvaluateVisitor
nodeWithSpan, () => arguments.verify(positional, MapKeySet(named)));

Future<Value> visitSelectorExpression(SelectorExpression node) async {
if (_styleRule == null) return sassNull;
return _styleRule.originalSelector.asSassList;
if (_styleRuleIgnoringAtRoot == null) return sassNull;
Copy link
Member

Choose a reason for hiding this comment

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

What determines when _styleRuleIgnoringAtRoot should be used instead of _styleRule?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The spec, mostly—in this case, & as an expression always ignores @at-root so you can interpolate it into selectors. Separating out these variables not only makes it more explicit which references respect @at-root and which don't, it also removes the implicit constraint that _inStyleRule means that _styleRule != null. Instead, we just check _styleRule == null, which is more visible to the type system.

lib/src/ast/selector/list.dart Outdated Show resolved Hide resolved
lib/src/util/nullable.dart Outdated Show resolved Hide resolved
pubspec.yaml Outdated Show resolved Hide resolved
@nex3 nex3 merged commit f681906 into master Apr 15, 2021
@nex3 nex3 deleted the nnbd branch April 15, 2021 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants