Acknowledged
5 months ago

SDK 8.0.0 beta: issue with conditionals from 7.4.1 is still present

There's a problem in early SDK 7.4.* versions (that have been pulled back) and in SDK 8.0.0 beta with enabled strict type checking:

import Toybox.Lang;

function f(x as Number) as Number? {
    // Object of type 'PolyType<Any or $.Toybox.Lang.Number>' does not match return type 'PolyType<Null or $.Toybox.Lang.Number>'.
    return x != null ? x : null;
}

Such code builds without errors in 7.3.1 and 7.4.3.

9811827
Parents
  • 5 months ago

    > But this code is incorrect IMHO. x should be declared as Number?, and it should also give you an error/warning about that (ok, that is maybe not garmin but only Prettier Monkey C)

    But it's not incorrect code in Monkey C (by definition, since it builds fine in 7.3.1 and 7.4.3, assuming that's intended behavior). Perhaps you mean that it *should* be incorrect (in your opinion? Or that it's incorrect code in PMC's judgement?

    Incidentally, this type of thing (comparing a non-nullable type to null) is not an error/warning in TypeScript either. (I mention this since Monkey Types clearly takes a lot of inspiration from TypeScript, and it's also interesting to see how this sort of thing is handled by a well-established language.)

    https://github.com/microsoft/TypeScript/issues/11920

    > disallow comparing to null and undefined unless they are valid cases in strict null mode #11920

    >> We intentionally allow this for the sake of defensive programming (i.e. defending against missing inputs from non-TS code). If there's enough demand we could add a flag or something.

    \

    To be fair, many of the commenters disagree with this behaviour.

    I can see a similar rationale for CIQ / Monkey C. It regularly comes up in forum discussions that many devs will write defensive code where they check various API fields for null values, even when the fields are typed to be non-nullable. In many cases, these checks may be superfluous, but in some cases they may actually be helpful (to cover the case when the field is actually null, due to either a bug in the implementation or a bug in the API documentation / type specification.)

    Isn't there a known issue where getHeartRateZones() can return null even though it's typed to not return null? It's been mentioned more than once.

    If Monkey Types actually enforced a rule where it's an error or warning to compare a non-nullable type to null, then that could break lots of existing apps (meaning the compiler would start returning warnings or errors for previously valid code), and actually lead to more bugs at runtime (in the case where a seemingly superfluous defensive null check would actually avoid a bug).

Comment
  • 5 months ago

    > But this code is incorrect IMHO. x should be declared as Number?, and it should also give you an error/warning about that (ok, that is maybe not garmin but only Prettier Monkey C)

    But it's not incorrect code in Monkey C (by definition, since it builds fine in 7.3.1 and 7.4.3, assuming that's intended behavior). Perhaps you mean that it *should* be incorrect (in your opinion? Or that it's incorrect code in PMC's judgement?

    Incidentally, this type of thing (comparing a non-nullable type to null) is not an error/warning in TypeScript either. (I mention this since Monkey Types clearly takes a lot of inspiration from TypeScript, and it's also interesting to see how this sort of thing is handled by a well-established language.)

    https://github.com/microsoft/TypeScript/issues/11920

    > disallow comparing to null and undefined unless they are valid cases in strict null mode #11920

    >> We intentionally allow this for the sake of defensive programming (i.e. defending against missing inputs from non-TS code). If there's enough demand we could add a flag or something.

    \

    To be fair, many of the commenters disagree with this behaviour.

    I can see a similar rationale for CIQ / Monkey C. It regularly comes up in forum discussions that many devs will write defensive code where they check various API fields for null values, even when the fields are typed to be non-nullable. In many cases, these checks may be superfluous, but in some cases they may actually be helpful (to cover the case when the field is actually null, due to either a bug in the implementation or a bug in the API documentation / type specification.)

    Isn't there a known issue where getHeartRateZones() can return null even though it's typed to not return null? It's been mentioned more than once.

    If Monkey Types actually enforced a rule where it's an error or warning to compare a non-nullable type to null, then that could break lots of existing apps (meaning the compiler would start returning warnings or errors for previously valid code), and actually lead to more bugs at runtime (in the case where a seemingly superfluous defensive null check would actually avoid a bug).

Children
No Data