Add ability to add custom context and optional plug-ins

[ilyavolodin]

I had exactly the same idea about implementing linter for javascript some time ago, I even started writing it (using esprima and per node iterators). But what I wanted to do differently from jshint/jslint is to add an ability to identify frameworks that are used in the code and run custom rules on them.

For example, identifying that user is using Backbone, and running custom rules that would require all models to have initialize function. Or checking for complex selectors if they are using jQuery.

[nzakas]

I'm not sure exactly what you're asking for in this issue. Can you explain
more?

On Sunday, July 14, 2013, Ilya Volodin wrote:

I had exactly the same idea about implementing linter for javascript some
time ago, I even started writing it (using esprima and per node iterators).
But what I wanted to do differently from jshint/jslint is to add an ability
to identify frameworks that are used in the code and run custom rules on
them.

For example, identifying that user is using Backbone, and running custom
rules that would require all models to have initialize function. Or
checking for complex selectors if they are using jQuery.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/36
.

---

Nicholas C. Zakas
@SlickNet

Author, Professional JavaScript for Web Developers
Buy it at Amazon.com:
http://www.amazon.com/Professional-JavaScript-Developers-Nicholas-Zakas/dp/1118026691/ref=sr_1_3

[ilyavolodin]

I would like to be able to set up context first, something that would look like this:

module.exports = function(context) {
    return {
        "MemberExpression": function(node) {
            if (node.object.name === "Backbone" && node.property.name === "Model") {
                context.setContext("Backbone");
            }
        }
    }
}

And then create rules that would apply only to the context "Backbone", for example.

[nzakas]

Ah, I see what you mean. I'll rephrase: you want to be able to set some shared information such that multiple rules can use it to do the right thing. Let me know think about how I would do this. I don't think setContext() is quite right, because you could be using Backbone, jQuery, Underscore, etc. all in one.

[ilyavolodin]

Right, that's better way of phrasing it. However, just to add on top of it a bit, I would not only like to share some information, I would also like to limit some of the rules to only be called when you are within the AST branch that is marked with certain information. Like if you are within the branch of code that starts with $, it would get marked with jQuery, and jQuery rules should be only executed within that branch. Once you are out that branch, no jQuery rules should apply anymore.

[nzakas]

Hmmm...okay, that complicates things a bit. I think what you're asking for
are context-aware rules, which is a bit out of scope for what I'm thinking
for version 1.0. That's not to say it can't be done, but there are a lot of
foundation pieces that have to be put in place first before I'd want to
tackle context-aware rules.

On Mon, Jul 15, 2013 at 1:17 PM, Ilya Volodin notifications@github.comwrote:

Right, that's better way of phrasing it. However, just to add on top of it
a bit, I would not only like to share some information, I would also like
to limit some of the rules to only be called when you are within the AST
branch that is marked with certain information. Like if you are within the
branch of code that starts with $, it would get marked with jQuery, and
jQuery rules should be only executed within that branch. Once you are out
that branch, no jQuery rules should apply anymore.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/36#issuecomment-21000469
.

---

Nicholas C. Zakas
@SlickNet

Author, Professional JavaScript for Web Developers
Buy it at Amazon.com:
http://www.amazon.com/Professional-JavaScript-Developers-Nicholas-Zakas/dp/1118026691/ref=sr_1_3

[ilyavolodin]

I think you will have to implement something like that even for the basic rules. It might not be context aware rules, but you will at least have to implement scope aware rules. Otherwise it will be very tough to create rules for something like that: http://jslinterrors.com/a-is-a-statement-label/ or http://jslinterrors.com/const-a-has-already-been-declared/ You can run your rule on the whole tree and search for all const declarations, but that means that in order to match even all the rules of the jshint/jslint, you will probably need to iterate over the AST 30-40 times, which isn't a scalable solution.

[nzakas]

Yes, scope awareness is something that will need to be built in (I'm
investigating escope), however, these rules will not be turned on or off
based on the scope.

On Mon, Jul 15, 2013 at 4:13 PM, Ilya Volodin notifications@github.comwrote:

I think you will have to implement something like that even for the basic
rules. It might not be context aware rules, but you will at least have to
implement scope aware rules. Otherwise it will be very tough to create
rules for something like that:
http://jslinterrors.com/a-is-a-statement-label/ or
http://jslinterrors.com/const-a-has-already-been-declared/ You can run
your rule on the whole tree and search for all const declarations, but that
means that in order to match even all the rules of the jshint/jslint, you
will probably need to iterate over the AST 30-40 times, which isn't a
scalable solution.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/36#issuecomment-21010712
.

---

Nicholas C. Zakas
@SlickNet

Author, Professional JavaScript for Web Developers
Buy it at Amazon.com:
http://www.amazon.com/Professional-JavaScript-Developers-Nicholas-Zakas/dp/1118026691/ref=sr_1_3

[ilyavolodin]

I think for the initial version, it would be fine to not turn off the context aware rules out of context, just give them awareness, just like you described and then short circuit them in the rule implementation. In the future version, it would be nice to only execute those rules when the context is set, to improve performance.

[nzakas]

Agree.

On Mon, Jul 15, 2013 at 4:36 PM, Ilya Volodin notifications@github.comwrote:

I think for the initial version, it would be fine to not turn off the
context aware rules out of context, just give them awareness, just like you
described and then short circuit them in the rule implementation. In the
future version, it would be nice to only execute those rules when the
context is set, to improve performance.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/36#issuecomment-21011701
.

---

Nicholas C. Zakas
@SlickNet

Author, Professional JavaScript for Web Developers
Buy it at Amazon.com:
http://www.amazon.com/Professional-JavaScript-Developers-Nicholas-Zakas/dp/1118026691/ref=sr_1_3

[jrfeenst]

Something like this could be done with an esquery based rules engine. I'd be interested in some other peoples thoughts on using it for rule definitions. With a selector based rules engine I can imagine rule sets only applying if a node matches a given selector (equivalent to a context).

[ilyavolodin]

I didn't know about esquery. @nzakas could we add this in? This would make it much easier to create complex rules. For example, there's a rule in jshint that prevents you from creating functions inside for loop.. Since it supports parent>child and descendant, events can be done in a much nicer way. I was actually planning on adding something like that to the events.

[nzakas]

I've chatted with @jrfeenst and we can definitely look at it down the road. I don't think either project is mature enough to really put a lot of work in integration at this point. I'd rather focus on getting to feature parity with JSHint/JSLint, do some stabilization, and then we can explore what the gaps and pain points are for people. That would be the time to start evaluating esquery seriously.

[michaelficarra]

@jrfeenst: @nzakas: @ilyavolodin: I created esdispatch for this. It will allow eslint rules to operate on esquery selectors rather than just node types. It still uses a single traversal, so should be no less efficient, will be backwards compatible with current rules (because node types are also valid queries), and will allow us to greatly simplify many of the eslint rules. Pull request coming soon.

(P.S. sorry for the double post, I accidentally posted this in the wrong issue the first time)

[ilyavolodin]

@michaelficarra Very nice! This would allow for much simpler rules. It will prevent us from doing things like inline comment parsing easily (like turning rules off/on based on the comments per line), but we can find some other way of doing that.