Item12136: scripts don't pass jslint - should it be a checkin gate?

I've been prototyping something new, which caused me to add some code at the end of Configure's scripts.js.

Debugging, I found that the existing code doesn't pass jslint. This doesn't seem like a good thing. 17 warnings. We do perltidy on the perl code as a gate to checkins; should we run a javascript validator on the js? (Doesn't have to be this one.)

FYI, here's the current list of "errors". I know, one can quibble about some, but that's the same with perltidy too

JavaScript Lint 0.3.0 (JavaScript-C 1.5 2004-09-24)
Developed by Matthias Miller (http://www.JavaScriptLint.com)

scripts.js
O:\scripts.js(218): lint warning: parseInt missing radix parameter
        inLink.defaultValue = 0+parseInt(unescape(inLink.title)).toString(8);
...............................................................^

O:\scripts.js(257): lint warning: comparisons against null, 0, true, false, or an empty string allowing implicit type conversion (use === or !==)
    if (inLink.oldValue != null) value = inLink.oldValue;
...............................^

O:\scripts.js(282): lint warning: parseInt missing radix parameter
            oldValue = 0+parseInt(elem.value).toString(8);
............................................^

O:\scripts.js(283): lint warning: parseInt missing radix parameter
            elem.value = 0+parseInt(value).toString(8);
.........................................^

O:\scripts.js(291): lint warning: comparisons against null, 0, true, false, or an empty string allowing implicit type conversion (use === or !==)
    if (inLink.oldValue == null) {
...............................^

O:\scripts.js(302): warning: function resetToDefaultValue does not always return a value
    return false;
................^

O:\scripts.js(336): lint warning: comparisons against null, 0, true, false, or an empty string allowing implicit type conversion (use === or !==)
    if (inValue.length == 0) {
...........................^

O:\scripts.js(431): lint warning: comparisons against null, 0, true, false, or an empty string allowing implicit type conversion (use === or !==)
    if (!el.title || el.title == '')
...................................^

O:\scripts.js(501): lint warning: missing semicolon
PageQuery.prototype.getKeyValuePairs = function() {
^

O:\scripts.js(507): lint warning: missing semicolon
PageQuery.prototype.getValue = function (s) {
^

O:\scripts.js(514): lint warning: missing semicolon
PageQuery.prototype.getParameters = function () {
^

O:\scripts.js(521): lint warning: missing semicolon
PageQuery.prototype.getLength = function() {
^

O:\scripts.js(525): lint warning: missing semicolon
function getUrlParam(inName) {
^

O:\scripts.js(545): lint warning: empty statement or extra semicolon
        } else {
........^

O:\scripts.js(637): lint warning: unexpected end of line; it is ambiguous whether these lines are part of the same statement
                    .removeClass('foswikiSubmitDisabled');
....................^

O:\scripts.js(640): lint warning: unexpected end of line; it is ambiguous whether these lines are part of the same statement
                    .addClass('foswikiSubmitDisabled');
....................^

O:\scripts.js(647): lint warning: missing semicolon
    $(window).scroll(function(){ imgOnDemand() });
...............................................^


0 error(s), 17 warning(s)
Press any key to continue...

-- TimotheLitt - 10 Oct 2012

While there is a tool to tidy perl code, there is no tool to lint js code. This is a manual and often tedious job. I'm afraid that the bar will become very high for aspiring developers.

-- ArthurClemens - 10 Oct 2012

Maybe for jslint, a UnitTest could be developed instead of a checkin gate. Similar to the HTML Validation tests. http://trac.foswiki.org/browser/trunk/UnitTestContrib/test/unit/HTMLValidationTests.pm

-- GeorgeClark - 10 Oct 2012

It's not just formatting, that I'm not religious about. It catches real bugs that can lie dormant for a long time.

At least we should have a standard that says "run a js validation tool". (Anyone who tires of waiving false errors will find a way to fix them efficiently.) And for the core components, like the editor and configure, we really should have a higher standard.

I'm not for making life difficult on developers. Or discouraging them. I find that running jslint over an edit before trying it in the browser saves me debugging time. Faster, clearer and easier to access feedback. So I see it as a tool to make life easier.

That's my 3 cents.

-- TimotheLitt - 11 Oct 2012

I'm a big fan of jslint too. I agree, it's helped me catch a number of stupid bugs which arise from coding JS while under the influence of other languages (for me perl & python)...

Presently, the autoBuildFoswiki.pl compiles a perlcritic log that nobody reads, which can be found at http://fosiki.com/Foswiki_trunk/Foswiki-PerlCritic.log

I'm not sure that perlcritic or jslint quibbles should block a checkin. But something that's almost as in-your-face would be nice. E-mail back to foswiki-svn on checkin? Or E-mail the developer who made the checkin? Is it possible for the svn post-commit hook to display a message to the git-svn/svn user? Hrmm.

-- PaulHarvey - 11 Oct 2012

No problem with JSLint, JSHint etc. but should they be compulsory? i don't think so. Regrading as Enhancement.

-- CrawfordCurrie - 21 Dec 2014

Commenting is disabled while not logged in