I was recently looking at tidying up a couple of deprecation warnings during the compileJava task of our build. In Groovy 5, we added for loops with index variables and as part of that change we now have a getValueVariable() method and a getIndexVariable() method instead of the old getVariable(), now deprecated. We were still calling the old method in a few spots. This is quite possibly related to churn while we were considering whether we could simplify certain for loop variants.
Anyway, as part of trying to remove the deprecated calls, I noticed that some of the implementation details leak out of the current abstractions. I was going to propose we fix with this: https://github.com/apache/groovy/pull/2378 I am keen on any feedback. There is an argument that we should apply a similar treatment to getIndexVariable(), i.e. provide a hasIndexVariable() method checking whether it is null. At the moment, there are various visitors that don't visit that node. Having something like hasIndexVariable() would simplify adding in such traversal. Having said that, there aren't any scenarios which strictly need it yet. We don't need ResolveVisitor to visit the type since we have a limited number of known types we support. We currently restrict the index variable from having annotations at the grammar level. If we ever added support for that, and we'd need to answer a bunch of questions first, we'd need to add in traversal support for a couple of more visitors. This would allow frameworks to have such annotations, though we know of none currently asking for this. So, to cut a long story short, I want to keep such a change out of this discussion and we can add that in a subsequent ticket if needed. Thoughts? Cheers, Paul.
