On Wed, 29 Sep 2021 10:39:28 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> FWIW, I'm still in two minds whether this should be a switch or overriding >> methods. It's ... er, disappointing ... that the lack of support for >> `PROVIDES` and `USES` didn't "automatically" show up in testing. That may >> suggest a lack of coverage in the `DocTree` tests, which suggests the need >> for a test to check coverage. > > For the sake of this review, using `switch` would be better: fewer changes, > less cognitive load. Later we can think of ways of improving that code. It > would be nice to structure it such that it would fail at compile time, if we > forgot a case. It's hard to know how to make it fail at compile time. There are essentially 3 groups of trees ... inline tags (and other trees that use `DCEndPosTree`), block tags (the big block of cases round about line 177), and "other". We could arguably move block tags into the `default` case and use a type test, which would improve the future-safety. We could arguably catch missing cases at runtime with suitable use of `IllegalStateException`. As for tests, we *do* have tests for `ProvidesTree` and `UsesTree`, but the new `RangeChecker` didn't catch the missing case labels because the basic checks in `RangeChecker` were satisfied by the code in the default case in the switch statement. ------------- PR: https://git.openjdk.java.net/jdk/pull/5510
