Bug 444120

Description Tobias Bertelsen 2014-09-15 09:50:49 EDT

Quick fix processors are asked for correction when there are no markers. This is because JavaCorrectionProcessor calls QuickFixProcessor#getCorrections on all quick fix processors when a there are no problem locations.

************************************
Steps to reproduce:
 1. Launch Eclipse in debug and set a break point in JavaCorrectionProcessor#collectCorrections
 2. Open a Java file, set the cursor on an empty line and press crtl+1
 3. In the debugger, note how QuickFixProcessor#getCorrections are called on all processors.

Alternative steps to reproduce (with user-visible output),
 1. Implement a simple QuickFixProcessors:
   A. Make sure that it 1. Only handle a simple or non-existing marker type (foo.marker)
   B: it should always provides the same fix, without checking the "locations" argument in QuickFixProcessor#getCorrections
 2. Launch an Eclipse with the QuickFixProcessor loaded.
 3. Open a Java file, set the cursor on an empty line and press crtl+1
 4. Note how the new suggestions are shown, even though the marker (foo.marker) is not present.

************************************
Solutions.
There are several solutions. I will recommend number 1 for efficiency.


*** Solution 1 ***

// CHANGE
public static IStatus collectCorrections(IInvocationContext context, IProblemLocation[] locations, Collection<IJavaCompletionProposal> proposals) {
  ContributedProcessorDescriptor[] processors= getCorrectionProcessors();
  SafeCorrectionCollector collector= new SafeCorrectionCollector(context, proposals);
  for (int i= 0; i < processors.length; i++) {
    ContributedProcessorDescriptor curr= processors[i];
    IProblemLocation[] handled= getHandledProblems(locations, curr);
    if (handled != null) {
      collector.setProblemLocations(handled);
      collector.process(curr);
    }
  }
  return collector.getStatus();
}
// TO
public static IStatus collectCorrections(IInvocationContext context, IProblemLocation[] locations, Collection<IJavaCompletionProposal> proposals) {
  ContributedProcessorDescriptor[] processors= getCorrectionProcessors();
  SafeCorrectionCollector collector= new SafeCorrectionCollector(context, proposals);
  if (locations == null || locations.length == 0) {
    return collector.getStatus();
  }
  for (int i= 0; i < processors.length; i++) {
    ContributedProcessorDescriptor curr= processors[i];
    IProblemLocation[] handled= getHandledProblems(locations, curr);
    if (handled != null) {
      collector.setProblemLocations(handled);
      collector.process(curr);
    }
  }
  return collector.getStatus();
}


*** Solution 2 ***

// CHANGE
public static IStatus collectCorrections(IInvocationContext context, IProblemLocation[] locations, Collection<IJavaCompletionProposal> proposals) {
  ContributedProcessorDescriptor[] processors= getCorrectionProcessors();
  SafeCorrectionCollector collector= new SafeCorrectionCollector(context, proposals);
  for (int i= 0; i < processors.length; i++) {
    ContributedProcessorDescriptor curr= processors[i];
    IProblemLocation[] handled= getHandledProblems(locations, curr);
    if (handled != null) {
      collector.setProblemLocations(handled);
      collector.process(curr);
    }
  }
  return collector.getStatus();
}
// TO
public static IStatus collectCorrections(IInvocationContext context, IProblemLocation[] locations, Collection<IJavaCompletionProposal> proposals) {
  ContributedProcessorDescriptor[] processors= getCorrectionProcessors();
  SafeCorrectionCollector collector= new SafeCorrectionCollector(context, proposals);
  for (int i= 0; i < processors.length; i++) {
    ContributedProcessorDescriptor curr= processors[i];
    IProblemLocation[] handled= getHandledProblems(locations, curr);
    if (handled != null && handled.length > 0) {
      collector.setProblemLocations(handled);
      collector.process(curr);
    }
  }
  return collector.getStatus();
}


*** Solution 3 ***

// In JavaCorrectionProcessor#getHandledProblems 
//CHANGE
  if (allHandled) {
    return locations;
  }
  if (res == null) {
    return null;
  }
  return res.toArray(new IProblemLocation[res.size()]);
}
// TO
  if (res == null) {
    return null;
  }
  if (allHandled) {
    return locations;
  }
  return res.toArray(new IProblemLocation[res.size()]);
}



************************************
Workaround for people who implement QuickFixProcessor.

Just check that locations is not empty, like this:

public IJavaCompletionProposal[] getCorrections(final IInvocationContext context, final IProblemLocation[] locations) throws CoreException {
  if (locations == null || locations.length == 0) {
    // Handle Eclipse bug, until it is fixed.
    return null;
  }
  // Your code...
}


Note that this is a workaround and not a fix. Clients implementing QuickFixProcessor's should be able to count on, that they will only be called for markers they have declared in plugin.xml they can handle.