From 4e25e499c8cf4670918f9ffc1c8a9baef436445f Mon Sep 17 00:00:00 2001 From: Nicholas Shahan Date: Fri, 9 Apr 2021 22:24:43 +0000 Subject: [PATCH] [ddc] Add library URIs to cast failure messages When the two types have the same name but are from different libraries showing the library URI will help users understand the failure better. Change-Id: I5ab4412e676272111d41f688ef2d1cc83afbe627 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/194116 Reviewed-by: Mark Zhou Reviewed-by: Sigmund Cherem Commit-Queue: Nicholas Shahan --- .../private/ddc_runtime/errors.dart | 19 +++++- tests/dartdevc/cast_error/lib_a.dart | 5 ++ tests/dartdevc/cast_error/lib_b.dart | 5 ++ .../library_uri_in_message_test.dart | 66 +++++++++++++++++++ tests/dartdevc_2/cast_error/lib_a.dart | 5 ++ tests/dartdevc_2/cast_error/lib_b.dart | 5 ++ .../library_uri_in_message_test.dart | 53 +++++++++++++++ 7 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 tests/dartdevc/cast_error/lib_a.dart create mode 100644 tests/dartdevc/cast_error/lib_b.dart create mode 100644 tests/dartdevc/cast_error/library_uri_in_message_test.dart create mode 100644 tests/dartdevc_2/cast_error/lib_a.dart create mode 100644 tests/dartdevc_2/cast_error/lib_b.dart create mode 100644 tests/dartdevc_2/cast_error/library_uri_in_message_test.dart diff --git a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart index 36ebdf8a18f..b798adf3399 100644 --- a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart +++ b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart @@ -102,8 +102,23 @@ String _castErrorMessage(from, to) { // } // } // } - return "Expected a value of type '${typeName(to)}', " - "but got one of type '${typeName(from)}'"; + var fromName = "'${typeName(from)}'"; + var toName = "'${typeName(to)}'"; + + var toType = to; + if (_jsInstanceOf(to, NullableType) || _jsInstanceOf(to, LegacyType)) { + toType = to.type; + } + var fromType = from; + if (_jsInstanceOf(from, NullableType) || _jsInstanceOf(from, LegacyType)) { + fromType = from.type; + } + + if (typeName(fromType) == typeName(toType)) { + fromName += ' (in ${getLibraryUri(fromType)})'; + toName += ' (in ${getLibraryUri(toType)})'; + } + return 'Expected a value of type $toName, but got one of type $fromName'; } /// The symbol that references the thrown Dart Object (typically but not diff --git a/tests/dartdevc/cast_error/lib_a.dart b/tests/dartdevc/cast_error/lib_a.dart new file mode 100644 index 00000000000..5c6dedf1795 --- /dev/null +++ b/tests/dartdevc/cast_error/lib_a.dart @@ -0,0 +1,5 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +class Animal {} diff --git a/tests/dartdevc/cast_error/lib_b.dart b/tests/dartdevc/cast_error/lib_b.dart new file mode 100644 index 00000000000..5c6dedf1795 --- /dev/null +++ b/tests/dartdevc/cast_error/lib_b.dart @@ -0,0 +1,5 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +class Animal {} diff --git a/tests/dartdevc/cast_error/library_uri_in_message_test.dart b/tests/dartdevc/cast_error/library_uri_in_message_test.dart new file mode 100644 index 00000000000..42d7b853cd4 --- /dev/null +++ b/tests/dartdevc/cast_error/library_uri_in_message_test.dart @@ -0,0 +1,66 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import "../utils.dart"; +import "lib_a.dart" as libA; +import "lib_b.dart" as libB; + +void main() { + // When the name of the class is the same the error message should include + // library URIs for the two classes. + var a = libA.Animal(); + try { + a as libB.Animal; + } on TypeError catch (error) { + var message = error.toString(); + expectStringContains( + "Expected a value of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc/cast_error/lib_b.dart)", + message); + expectStringContains( + "but got one of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc/cast_error/lib_a.dart)", + message); + } + // Verify the libraries are properly ordered. + var b = libB.Animal(); + try { + b as libA.Animal; + } on TypeError catch (error) { + var message = error.toString(); + expectStringContains( + "Expected a value of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc/cast_error/lib_a.dart)", + message); + expectStringContains( + "but got one of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc/cast_error/lib_b.dart)", + message); + } + + // Shows library URIs when one of the types is nullable. + try { + b as libA.Animal?; + } on TypeError catch (error) { + var message = error.toString(); + expectStringContains( + "Expected a value of type 'Animal?' " + "(in org-dartlang-app:/tests/dartdevc/cast_error/lib_a.dart)", + message); + expectStringContains( + "but got one of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc/cast_error/lib_b.dart)", + message); + } + + // URIs are not displayed when the class names are different. + try { + a as String; + } on TypeError catch (error) { + var message = error.toString(); + expectStringContains( + "Expected a value of type 'String', but got one of type 'Animal'", + message); + } +} diff --git a/tests/dartdevc_2/cast_error/lib_a.dart b/tests/dartdevc_2/cast_error/lib_a.dart new file mode 100644 index 00000000000..5c6dedf1795 --- /dev/null +++ b/tests/dartdevc_2/cast_error/lib_a.dart @@ -0,0 +1,5 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +class Animal {} diff --git a/tests/dartdevc_2/cast_error/lib_b.dart b/tests/dartdevc_2/cast_error/lib_b.dart new file mode 100644 index 00000000000..5c6dedf1795 --- /dev/null +++ b/tests/dartdevc_2/cast_error/lib_b.dart @@ -0,0 +1,5 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +class Animal {} diff --git a/tests/dartdevc_2/cast_error/library_uri_in_message_test.dart b/tests/dartdevc_2/cast_error/library_uri_in_message_test.dart new file mode 100644 index 00000000000..c278d15cf46 --- /dev/null +++ b/tests/dartdevc_2/cast_error/library_uri_in_message_test.dart @@ -0,0 +1,53 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// @dart = 2.9 + +import "../utils.dart"; +import "lib_a.dart" as libA; +import "lib_b.dart" as libB; + +void main() { + // When the name of the class is the same the error message should include + // library URIs for the two classes. + var a = libA.Animal(); + try { + a as libB.Animal; + } on TypeError catch (error) { + var message = error.toString(); + expectStringContains( + "Expected a value of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc_2/cast_error/lib_b.dart)", + message); + expectStringContains( + "but got one of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc_2/cast_error/lib_a.dart)", + message); + } + // Verify the libraries are properly ordered. + var b = libB.Animal(); + try { + b as libA.Animal; + } on TypeError catch (error) { + var message = error.toString(); + expectStringContains( + "Expected a value of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc_2/cast_error/lib_a.dart)", + message); + expectStringContains( + "but got one of type 'Animal' " + "(in org-dartlang-app:/tests/dartdevc_2/cast_error/lib_b.dart)", + message); + } + + // URIs are not displayed when the class names are different. + try { + a as String; + } on TypeError catch (error) { + var message = error.toString(); + expectStringContains( + "Expected a value of type 'String', but got one of type 'Animal'", + message); + } +}