Skip to content

Throwing a null should raise TypeError, not NullThrownError #1553

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

Closed
chloestefantsova opened this issue Nov 24, 2022 · 2 comments
Closed

Throwing a null should raise TypeError, not NullThrownError #1553

chloestefantsova opened this issue Nov 24, 2022 · 2 comments
Assignees
Labels
bad-test Report tests in need of updates. When closed, the tests should be considered good

Comments

@chloestefantsova
Copy link
Contributor

In the following text it is expected that throwing the null value results in NullThrownError. In NullSafe mode an attempt to throw null should raise TypeError instead.

  • co19/Language/Expressions/Throw/evaluation_t04
  • co19/LibTest/async/Future/Future.sync_A01_t03
  • co19/Utils/tests/Expect/throws_A01_t03

cc @lrhn

@sgrekhov sgrekhov added the bad-test Report tests in need of updates. When closed, the tests should be considered good label Nov 24, 2022
@sgrekhov sgrekhov self-assigned this Nov 24, 2022
sgrekhov added a commit to sgrekhov/co19 that referenced this issue Nov 25, 2022
@lrhn
Copy link
Member

lrhn commented Nov 25, 2022

The null safety specification says:

It is an error if the static type of e in the expression throw e is not assignable to Object.

It doesn't explicitly say what happens for null typed as dynamic, but I believe it should be treated the same way as other places where we have a required type:

  • The context type schema for e is Object.
  • It's a run-time type error if the value of e is not an instance of a subtype of Object (aka. "implicit downcast from dynamic").

@leafpetersen Do you agree with this?

Example showing that we have bool as context type in conditions, but not Object in throw:

void main() async {
  if (ct(false)) print("ok"); // "CT: bool"
  throw ct("a"); // "CT: dynamic"
}

// Captures static context type.
T ct<T>(Object? value) {
  print("CT: $T");
  return value as T;
}

@leafpetersen
Copy link
Member

The preference would be to use Object as the context type, as discussed in the original issue here. It looks like we neither specified it as such, nor implemented it as such though. Technically breaking to change it now.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Nov 30, 2022
2022-11-29 [email protected] Fixes dart-lang/co19#1553. Replace NullThrowError by TypeError (dart-lang/co19#1557)
2022-11-29 [email protected] dart-lang/co19#1401. [Patterns] List pattern tests added (dart-lang/co19#1552)
2022-11-29 [email protected] Fixes dart-lang/co19#1562. Remove deprecated FallThroughError tests (dart-lang/co19#1563)
2022-11-29 [email protected] Fixes dart-lang/co19#1526. [Records] Expect error for field names that start with underscore (dart-lang/co19#1527)

Change-Id: Ia99d047bc85db40b0db3448fbc0aae54ea8a16d4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272641
Commit-Queue: Michael Thomsen <[email protected]>
Reviewed-by: Erik Ernst <[email protected]>
@sgrekhov sgrekhov reopened this Dec 2, 2022
sgrekhov added a commit to sgrekhov/co19 that referenced this issue Dec 2, 2022
@eernstg eernstg closed this as completed in 97d07c0 Dec 2, 2022
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Dec 9, 2022
2022-12-09 [email protected] dart-lang/co19#1401. [Patterns] List's rest pattern tests (dart-lang/co19#1578)
2022-12-08 [email protected] dart-lang/co19#1575. Update test expectations for analyzer (dart-lang/co19#1576)
2022-12-07 [email protected] dart-lang/co19#1401. [Patterns] Map patterns tests updated. Check primitive == for keys (dart-lang/co19#1574)
2022-12-07 [email protected] dart-lang/co19#1401. [Patterns] Logical and relational patterns tests refactored (dart-lang/co19#1573)
2022-12-06 [email protected] dart-lang/co19#1401. [Patterns] Map pattern tests (dart-lang/co19#1572)
2022-12-02 [email protected] Fixes dart-lang/co19#1553. Description wording fixed (dart-lang/co19#1569)
2022-12-02 [email protected] Fixes dart-lang/co19#1568. Exclude VM-specific test form web-platforms (dart-lang/co19#1570)

Change-Id: I69e3d49ac0b244ed12c266bd00a1c3ec9f8e1b45
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/274660
Reviewed-by: Alexander Thomas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bad-test Report tests in need of updates. When closed, the tests should be considered good
Projects
None yet
Development

No branches or pull requests

4 participants