NPE nullable warning toggles

This code snippet generates a warning about NPE at withEtx.length inside for()
A “NullPointerException” could be thrown; “withEtx” is nullable here.

// Java
final byte[] withEtx =
	ByteBuffer.allocate(input.length + Constants.ETX_LENGTH)
		.put(input)
		.put(Constants.ETX)
		.array();

if (log.isTraceEnabled())
	log.trace("Data with ETX added: {}", bytesToHex(withEtx));

byte lrc = 0;
for (int i = 0; i < withEtx.length; ++i) {
	lrc ^= withEtx[i];
}

When I remove log.trace() the warning disappears:

// Java
final byte[] withEtx =
	ByteBuffer.allocate(input.length + Constants.ETX_LENGTH)
		.put(input)
		.put(Constants.ETX)
		.array();

byte lrc = 0;
for (int i = 0; i < withEtx.length; ++i) {
	lrc ^= withEtx[i];
}

Method bytesToHex() has final modifier and bytes cannot be set to null

// Java
static String bytesToHex(final byte[] bytes) {
	// some work
}

Hi @H.Lo,
We agree there is an inconsistency here, and we suspect the presence of a null check is tripping the check.

Answering these questions would help:

  1. Is the rule raising the issue S2259?
  2. Is there a null check in bytesToHex?
  3. Are there annotations on the package, class or method that indicate that the parameter passed to bytesToHex is non-null by default?

Hi!

The method bytesToHex() has a parameter that has final modifier and passing non null inside will leave it non null no matter what method do if you have null check or not.

Also the final variable withEtx is created by ByteBuffer.array() and cannot create null value which makes it non null.
If there was no final modifier on variable or method parameter, the situation would be unknown.
And that’s the main reason I use finals as much as possible - to keep things immutable.

BTW This is not a problem for me, I just wrote this ticket to let you know and to let you decide what to do with it. For sure I’m not giving up on SonarLint :slightly_smiling_face:
BR,
Hrvoje

Thanks for the extra info (and sticking with Sonarlint :wink: ), I didn’t think about the parameter being marked as final but indeed that does not seem to change the way the rule behaves.

I think we can discard any misunderstanding around .array() but I would like to dig a bit deeper around bytesToHex: Is there a null check in there or a method invoked within bytesToHex that would do such a thing?

  static String bytesToHex(final byte[] withEtx) {
    if (withEtx == null) {
      return "";
    }
    return withEtx.toString();
  }

In the example above, the null check is sufficient to raise an issue but when it is removed the issue disappears.

1 Like

Hi!
Now that I give it a second thought, - yes, there is logical explanation about mentioned behaviour.
I have null check inside method and it returns null in case of null input.
I never send null into this method, but of course, how could you possibly know - I realize it now!

public static String bytesToHexLog(final byte[] bytes) {
		if (bytes == null) {
			return null;
		}
		final StringBuilder sb = new StringBuilder();
		for (int j = 0; j < bytes.length; ++j) {
			// .....
		}
		return sb.toString();
	}

@Dorian_Burihabwa
Hi Dorian!
Just to avoid opening new issue - please can you tell me am I the only one getting warning “Add a private constructor to hide the implicit public one.” on Java abstract class? I mean you cannot create an instance because it’s abstract class and no constructor is needed at all.
Is there some catch I don’t see?
Thanks!

Can you open a new thread about this? That would be helpful for people looking up the same problem in the future.

@Dorian_Burihabwa No problem at all. I thought you had enought of me so far :slight_smile:

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.