-
Notifications
You must be signed in to change notification settings - Fork 170
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
extend cases of unnecessary_null_checks/null_check_on_nullable_type_parameter #3258
Changes from 3 commits
5081ed4
401eac1
1e8b797
1227225
2695e5a
545ba5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,10 @@ DartType? getExpectedType(PostfixExpression node) { | |
var realNode = | ||
node.thisOrAncestorMatching((e) => e.parent is! ParenthesizedExpression); | ||
var parent = realNode?.parent; | ||
var withAwait = parent is AwaitExpression; | ||
if (withAwait) { | ||
parent = parent!.parent; | ||
} | ||
|
||
// in return value | ||
if (parent is ReturnStatement || parent is ExpressionFunctionBody) { | ||
|
@@ -48,7 +52,27 @@ DartType? getExpectedType(PostfixExpression node) { | |
return null; | ||
} | ||
var staticType = parentExpression.staticType; | ||
return staticType is FunctionType ? staticType.returnType : null; | ||
if (staticType is! FunctionType) { | ||
return null; | ||
} | ||
if (withAwait || parentExpression.body.keyword?.lexeme == 'async') { | ||
// return type is obligatorily a Future<X> | ||
return (staticType.returnType as ParameterizedType?)?.typeArguments.first; | ||
} else { | ||
return staticType.returnType; | ||
} | ||
} | ||
// in yield value | ||
if (parent is YieldStatement) { | ||
var parentExpression = parent.thisOrAncestorOfType<FunctionExpression>(); | ||
if (parentExpression == null) { | ||
return null; | ||
} | ||
var staticType = parentExpression.staticType; | ||
if (staticType is! FunctionType) { | ||
return null; | ||
} | ||
return (staticType.returnType as ParameterizedType).typeArguments.first; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. Having a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, having a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my bad, sorry and thanks |
||
} | ||
// assignment | ||
if (parent is AssignmentExpression && | ||
|
@@ -71,6 +95,20 @@ DartType? getExpectedType(PostfixExpression node) { | |
} | ||
return parentElement.parameters.first.type; | ||
} | ||
// as member of list | ||
if (parent is ListLiteral) { | ||
return (parent.staticType as ParameterizedType?)?.typeArguments.first; | ||
} | ||
// as member of set | ||
if (parent is SetOrMapLiteral && parent.isSet) { | ||
return (parent.staticType as ParameterizedType?)?.typeArguments.first; | ||
} | ||
// as member of map | ||
if (parent is MapLiteralEntry) { | ||
var typeParameters = | ||
(parent.parent! as SetOrMapLiteral).staticType as ParameterizedType?; | ||
return typeParameters?.typeArguments[parent.key == node ? 0 : 1]; | ||
} | ||
// as parameter of function | ||
if (parent is NamedExpression) { | ||
realNode = parent; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type could also be
void
,dynamic
, orNever
, so it isn't safe to just cast it to aParameterizedType
; doing so could cause an exception to be thrown.Even when it's a
ParameterizedType
, you can't assume that it has type arguments. EveryInterfaceType
is aParameterizedType
, but not everyInterfaceType
has type arguments. So asking for the first type argument without first checking that the list isn't empty will result in an exception being thrown.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a host of negative tests would be good here?