Skip to content
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

Genercis can solve to Never with final/sealed classes when it could never resolve #56964

Open
FMorschel opened this issue Oct 25, 2024 · 7 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). type-enhancement A request for a change that isn't a bug

Comments

@FMorschel
Copy link
Contributor

FMorschel commented Oct 25, 2024

If you write code like the following:

void foo() {
  String s1 = bar(0);  // <-- invalid_assignment
  String s2 = baz(0);  // <-- argument_type_not_assignable
}

num bar(num n) => n;

T baz<T extends num?>(T t) => t;

At bar(0) you get the following message:

A value of type 'num' can't be assigned to a variable of type 'String'.
Try changing the type of the variable, or casting the right-hand type to 'String'.

At 0 (inside baz) you get the following message:

The argument type 'int' can't be assigned to the parameter type 'Never'. 

In cases with final/sealed types (like String and num) couldn't we calculate up to invalid_assignment instead of calculating the generics to Never (we could still do that but it would be a bit counter intuitive)?


Edit

Found this in a code that simply returned T without any parameter (it would look somewhere else for the value) and then the problem was happening inside when the return was happening.

@dart-github-bot
Copy link
Collaborator

Summary: The issue reports that Dart's type inference for generics with final/sealed classes like String and num leads to counterintuitive errors. When assigning a num to a String variable, the error message suggests casting, while a generic function with a Never type parameter results in an error about the argument type being incompatible with Never. The user proposes that the type inference should prioritize the invalid_assignment error over the Never inference.

@dart-github-bot dart-github-bot added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug labels Oct 25, 2024
@eernstg
Copy link
Member

eernstg commented Oct 25, 2024

This is working as intended. baz(0) is inferred as baz<Never>(0) because it occurs with context type String (so the chosen value of T must be a subtype of String), and the bound requires that T is a subtype of num?, and no other type than Never satisfies those two requirements.

This means, in turn, that the actual argument 0 isn't type correct, because int isn't assignable to Never.

couldn't we calculate up to invalid_assignment instead of calculating the generics to Never.

It sounds like this could only be done if static analysis would somehow decide that the type of baz(0) is some other type than Never, which means that it could be a type S such that S <: num?, and also int <: S such that 0 is a type correct actual argument. This is actually how it would proceed if there hadn't been a context type.

But when there is a context type it is considered during inference, and this means that we do have those two subtype constraints where Never is the only solution.

@FMorschel
Copy link
Contributor Author

FMorschel commented Oct 25, 2024

This is working as intended.

I'm aware. Edited above:

Found this in a code that simply returned T without any parameter (it would look somewhere else for the value) and then the problem was happening inside when the return was happening.

Say if the implementation of baz was: T baz<T extends num?>() => 0 as T;

So in cases like this, I would only find out about the problem when running the code and getting to that point. Since we know that there is no valid return type that would be assignable for String that implements num, I'm asking that we throw invalid_assignment instead.

@eernstg
Copy link
Member

eernstg commented Oct 25, 2024

Just saw the edit. ;-) And the next comment.

Say if the implementation of baz was: T baz<T extends num?>() => 0 as T;

Right, as T certainly allows for run time failures. We even know statically that it must fail in this case:

T baz<T extends num?>() => 0 as T;

void foo() {
  String s = baz();
  print('Hello, world!'); // Warning: Dead code!
}

It might be useful to report on expressions of type Never in other ways, but it's very often causing subsequent code to be flagged as dead code, which could be taken as a strong hint because there is no other reason why we wouldn't reach that print statement. In many situations, it's actually desirable that a given expression has type Never, exactly because we want flow analysis to understand that it won't complete normally:

Never doThrowSomething() => throw "Something";

int foo(int? i) => i == null ? doThrowSomething() : i;

I think this illustrates that we can't simply decide that expressions should never have the type Never, so the question is how you would decide that we should blame the type Never rather than blaming the type of the actual argument in the original example for the invocation baz(0) with context String.

@FMorschel
Copy link
Contributor Author

I would rely on the expected type (say String) being final/sealed and having a generics with some other type that would resolve to Never, not an explicit Never result. I would still allow the person to add as String if this was intended to happen since that would stop the warning (not sure if there is any code where that was expected but it would be possible).

it's very often causing subsequent code to be flagged as dead code, which could be taken as a strong hint because there is no other reason why we wouldn't reach that print statement.

I saw that when creating the repro (swapped both lines on foo) but in my case, for example, I was using it inside a late final field like:

class A {
  late final String str = baz();
}

So fortunately I thought it was weird that there was no errors and added .toString() but there was no indication of it being wrong (hence I opened this issue). This would also happen in expression body code for a getter or something similar.

@bwilkerson
Copy link
Member

I don't believe that reporting an invalid_assignment here would significantly improve the UX, but I do agree that reporting an invalid_assignment isn't very helpful either.

If we could get enough information out of type inference to know that the type Never was the result of the fact that the context type and the bound are not compatible, then we could report that as the problem and provide some real help for the user to understand the problem and its solution.

This might even help in the second case, where an as is hiding the problem. With that information the analyzer could report a warning suggesting that the type incompatibility might be masking an issue (or a lint if we think such a check would have too many false positives).

@stereotype441 Is there a way for the analyzer to get this information?

@FMorschel
Copy link
Contributor Author

I already had proposed something similar about casting unrelated types at dart-lang/linter#5062.

@a-siva a-siva removed the triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. label Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants