Avoid exception when tree parameter is null

[pedro-w]

This will happen when scanning the default: clause of a switch block.

This PR is intended to fix NETBEANS-4569

[pedro-w]

This is not the correct solution. I will re-work it.

[sdedic]

Would it be possible to also write a test for the change ?

[pedro-w]

There is one already actually, it's in org.netbeans.modules.java.source.save.FormatingTest in the "Java Source Base" module. At the moment it crashes and with my incorrect solution in this PR is doesn't crash but it fails(!). But I have got it to pass now.

[pedro-w]

I've re-done it now so the individual test passes. However there are some other tests in the same 'FormatingTest' file which fail (they did before and still do). I haven't studied each one thoroughly but for example one is the formatting for switch expressions (using arrow syntax). Here the syntax tree passed to the reformatter contains elements described as "(ERROR)" - I assume these are because I am using JDK 11 which doesn't have switch expressions. Is there a way to skip tests based on the JDK used (or another way to fix, or actually do you want them to stay as failures)? Thanks!

[sdedic]

@pedro-w not sure, the testsuite should be run (?) with nb-javac that should understand switch exprs even on JDK11 ... // cc: @jlahoda

[pedro-w]

It is possible, though, to run it without nb-javac and I believe it does make sense to test the code without as the users might be running NB without nb-javac.
What I found is that only one additional test was failing (the switch expression one) and that test did not have a version check in it, whereas the other tests in the same file did. I guess it was just missed out by accident?
So, I copy-pasted the check from another function in the file and that is commit 8add5a2
With this, all tests in the file pass for me.
I also added another clean up commit to the test file - 233ae40 - I limited this to just the things the IDE flagged up (obsolete API, unused functions etc.) I don't know if you want this as it doesn't add a huge amount of benefit to the code!
Please let me know,

[jlahoda]

I don't think CaseTree.getExpressions should return list which contains null. The bug is more likely in java/java.source.base/src/org/netbeans/modules/java/source/TreeShims.java, method getExpressions, in case of NoSuchMethodException, it returns:

return Collections.singletonList(node.getExpression());

But it should rather do something like:

return node.getExpression() != null ? Collections.singletonList(node.getExpression()) : Collections.emptyList();

As CaseTree.getExpression() historically returned null for default, but getExpressions() is intended to return empty lists instead.

[pedro-w]

@jlahoda is your recommendation to drop this PR altogether and modify TreeShims.java instead? That looks straightforward for me to do.

[pedro-w]

I've implemented it the way that @jlahoda suggested, please see master...pedro-w:ALT-NETBEANS-4569 . Is that a better fix than this one?
Let me know,

[mbien]

since NetBeans 13 uses javac 17 as new baseline, this fix is no longer needed as far as I see. Most of TreesShims is now a comment (which should be probably removed some time in future).

If any of the participants here think otherwise, feel free to reopen.