Support static analysis of required operations

[eernstg]

The Java programming language has a try-with-resources feature delivering guarantees for the execution of specific operations on a given object in a specific scope.

try (Connection conn = dataSource.getConnection()) {
  conn.executeQuery("a query");
}

This works like a syntactic transformation when conn has a type which is a subtype of AutoCloseable, adding a finally block where conn.close() is called.

I think it would be useful to have a similar mechanism in Dart.

This issue is a proposal to generalize local variable declarations such that they support a similar kind of guarantee, but somewhat more general, and also somewhat more manually supported.

For example:

class File {
  void close() {}
}

void main() {
  final f = File() do close;
  if (condition) {
    f.close();
  } else {
    ... // Does not invoke `f.close()`.
  }
  if (otherCondition) throw "Whatever";
}

This declaration would give rise to diagnostic messages indicating that f.close is not guaranteed to have been executed at the throw expression, and the same holds for the end of the function body. This outcome is based on a static analysis which is similar to definite assignment analysis for local variables, but it keeps track of whether the statically known member close has been invoked on f. For any location where the function body is exited, and no such invocation of close is guaranteed to have occurred for all control flow paths, the diagnostic is emitted.

The feature is more general than the Java try-with-resources mechanism. It is associated with a local variable declaration rather than a try statement, which means that it needs to be located inside a regular try statement in order to be sound in the case where the function body is exited because of a throwing completion caused by code outside the current function body. However, this is a choice which is made by the developer, and I expect the approach where it is assumed that no such exception will occur is useful as well. So we allow this particular kind of unsoundness to exist, and rely on developers to write the try statement if they need to enforce the given discipline also when the code is exited because of an exception.

Next, the analysis would be able to determine the case where an operation is executed at least once on every control flow path, but it would also be able to determine that it has been executed exactly once, or at most once. It seems useful to make these distinctions, and the feature supports the modifiers once and maybe to handle these variants.

Finally, note that there's no need to connect this mechanism to the lifespan of any given object. In particular, we may wish to keep track of the fact that a particular object will invoke its init method exactly once on all control flow paths, but we don't want to rename init to close in order to satisfy the requirements that the type of the object must be AutoCloseable or anything like that, and we also don't wish to enforce that the object remains local (non-leaked) to the current function body, it's perfectly fine if we enforce that init is called once on a newly created object, and then it's free to be leaked (returned!) to other parts of the program.

Based on these considerations, the feature supports arbitrary lists of identifiers denoting statically known members of the receiver.

void main() {
  final f = File(...);
  try {
    final contents = f.readAsStringSync();
    final thingy = Thingy.parse(contents) do init once;
    L: if (condition) {
      thingy.init(...);
    } else {
      if (otherCondition) break L;
      thingy.init(...);
    }
    // Warning: We may have invoked `thingy.init` zero times.
  } finally {
    f.close(); // Warning at end of function if we forget this.
  }
}

Syntax

<localVariableDeclaration> ::= // Modified rule: Adding one alternative at the end.
    <metadata> <initializedVariableDeclaration> ';'
  | <metadata> <patternVariableDeclaration> ';'
  | <metadata> <doVariableDeclaration> ';'

<doVariableDeclaration> ::=
    'final' <type>? <identifier> ('=' <expression>)? 'do' <doMemberList> ';'

<doMemberList> ::=
    <doMemberElement> (',' <doMemberElement>)*

<doMemberElement> ::=
    <identifier> ('maybe' | 'once')?

Static analysis

The flow analysis is extended with the ability to maintain information about local variables declared as 'do' variables with respect to the declared member names, similar to the information about definite assignment.

A warning is emitted for each point where the current function body is exited, and one of the following conditions is satisfied:

• A do-variable v has a do-member m with no modifiers, and one or more control flow paths to the current location does not include an invocation of m with v as the receiver.
• A do-variable v has a do-member m with the modifier once, and one or more control flow paths to the current location does not include an invocation of m with v as the receiver, or it contains more than one such invocation.
• A do-variable v has a do-member m with the modifier maybe, and one or more control flow paths to the current location contains more than one invocations of m with v as the receiver.

The diagnostic message is emitted for each of these points, but it may be emitted at an earlier location, which may indicate the source of the problem more clearly. For instance, it may be known much earlier that a particular control flow path will executed a once do-variable member twice.

An invocation of a member is an invocation of a method (not a tear-off), and invocation of a getter or a setter. We could include a tear-off of a method as an invocation, and we could require that a getter whose return type is a function type is invoked and the returned function object is also invoked, but we've chosen to include only the most typical kinds of invocations.

[craiglabenz]

This would be cool to see.

