[new lint]: `avoid_types_as_type_parameter_names`

[pq]

See https://github.com/dart-lang/sdk/issues/32042#issuecomment-568497057.

Related to: http://dart-lang.github.io/linter/lints/avoid_types_as_parameter_names.html

[pq]

(Looking at this now.)

[pq]

Looking at the related lint, I see this comment:

https://github.com/dart-lang/linter/blob/e57b9cd870cd0d15a8c2a291283eccf10b29f0e0/lib/src/rules/avoid_types_as_parameter_names.dart#L53-L54

and then a heuristic:

https://github.com/dart-lang/linter/blob/e57b9cd870cd0d15a8c2a291283eccf10b29f0e0/lib/src/rules/avoid_types_as_parameter_names.dart#L55-L62

@scheglov, @bwilkerson: thoughts on a better way? Can we "synthesize" a type object for the identifier and see if it resolves? (Not saying this right I'm sure...)

[bwilkerson]

We would need to build a Namespace for the scope surrounding the function declaration and then attempt to resolve the name in that scope. As far as I know we don't have any public API to allow us to do that.

It would be useful to have such an API. We could use it here, and we could also use it when suggesting names to ensure that the name we're suggesting would not hide an existing name.

I don't really want to make Namespace part of the public API, but one alternative might be to add a method to ResolvedUnitResult to resolve a name at a given offset (or return null if the name is not defined in that scope). That would allow us to answer both of the queries above, but might be inefficient for other uses. A slight variation would be to take a list of names and return a map.

[pq]

@scheglov: do you have any preferences given Brian's proposals?

[scheglov]

Coincidentally I was thinking how to update clients that currently use ScopedVisitor (other than just break them because they should not do this without our explicit permission). There are three internal clients that do this, and look into nameScope.

Yes, I think we could add a new method to ResolvedUnitResult, and also to LinterContextUnit. Something like Element resolveScopeName(AstNode atNode, String name). Using atNode instead of atOffset seems better, because clients that need this functionality try to make some decision given a node.

[bwilkerson]

Perhaps resolveNameInScope(String name, AstNode scope)?

We'll need to carefully define what scope is being used by each class of node. For example, if the node is a ForStatement, does the name get looked up in the scope containing the for statement or the scope of the body of the for statement? My intuition is that it should be the scope containing the node, but that might make some scopes difficult or impossible to access (such as the type-parameter scope of a class).

[scheglov]

Sorry, I thought some more about this issue, and think that maybe we should go with more specific services in LinterContext.

For unnecessary_this we need to check if some class member element can be referenced without this. prefix at some location. If it is a member in the same class, we only have to check that it is not shadowed in the local scope. If it is an inherited member, or an extension member, we also need to check that the library scope does not shadow it.

extension E on A {
  int get foo => 0;
}

int foo = 0;

class A {
  void bar() {
    this.foo;
  }
}

So, maybe something like this?

  /// If the given [element] is declared in a class, mixin, or extension,
  /// return `true` if it can be referenced by its name without `this.` prefix
  /// at the location of the [node].
  bool canReferenceElementWithoutThisPrefix(Element element, AstNode node);

And for this new lint, provide exactly what the linter needs.

  /// Return `true` if the [name] if the name of a type defining element
  /// in the library scope, i.e. of a class, mixin, or a function type alias.
  ///
  /// Return `false` for names of other declarations - local variables and
  /// functions, top-level functions, extensions, import prefixes, etc; or when
  /// the name is not a name of any element.
  bool isTypeName(String name);

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/129819 implements canReferenceElementWithoutThisPrefix.

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/129819 added LinterContext.resolveNameInScope(String id, bool setter, AstNode node), which is enough to stop using ScopedVisitor in linter for UnnecessaryThis. I have not tried to use it for avoid_types_as_type_parameter_names and the other lint, some additional scope remembering might be necessary.

[pq]

Fantastic.

When @stereotype441 is back and can do an analyzer push, let's get this all updated to use the new APIs. 👍