[analyzer] extend unused_element for public declarations inside private ones

[asashour]

Currently, unused_element doesn't consider public declarations which are inside private ones.

For example:

class _A {
  m() {}
}

Class _A is private, so is not visible outside the defining library.

Method m is not used (in this example), and can not be used outside.

Would it make sense to extend unused_element to cover such cases?

[bwilkerson]

I believe it would be safe to say that it's unused if

• the class is private,
• there are no invocations of m inside the library,
• the member does not override a member from a superclass, and
• no instance of the class is ever accessible from outside the library.

But the last case is fairly involved to check and I suspect that the number of cases where we could definitely conclude that the member can't be used is fairly small, so I'm not sure it's worth the effort (either in terms of our time to write the code or in terms of the processor time to run the code for every public member in every private class).

[asashour]

• no instance of the class is ever accessible from outside the library.

I thought that this is not possible, because the class is private.

But I can see that it can be exposed by

class P {
  _A get() => _A();
}

class _A {
  m() {}
}

In this case, we can also check if the the private class is exposed by a public method (in a public declaration).

Is there any other way to expose a private class to be used from outside the library?

... number of cases ... is fairly small, ..in terms of the processor time ...

In fact, this was detected, for a method inside a Flutter widget State, which is private by default, so I would imagine it adds a value to many users.

And in regards the processor time, the following case is already handled, so my guess it wouldn't harm to also add the public methods to the same check (with not so big coding effort, hopefully).

class _A {
  _m() {}
}

[eernstg]

Note the similarities between this topic and that of dart-lang/language#2020, which is about the ability to override a private member. That issue mentions several different situations where an override may occur, even though it is perhaps tempting to assume that it is not possible.

Here is one situation which is taken from that issue and adjusted for this context: Any public subtype B of _A would make m part of a public interface, so if anyone invokes m with a receiver of type B (or some supertype whose interface contains a member named m) then m is no longer unused. Presumably, no messages would be emitted in this situation, because we can't assume that this will never happen.

The ability to invoke a member like m using a dynamic invocation is more difficult to handle, because this makes it an undecidable question whether or not m could be invoked from another library, and, again, if we have an open world assumption we can't rule out that there could exist a dynamic invocation of m.

It is possible that we could ignore dynamic invocations, though: We're looking for situations where m is (apparently) unused, and if it is never invoked with a statically typed receiver then we could perhaps say that it "is never used properly" ;-), and hence it's OK to report that situation to the authors of m.

[asashour]

The mentioned issue dives so deep in the language ocean.

I understand the burden of the open/closed world analysis.

How about the minimal level, in which only the library in question is analyzed, and in which no exposure of the parent class. I would refer to the usual Flutter stateful widget:

class My extends StatefulWidget {

  State<My> createState() => _MyState();
}

class _MyState extends State<My> {

  Widget build(BuildContext context) => Container();

  void myPublic() {}
}

Of course, any feature that can lead to a potential public access, would not trigger the diagnostic. Later steps could follow, similar to the mentioned topic.

[srawlins]

The algorithm would definitely have to do more work to identify if a private type is exposed publicly. I don't think any of our current analyses do this. Tricky as well (meaning, high potential for bugs, and definite maintenance cost). Case examples:

• typedefs (typedef A = _A; and a public element returns an A)

• public top-level field typed with the private class

• public top-level functions return-typed with the private class

• public member of public library element (class, mixin, extension), with analogous cases to the above

• public members of private library elements! Yikes! E.g.

  class _A {
    void m() {} // Used?
  }
  class _B {
    _A m2() => _A(); // Uh oh, is [_B] exposed publicly?
  }

• type variables - here's a fun one where _A can escape:

  T foo<List<T>>(T p) => p.first;
  class MyList implements List<_A> {}

It's a solvable problem, but I think requires making a reference graph, and a very tricky walk of the graph to find private types which escape.