Summary: | Fup on bug 102305 - align all ICompilationUnit#getContents implementations on a 'never null' behavior | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Maxime Daniel <maxime_daniel> | ||||||||||
Component: | Core | Assignee: | Frederic Fusier <frederic_fusier> | ||||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||||
Severity: | normal | ||||||||||||
Priority: | P3 | CC: | frederic_fusier, jerome_lanneluc, Olivier_Thomann, philippe_mulet | ||||||||||
Version: | 3.1.1 | ||||||||||||
Target Milestone: | 3.3 M4 | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Linux | ||||||||||||
Whiteboard: | |||||||||||||
Attachments: |
|
Description
Maxime Daniel
2005-09-27 05:43:26 EDT
Potential breakages: - org.eclipse.jdt.internal.core.CompilationUnit: anonymous class into buildStructure(); - org.eclipse.jdt.internal.core.search.matching.PossibleMatch. No test case of our base gets the said methods to really return null. Some calling points merely try to access .length on the result without testing for null. Will hence return CharOperation.NO_CHAR in the two identified instances where we would return null. Created attachment 53801 [details]
Suggested fix
Released for 3.3 M4. Also amended the javadoc of ICompilationUnit#getContents to warn implementors. Forgot to change the state... Shouldn't org.eclipse.jdt.internal.core.ClassFileWorkingCopy also change its getContents() implementation ? It can potentially return null. Created attachment 55428 [details]
Proposed fix
Frederic, Would you take care of releasing it? Created attachment 55468 [details]
Other proposed fix
Looking at implementation in Buffer, getCharacters() returns always a non-null array as soon as the buffer is opened. getContents() on ClassFileWorkingCopy calls getBuffer() and this method returns an opened buffer or null, I think it's better to test whether the buffer is null and return an empty array in this case...
Jerome, can you double-check? Thanks Comment on attachment 55468 [details]
Other proposed fix
Jerome rightly highlighted the fact that IBuffer might be implemented by clients and so we has to rely on spec for getCharacters() method return. As it may be null, we also need to return empty char array as previous Olivier's patch did.
I'll put a new one merging the two changes...
Created attachment 55510 [details]
New proposed patch
Merge patches + fix same potential issue (IBuffer.getCharacters() may return null) in initial suggested patch...
All JDT/Core tests pass with this patch. Released for 3.3 M4 in HEAD. We will not contribute again for the additional patch. However, as there's no specific test case for this bug, just consider it as verified for 3.3 M4 using HEAD stream... |