Skip to content

Commit

Permalink
Revert "Analyzer: Report on return types of Future.catchError function"
Browse files Browse the repository at this point in the history
This reverts commit d5ee021.

Reason for revert:
Breaks google3: b/178222419

Original change's description:
> Analyzer: Report on return types of Future.catchError function
>
> The type of the `onError` parameter of Future<T>.catchError is just Function,
> but the function can either have signature `FutureOr<T> Function(dynamic)` or
> `FutureOr<T> Function(dynamic, StackTrace)`. This change adds checks for return
> statements in a function literal passed to `onError`, and the return type of a
> function-typed expression passed to `onError`.
>
> We still need to check parameter types.
>
> #35825
>
> Change-Id: I3a1d1c444e298c5816fcd1d4bc537f7b87fa3da1
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/176221
> Commit-Queue: Samuel Rawlins <srawlins@google.com>
> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

TBR=scheglov@google.com,brianwilkerson@google.com,srawlins@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I8ebbaac0a4b44293576809baa2dc2c3fdcf35379
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/180822
Commit-Queue: David Morgan <davidmorgan@google.com>
Reviewed-by: David Morgan <davidmorgan@google.com>
  • Loading branch information
emmanuel-p authored and commit-bot@chromium.org committed Jan 25, 2021
1 parent 341213b commit ba01596
Show file tree
Hide file tree
Showing 12 changed files with 9 additions and 486 deletions.
2 changes: 0 additions & 2 deletions pkg/analyzer/lib/error/error.dart
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,6 @@ const List<ErrorCode> errorCodeValues = [
HintCode.PACKAGE_IMPORT_CONTAINS_DOT_DOT,
HintCode.RECEIVER_OF_TYPE_NEVER,
HintCode.RETURN_OF_DO_NOT_STORE,
HintCode.RETURN_OF_INVALID_TYPE_FROM_CATCH_ERROR,
HintCode.RETURN_TYPE_INVALID_FOR_CATCH_ERROR,
HintCode.SDK_VERSION_AS_EXPRESSION_IN_CONST_CONTEXT,
HintCode.SDK_VERSION_ASYNC_EXPORTED_FROM_CORE,
HintCode.SDK_VERSION_BOOL_OPERATOR_IN_CONST_CONTEXT,
Expand Down
22 changes: 0 additions & 22 deletions pkg/analyzer/lib/src/dart/error/hint_codes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1745,28 +1745,6 @@ class HintCode extends AnalyzerErrorCode {
"'{1}' is also annotated.",
correction: "Annotate '{1}' with 'doNotStore'.");

/**
* Parameters:
* 0: the return type as declared in the return statement
* 1: the expected return type as defined by the type of the Future
*/
static const HintCode RETURN_OF_INVALID_TYPE_FROM_CATCH_ERROR = HintCode(
'RETURN_OF_INVALID_TYPE',
"A value of type '{0}' can't be returned by the 'onError' handler because "
"it must be assignable to '{1}'.",
uniqueName: 'RETURN_OF_INVALID_TYPE_FROM_CATCH_ERROR',
);

/**
* Parameters:
* 0: the return type of the function
* 1: the expected return type as defined by the type of the Future
*/
static const HintCode RETURN_TYPE_INVALID_FOR_CATCH_ERROR = HintCode(
'RETURN_TYPE_INVALID_FOR_CATCH_ERROR',
"The return type '{0}' isn't assignable to '{1}', as required by "
"'Future.catchError'.");

/**
* No parameters.
*/
Expand Down
6 changes: 0 additions & 6 deletions pkg/analyzer/lib/src/error/best_practices_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import 'package:analyzer/src/dart/element/type_system.dart';
import 'package:analyzer/src/dart/resolver/body_inference_context.dart';
import 'package:analyzer/src/dart/resolver/exit_detector.dart';
import 'package:analyzer/src/dart/resolver/scope.dart';
import 'package:analyzer/src/error/catch_error_verifier.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/error/deprecated_member_use_verifier.dart';
import 'package:analyzer/src/error/must_call_super_verifier.dart';
Expand Down Expand Up @@ -73,8 +72,6 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {

final MustCallSuperVerifier _mustCallSuperVerifier;

final CatchErrorVerifier _catchErrorVerifier;

/// The [WorkspacePackage] in which [_currentLibrary] is declared.
final WorkspacePackage _workspacePackage;

Expand Down Expand Up @@ -112,8 +109,6 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
_deprecatedVerifier =
DeprecatedMemberUseVerifier(workspacePackage, _errorReporter),
_mustCallSuperVerifier = MustCallSuperVerifier(_errorReporter),
_catchErrorVerifier =
CatchErrorVerifier(_errorReporter, typeProvider, typeSystem),
_workspacePackage = workspacePackage {
_deprecatedVerifier.pushInDeprecatedValue(_currentLibrary.hasDeprecated);
_inDoNotStoreMember = _currentLibrary.hasDoNotStore;
Expand Down Expand Up @@ -593,7 +588,6 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
void visitMethodInvocation(MethodInvocation node) {
_deprecatedVerifier.methodInvocation(node);
_checkForNullAwareHints(node, node.operator);
_catchErrorVerifier.verifyMethodInvocation(node);
super.visitMethodInvocation(node);
}

Expand Down
107 changes: 0 additions & 107 deletions pkg/analyzer/lib/src/error/catch_error_verifier.dart

This file was deleted.

16 changes: 2 additions & 14 deletions pkg/analyzer/lib/src/error/return_type_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,7 @@ class ReturnTypeVerifier {
var S = expression.staticType;

void reportTypeError() {
if (enclosingExecutable.catchErrorOnErrorReturnType != null) {
_errorReporter.reportErrorForNode(
HintCode.RETURN_OF_INVALID_TYPE_FROM_CATCH_ERROR,
expression,
[S, T],
);
} else if (enclosingExecutable.isClosure) {
if (enclosingExecutable.isClosure) {
_errorReporter.reportErrorForNode(
CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_CLOSURE,
expression,
Expand Down Expand Up @@ -262,13 +256,7 @@ class ReturnTypeVerifier {
var S = expression.staticType;

void reportTypeError() {
if (enclosingExecutable.catchErrorOnErrorReturnType != null) {
_errorReporter.reportErrorForNode(
HintCode.RETURN_OF_INVALID_TYPE_FROM_CATCH_ERROR,
expression,
[S, T],
);
} else if (enclosingExecutable.isClosure) {
if (enclosingExecutable.isClosure) {
_errorReporter.reportErrorForNode(
CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_CLOSURE,
expression,
Expand Down
21 changes: 7 additions & 14 deletions pkg/analyzer/lib/src/generated/error_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ class EnclosingExecutableContext {
final bool inFactoryConstructor;
final bool inStaticMethod;

/// If this [EnclosingExecutableContext] is the first argument in a method
/// invocation of [Future.catchError], returns the return type expected for
/// `Future<T>.catchError`'s `onError` parameter, which is `FutureOr<T>`,
/// otherwise `null`.
final InterfaceType catchErrorOnErrorReturnType;

/// The return statements that have a value.
final List<ReturnStatement> _returnsWith = [];

Expand All @@ -69,10 +63,8 @@ class EnclosingExecutableContext {
/// for the kind of the function body, e.g. not `Future` for `async`.
bool hasLegalReturnType = true;

EnclosingExecutableContext(this.element,
{bool isAsynchronous, this.catchErrorOnErrorReturnType})
: isAsynchronous =
isAsynchronous ?? (element != null && element.isAsynchronous),
EnclosingExecutableContext(this.element)
: isAsynchronous = element != null && element.isAsynchronous,
isConstConstructor = element is ConstructorElement && element.isConst,
isGenerativeConstructor =
element is ConstructorElement && !element.isFactory,
Expand Down Expand Up @@ -112,7 +104,7 @@ class EnclosingExecutableContext {

bool get isSynchronous => !isAsynchronous;

DartType get returnType => catchErrorOnErrorReturnType ?? element.returnType;
DartType get returnType => element.returnType;

static bool _inFactoryConstructor(ExecutableElement element) {
if (element is ConstructorElement) {
Expand Down Expand Up @@ -754,12 +746,13 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
@override
void visitFunctionExpression(FunctionExpression node) {
_isInLateLocalVariable.add(false);
if (node.parent is FunctionDeclaration) {
super.visitFunctionExpression(node);
} else {

if (node.parent is! FunctionDeclaration) {
_withEnclosingExecutable(node.declaredElement, () {
super.visitFunctionExpression(node);
});
} else {
super.visitFunctionExpression(node);
}

_isInLateLocalVariable.removeLast();
Expand Down
2 changes: 0 additions & 2 deletions pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ abstract class Future<T> {
throw 0;
}
Future<T> catchError(Function onError, {bool test(Object error)});
Future<R> then<R>(FutureOr<R> onValue(T value));
Future<T> whenComplete(action());
Expand Down
Loading

0 comments on commit ba01596

Please sign in to comment.