Lint source code.

[cscott]

Update the version of jshint used by this package, and delint the code to get rid of ==, unused variables, and maps which inherit from Object.prototype (and thus contain spurious keys like hasOwnProperty).

[cscott]

Note that this PR also contains PR #180, along with an equivalent patch to PR #169.

[XhmikosR]

Can you make a PR with the patches I comment first and then we leave the rest after a rebase?

[XhmikosR]

On a side note @cscott, I'd be very interested if you make a PR to switch to Grunt instead of ant. That change was made for CSS Lint some time ago and we have cleaned a lot of bloat.

After that, personally, I'd like to switch to ESLint for both projects but one step at a time.

[cscott]

Ok. Separate patches split from this PR: #180, #188, #189.

[cscott]

@XhmikosR wrt c470821 -- there are a number of cases of this pattern in the code:

x == "literal"

where the value of x could be null or a Token or Expression type. String(x) === "literal" makes the cast clearer. Yes, the values could be null; I did consider changing the tests to something like:

(x && x.toString()) === "literal"

but it seemed like the String cast read a little bit better.

Strictly speaking, String(null) === "null" and String(undefined) === "undefined", but since we're comparing to a string literal, this works as expected. And it's roughly equivalent to what the == test is actually doing, it's just a little more explicit about it.

[XhmikosR]

@cscott: all right then. Please fetch and rebase and while at it, run the new jshint uglifyjs --compress --mangle. I wonder, why is jshint even included, but I guess with Grunt we won't have to worry anymore. We could also use it via node_modules locally in the meantime.

[cscott]

@XhmikosR could you look at #157, #180, and #186? In particular, #180 is included as part of this stack; it would be nicer to merge that separately and then rebase; and #157 conflicts in a trivial way with this patch stack, so it would be better to merge that first and then rebase this on top to resolve the conflict. #186 is just gravy.

And I'm assuming the uglifyjs --compress --mangle request is regarding lib/jshint.js (aka 0b774bf)?

[XhmikosR]

@cscott: I merged what I could review; the rest will probably have to wait for someone else.

And, yeah, I was talking about jshint.js.

[cscott]

@XhmikosR ok, I'm going to hang tight and see if we can get #157 and #180 merged, then, before I rebase. If things seem to stall, I can factor out #180 from this patch (adding an explicit /* jshint evil:true */), but that would require making a new PR for #157 and rebasing #180, so I'd like to avoid that if I can.

[cscott]

Rebased on top of PR #180.

[XhmikosR]

I still believe you should minify jshint.js. It's so many bloat for the repo.

[cscott]

@XhmikosR I've minified jshint.js and updated this PR.

[XhmikosR]

How about we switch to ESLint and getting rid of ant? I just find JSHint way less powerful these days.

Also, I'm not sure about the __proto__ changes. And the Properly parse CSS strings with escape sequences. patch shouldn't be part of this.

[cscott]

The __proto__ changes prevent us accepting (for example) hasOwnProperty as a valid CSS property name. And I agree that PR #180 shouldn't be a part, but it causes conflicts (and causes jshint to fail) if not included. The idea is to merge PR #180 before this one.

I'm all for switching to eslint/jscs/whatever eventually, but please merge this first. A bunch of later patches depend on it, and I'm spending too much time rebasing and resolving conflicts.

[XhmikosR]

As long as you are around to take care of any potential regressions, I'd like to move forward to better build system.

[cscott]

@XhmikosR thanks! I;ve got one more patch set for you to look at (#203) and I'll start work on de-ant-ifying the build process today. I'll be around post-release to handle any issues that arise.