It's probably worth pointing out that realistic Flutter applications are much more convoluted than the typical file handler management case that Java, Python, and Go handle so well (and which is in your sample).

In a Flutter widget, a long-lived Controller might be initialized in one life cycle function and then closed in a different life cycle function; all of which can only be understood to be guaranteed-to-be-called if one groks a large percentage of Flutter Elements. I don't know whether it would be better to attempt to do that or to hard code awareness of more specific situations.

[julemand101]

One concern would be that this thing looks very much to be something the user of an API needs to already know about. E.g. the user needs to already know the close() method needs to be called since they need to specify that requirement when getting the instance of the object.

With AutoCloseable, it is the designer of the API that have made the decision that close() should be called when done. Not sure if this feature would have a way for API designers to express their intended usage of the API.

[lrhn]

It's probably undecidable in general whether you call a specific method or not.

(If you up-cast first and call .close() on that, is it the same value? What if the superclass had no close member, but there is an extension method, so it's not the same method? What if ... any number of things.)

If you allocate something, and then return it, you're probably a helper-allocator-wrapper-function.
Then it's the caller which must close/dispose/init/whatever the thing.

It sounds like something you could do with a flow analysis and annotations.

class SomethingDisposable {
  @mustInvoke(#close)
  SomethingDisposable() ...
}

and then whoever gets that value must call close, on that value or pass the requirement on to someone else:

   @mustInvoke(#close)
   static SomethingDisposable allocHelper(args) { // Doesn't call, close, but propagates the requirement.
     var value = SomethingDisposable(...);
     value.configure(...);
     return value;
   }

   static void disposeHelper(@mustInvoke(#close) SomethingDisposable value) {
      value.deconfigure(...);
      value.close();  // Commitment ends here.
   }

   static void withSomething(void Function(SomethingDisposable) action) {
     var value = allocHelper(...); // Requirement introduced.
     try {
       action(value);
     } finally {
       disposeHelper(value); // Requirement satisfied.
     }
   }

It's still flow analysis, which is tricky.
If anything can throw, then you need to guard the paths with finally.

Or it can be allowed to be "best effort" and assume things don't throw.

Doesn't feel like something that can be adequately handled by a language feature, because then failing a requirement is an error, not just a warning, and then it become much, much harder to write safe code. (Checked execeptions for all values!)

[eernstg]

It's probably undecidable in general whether you call a specific method or not.

Definitely, but some situations are amenable to static analysis and we may then be able to prove that the method has been called, or it has not been called, and possibly that it has been called a specific number of times. (As described in this issue, we'd assume that no expression evaluation completes throwing an exception unless it is a throw expression. However, if you wrap the code in a try/finally statement then this is covered soundly.)

Other situations are beyond the power of the given static analysis, and they just have to be declared as unsafe (by emitting a warning). This is no different from the situation with a final variable which has or hasn't been initialized when it is first used. For example:

void main() {
  final int x, y;
  bool b = true as dynamic; // Not known statically.
  if (b) {
    x = 1;
  } else {
    x = 2;
  }
  if (b) {
    y = 1;
  }
  if (!b) {
    y = 2; // Error, `y` might have been initialized already.
  }
  print(x + y); // `x` is OK, `y` is not definitely assigned.
}

If you up-cast first and call .close() on that, is it the same value?

The analysis only works relative to a specific local variable (and it must even be final). You can cheat it, including: Create an alias (var other = v;) and then operate on that other variable (e.g., executing other.foo(), violating a constraint that v.foo must be executed exactly once).

So you're definitely expected to write the code in a way that doesn't try to cheat. That shouldn't be too hard, though, just keep it simple.

class SomethingDisposable {
@mustInvoke(#close)
SomethingDisposable() ...
}

Interesting! This could be connected to the do-variable mechanism using a lint:

void bar() {
  final thing = SomethingDisposable() do close; // Linted if we omit 'do close'.
  ..
}

However, it's not a given that this generalization is helpful. For example, we might be in a situation where the newly created SomethingDisposable is created outside bar and provided as an actual argument, or we might not want to close it inside bar because it's supposed to have a much longer life and be accessed by many pieces of code outside bar. And so on.

Or it can be allowed to be "best effort" and assume things don't throw.

Yes, that's exactly the approach I've proposed.

Doesn't feel like something that can be adequately handled by a language feature

I can't see why it wouldn't be possible (or appropriate) for a language mechanism to give rise to warnings. For instance, dead_code does this. Also, it does need to be wired into the flow analysis in order to avoid duplicating the entire flow analysis framework. But nothing stops us if we insist on expressing the do requirements using metadata, and then we can of course say that "it isn't a language mechanism". ;-)