Updated JSLint to the latest version and fixed all the new errors

[TomMalbran]

This might be a big request, but all the changes done are very simple.

1. Updated the JSlint directives to: /*jslint vars: true, plusplus: true, devel: true, nomen: true, todo: true, unparam: true, indent: 4, maxerr: 50 */ on every file and added regexp: true, forin: true, browser: true, node: true after the maxerror directive when required in the file. The new things here are todo: true to allow TODO comments and unparam: true to fix issues were one of the first parameter isn't used, but one of the last one is, for example, with event listeners. If JSLint would get updated to support this things, we could probably remove this directive.
2. The rest of the fixes are:
   a. Removed unused variables and some functions.
   b. Fixed the use of else after a return or break.
   c. Empty functions aren't allowed, but adding return undefined; keeps the same idea of an empty function.
   d. Removed assignments inside the while guard.
   e. Moved variables declared inside a loop outside of it.
   f. Removed parenthesis around variables.
   g. Removed some unused parameters.
   h. Removed empty callbacks when using brackets.fs.unlink since is no optional.
   i. Plus other minor changes.

I tested and everything seems to work fine. All tests are passing, with the exception of the first JSLint test which I couldn't fix.

There are still errors in the files: src/LiveDevelopment/Agents/RemoteFunctions.js, test/SpecRunner.js and test/spec/LiveDevelopment-test.js. Where an instance of a class is created and assigned to a variable, but then never used. Removing the variable would mean that the instance is created but not stored, which also gives an error. So wasn't sure how to fix those errors.

The new warnings for unused variables and using spaces instead of tabs are great.

[TomMalbran]

@peterflynn Let me know when you want me to fix the conflict issues. Since this request touches almost every file, most merges might create errors, so better to fix them when required.

[njx]

I nominated this PR for us to look at relatively soon.

[TomMalbran]

Thanks. I'll try to update it this weekend and I could even update the jslint submodule.

[zaggino]

@TomMalbran you may want to consider adding JSLint to Grunt so the PRs are checked for JSLint errors automatically - 9dde6a2

[peterflynn]

@TomMalbran We talked about this a bit today and have begun thinking that maybe we should make the switch over to JSHint instead. Some of the newer JSLint requirements are kind of frustrating to deal with, so the configurability of JSHint would be an advantage (compared to JSHint, where our code already cleanly passes the latest version -- we know that since it's part of the Travis build). JSHint also seems to be what more users would prefer we bundle by default.

What do you think?

[TomMalbran]

@peterflynn Sure. That makes sense. Anyway we want to do it is fine, but we should update the linters to check for more errors that we might want to avoid.

[TomMalbran]

I'll close this PR then. It is actually so old, that it might be hard to update it for the latest code.

If we are still going to ship with JSLint as a default extension, we should still update it, but that would be a lot easier to do.

[peterflynn]

@TomMalbran We created a card here: https://trello.com/c/SQhvGHri/498-jshint-linting-support. One of the tasks is to release a JSLint extension that's actually up to date. We wouldn't ship it in core alongside JSHint until potentially later, when we introduce UI/preferences for enabling & disabling installed linters. (If there were strong pushback on having the out-of-box Brackets experience switch to JSHint, then I suppose we'd do the opposite -- keep JSLint in core, update it, but have Brackets contributors all install a JSHint extension).

Re taking advantage of newer JSHint/Lint features to check for more errors -- do you have any suggestions about what newer checks to enable? I haven't kept up with the new settings very well...

[TomMalbran]

@peterflynn Once we have a UI/preferences for enabling & disabling installed linters (wasn't someone working on that?) it would be really easy to just provide both linters and make them disabled by default but have jshint enabled by default for brackets as a project preference.

One of the nice new checks for jslint was to check for unused variables and variables declarations in loops, which can be activated in jshint.

[peterflynn]

I think @busykai was planning to take a stab at a preferences-layer implementation at some point, and then we'd add UI for it at a later point?

But I assume we'd always want Brackets out of the box to have one of the linters enabled by default, even if we ship with both (@TomMalbran the way I read what you wrote above, it sounds like you were thinking we'd have both disabled and there'd be no linting until the user picks one?)

Re the new unused vars warning -- is that decoupled from the unused args warning? If not, we might not be able to use it because the unused args warning is problematic for stub methods (like those on superclasses), which in turn complicates writing the correct JSDocs...

[TomMalbran]

Anyway we do it, if there are preferences to enable/disable linters, then those can be changed for Brackets so that we use the linter that we want.

I found that /** jslint unused:vars */ will check for unused variables but not unused parameters.

[peterflynn]

Oh cool, good to know. I'll mention that on the card.

[marcelgerber]

If we decide to use JSHint, we should probably consider using a .jshintrc (or make Brackets preferences work) instead of those comments on each file.
A good example is brackets-gits'.

[busykai]

@SAplayer we already run jshint as part of CI checks and there's [.jshintrc]|(https://github.com/adobe/brackets/blob/master/.jshintrc) that is used to do that. Is that what you meant?

@peterflynn yes, #7362 is a functioning PR to address configurability of linters, but it was pointed out that language layer (#6558) is a better way to accomplish that which is on my todo list.

[marcelgerber]

@busykai Oh, didn't know about that. It's a bit odd that we run JSLint client-side and JSHint server-side (it could be that there are errors on just one site).