Bugzilla – Full Text Bug Listing
| Summary: | type.move() returns unclear exception "invalid destination" | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Michael Pradel <michael> | ||||||
| Component: | Core | Assignee: | Jay Arthanareeswaran <jarthana> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | minor | ||||||||
| Priority: | P3 | CC: | akiezun, Olivier_Thomann, srikanth_sankaran | ||||||
| Version: | 3.1.2 | ||||||||
| Target Milestone: | 3.6 M1 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Michael Pradel
I think the destination is invalid. Types cannot be moved to packages. The spec says nothing, however, about what the valid destinations for various elements are. So this bug is about the API. (there's also the problem of not informative error message, but I guess good API comments should be priority here) Jay, could you please clarify the documentation inside: org.eclipse.jdt.core.IJavaModel.move(IJavaElement[], IJavaElement[], IJavaElement[], String[], boolean, IProgressMonitor) Olivier, Part of the documentation about the INVALID_DESTINATION is available in CopyElementsOperation#verify(IJavaElement). I was wondering if we can add a @see reference to this method. This is a protected method, by the way. Created attachment 142112 [details]
Patch with updated documentation
Updated the documentation relevant to INVALID_DESTINATION exception for the following methods:
(i) org.eclipse.jdt.core.IJavaModel.move(IJavaElement[], IJavaElement[], IJavaElement[], String[], boolean, IProgressMonitor)
(ii) org.eclipse.jdt.core.IJavaModel.copy(IJavaElement[], IJavaElement[], IJavaElement[], String[], boolean, IProgressMonitor)
(In reply to comment #2) > Jay, could you please clarify the documentation inside: > org.eclipse.jdt.core.IJavaModel.move(IJavaElement[], IJavaElement[], > IJavaElement[], String[], boolean, IProgressMonitor) It would appear the concerned method is org.eclipse.jdt.core.ISourceManipulation.move(IJavaElement, IJavaElement, String, boolean, IProgressMonitor) and not org.eclipse.jdt.core.IJavaModel.move(IJavaElement[], IJavaElement[], IJavaElement[], String[], boolean, IProgressMonitor) though the latter could also benefit the additional clarification. Jay, while we are at this, could you also fix the typo ("destinaion") in the javadoc for org.eclipse.jdt.internal.core.CopyElementsOperation.verify(IJavaElement) Does it make sense to add the clarification to the javadoc for org.eclipse.jdt.core.IJavaModelStatusConstants.INVALID_DESTINATION and refer to it with a @see from ISourceManipulation, IJavaModel ? If we decide otherwise, does it make sense to add this clarification to both move and copy methods instead referring one from the other ? (In reply to comment #5) > Does it make sense to add the clarification to the javadoc for > org.eclipse.jdt.core.IJavaModelStatusConstants.INVALID_DESTINATION > and refer to it with a @see from ISourceManipulation, > IJavaModel ? Sounds good to me. > If we decide otherwise, does it make sense to add this clarification > to both move and copy methods instead referring one from the other ? We could reference the constant in both methods. One explication is enough as long as we have one. Created attachment 142347 [details]
Updated patch
Moved the documentation to IJavaModelStatusConstants and added @see references in copy/move methods of IJavaModel and ISourceManipulation. Also corrected the typo in CopyElementsOperation's javadoc.
Released Jay's clarifications to the documentation on HEAD for 3.6M1 Verified for 3.6M1 IMO, I think it would have been better to separate the additional explanations from the first comment sentence using the <p></p> html tags, e.g.: * Status constant indicating that a destination provided for a copy/move/rename operation * is invalid. * <p> * The destination for a package fragment... |