Skip to content

Commit

Permalink
fix #29108, improve inference error messages
Browse files Browse the repository at this point in the history
R=leafp@google.com

Review-Url: https://codereview.chromium.org/2757233004 .
  • Loading branch information
Jennifer Messerly committed Mar 21, 2017
1 parent 9f2668c commit 4f9fff9
Show file tree
Hide file tree
Showing 12 changed files with 322 additions and 87 deletions.
102 changes: 70 additions & 32 deletions pkg/analyzer/lib/src/generated/type_system.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,7 @@ class StrongTypeSystemImpl extends TypeSystem {
// subtypes (or supertypes) as necessary, and track the constraints that
// are implied by this.
var inferrer = new _GenericInferrer(typeProvider, this, fnType.typeFormals);

// Since we're trying to infer the instantiation, we want to ignore type
// formals as we check the parameters and return type.
var inferFnType =
fnType.instantiate(TypeParameterTypeImpl.getTypes(fnType.typeFormals));
inferrer.constrainReturnType(inferFnType, contextType);
inferrer.constrainGenericFunctionInContext(fnType, contextType);

// Infer and instantiate the resulting type.
return inferrer.infer(fnType, fnType.typeFormals,
Expand Down Expand Up @@ -1603,6 +1598,19 @@ class _GenericInferrer {
_matchSubtypeOf(declaredType, contextType, null, origin, covariant: true);
}

/// Constrain a universal function type [fnType] used in a context
/// [contextType].
void constrainGenericFunctionInContext(
FunctionType fnType, DartType contextType) {
var origin = new _TypeConstraintFromFunctionContext(fnType, contextType);

// Since we're trying to infer the instantiation, we want to ignore type
// formals as we check the parameters and return type.
var inferFnType =
fnType.instantiate(TypeParameterTypeImpl.getTypes(fnType.typeFormals));
_matchSubtypeOf(inferFnType, contextType, null, origin, covariant: true);
}

/// Apply an argument constraint, which asserts that the [argument] staticType
/// is a subtype of the [parameterType].
void constrainArgument(
Expand Down Expand Up @@ -1851,20 +1859,31 @@ class _GenericInferrer {
var constraints = _constraints[typeParam.element];
var typeParamBound =
typeParam.bound.substitute2(inferredTypes, fnTypeParams);
if (!typeParamBound.isDynamic) {
constraints
.add(new _TypeConstraint.fromExtends(typeParam, typeParamBound));
}

var inferred = inferredTypes[i];
if (constraints.any((c) => !c.isSatisifedBy(_typeSystem, inferred))) {
// Heuristic: keep the erroneous type, it should satisfy at least some
// of the constraints (e.g. the return context). If we fall back to
// instantiateToBounds, we'll typically get more errors (e.g. because
// `dynamic` is the most common bound).
knownTypes[typeParam] = inferred;
errorReporter?.reportErrorForNode(StrongModeCode.COULD_NOT_INFER,
errorNode, [typeParam, _formatError(inferred, constraints)]);
} else if (UnknownInferredType.isKnown(inferred)) {
bool success =
constraints.every((c) => c.isSatisifedBy(_typeSystem, inferred));
if (success && !typeParamBound.isDynamic) {
// If everything else succeeded, check the `extends` constraint.
var extendsConstraint =
new _TypeConstraint.fromExtends(typeParam, typeParamBound);
constraints.add(extendsConstraint);
success = extendsConstraint.isSatisifedBy(_typeSystem, inferred);
}

if (!success) {
errorReporter?.reportErrorForNode(
StrongModeCode.COULD_NOT_INFER,
errorNode,
[typeParam, _formatError(typeParam, inferred, constraints)]);

// Heuristic: even if we failed, keep the erroneous type.
// It should satisfy at least some of the constraints (e.g. the return
// context). If we fall back to instantiateToBounds, we'll typically get
// more errors (e.g. because `dynamic` is the most common bound).
}

if (UnknownInferredType.isKnown(inferred)) {
knownTypes[typeParam] = inferred;
}
}
Expand Down Expand Up @@ -2039,22 +2058,23 @@ class _GenericInferrer {
return result;
}

String _formatError(
DartType inferred, Iterable<_TypeConstraint> constraints) {
var intro = "Inferred type '$inferred' does not work with constraints:";
String _formatError(TypeParameterType typeParam, DartType inferred,
Iterable<_TypeConstraint> constraints) {
var intro = "Tried to infer '$inferred' for '$typeParam'"
" which doesn't work:";

var constraintsByOrigin = <_TypeConstraintOrigin, List<_TypeConstraint>>{};
for (var c in constraints) {
constraintsByOrigin.putIfAbsent(c.origin, () => []).add(c);
}

// Only report unique constraint origins.
Iterable<_TypeConstraint> isSatisified(bool expected) => constraintsByOrigin
.values
.where((l) =>
l.every((c) => c.isSatisifedBy(_typeSystem, inferred)) == expected)
.expand((i) => i);

// Only report unique constraint origins.
String unsatisified = _formatConstraints(isSatisified(false));
String satisified = _formatConstraints(isSatisified(true));

Expand All @@ -2074,16 +2094,19 @@ class _GenericInferrer {
.toList();

int prefixMax = lineParts.map((p) => p[0].length).fold(0, math.max);
int middleMax = lineParts.map((p) => p[1].length).fold(0, math.max);

// Use a set to prevent identical message lines.
// (It's not uncommon for the same constraint to show up in a few places.)
var messageLines = new Set<String>.from(lineParts.map((parts) {
var prefix = parts[0];
var middle = parts[1];
var prefixPad = ' ' * (prefixMax - prefix.length);
var middlePad = ' ' * (middleMax - middle.length);
return ' $prefix$prefixPad $middle$middlePad ${parts[2]}'.trimRight();
var middlePad = ' ' * (prefixMax);
var end = "";
if (parts.length > 2) {
end = '\n $middlePad ${parts[2]}';
}
return ' $prefix$prefixPad $middle$end';
}));

return messageLines.join('\n');
Expand Down Expand Up @@ -2121,13 +2144,13 @@ class _TypeConstraintFromArgument extends _TypeConstraintOrigin {
// "Map value"
prefix = "${genericType.name} $parameterName";
} else {
prefix = "Argument '$parameterName'";
prefix = "Parameter '$parameterName'";
}

return [
prefix,
"inferred as '$argumentType'",
"must be a '$parameterType'."
"declared as '$parameterType'",
"but argument is '$argumentType'."
];
}
}
Expand All @@ -2143,7 +2166,23 @@ class _TypeConstraintFromReturnType extends _TypeConstraintOrigin {
return [
"Return type",
"declared as '$declaredType'",
"used where a '$contextType' is required."
"used where '$contextType' is required."
];
}
}

class _TypeConstraintFromFunctionContext extends _TypeConstraintOrigin {
final DartType contextType;
final DartType functionType;

_TypeConstraintFromFunctionContext(this.functionType, this.contextType);

@override
formatError() {
return [
"Function type",
"declared as '$functionType'",
"used where '$contextType' is required."
];
}
}
Expand All @@ -2158,8 +2197,7 @@ class _TypeConstraintFromExtendsClause extends _TypeConstraintOrigin {
formatError() {
return [
"Type parameter '$typeParam'",
"declared to extend '$extendsType'.",
""
"declared to extend '$extendsType'."
];
}
}
Expand Down
187 changes: 187 additions & 0 deletions pkg/analyzer/test/generated/strong_mode_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,176 @@ class StrongModeLocalInferenceTest extends ResolverTestCase {
_isInstantiationOf(_hasElement(elementC))([_isDynamic])(cType);
}

test_inference_error_arguments() async {
Source source = addSource(r'''
typedef R F<T, R>(T t);
F<T, T> g<T>(F<T, T> f) => (x) => f(f(x));
test() {
var h = g((int x) => 42.0);
}
''');
await computeAnalysisResult(source);
_expectInferenceError(
source,
[
StrongModeCode.COULD_NOT_INFER,
StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE
],
r'''
Couldn't infer type parameter 'T'.
Tried to infer 'double' for 'T' which doesn't work:
Parameter 'f' declared as '(T) → T'
but argument is '(int) → double'.
Consider passing explicit type argument(s) to the generic.
''');
}

test_inference_error_arguments2() async {
Source source = addSource(r'''
typedef R F<T, R>(T t);
F<T, T> g<T>(F<T, T> a, F<T, T> b) => (x) => a(b(x));
test() {
var h = g((int x) => 42.0, (double x) => 42);
}
''');
await computeAnalysisResult(source);
_expectInferenceError(
source,
[
StrongModeCode.COULD_NOT_INFER,
StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE,
StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE
],
r'''
Couldn't infer type parameter 'T'.
Tried to infer 'num' for 'T' which doesn't work:
Parameter 'a' declared as '(T) → T'
but argument is '(int) → double'.
Parameter 'b' declared as '(T) → T'
but argument is '(double) → int'.
Consider passing explicit type argument(s) to the generic.
''');
}

test_inference_error_extendsFromReturn() async {
// This is not an inference error because we successfully infer Null.
Source source = addSource(r'''
T max<T extends num>(T x, T y) => x;
test() {
String hello = max(1, 2);
}
''');
var analysisResult = await computeAnalysisResult(source);
assertErrors(source, [
StrongModeCode.INVALID_CAST_LITERAL,
StrongModeCode.INVALID_CAST_LITERAL
]);
var unit = analysisResult.unit;
var h = (AstFinder.getStatementsInTopLevelFunction(unit, "test")[0]
as VariableDeclarationStatement)
.variables
.variables[0];
var call = h.initializer as MethodInvocation;
expect(call.staticInvokeType.toString(), '(Null, Null) → Null');
}

test_inference_error_extendsFromReturn2() async {
Source source = addSource(r'''
typedef R F<T, R>(T t);
F<T, T> g<T extends num>() => (y) => y;
test() {
F<String, String> hello = g();
}
''');
await computeAnalysisResult(source);
_expectInferenceError(
source,
[
StrongModeCode.COULD_NOT_INFER,
],
r'''
Couldn't infer type parameter 'T'.
Tried to infer 'String' for 'T' which doesn't work:
Type parameter 'T' declared to extend 'num'.
The type 'String' was inferred from:
Return type declared as '(T) → T'
used where '(String) → String' is required.
Consider passing explicit type argument(s) to the generic.
''');
}


test_inference_error_genericFunction() async {
Source source = addSource(r'''
T max<T extends num>(T x, T y) => x < y ? y : x;
abstract class Iterable<T> {
T get first;
S fold<S>(S s, S f(S s, T t));
}
test(Iterable values) {
num n = values.fold(values.first as num, max);
}
''');
await computeAnalysisResult(source);
_expectInferenceError(
source,
[
StrongModeCode.COULD_NOT_INFER,
StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE
],
r'''
Couldn't infer type parameter 'T'.
Tried to infer 'dynamic' for 'T' which doesn't work:
Function type declared as '<T extends num>(T, T) → T'
used where '(num, dynamic) → num' is required.
Consider passing explicit type argument(s) to the generic.
''');
}

test_inference_error_returnContext() async {
Source source = addSource(r'''
typedef R F<T, R>(T t);
F<T, T> g<T>(T t) => (x) => t;
test() {
F<num, int> h = g(42);
}
''');
await computeAnalysisResult(source);
_expectInferenceError(
source,
[StrongModeCode.COULD_NOT_INFER],
r'''
Couldn't infer type parameter 'T'.
Tried to infer 'num' for 'T' which doesn't work:
Return type declared as '(T) → T'
used where '(num) → int' is required.
Consider passing explicit type argument(s) to the generic.
''');
}

test_inference_hints() async {
Source source = addSource(r'''
void main () {
Expand Down Expand Up @@ -2137,6 +2307,23 @@ num test(Iterable values) => values.fold(values.first as num, max);
check("f3", _isListOf((DartType type) => _isListOf(_isInt)(type)));
}

/// Verifies the source has the expected [errorCodes] as well as the
/// expected [errorMessage].
void _expectInferenceError(
Source source, List<ErrorCode> errorCodes, String errorMessage) {
assertErrors(source, errorCodes);
var errors = analysisResults[source]
.errors
.where((e) => e.errorCode == StrongModeCode.COULD_NOT_INFER)
.map((e) => e.message)
.toList();
expect(errors.length, 1);
var actual = errors[0];
expect(actual,
errorMessage, // Print the literal error message for easy copy+paste:
reason: 'Actual error did not match expected error:\n$actual');
}

/// Helper method for testing `FutureOr<T>`.
///
/// Validates that [code] produces [errors]. It should define a function
Expand Down
Loading

0 comments on commit 4f9fff9

Please sign in to comment.