[dojo-contributors] No Side Effects Rejection

Mike Wilcox mwilcox at sitepen.com
Mon May 11 12:16:41 EDT 2009


Good info as always Pete. I'll fix my style violations as soon as time  
allows.

Mike Wilcox
mwilcox at sitepen.com
http://www.sitepen.com
work: 650.968.8787 x218
cell:	  214.697.4872

On May 11, 2009, at 11:08 AM, Peter E Higgins 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
>



More information about the dojo-contributors mailing list