dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.2k stars 1.57k forks source link

Analyzer not reporting compile-time type error #54011

Open fgaudo opened 11 months ago

fgaudo commented 11 months ago

New project with the following analysis_options.yaml:

# analysis_options.yaml

include: package:lints/recommended.yaml

analyzer:
  language:
    strict-casts: true
    strict-inference: true
    strict-raw-types: true

Given the following dart file content:

// example/bug_example.dart

typedef IO<A> = A Function();
typedef Reader<R, A> = A Function(R);
typedef ReaderIO<R, A> = Reader<R, IO<A>>;

Reader<R, B> Function<R>(Reader<R, A>) map<A, B>(
  B Function(A) f,
) =>
    <R>(ra) => (r) => f(ra(r));

ReaderIO<R, B> Function(ReaderIO<R, A>) flatMap<R, A, B>(
  ReaderIO<R, B> Function(A) f,
) =>
    map<IO<A>, IO<B>>((io) => f(io()));   // returning f(io()) should be an error

The analyzer outputs:

$ dart analyze example/bug_example.dart 
Analyzing bug_example.dart...          0.2s
No issues found!

Running the file:

$ dart run example/bug_example.dart 
example/bug_example.dart:15:32: Error: A value of type 'B Function() Function(R)' can't be returned from a function with return type 'B Function()'.
    map<IO<A>, IO<B>>((io) => f(io())); // returning f(io()) should be an error
                               ^

Info:

$ dart info

If providing this information as part of reporting a bug, please review the information
below to ensure it only contains things you're comfortable posting publicly.

#### General info

- Dart 3.1.5 (stable) (Tue Oct 24 04:57:17 2023 +0000) on "linux_x64"
- on linux / Linux 6.1.0-13-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.55-1 (2023-09-29)
- locale is it_CH.UTF-8

#### Project info

- sdk constraint: '^3.1.5'
- dependencies: 
- dev_dependencies: lints, test

#### Process info

| Memory |  CPU | Elapsed time | Command line                                                                    |
| -----: | ---: | -----------: | ------------------------------------------------------------------------------- |
| 440 MB | 1.8% |        13:51 | dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.76.1 |
lrhn commented 11 months ago

That does look like a bug, like the analyzer simply gives up at some point.

If I introduce some static type checks on the sub-expressions (and unfold the type aliases, so it's easier to see the function structure), I get:

B Function(R) Function<R>(A Function(R)) map<A, B>(
  B Function(A) f,
) =>
    <R>(A Function(R) ra) => (R r) => f(ra(r));

B Function() Function(R) Function(A Function() Function(R)) flatMap<R, A, B>(
  B Function() Function(R) Function(A) f,
) =>
    map<A Function(), B Function()>(
       ((A Function() io) => f(io())..st<E<B Function() Function(R)>>())
          ..st<E<B Function() Function(A Function())>>);   // returning f(io()) should be an error

void main() {}

extension <T> on T {
  void st<X extends E<T>>(){}
}
typedef E<T> = T Function(T);

If I make a mistake in the typing of the .st<E<...>> types, then the front-end complains, but the analyzer is silent. If I change f(io) to, fx, f(io + 1), the analyzer does tell me that the type doesn't have a +, so it gets there. It just gives up, before (or when) checking the types of the functions.

bwilkerson commented 11 months ago

@scheglov

scheglov commented 11 months ago

I hope this isn't too much of an imposition, and I apologize in advance if my suggestion comes across as offensive or overstepping my boundaries. I've taken a quick glance at the code, and it seems to me that @srawlins might have a better understanding of the context here.

srawlins commented 11 months ago

Thanks for the repro and expanded repro. I don' think this has anything to do with the strict modes. This isn't my area of expertise 😊 but it's probably something I could look into when it gets prioritized.

eernstg commented 11 months ago

Perhaps a change in the analyzer that resolves https://github.com/dart-lang/sdk/issues/54055 would also have an effect on the behavior seen here.

lrhn commented 11 months ago

54055 could look related.

More reproduction here. It seems it is the generic instantiation of a returned type variable (unused here) which turns of the validation of the return value. Or, more likely, it turns of error-reporting or type analysis.

In particular the

  map((C _) => ctx(D())..st<Ex<F>>)<G>;  // Not checking `st<Ex<F>>` *at all*.

should give an error for any reasonable static type of the expression ctx(D()), which has the type of the context type, because whatever it is, it is definitely not exactly F. The only way to not get an error is not analyze at all, or to suppress the error.

// ignore_for_file: prefer_function_declarations_over_variables

void main() {// Returns generic function, takes function as argument.
  B Function<R>(A) map(D Function(C) f) => <R>(A _) => B();
  // Explicit or implicit type instantiation, removes return-type check,
  // or more likely context type and inferred return type, from argument.

  map((C _) => E())<G>; // Explicit instantiation. No error.
  B Function(A) f1 = map((C _) => E()); // Implicit instantiation. No error.

  map((C _) => ctx(D())..st<Ex<D>>);     // Context type is D!
  map((C _) => ctx(D())..st<Ex<F>>)<G>;  // Not checking `st<Ex<F>>` *at all*.
  // It doesn't complain that `st` is not defined, as it should for `dynamic` or `Never`,
  // It also doesn't check that the static type of ctx(...) is exactly `F`, because it isn't!
  // It is as if type checking *stops* at the return of the function argument.

  map((C _) {
    if (C() == C()) return E(); // No error.
    // String x = 42; // But not before the return, this is checked.
    return ctx(D())
      ..st<Ex<F>>()  // No error, on context type.
      ..toList().st<Ex<List<F>>>() // No error, on List<context type>
      ..toInt().st<Ex<F>>() // No error on int (?!?)
      // ..accept(E()) // "Argument E cannot be assigned to parameter type D"
      // ..arglebargle   // Getter "arglebargle is not defined for the type D"
      ;
  })<G>; // Explicit instantiation. No error.
  // If remove the `<G>`, all the errors above start showing.

  (map((C _) => E())..call<F>(A()))<G>; // Explicit, even through a cascade!

  var b = DateTime.now().millisecondsSinceEpoch < 0; // Aka. false.
  (b ? map((C _) => E()) : map((C _) => E()))<G>; // Also disables the check for both branches

  T id<T>(T value) => value;
  id(map((C _) => E()))<G>; // Explicit. No error.

  B Function<R>(A) idFunc(B Function<R>(A) f) => f;
  idFunc(map((C _) => E()))<G>; // Explicit. No error.

  //   map((C _) => E()); // Not instantiated, type error.
  //   map((E _) => D())<G>; // Parameter types are checked, this is still error.
}

// Unrelated  exmaple types.
class A {}
class B {}
class C {}
class D {}
class E {}
class F {}
class G {}

extension <T> on T {
  // Checks static type is exactly T.
  void st<X extends Ex<T>>(){}
  List<T> toList() => [this];
  int toInt() => 42;
  void accept(T value) {}
}
// Exactly, invariantly, T.
typedef Ex<T> = T Function(T);
// Captures context type as expression with that static type.
T ctx<T>(Object? v) => v as T;