* Set up Grunt via package.json (run `npm install` in mediawiki-core)
* Add grunt task for node-jscs (NEW)
This is a style checker (as opposed to jshint, which is for
code quality). There are a few small style-related things that
JSHint can check (camelcase, onevar etc.) but those are being
deprecated in JSHint v3, people should use more sophisticated
tools like node-jscs for this instead. As such this commit
removes moves of those options from our jshint configuration.
See: http://jshint.com/blog/jshint-3-plans/
* Add grunt task for jshint
This will use the same jshint configuration as we use on
Jenkins but makes it easier to run locally from the command
line by being part of the same `$ grunt test` task list.
Also:
* Changed jshintignore to use "dir/**"" instead of "/dir" or "dir"
because the latter is not compatible with Grunt for some reason.
See also https://github.com/gruntjs/grunt-contrib-jshint/issues/126.
Examples of coding style rules that were being violated that we
can now catch in node-jscs:
* Operator "," should stick to preceding expression
* Missing space after "if" keyword
* Multiple line break
* Empty block (in jquery.textSelection and mediawiki.language)
Bug: 54218
Change-Id: Ib9d7eab9f0d5cea5fb33f0b9f82e5554897fdfe0
There were still some files not passing jshint, and for files
that did, we managed to screw 'em up again.
Added more explicit settings in .jshintrc to avoid relying on a
kind of default somewhere. There are too many default-factors:
closest(.jshintrc), ~/.jshintrc, IDE/editor, node-jshint..
Added node_modules/ and extensions/ to .jshintignore.
Previously "$ jshint ." would recurse over all kinds of
unrelated code. Extensions should have their own jshint
dotfiles. When linting from Jenkins this won't be a problem as
those will be ran per repo (so when linting core it will skip extensions and when in an extension dir, the core dotfiles
don't apply as they'll be out of scope).
Some of our modules are really messy and should be refactored
to be less spaghetti-ish and have more descriptive variable
names and more manageable function-level complexity.
But for this commit, I'm keeping it as much as-is as possible,
because its hard/large enough to review as it is.
A few errors are cited below to give an impression of the kind
of warnings I addressed (for cases where the diff isn't
so obvious):
* jquery.hidpi.js: line 110, col 15, Empty block.
* mediawiki.jqueryMsg.js: line 34, col 17, Too many var statements.
* mediawiki.jqueryMsg.js: line 145, col 33, Strings must use singlequote.
* mediawiki.action.edit.js: line 74, col 73, 'selectText' is defined but never used.
* startup.js: line 19, col 22, 'isCompatible' is defined but never used.
* jquery.byteLength.test.js: line 26, col 9, Identifier 'U_00A2' is not in camel case.
* jquery.localize.test.js: line 63, col 29, 'attrs' is defined but never used.
* mediawiki.cldr.test.js: line 72, col 27, 'mw' is not defined.
* mediawiki.jscompat.test.js: line 6, col 17, Strings must use singlequote.
* mediawiki.api.parse.test.js: line 9, col 17, Strings must use singlequote.
* mediawiki.api.parse.test.js: line 7, col 15, 'mw' is not defined.
* mediawiki.api.parse.test.js: line 14, col 24, '$' is not defined.
* mediawiki.api.test.js: line 43, col 28, 'data' is defined but never used.
Other fixes:
* Add closures fix implied global errors ($, mw and more),
and prevents local variables from becoming globals.
* Avoid jQ magic map arg etc. (per code conventions).
* Fix incorrect usage of jQuery methods (prop instead of attr,
prop false instead of removeProp, ..).
* Unquote keys in object literals for consistency, and
enforce single quotes (no magic quotes in javascript, as much
as we might think "\n" and '/n' are really exactly the same).
Chose single quotes over double quotes since that's what most
code already had and what conventions seemed to prefer
(both the old generic ones and the new per-lang ones since
2011/2012).
* Enforce camelCase variable names with jshint per code
conventions.
* $foo.on('x', fn).trigger('x') -> $foo.on('x', fn); fn()
(No event simulation overhead, unless intended of course)
* Incorrect indentation (ignore whitespace in the diff!).
* Avoid proprietary selectors like ':first' when .eq(0)
afterwards is just as possible (significantly faster in
jQuery due to mostly avoiding the Sizzle engine and going
native in modern browsers).
* When at it, convert deprecated jQuery methods to new ones.
Mostly just .delegate(sel, type, fn) -> .on(type, sel, fn).
* Addressed whitespace here and there.
Interesting:
* mediawiki.js: local function "compare" wasn't used anymore
(hasn't been in a while!) removed per jshint warning.
* mediawiki.special.recentchanges.js: Was a mess, only a few
lines of code, rewritten.
Pfew, let's hope it's the last one before we lint from Jenkins!
Change-Id: I23ad60a1d804c542d9b91454aaa20ce7be4ff289
* .jshintrc
- Update properties to reflect our conventions more
* Re-pass resources/jquery/*
* Pass resources/mediawiki/*
- Trailing whitespace
- Whitelist the one usage of document.write with
a local /*jshint evil:true */ in the function that
we allow to use it.
- Get rid of dangling _ in var names and undescriptive
instances of '_this'.
- More code conventions
* Add a few documentation comments while at it
Change-Id: Ic4f2b5d473a440667a40e4d5f12f40877386b02f
Changed written by Timo and reviewed by Hashar. This should be harmless.
To enable the feature:
$wgEnableJavaScriptTest = true;
Then head to:
[[Special:JavaScriptTest/qunit]]
James D. Forrester
Timo Tijhof
Fomafix
Timo Tijhof
Antoine Musso