GH-10083: Add Nullability to support.management #10399
Conversation
| @@ -300,7 +302,7 @@ public static final class CommandMethod { | |||
|
|
|||
| private final Class<?>[] parameterTypes; | |||
|
|
|||
| private String description; | |||
| private @Nullable String description; | |||
There was a problem hiding this comment.
I would prefer NullAway.Init.
See the logic in the populateDescriptionIntoCommand().
So, whenever this CommandMethod is initialized after populateCommandMethod(), the description is not null.
| @@ -132,7 +133,7 @@ public record ControlBusBean(String beanName, List<ControlBusCommand> commands) | |||
|
|
|||
| } | |||
|
|
|||
| public record ControlBusCommand(String command, String description, List<Class<?>> parameterTypes) { | |||
| public record ControlBusCommand(String command, @Nullable String description, List<Class<?>> parameterTypes) { | |||
There was a problem hiding this comment.
Therefore the change in this class will go away.
| @@ -273,7 +274,7 @@ else if (conversionService.canConvert(key.getClass(), String.class)) { | |||
| throw new MessagingException("Unsupported channel mapping type for router [" | |||
| + key.getClass() + "]"); | |||
| } | |||
|
|
|||
| Assert.notNull(channelKey, "'channelKey' must not be null"); | |||
There was a problem hiding this comment.
I think this is a false impression.
See the logic:
else if (conversionService.canConvert(key.getClass(), String.class)) {
channelKey = conversionService.convert(key, String.class);
}
I believe, according to that canConvert() Javadocs, the convert result would never be null.
So, we probably need @SuppressWarnings("NullAway") instead.
There was a problem hiding this comment.
Yeah... This is a critical path execution for the router logic, so I'd rather bite a bullet and suppress than extra code.
| @@ -43,6 +43,7 @@ | |||
| * The REST Controller to provide the management API for Control Bus pattern. | |||
| * | |||
| * @author Artem Bilan | |||
| * @author Glenn Renfro | |||
There was a problem hiding this comment.
Sorry, my friend, but looks like this is only change in this class 🥶
There was a problem hiding this comment.
I debated which direction on both instances. There was a remote path where they could be null. So I chose the code path vs the SuppressWarning. But if it has no chance at null, then we are good to Suppress. And for performance for sure.
There was a problem hiding this comment.
Doh! But I wanted my name here 😄
This is for performance reasons
No description provided.