Bug 317281

Comment 1 Jacek Pospychala 2010-06-18 05:59:36 EDT

Created attachment 172202 [details]
screenshot

Comment 2 Jacek Pospychala 2010-06-18 06:01:05 EDT

Created attachment 172203 [details]
patch

remove the $->'.' conversion

Comment 3 Chris Jaun 2010-06-22 13:13:55 EDT

Let's try and get this in 3.2.1.

Jacek...do you know if there are any cases where it may be valid to change $ to .?

Under what circumstances to signatures get generated with $? Do you think that is that just something left over from JDT?

Comment 4 Jacek Pospychala 2010-06-23 05:18:07 EDT

I think it's left over from JDT. In JavaScript, thanks to their hashmap-ish nature, you can use pretty much anything as a field name. E.g.

org.jQuery = function () {}; // valid
org['jQuery'](); // valid, invokes function defined above
org['jQ$&*/\@ery'] = function () {}; // valid
org.jQ$&*/\@ery(); // invalid, parser will complain
org['jQ$&*/\@ery'](); // valid, parser won't complain :-)

At least Prototype and jQuery make use of such strange names, i.e. $W, $A, $$, $H, jQuery selectors: >, ., *=, etc.


However, there may be more left overs in JSDT, that also specially treat $. So far I didn't noticed any, and I'm running with patch applied.

Comment 5 Chris Jaun 2010-06-29 09:54:52 EDT

Patch checked in.

Comment 6 Nitin Dahyabhai 2010-06-29 10:14:56 EDT

Thanks, Jacek.

Comment 7 Chris Jaun 2010-07-08 11:13:51 EDT

This patch is getting backed out of 3.2.1.

Will re-target this bug to 3.2.2.

I discovered that it break's content assist for namespaced types.

Comment 8 Jacek Pospychala 2010-07-08 11:21:45 EDT

any example where it breaks?

Comment 9 Chris Jaun 2010-07-09 09:41:54 EDT

Let me work up an example for you Jacek. If you can fix the patch by next Wednesday we may still be able to get it in 3.2.1, but it will have to be very well tested.

Comment 10 Chris Jaun 2010-07-12 13:53:24 EDT

Created attachment 174074 [details]
Updated Patch

Comment 11 Chris Jaun 2010-07-12 13:55:31 EDT

Updated the patch to make sure signatures don't get generated with $ in them.

Looks like a mistake made in BUG305694 allowed that to happen for namespaced types.

Nitin, marked you for review. Not sure about putting this in 3.2.1 or not. Need to test it a lot.

Comment 12 Chris Jaun 2010-07-12 15:10:31 EDT

Jacek, I marked you for review as well.

Comment 13 Chris Jaun 2010-07-12 15:11:51 EDT

Here is some example code:

var obj = {};
obj.form = {};
obj.form.CheckBox = function() {};
obj.form.CheckBox.prototype = new Object();

Put that in one file and attempt content assist in another file. Type to filter down the proposals.

Comment 14 Jacek Pospychala 2010-07-13 03:15:53 EDT

hi, patch looks good to me, altough searching jsdt.core for "'$'" gives another 90 results, out of which at least few are "replace('.', '$')" or "replace('$', '.')". Not sure if they're ever reached though

Comment 15 Chris Jaun 2010-07-13 09:37:57 EDT

Yeah, in my testing I did not see any other problems and the junits pass.

I think this specific case was causing a problem due to the error I made in BUG305694.

Comment 16 Chris Jaun 2010-07-13 15:51:16 EDT

After deciding we aren't sure exactly all the possible uses of $ in type signatures Nitin and I decided to move this one out to 3.2.2.

Comment 17 Nitin Dahyabhai 2011-01-26 22:40:52 EST

Possibly something we can fix once bug 306958 is addressed?

Comment 18 Chris Jaun 2011-04-27 17:54:31 EDT

Fixed this as part of https://bugs.eclipse.org/bugs/show_bug.cgi?id=343691.