Use LinkedHashMap and LinkedHashSet in configuration properties#50367
Conversation
Signed-off-by: Venkata Naga Sai Srikanth Gollapudi <42247688+GollapudiSrikanth@users.noreply.github.com>
… Arch rule Signed-off-by: Venkata Naga Sai Srikanth Gollapudi <42247688+GollapudiSrikanth@users.noreply.github.com>
spring-projects-issues
added
the
status: waiting-for-triage
Signed-off-by: Venkata Naga Sai Srikanth Gollapudi <42247688+GollapudiSrikanth@users.noreply.github.com>
Signed-off-by: Venkata Naga Sai Srikanth Gollapudi <42247688+GollapudiSrikanth@users.noreply.github.com>
snicoll
left a comment
snicoll
left a comment
There was a problem hiding this comment.
Thanks for the PR, I've left some comments for your consideration.
| @Override | ||
| public boolean test(CodeUnitCallTarget target) { | ||
| String ownerName = target.getOwner().getName(); | ||
| return ("java.util.HashMap".equals(ownerName) || "java.util.HashSet".equals(ownerName)) |
There was a problem hiding this comment.
This explicitly check for the presence of those two and would miss all other cases. It'd be better if the rule checks that field of type Map or Set, if initialized, uses LinkedHashMap or LinkedHashSet respectively. I also don't think we need to link those two together. We could have a "is property of type X initialized, make sure it is Y. With that we can have three rules for List, Set and Map that call the same logic with different arguments.
There was a problem hiding this comment.
Thanks for the feedback.
That makes sense. I'll rework the rule to validate the expected implementation based on the field type rather than checking specifically for HashMap and HashSet. I'll also look at extracting the common logic so that List, Set and Map can share the same validation mechanism with the expected implementation passed as an argument.
| * | ||
| * @author Stephane Nicoll | ||
| * @author Madhura Bhave | ||
| * @author Venkata Naga Sai Srikanth Gollapudi |
There was a problem hiding this comment.
Please revert the author tag for every configurationproperties you've touched that only changes the type. This isn't considered as a significant change.
There was a problem hiding this comment.
Thanks @snicoll , I'll revert the author tag changes and keep the authorship limited to the new rule implementation and any substantial additions.
snicoll
added
the
status: waiting-for-feedback
3 participants
This PR updates @ConfigurationProperties classes to use LinkedHashMap and LinkedHashSet consistently instead of HashMap and HashSet.
This aligns with the preferred convention discussed in #43919 and provides consistent ordering behavior across collection and map configuration properties.
While working on this, I also noticed a number of cases involving final nested/collection/map properties that would require broader changes to align with the preferred option 2 style (initialized field with getter/setter). To keep this PR focused and easier to review, those changes have been left for potential follow-up work.
Closes #43919