squid:S2259 - Null pointers should not be dereferenced #200
Conversation
| @@ -134,10 +134,14 @@ static void registerExpander(String macroName, Class<? extends JobConfigExpander | |||
| } catch (InstantiationException | IllegalAccessException e) { | |||
| log.warn("Class '" + clazz + "' registered for '" + macroName + "' cannot be initialized: " + e, e); | |||
| } catch (ClassCastException e) { | |||
| log.warn("Class '" + clazz + "' registered for '" + macroName + "' is not JobConfigExpander but '" + o.getClass() + "'"); | |||
| log.warn("Class '" + clazz + "' registered for '" + macroName + "' is not JobConfigExpander but '" + getObjectClassIfNotNull(o) + "'"); | |||
There was a problem hiding this comment.
o is guaranteed to be not null, so the extra guard is unnecessary here.
There was a problem hiding this comment.
Maybe it would be better to split the try block into two. A bit more verbose, but it would be clearer which catch blocks are guarding which line. Something something clarity is great.
It's been an awfully long time, so I suggest closing this PR (with gratitude), and then fixing whatever you want from a cleaner commit.
| @@ -33,7 +33,7 @@ public JobTask getTask() { | |||
|
|
|||
| @Override | |||
| public boolean equals(Object o) { | |||
| if (o.getClass() != getClass()) { | |||
| if (o == null || o.getClass() != getClass()) { | |||
There was a problem hiding this comment.
A fun alternative here is a humble instanceof call. It'll return false for both nulls and guys two lines away from a class cast exception.
One time I checked the jvm bytecode specifically to spite an old professor and at the time at least, it was also several instructions more efficient.
This pull request is focused on resolving occurrences of Sonar rule
squid:S2259 - Null pointers should not be dereferenced.
You can find more information about the issue here:
https://dev.eclipse.org/sonar/coding_rules#q=squid%3AS2259
Please let me know if you have any questions.
George Kankava