Patch for MoveRefactoring null error fix

[shivam71]

• The MoveRefactoring with the default visibility option as "Escalate" was not working .
• The issue seems to be from an uncaught null pointer exception arising somewhere else .
• It got fixed by minimal changes on NB .
  Netbeans PR link #7923

[sid-srini]

Thanks. Looks fine to me. 👍

Would you be able to confirm if there are any tests in the netbeans test cases that could cover this NPE flow? Or whether a small test case could be added as part of the upstream PR?

[Achal1607]

Can we add any tests around this on NetBeans side?

[shivam71]

Hmm I will check for the tests though the change seems almost harmless we are catching an exception which was not being caught where the later is more dangerous . I am not sure of the tests but this NPE is always caught because initially an empty HTML form gets created with none of the fields selected so this always happens by default every time the webview is created .

[sid-srini]

Hmm I will check for the tests though the change seems almost harmless we are catching an exception which was not being caught where the later is more dangerous.

Agreed. In this case the method contract is already set to return null in case of a bad URI.

However, it might be better if instead of letting the NullPointerException be thrown, we could test the method argument for null.

        private static Project getSelectedProject(NamedPath selectedProject) {
            String path = selectedProject == null ? null : selectedProject.getPath();
            if (path == null) return null;
            FileObject file;
            try {
                file = Utils.fromUri(path);
            } catch (MalformedURLException ex) {
                file = null;
            }
            return file == null ? null : FileOwnerQuery.getOwner(file);
        }

        private static FileObject getSelectedRoot(NamedPath selectedRoot) {
            String path = selectedRoot == null ? null : selectedRoot.getPath();
            if (path == null) return null;
            try {
                return Utils.fromUri(path);
            } catch (MalformedURLException ex) {
                return null;
            }
        }

This would be especially relevant if this is a common scenario like you mentioned, whereby a simple null check outside the try-catch could reduce the slight overhead of setting up the guards.

Thoughts?

[sid-srini]

Thanks @shivam71. Looks ok to me for the interim, until the upstream PR is reviewed and approved. At that point, please update the patch with any changes, as a fresh PR here.