Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 490828

Summary:	[eslint] Annotation for 'not loaded plugin / env directive' is in odd place
Product:	[ECD] Orion	Reporter:	Michael Rennie <Michael_Rennie>
Component:	JS Tools	Assignee:	Michael Rennie <Michael_Rennie>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	curtis.windatt.public
Version:	12.0
Target Milestone:	12.0
Hardware:	PC
OS:	Mac OS X
Whiteboard:

Description Michael Rennie

2016-03-31 17:10:11 EDT

The new linting rule that tells you when you have specified an eslint-env entry that references a not-loaded plugin should tag the entry in question (rather than just be stuck at the top of the file), then the message would not have to mention it by name.

So we could go from:

"To get support for the 'express' environment, the 'express' plugin must be started. 'express' has not been set in the plugins entry of your .tern-project file."

to something a bit more consumable like:

"This environment references a Tern plugin that is not currently loaded."

The quickfix text could stay the same, or be updated to "Update .tern-project file".

We can use Finder.findDirective(ast, envname) to get the comment node and then find the name in the comment from there.

Comment 1 Curtis Windatt

2016-03-31 17:24:07 EDT

I was already looking at doing this, but nice to know there is a function already to find the right comment. I'm not going to remove the plugin name from the text: a) The plugin name doesn't always match the env directive and b) The quickfix uses the name from the annotation title for the fix (same as eqeqeq quickfix).

Comment 2 Curtis Windatt

2016-03-31 17:41:22 EDT

findDirective only gets you the entire eslint-env comment.  You also can't pass a comment to report as the node.  This will require writing a new Finder.findESLintEnv function with tests and then reporting a specific start/end range + line/col the same way unnecessary-nls rule does.

Comment 3 Michael Rennie

2016-03-31 18:18:49 EDT

(In reply to Curtis Windatt from comment #2)
>  This will require writing a new
> Finder.findESLintEnv function with tests and then reporting a specific
> start/end range + line/col the same way unnecessary-nls rule does.

Thats overkill. Just use Finder.findDirective to find the comment node (as I mentioned) then look in comment.value for the name you have reported. Use the range array from the comment node to compute the range for the word to mark and create a node to pass in to context.report.

Have a look at the no-jslint rule for an example.

Comment 4 Michael Rennie

2016-04-01 13:24:09 EDT

Fixed in:

http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=ce903e4dcf36fcdcea1d091306b140c14a793f64

The fix includes:

1. exported the optional plugins mapping from the tern context
2. removed the unnecessary tern request, which was also causing a timing problem where the linting would only show up 25% of the time
3. stopped re-exporting the entire tern server into the eslint context
4. adds missing mapping of 'mongo' to 'mongodb' - eslint provides mongo, we should detect this and be able to load the mongodb plugin as necessary
5. marks the offending name in the directive correctly
6. fixes the quickfix to not rely on regexing the annotation text (which also would have relied on the warning being in english)
7. updated the tests to check for the new 'EnvNode' instead of 'Program'

I did not change the message, I opened bug 490932 for that.

All tests pass.