Community
Participate
Working Groups
JDT/Core uses source ranges to indicate the range of elements in source. Since most of the JDT/UI can deal with source range from IJavaElements and DOM/AST we implemented our own source range with implements ISoureRange to not be forced to copy source ranges retrieved from JDT/Core. Would it be possible to allow implementation of ISourceRange ?
A class SourceRange that we can extend would do the trick as well. This might be saver since you can still change the interface then.
Deferring post 3.1
*** Bug 67140 has been marked as a duplicate of this bug. ***
JDT/UI still implements this interface (illegally). JDT/Core should provide a public class SourceRange and add a helper method to test for bug 18311 (like we do in our implementation: public static boolean isAvailable(ISourceRange range) { return range != null && range.getOffset() != -1; } )
Could you please fix this? It's one of the last API problems we have in JDT/UI.
Reopen for consideration. Jerome - what do you think ?
Jerome - what do you think ?
That makes sense.
Not for 3.5
This should be trivial to provide. We only need the existing org.eclipse.jdt.internal.core.SourceRange as API (OK if not subclassable) plus the helper mentioned in comment 4, though I'd prefer to have a non-static isValid() (or similar) method.
> [..] though I'd prefer to have a non-static isValid() (or similar) method. Unfortunately, the method has to be static, since invalid ranges are either null or [-1, 0] (bug 18311). You should also mention the helper method in the Javadoc of ISourceRange (to make clients aware that e.g. ISourceRange#getOffset() can return -1).
Srikanth, can you please move SourceRange to be API (org.eclipse.jdt.core.model.SourceRange)? It should be tagged with @noextend and contain the static method mentioned in comment 4
(In reply to comment #12) > Srikanth, can you please move SourceRange to be API > (org.eclipse.jdt.core.model.SourceRange)? > It should be tagged with @noextend and contain the static method mentioned in > comment 4 comment #1 and comment#3 mention a subclassable API SourceRange, while comment #10 says it is OK if not subclassable. So does comment #10, by virtue of being the latest (and there not being any debate around it) take precedence ?
Yes, at a minimum we *could* live with that but of course it would serve us much more if we were allowed to subclass (you could make your methods private and the fields protected) because otherwise we have to move the code we have in our copy into some helper and redirect the calls. So, unless you have strong objections we'd prefer to be able to subclass it.
FYI: this is our current copy: org.eclipse.jdt.internal.corext.SourceRange
Created attachment 139967 [details] Proposed fix and tests The attached patch : - Moves SourceRange to be API under the package hierarchy structure org.eclipse.jdt.core.model.SourceRange - This is tagged with @since 3.6. - SourceRange is subclassable. - The static helper method isAvailable(ISourceRange range) has been added per comment #4 - Javadoc on ISourceRange#getOffset mentions that -1 may be returned. - Tests have been added to verify the availbility and correctness of the API.
I looked at the patch from an API/client standpoint. It works for us and all our tests pass with our adjusted client code. Here some things to improve: - do not introduce 'org.eclipse.jdt.core.model' as this is confusing since all other model related types are not in this package and adding it for just one class is overkill ==> put it into 'org.eclipse.jdt.core' - make the fields private (we don't need to access them and it makes the class safer if it can't be modified after creation) - the formatting of SourceRange seems odd: the method declarations start at column 1 instead of being indented - there is no Javadoc comment for the type - the Javadoc comment for isAvailable(...) should mention why this method is needed at all (link to bug 130161) - copyright is wrong, it should be 2009 or otherwise mention in the Javadoc that it existed before but in another package
Created attachment 140936 [details] Improved patch
Released in HEAD for 3.6M1
Verified in HEAD and adopted the API in JDT Text and JDT UI.
