[dojo-contributors] No Side Effects Rejection

Peter E Higgins dante at dojotoolkit.org
Mon May 11 15:07:54 EDT 2009


Dustin also turned up a thinger called 'jsure' which catches all
trailing commas and a host of other quirky things (it doesn't hurt your
feelings, it is quite polite in fact and very verbose in it's
explanations) but unfortunately it outputs entirely in ANSI style escape
sequences for console colors, and doesn't pipe well (no options i've
seen to disable colors, and source is not available afaik)

http://www.jsure.org/

Regards


Nathan Toone wrote:
> You know, I have found that jsl (which isn't related to Crockford's
> jslint) is actually pretty good at finding things like trailing
> commas, and it is quite configurable.  We use it internally for the
> code that we write - and it catches a lot of things that might
> otherwise be missed.  But it doesn't seem to hurt your feelings as
> often as jslint does.  :)
>
> http://www.javascriptlint.com
>
> -Nathan
>
> On May 11, 2009, at 12:28 PM, Adam Peller wrote:
>
>> Just so you know, none of this is "jslint".  We're currently running
>> everything through Rhino with the "-strict" switch, which implements
>> only a handful of checks, some of them reminiscent of jslint, but no
>> where near as comprehensive (or offensive).  We started out just
>> running checkins through Rhino's parser to reject really obvious
>> stuff, like typos which would cause syntax errors.  I turned on the
>> strict mode mostly hoping it would catch the dreaded trailing comma,
>> as advertised.  Unfortunately, it only catches them in literals, not
>> in code :-(
>>
>> Yeah, I've been scratching my head over the side-effects message too
>> -- sometimes it doesn't seem to make any sense.  Anyhow, this is why
>> we have an override with "!strict"  Until someone finds something
>> better, I think it's doing more good than harm to the codebase.
>>
>> Also, FWIW, there are some really nasty examples out there where a
>> stray semi or return can actually alter the meaning of JavaScript.  I
>> don't know if this particular rule has anything to do with that, but
>> putting a semi after a code block simply shouldn't be allowed.
>>
>> -Adam
>>
>>
>> On Mon, May 11, 2009 at 12:08 PM, Peter E Higgins
>> <dante at dojotoolkit.org> wrote:
>>> Mike Wilcox wrote:
>>>> You are suggesting it's a format violation which is fine, but that's
>>>> not what was throwing errors.
>>>>
>>>>
>>> I know, but my comment was that the issue was moot because the patch
>>> should have been rejected based on the style guidelines alone.  The
>>> warning was just a warning, and not an error, so it is just lint
>>> talking
>>> to you. Our lint is clearly more strict than Komodo/Textmate/etc, and I
>>> agree it is overly aggressive.
>>>
>>> The issue is the ; is NOT required, and lint is complaining the "next
>>> line" has no side effect. For example, the following commit attempt
>>> failed. The number of Errors is equal to the number of excessive ;'s
>>>
>>> Index: FileUploader.js
>>> ===================================================================
>>> --- FileUploader.js     (revision 17515)
>>> +++ FileUploader.js     (working copy)
>>> @@ -18,7 +18,7 @@
>>>
>>>        var mixin = function(o1,o2){
>>>                // custom mixin
>>> -               var o = {}, nm;
>>> +               var o = {}, nm;;;;;;
>>>                for(nm in o1){
>>>                        if(dojo.isObject(o1[nm])){
>>>                                o[nm] = mixin({}, o1[nm]);
>>> sugr:form dante$ svn commit -m "refs #7340 - this is a test"
>>> Sending        form/FileUploader.js
>>> Transmitting file data .svn: Commit failed (details follow):
>>> svn: MERGE request failed on '/src/dojox/trunk/form'
>>> svn: 'pre-commit' hook failed with error output:
>>> Error at dojox/trunk/form/FileUploader.js:21 Code has no side effects
>>> Error at dojox/trunk/form/FileUploader.js:21 Code has no side effects
>>> Error at dojox/trunk/form/FileUploader.js:21 Code has no side effects
>>> Error at dojox/trunk/form/FileUploader.js:21 Code has no side effects
>>> Error at dojox/trunk/form/FileUploader.js:21 Code has no side effects
>>> ERROR: Syntax errors. Checkin aborted. To override strict mode errors,
>>> use "!strict" in comment.
>>>
>>> sugr:form dante$
>>>
>>>> As you proved though, it was the semi-colon in the wrong place. I
>>>> guess I can see "return" being invalid compared to "return;"
>>>>
>>>>
>>> I think it would have been fine if it were:
>>>
>>> if(this._formNode){ return }
>>>
>>> (note the lack of a ; after the }, which isn't an object or a var, its
>>> the 'if' block)
>>>
>>> Another interesting quirk here (don't quote me) return's cannot have a
>>> newline on the 'return' line:
>>>
>>> function bar(a){
>>>    var b = 1;
>>>    return
>>>       a || b;
>>> }
>>>
>>> var x = bar(2); // expects 2
>>> var y = bar(); // expects 1
>>>
>>> console.log(x, y); // undefined, undefined
>>>
>>> moving a || b to " return a || b; " works fine.
>>>
>>> As it is all over our code, this is completely valid:
>>>
>>> return someTruthy ?
>>>    trueValue :
>>>    falseValue;
>>>
>>>> And as it turned out, a semi-colon after my try-catch was breaking
>>>> embed.Flash:
>>>>
>>>> try{..}catch(e){...); <----- that semi colon wouldn't allow a commit!!
>>>>
>>>>
>>> Yep, same case as above.
>>>> I guess I would have to blame the validator for the improper message
>>>> on that second one. is it reading that as two statements?
>>>>
>>>>
>>> It is reading it as a line of code ";" that "does nothing", and "has no
>>> side effects".
>>>
>>> Please look at Shane's output, and ensure all modules you (all of us)
>>> own pass the basic style guideline rules in place and already being
>>> spotted safely.
>>>
>>> http://skynet.ie/~sos/misc/dojoCheckstyle/util/buildscripts/checkstyleReport.html
>>>
>>>
>>> Regards
>>>
>>> -- 
>>> Peter E Higgins
>>> Dojo Project Lead : http://dojotoolkit.org
>>>
>>> _______________________________________________
>>> dojo-contributors mailing list
>>> dojo-contributors at mail.dojotoolkit.org
>>> http://mail.dojotoolkit.org/mailman/listinfo/dojo-contributors
>>>
>> _______________________________________________
>> dojo-contributors mailing list
>> dojo-contributors at mail.dojotoolkit.org
>> http://mail.dojotoolkit.org/mailman/listinfo/dojo-contributors
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> dojo-contributors mailing list
> dojo-contributors at mail.dojotoolkit.org
> http://mail.dojotoolkit.org/mailman/listinfo/dojo-contributors
>   


-- 
Peter E Higgins
Dojo Project Lead : http://dojotoolkit.org 



More information about the dojo-contributors mailing list