Fix nextKey skipping a key and throwing for keys not in the map

[rootvector2]

AbstractSortedMapDecorator.nextKey walks tailMap(key) and discards the first element to step over key itself, but never checks that key is present. When key is absent that first element is the successor, so it gets dropped and the wrong key comes back; when key is at or past the end tailMap is empty and it.next() throws NoSuchElementException instead of returning null. DualTreeBidiMap.nextKey repeats the same tailMap().iterator().next() shape and fails the same two ways, since its isEmpty() guard only covers a fully empty map.

Return null when the map does not contain key, matching the documented null if no match contract and every other nextKey in the library (AbstractLinkedMap, ListOrderedMap, TreeBidiMap, PatriciaTrie all return null for an absent key); the skip-first logic then runs only for a present key, the case it was written for. FixedSizeSortedMap and UnmodifiableSortedMap inherit the decorator fix. Found by auditing the OrderedMap.nextKey implementations against the contract.

• Read the contribution guidelines for this project.
• Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
• I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
• Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
• Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

[garydgregory]

Hello @rootvector2

Thank you for the PR. How about adding tests for:

• An explicit test for DualTreeBidiMap.nextKey() with an absent key that is between existing keys. The abstract test uses getOtherKeys() which are outside the sample range, so the bug would still be hidden if the key were in-range.
• An explicit test for UnmodifiableSortedMap.nextKey(). It inherits the decorator fix but has no dedicated test.

Should the code be using NavigableMap.higherKey() when available? TreeMap implements NavigableMap, so higherKey(key) would give the correct successor in one call and would be null for absent keys automatically.

Should we guard the OrderedMap branch in DualTreeBidiMap.nextKey() for consistency; either ensure the delegated implementation also checks presence or apply the same containsKey guard before delegation.

Isn't there a similar problem with previousKey()? That could be addressed in a different PR.

[garydgregory]

@rootvector2 ping 🔔

[rootvector2]

Pushed the requested changes:

• DualTreeBidiMapTest#testNextKeyAbsentKey covers an absent key between existing keys ("b" in {a, c, e}) plus one past the end, and UnmodifiableSortedMapTest#testNextKey exercises the inherited decorator fix directly.
• Switched both implementations to NavigableMap.higherKey when the underlying map supports it. The containsKey guard has to stay though: higherKey returns the least key strictly greater than the argument whether or not it's present, so an absent in-range key would still get the successor instead of null. It only handles the past-the-end case automatically.
• Hoisted the guard in DualTreeBidiMap.nextKey above the OrderedMap delegation so both branches agree on absent keys.
• previousKey has the same absent-key inconsistency: it returns the predecessor instead of null (no throw, since headMap already excludes the key). I'll send that in a separate PR as you suggested.

Full default mvn goal is green.

[garydgregory]

@rootvector2 Thank you, merged 🚀