Analyzer API: Could the potential types of an AstNode/Element property be documented?

[rrousselGit]

Hello!

Problem

This is general feedback about the Analyzer API. It's working great, but I believe there's some usability concerns related to knowing what are the potential types of a given property.

More specifically, the Analyzer API largely relies on subclasses. For example Element vs Expression vs AssignmentExpression.
And due to the power of Dart, there a lot of options.

The issue is, when we look at a specific variable exposed by the Analyzer, it is often typed as a lowr-level interface.

For example, AssignmentExpression.leftHandSide is typed as Expression. But that's not very clear. Because there are a ton of subclasses for Expression, but many of which will never be used by leftHandSide. One case could be: leftHandSide will never be a ListLiterral, As [] = value is not a valid Dart syntax

Currently, we can only make an educated guess about what the concrete types for a variable is.
For example, my assumption is that AssignmentExpression.leftHandSide is in fact Identifier | IndexExpression, respectively for variable = 42 vs list[0] = 42. But that's only my guess, and it's very likely I missed one option.

Proposal

Would it be possible to list the potential concrete types in the dartdoc of any given property?

For example, the doc of AssignmentExpression.leftHandSide could be:

abstract class AssignmentExpression {
  /// Return the expression used to compute the left hand side.
  ///
  /// Can be an instance of either:
  /// - [Identifier], when writing `variable = value;`
  /// - [IndexExpression]`, when writing `object[key] = value`
  Expression get leftHandSide;
}

Alternatively, but also involving a lot more work and possibly breaking, an option would be to avoid using shared interfaces like Expression and instead define new unique interfaces like LefHandSideExpression. Such that we'd have:

abstract class AssignmentExpression {
  /// Return the expression used to compute the left-hand side.
  LeftHandSideExpression get leftHandSide;
}

Then, it becomes possible to use search options like "Search all references"/"Search all implementations".
With such an approach, instead of having the list of all Expressions pop up, only the ones that we are concerned with would show up.

This could also later be adapted to use the upcoming pattern-matching feature. Using some form of sealed class, it could become possible to write an exhaustive check like:

switch (assignment.leftHandSide) {
  case Identifier():
  case IndexExpression():
}

But of course, the dartdoc option is probably faster to implement for now.

[bwilkerson]

But that's only my guess, and it's very likely I missed one option.

Proving your point, there are two more that I can think of easily: PrefixedIdentifier and PropertyAccess.

Would it be possible to list the potential concrete types in the dartdoc of any given property?

Yes, it's possible. My concern would be that the analyzer team isn't very good about updating such documentation (I'm not claiming that that's ok, only that it's historically true), so it would likely get out of date fairly quickly, which would probably be worse than having no documentation.

... avoid using shared interfaces like Expression and instead define new unique interfaces like LefHandSideExpression.

That's an interesting idea, but I'd want to understand the implications of such a change in terms of the number of new interfaces that would need to be added. But at least the type system would partially help us stay honest.

[rrousselGit]

@bwilkerson

That's an interesting idea, but I'd want to understand the implications of such a change in terms of the number of new interfaces that would need to be added. But at least the type system would partially help us stay honest.

If having too many interfaces is an issue, what about an in-between?
Those interfaces could be private, and used to automatically generate the doc.

For example we could have:

abstract class AssignmentExpression {
  Expression get leftHandSide;
}

class _AssignmentExpressionImpl implements AssignmentExpression {
  @override
  _AssignmentExpressionLeftHandSide get leftHandSide;
]

abstract class _AssignmentExpressionLeftHandSide implements Expression {}

class IndexExpression implements _AssignmentExpressionLeftHandSide {}

Then, the dartdoc of AssignmentExpression.leftHandSide could list all subclasses of _AssignmentExpressionLeftHandSide.
This would be statically computable, making sure docs are up-to-date. At the same time, those interfaces aren't public.

I'd personally prefer making those interfaces public though.

[bwilkerson]

Definitely something to consider.

We'll also need to think about how this interacts with the sealed and type-modifiers language proposals. Not sure at the moment whether/how we'll want to update the ASTNode hierarchy to make use of those features.