Item12136: scripts don't pass jslint - should it be a checkin gate?
Priority: Enhancement
Current State: Proposal Required
Released In: n/a
Target Release: n/a
Applies To: Engine
Component: Configure
Branches:
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