Results of strict mode on 'env.js'
Reported by nickg | March 28th, 2010 @ 06:36 PM
From gpsee http://code.google.com/p/gpsee/ and a newer snapshot of mozilla's tracemonkey branch and the new nifty ECMA5 strict mode.
A quick survey suggest the major gotchas are:
- forgetting a
var(oops) deletelocalvar (deprecated and should do nothing)- using
varon a variable name that is an argument name - a function returns something in one control path, but not in
another (prob want to
return null)
cool huh?
-nickg
$ gsr -f 'env-js/dist/env.js'
JS strict warning #127 in env.js at line 2557 ch 12 - variable value redeclares argument
JS strict warning #127 in env.js at line 7032 ch 12 - variable newChild redeclares argument
JS strict warning #127 in env.js at line 7038 ch 12 - variable newChild redeclares argument
JS strict warning #157 in env.js at line 8545 ch 8 - anonymous function does not always return a value
JS strict warning #157 in env.js at line 8554 ch 8 - anonymous function does not always return a value
JS strict warning #157 in env.js at line 9525 ch 149 - anonymous function does not always return a value
JS warning #235 in env.js at line 9710 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated
JS warning #235 in env.js at line 9716 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated
JS warning #235 in env.js at line 9718 ch 4 - Applying the 'delete' operator to an unqualified name is deprecated
JS warning #235 in env.js at line 9772 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated
JS warning #235 in env.js at line 9785 ch 12 - Applying the 'delete' operator to an unqualified name is deprecated
JS warning #235 in env.js at line 9787 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated
JS warning #235 in env.js at line 9788 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated
JS strict warning #157 in env.js at line 10622 ch 26 - anonymous function does not always return a value
JS strict warning #157 in env.js at line 10869 ch 29 - anonymous function does not always return a value
JS strict warning #157 in env.js at line 11861 ch 12 - anonymous function does not always return a value
JS warning #235 in env.js at line 11884 ch 16 - Applying the 'delete' operator to an unqualified name is deprecated
JS strict warning #163 in env.js at line 11885 - useless expression
JS strict warning #156 in env.js at line 7768 - assignment to undeclared variable HTMLParagraphElement
JS strict warning #156 in env.js at line 9083 - assignment to undeclared variable $css2properties
JS strict warning #156 in env.js at line 5129 - assignment to undeclared variable node
JS strict warning #156 in env.js at line 11393 - assignment to undeclared variable $top
JS strict warning #156 in env.js at line 11394 - assignment to undeclared variable $left
JS strict warning #156 in env.js at line 11395 - assignment to undeclared variable $availTop
JS strict warning #156 in env.js at line 11396 - assignment to undeclared variable $availLeft
JS strict warning #162 in env.js at line 11355 - reference to undefined property Envjs.revision
Comments and changes to this ticket
-
nickg March 28th, 2010 @ 06:45 PM
I couldn't help myself.
http://github.com/thatcher/env-js/commit/7879a696e8e7bfaa6259bc626e...
fixes
JS strict warning #156 in env.js at line 5129 - assignment to undeclared variable node JS strict warning #156 in env.js at line 7768 - assignment to undeclared variable HTMLParagraphElementThe first of which is semi-serious depending how the async/parser works, and certainly pollutes global namespace with a common name. (this is in the main document.createElement function)
The second is just cause I probably caused the error.
-
Steven Parkes March 28th, 2010 @ 08:32 PM
Nice.
I found the node one with a tweak I've been playing with on my fork: loading the DOM classes into their own global object and importing them into individual windows. Highlighted every global that wasn't supposed to be there. This is the only one that I really remember. I thought I checked to see if it had been fixed in 1.2 but obviously I messed up some how.
I run all my code through jslint on every save. Unfortunately, until Crockford (or someone else) decides to implement getters/setters, it's a non-starter for envjs.
The emacs JS2 mode actually finds things like unmatched returns.
I think I tried running envjs compressed by YUI and Closure and it failed afterwards. Be nice if we could make that work ... I wonder if it would speed anything up? Perhaps not, but ...
-
nickg March 29th, 2010 @ 09:55 AM
yeah crockford ain't too keen on adding support for getters/setters, huh? I saw your post to the group too. He's been cranky about them for years. Oh well.
I attempted a patch, it's close, but not quite close enough. The problem with parsers like these is that unless you really dig into and work with it every day, it's really tough to understand. And honestly, he'll never integrate it.
But I have an even better hack which allows jslint to work. Quick tests show:
* the division of directories into separate files (e.g. html.js, dom.js, console.js, etc) don't really work well since they have dependencies on other modules. * each module has some ordering problems in terms of how the files are arranged. * lots of warnings about==vs.===* Missing semi-colonsIn other words, the usual stuff!
I'll try and set up an automated dump of the results shortly.
--nickg
-
nickg March 29th, 2010 @ 10:29 AM
ok attached jslint output.
A lot of these could be avoided by better ordering of the code. I post my script that generated this later...
-
nickg March 29th, 2010 @ 10:46 AM
Executive summary! 592 total errors, with a very loose set of options for jslint, and the HTMLParser stuff ripped out.
$ ./jslintparser.py < lintout.txt Missing '()' invoking a constructor. 71 '__ownerDocument__' is not defined. 46 Missing semicolon. 37 Unnecessary semicolon. 33 Mixed spaces and tabs. 30 Expected '{' and instead saw 'a'. 18 Expected '{' and instead saw 'return'. 15 '__eval__' is not defined. 12 Wrap the entire immediate function invocation in parens. 12 Move the invocation into the parens that contain the function. 12 The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype. 11 Read only. 8 'window' is not defined. 8 '$css2properties' is not defined. 7 'e' is already defined. 6 'logFormatted' is not defined. 6 'i' is already defined. 6 Use '===' to compare with 'null'. 6 Expected '{' and instead saw 'this'. 5 'i' is not defined. 5 Expected an assignment or function call and instead saw an expression. 5 Missing radix parameter. 5 Use '===' to compare with '0'. 4 'appendText' is not defined. 4 Bad escapement. 4 Don't make functions within a loop. 4 'go' is not defined. 4 '__isAncestor__' is not defined. 3 '__toCamelCase__' is not defined. 3 'override' is not defined. 3 '__fieldValue__' is not defined. 3 '__isValidNamespace__' is not defined. 3 '__findNamedItemIndex__' is not defined. 3 Unnecessary escapement. 3 '__findNamedItemNSIndex__' is not defined. 3 Expected '{' and instead saw 'Envjs'. 3 Do not use 'new' for side effects. 3 Expected '{' and instead saw 'aspect'. 3 'Window' is not defined. 3 Expected '{' and instead saw 'element'. 3 '__windows__' is not defined. 3 Bad line breaking before '&&'. 3 'XML' is not defined. 3 Expected a 'break' statement before 'default'. 3 Expected '{' and instead saw 'appendText'. 3 Expected '{' and instead saw 'continue'. 3 'console' is not defined. 3 '__cssTextToStyles__' is not defined. 2 Expected '{' and instead saw 'appendNull'. 2 Use '===' to compare with 'undefined'. 2 Extra comma. 2 'newChild' is already defined. 2 'appendObject' is not defined. 2 Use '!==' to compare with '0'. 2 Expected '{' and instead saw 'attrstring'. 2 Expected an identifier and instead saw 'eval' (a reserved word). 2 '__isValidString__' is not defined. 2 Expected '{' and instead saw 'accumulateText'. 2 'appendInteger' is not defined. 2 '$left' is not defined. 2 Expected '{' and instead saw 'array'. 2 Expected '{' and instead saw 'html'. 2 Use '!==' to compare with 'null'. 2 '__isIdDeclaration__' is not defined. 2 '__toDashed__' is not defined. 2 Expected '.' and instead saw ';'. 2 Expected '{' and instead saw 'ns'. 2 '__param__' is not defined. 2 Expected '{' and instead saw '_window'. 2 '$top' is not defined. 2 ['BUBBLING'] is better written in dot notation. 2 Expected an identifier and instead saw 'arguments' (a reserved word). 2 ['CAPTURING'] is better written in dot notation. 2 '__exchangeHTMLDocument__' is not defined. 2 '$availTop' is not defined. 2 Confusing use of '!'. 2 'Cookies' is not defined. 2 Expected '{' and instead saw 'successful'. 1 'HTMLDocument' is not defined. 1 '__bubbleEvent__' is not defined. 1 '__submit__' is not defined. 1 'max' is not defined. 1 'trim' is not defined. 1 'console' was used before it was defined. 1 'appendFloat' is not defined. 1 'inserted' is already defined. 1 'value' is already defined. 1 'object' is already defined. 1 'HTMLDocument' was used before it was defined. 1 Bad for in variable 'name'. 1 '__window__resizeTo' is not defined. 1 'appendObjectFormatted' is not defined. 1 'NamespaceNodeMap' is not defined. 1 '__removeEventListener__' is not defined. 1 'data' is not defined. 1 'pointcut' is not defined. 1 Bad for in variable 'index'. 1 Use '===' to compare with 'false'. 1 '__top__' is not defined. 1 Redefinition of 'clearTimeout'. 1 A constructor name should start with an uppercase letter. 1 'XMLList' is not defined. 1 Expected '{' and instead saw 'value'. 1 Expected '{' and instead saw 'css'. 1 Redefinition of 'clearInterval'. 1 Use '===' to compare with 'true'. 1 '__add__' is not defined. 1 '__recursivelyGatherText__' is not defined. 1 Expected '{' and instead saw 'appendSelector'. 1 '__formSerialize__' is not defined. 1 're_invalidStringChars' is not defined. 1 Expected '{' and instead saw 'appendString'. 1 'parseFormat' is not defined. 1 Use '!==' to compare with 'undefined'. 1 Missing 'new' prefix when invoking a constructor. 1 Expected '{' and instead saw 'appendObject'. 1 Expected '{' and instead saw 'appendInteger'. 1 'node' is not defined. 1 Expected '{' and instead saw 'appendObjectFormatted'. 1 're_validName' is not defined. 1 'url' is not defined. 1 ['height'] is better written in dot notation. 1 Redefinition of 'setInterval'. 1 Expected '{' and instead saw 'parent'. 1 'value' is not defined. 1 'leftover' used out of scope. 1 Redefinition of 'XMLHttpRequest'. 1 '$availleft' is not defined. 1 '__toDomNode__' is not defined. 1 Expected '{' and instead saw 'appendNode'. 1 'ns' is already defined. 1 Expected '{' and instead saw 'console'. 1 '__dispatchEvent__' is not defined. 1 '__supportedStyles__' is not defined. 1 '__getElementsByTagNameRecursive__' is not defined. 1 '__updateCss2Props__' is not defined. 1 '__addEventListener__' is not defined. 1 '__remove__' is not defined. 1 Expected '{' and instead saw 'break'. 1 Expected '{' and instead saw 'throw'. 1 Expected '{' and instead saw 'text'. 1 'Window' was used before it was defined. 1 'HTMLULElement' is not defined. 1 'nodeList' is not defined. 1 '__clearField__' is not defined. 1 '$availLeft' is not defined. 1 '__captureEvent__' is not defined. 1 'HTMLParser' is not defined. 1 'tmp' is already defined. 1 Redefinition of 'Image'. 1 ['width'] is better written in dot notation. 1 Redefinition of 'Option'. 1 Expected '{' and instead saw 'event'. 1 '$implementation' is not defined. 1 'leftover' is already defined. 1 Expected '{' and instead saw 'returnedHeaders'. 1 Line breaking error 'break'. 1 'input' is not defined. 1 Redefinition of 'setTimeout'. 1 This 'switch' should be an 'if'. 1 '__getElementsByTagNameNSRecursive__' is not defined. 1 Expected '{' and instead saw 'appendFunction'. 1 Expected '{' and instead saw 'bubbles'. 1 'XPathExpression' is not defined. 1 'appendSelector' is not defined. 1 TOTAL ERRORS: 592 -
Steven Parkes March 29th, 2010 @ 11:22 AM
I haven't mentioned it, but I kinda don't like the single mother of all closures. Having closures span files confuses some tools and I find it makes it easy to create pseudo-globals that have a lot of the negative consequences of real globals: hard to track down modifications, easy to have collisions, etc.
Seems like it'd be easy enough to create a global object in an intro.js and delete it in an outro.js. Files could have individual closures to capture it. And it could be mocked for testing.
Seems like scoping between files would be a lot clearer.
I don't know if this a really good idea or a really bad idea ...
-
nickg March 29th, 2010 @ 11:56 AM
well I committed some easy fixes between meeting. Only 482 to go. Actually, I don't care about 100% "correct" in jslint's eyes, but it's better than nothing. Certainly trying to set all "Missing semi-colons" and "eval is a keyword" fixed up in attempt to allow this to be minified correctly.
In case you think this is all bs:
'node' is not defined.is an actually bug that would crash envjs.
Anyways, fun stuff.
-
nickg March 29th, 2010 @ 11:57 AM
@steve: re: I haven't mentioned it, but I kinda don't like the single mother of all closures
I more or less agree. I have to run but let's talk later on this.
-
nickg April 15th, 2010 @ 04:35 AM
- State changed from “new” to “resolved”
I declare minor victory in this ticket. The first 4 are due to html5parser. And the later ones are bogus
deleteoperations, which is covered by another ticket.JS strict warning #110 in env.js at line 10654 ch 2 - function $makeException does not always return a value JS strict warning #112 in env.js at line 10967 ch 33 - test for equality (==) mistyped as assignment (=)? ACTUALLY CORRECT TO MY AMAZEMENT JS strict warning #157 in env.js at line 11271 ch 4 - anonymous function does not always return a value JS strict warning #157 in env.js at line 16103 ch 4 - anonymous function does not always return a value JS warning #235 in env.js at line 20480 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated JS warning #235 in env.js at line 20486 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated JS warning #235 in env.js at line 20488 ch 4 - Applying the 'delete' operator to an unqualified name is deprecated JS warning #235 in env.js at line 20552 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated JS warning #235 in env.js at line 20566 ch 12 - Applying the 'delete' operator to an unqualified name is deprecated JS warning #235 in env.js at line 20568 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated JS warning #235 in env.js at line 20569 ch 8 - Applying the 'delete' operator to an unqualified name is deprecated JS warning #235 in env.js at line 22758 ch 16 - Applying the 'delete' operator to an unqualified name is deprecated JS strict warning #163 in env.js at line 22759 - useless expression
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
a javascript browser environment
nickg