Nodeclipse / nodeclipse

Nodeclipse-1 : Eclipse plugin for Node.js, PhantomJS development (Nodeclipse core plugin); Maven and Gradle (with Android) plugins
https://nodeclipse.github.io/
158 stars 76 forks source link

EcmaScript 5 support (Express server.delete - Syntax error on token ".") #70

Open ghost opened 11 years ago

ghost commented 11 years ago

Syntax error raised with this code:

  server.delete('/accounts/:id/contact',function(req,res) { 
  });

where server was instantiated like this:

   var express = require('express');
   var server = express(); 

server.get() or server.post() are doing fine but server.delete() is raising a syntax error in Nodeclipse which is hardly correct to me since delete() is a valid route for express objects (see: http://expressjs.com/api.html#app.VERB - the app.routes section)

paulvi commented 11 years ago

The cause is that delete is reserved word.

This issues seems to be a part of a bigger one.

ghost commented 11 years ago

Actually the issue is even bigger than I first thought it was:

var obj = { delete: 1 };

would raise a reserved word error as well.

But this is wrong after EcmaScript 5: v8 (therefore nodejs) is now implementing ES5 and in ES5 reserved word applies to identifiers - not to properties - not anymore.

In other words - it seems to me that that nodeclipse code validators are still based on ES3 - which means they are now obsolete and require a fix urgently.

I could even volonteer myself to give it a shot if I only knew what to look for: any pointers or advices to help would be very much welcomed.

paulvi commented 11 years ago

Nodeclipse is using Eclipse standard JSDT (JavaScript Development Tools) that is not yet ES5 aware.

es5-support

Before raising issue on Eclipse Bugzilla https://bugs.eclipse.org/bugs/enter_bug.cgi?product=JSDT I would ask on Stackoverflow.com and do a bit research.

Frankly, I am not thinking in this direction, but I would follow. ES5 support is essential nowdays.

paulvi commented 11 years ago

Good link about ES5 http://ejohn.org/blog/ecmascript-5-strict-mode-json-and-more/

Wikipedia http://en.wikipedia.org/wiki/ECMAScript

From http://wiki.eclipse.org/JSDT says, that there was work on es4 support in Eclipse that was thrown away, because es4 specification was abandoned. That why Eclipse has dropdown list with only 1 value.

paulvi commented 11 years ago

Solution may be to disable JSDT validation and use JSHint Eclipse.

Here is options for JSHint http://www.jshint.com/docs/options/#strict, http://www.jshint.com/docs/options/#node

That solutions needs time to to try. @nodeleaf Could you?

Template to look at https://gist.github.com/haschek/2595796

ghost commented 11 years ago

@PaulVI

I shall but later today

ghost commented 11 years ago

About JSHint - this is an interesing feature in Nodeclipse but it's not shadowing JS syntax validators.

By example - setting JSHint 'strict' mode is NOT bringing ES5 validation up the stack since the ES5 issue reported above remains:

screen shot 2013-09-26 at 16 21 26

Now - from my testings I have mixed impressions about Eclipse JSDT validators.

First of all, when the JS validator is disabled, it seems to me that Nodeclipse is still performing 'some' JS code validation as I am getting exactly same errors in the test case shown above when it's enabled or disabled here:

screen shot 2013-09-26 at 16 20 19

Looks like only JS validator options shown in Preferences could be disabled / enabled but that it's still validating code under the hood even when all JS validator options were turned OFF in Preferences. I had a feeling this was related to syntax coloring as the delete property in tu object was colorized as a keyword when it shouldn't since it's not an identifier but a property (tu being the identifier here)...

I also found a similar issue was already reported to Eclipse Bugzilla in... 2008! And that it was never addressed since then: https://bugs.eclipse.org/bugs/show_bug.cgi?id=223131

To summarize - Eclipse JSDT is only supporting ES3 making its validators obsolete nowadays since ES3 was released in 1999 when ES6 should reach us this year of 2013.

This could be also a Nodeclipse issue since it's still validating code after the JS validators were disabled in Preferences.

JSHint might be an interesting workaround but - as it seems to me that Nodeclipse is still performing 'some' code validation even when the feature was turned OFF in Preferences (could anyone else confirm that?) - this workaround won't be operational as long as 'disabled' JS validators are still messing up JSHint warnings.

paulvi commented 11 years ago

Thanks, that was nice reading for me.

Please notice that what you find under JavaScript in Preferences is pure JSDT functionality. No work have been done in Nodeclipse for extending validation/warnings.

I have voted for https://bugs.eclipse.org/bugs/show_bug.cgi?id=223131 Please do the same to show Eclipse that we care.

paulvi commented 11 years ago

Nodeclipse 0.7 will switch to JSHint, where ES 5 is default, jshint/jshint#1312 -> TODO ask on mailing list https://groups.google.com/forum/#!forum/jshint

However delete will still be as keyword.

bcochran-shutterstock commented 11 years ago

Hey Paul, does that mean it'll still highlight AND show an error, or just highlight?

ghost commented 11 years ago

Since a new release seems to be on its way - maybe it's an opportunity to also address another minor issue related to Help > Check for Updates?

In short if you'll check for updates when no updates were available then it raises a "Problem Occured" alert: useless at best - the "No updates were found" information message was far enough to me :)

screen shot 2013-10-18 at 17 29 58

paulvi commented 11 years ago

This is not related to ES5. And as this is Eclipse platform behavior, bugs should be raise on https://bugs.eclipse.org/bugs/ for Eclipse Platform/Workbench.

On 18.10.2013 23:43, nodeleaf wrote:

Since a new release seems to be on its way - maybe it's an opportunity to also address another minor issue related to Help > Check for Updates?

In short if you'll check for updates when no updates were available then it raises a "Problem Occured" alert: useless at best - the "No updates were found" information message was far enough to me :)

screen shot 2013-10-18 at 17 29 58 https://f.cloud.github.com/assets/3315320/1361647/e9846884-380a-11e3-8f0d-e1e56da7d60c.png

— Reply to this email directly or view it on GitHub https://github.com/Nodeclipse/nodeclipse-1/issues/70#issuecomment-26606161.

ghost commented 11 years ago

@PaulVI

Indeed this is not ES5 issue but lack of ES5 support was also an eclipse issue if I've got it right so where the line is drawn is not so clear to me: I'm always glad to be teached what's node's and what's eclipse's in this projet but I have to confess I still don't see why it should make any difference to a nodeclipse genuine user...

About ES5 support - I am afraid JSHint is far below the mark about ES5 since it's only checking syntax against ES5 standard when in my understanding JSDT's still checks and colorizes syntax for ES3 at the same time.

Well - that's what the current release does...

How nodeclipse will be switched to JSHint in 0.70 to solve this issue is not obvious to me: does it means JSDT ES3 will be removed? Turned optionnal? In such a case where the syntax coloring will come from? How could it still take an ES5 property for an ES3 identifier if ES3 was disabled?

In my understanding, JSHint is only issuing warnings, it's best feature being its "strict" ES5 mode but you can't expect every node projects out there to be "strictified" - so I suppose JSHint strict mode will remain optionnal...

I know Prefection is the Ennemy of Good but there was another road to solve this issue I believe: what about getting ES3 module source from JSDT to iterate a new ES5 module ourselves? Or with them?

Doing it that way we would select a new ES5 module in nodeclipse preferences to get coherent color syntax and code checker for NodeJS - period.

And we would activate a JSHint option to get ES5 "strict" mode at will.

In short: it would be ES5 color syntax and code checker for all of us and for "strict" ES5 we'll have to turn JSHint "strict" mode ON.

That would be quite a nice achievement don't you think?

This is not related to ES5. And as this is Eclipse platform behavior, bugs should be raise on https://bugs.eclipse.org/bugs/ for Eclipse Platform/Workbench.

paulvi commented 11 years ago

Just try out 0.7 (clone, mvn package then install from local .zip)

@bcochran-shutterstock @nodeleaf Well, syntax highlight will stay the same. Error and warning will be JSHint configured as defined in project .jshintrc

Some related link http://wiki.eclipse.org/JSDT

http://wiki.eclipse.org/JSDTDevelopment - how to become a JSDT Committer

Interesting topic: Remove default JSDT in Luna [4.4] or Fix it? http://www.eclipse.org/forums/index.php/t/544073/

Well as for implementing ES5 for JSDT I even don't know if will be 1 month or 3 months And I don't have such order of magnitude spare time to devote for free to Nodeclipse. So I don't start, I just do a little research every time someone asks about it (actually thanks to @nodeleaf ).

Also making ES5 support outside of Eclipse JSDT will become wasted time once Eclipse team does it. So to start with, one should learn how JSDT is going and understand those people who drive the JSDT project now.

paulvi commented 10 years ago

For those interested, should follow https://bugs.eclipse.org/bugs/show_bug.cgi?id=223131 It was just marked as planed for 3.6, that is Eclipse 4.4 Luna release summer 2014

JSDT team now has 6 commiters.

paulvi commented 10 years ago

provided with wizards .jshintrc has line

"es5" : false, // Allow EcmaScript 5 syntax.

TODO check if it helps

paulvi commented 10 years ago

also asked on http://stackoverflow.com/questions/26768829/nodeclipse-enide-delete-keyword-issue