Community
Participate
Working Groups
Build Identifier: M20100909-0800 A source folder that uses an exclamation point as the first character cannot be deleted from a Java project. Reproducible: Always Steps to Reproduce: 1. Create a new Java project 2. Create a new source folder named '!', a single exclamation point 3. Delete the source folder named '!'
Created attachment 184525 [details] Errors logged as a result of bug
Reproduced in HEAD. The bug occurs when a name starting with '!' is used for source folders. Looks like the '!' is considered as a token instead of part of the element name, which results in the error. Will see what needs to be done here.
Whenever a package fragment root name (source folder in this case) starts with any of the memento token names, an escape character is added. Apart from '!', this can also be seen for ^, < etc. However, only '!' causes any problem because of the check we have in JavaProject#getHandleFromMemento(). The calls to escapeMementoName from several places including PackageFragmentRoot.getHandleMemento() add the escape character to the element's name if they begin with any of these special characters. I doubt this is what we want. Investigating further.
(In reply to comment #3) > Whenever a package fragment root name (source folder in this case) starts with > any of the memento token names, an escape character is added. Apart from '!', > this can also be seen for ^, < etc. However, only '!' causes any problem > because of the check we have in JavaProject#getHandleFromMemento(). > > The calls to escapeMementoName from several places including > PackageFragmentRoot.getHandleMemento() add the escape character to the > element's name if they begin with any of these special characters. I doubt this > is what we want. Investigating further. That's a wrong understanding of the escape characters in the context of memento. The actual cause is MementoTokenizer#nextToken strips out the escape character. Hence, JavaProject#getHandleFromMemento is unable to determine what the '!' represents (a token or the element name)
Created attachment 184844 [details] Proposed patch Only the fix in the patch. Tests to be added.
Created attachment 184880 [details] Patch with test Test added in MementoTests#testBug331821()
Srikanth, can you review this patch, please?
Ayush, can I request you to review this please ? (I am tied up this week and the next.) Thanks.
It looks like the fix changes the semantics in org.eclipse.jdt.internal.core.JavaProject.getHandleFromMemento(String, MementoTokenizer, WorkingCopyOwner), where earlier we were consciously trying to break only in cases where the token starts with '!' or '<', we now only break when the token itself is '!' or '<'. This would be ok if this is what we'd intended to do, but i'm not sure. Also, the line just below the while loop if (token != null && token.charAt(0) == JEM_PACKAGEFRAGMENT) also needs to be written as if (token != null && token == MementoTokenizer.PACKAGEFRAGMENT)
(In reply to comment #9) > It looks like the fix changes the semantics in > org.eclipse.jdt.internal.core.JavaProject.getHandleFromMemento(String,... The code is only trying to extract the package fragment root from the supplied string. Apparently this particular implementation of getHandleFromMemento is not interested in the occurrence count. I have changed only the condition that determine whether the token is any of the two concerned. The rest of the code remains. > Also, the line just below the while loop > if (token != null && token.charAt(0) == JEM_PACKAGEFRAGMENT)... This could have been a problem too. But I just checked and it appears neither eclipse nor Windows allow '<' (represents JEM_PACKAGEFRAGMENT) in folder names.
(In reply to comment #10) > [..] > > Also, the line just below the while loop > > if (token != null && token.charAt(0) == JEM_PACKAGEFRAGMENT)... > > This could have been a problem too. But I just checked and it appears neither > eclipse nor Windows allow '<' (represents JEM_PACKAGEFRAGMENT) in folder names. If it is not allowed, why are we checking for it at two places in this particular method? Shouldn't the two checks be removed? Or if not, they should both be token == MementoTokenizer.PACKAGEFRAGMENT ?
(In reply to comment #11) > If it is not allowed, why are we checking for it at two places in this > particular method? Shouldn't the two checks be removed? Or if not, they should > both be token == MementoTokenizer.PACKAGEFRAGMENT ? What I meant was, the character '<' cannot be part of a folder name. However, the memento we are dealing with might have the PACKAGEFRAGMENT delimiter, which is represented by '<' and with an escape character. For e.g. the memento could be something like "=TestProject/!<com.abc" and since '<' is not allowed as part of the folder name (and package fragment name), it is considered as the token delimiter By the way, I happened to look at PackageFragmentRoot.getHandleFromMemento(String, MementoTokenizer, WorkingCopyOwner), which might have to be fixed as well.
(In reply to comment #12) > By the way, I happened to look at > PackageFragmentRoot.getHandleFromMemento(String, MementoTokenizer, > WorkingCopyOwner), which might have to be fixed as well. We don't allow '!' and other delimiters in package names. So, this is alright.
(In reply to comment #13) > (In reply to comment #12) > > By the way, I happened to look at > > PackageFragmentRoot.getHandleFromMemento(String, MementoTokenizer, > > WorkingCopyOwner), which might have to be fixed as well. > > We don't allow '!' and other delimiters in package names. So, this is alright. Ok. Good to go then!
Released in HEAD for 3.7M5.
Verified for 3.7M5 using build I20110124-1800
