Bug 129884

Created attachment 58225 [details]
Diff

Y01 It would imho be better to have static method asResult for converting the results (P4), this kind of protection - 
e.g. protected method with no needed instance context in a publicly subclassable class reminds me 
TextAction.getLastComponent() - can be easily workarounded

Y02 You might use instanceof check in the asResult method to return the argument directly if it is already of the 
right type.

Thanks Jarda.
Y1: I m not very friend of static support classes. The instance context seems to me as the best solution.
This method is needed only by proxy SFBQImpls - currently there are two, if we make this method public I am afraid someone will use
it for something completely else.
Y2: Right, I agree and I will add it. 

Looks good to me. Just a small nitpick, not meant as an integration blocker. I'm not a big fan of methods that allow
null as a valid parameter value and begin with:

if (parameter == null) { return null; }

Yes, I understand this. Originally I hadn't null as a valid argument. But allowing null asa valid argument simplified ProjectSFBQ code from:

Result r = delegate.findSourceRoot (url);
if (r == null) {
    return null;
}
return asResult (r);

to:
return asResult (delegate.findSourceRoot (url));

looks good to me.

I am going to integrate it.

Re. desc6:

Result r = delegate.findSourceRoot (url);
return r != null ? asResult(r) : null;

is a bit worse than your way, but OTOH it makes the API more clear.

ab: OK, I have no strong opinion on this. I will change this - null will not be allowed.

Fixed in:
b5134017b065
33d5c280f4be