rewrite-migrate-java
rewrite-migrate-java copied to clipboard
Safely expand SequencedCollection `getFirst/Last` to also convert lists returned from method calls
The recipe at https://docs.openrewrite.org/recipes/java/migrate/util/sequencedcollection can recognize a match where a simple index value is used
- TokenAdaptor tokenAdaptor = cachedTokens.get(0);
+ TokenAdaptor tokenAdaptor = cachedTokens.getFirst();
but it misses code where the size of the collection being operated on is given (which an Intellij inspection will convert) such as
- getHistory().remove(getHistory().size() - 1);
+ getHistory().removeLast();
or even where a constant is used, such as
- var storedTransactionLogAdaptor = (StoredTransactionLogAdaptor) resultVector.get(SettlementTransactionTokenSubTransactionConstants.EXPECTED_RESULT_SET_RECORD);
+ var storedTransactionLogAdaptor = (StoredTransactionLogAdaptor) resultVector.getFirst();
Calculating the index based on a constant value may be asking too much.
getLast() should work when a variable is used, but with method calls we hadn't yet covered those out of caution. In theory there might be side effects to that method call, hence why we're being safe. While it ought to be rare that folks call a method twice and expect a different collection, we can't rule it out and hence have to opt for the safest mode of not making code changes.
Thanks again for the suggestion @delanym , but I don't really see a good way forward to cover this in a guaranteed safe way for add/get/removeLast. Note that we already added support for add/get/removeFirst in
- https://github.com/openrewrite/rewrite-migrate-java/issues/423
- https://github.com/openrewrite/rewrite-migrate-java/issues/424
Best then to close this issue such that we don't make any unsafe changes.