Permalink
Browse files

New post: JSCritic

  • Loading branch information...
kangax committed Mar 27, 2014
1 parent d18a51c commit 5eac9cc3e5975a02700475daeb584b83f5fda433
@@ -1,7 +1,6 @@
---
layout: post
title: Moving from Wordpress to Github Pages
-new: new
tags:
- other
---
@@ -1,7 +1,6 @@
---
layout: post
title: State of function decompilation in Javascript
-new: new
tags:
- js
---
@@ -0,0 +1,189 @@
+---
+layout: post
+title: JSCritic
+tags:
+ - js
+new: new
+---
+
+# JSCritic
+
+Choosing a good piece of Javascript is hard.
+
+Every time I come across a newly-released, shiny plugin or library, I wonder what's going on underneath. Yes, it looks pretty and convenient but what does underlying code look like? Does it browser sniff or extend the DOM? Does it pollute global scope? What about compatibility with older browsers; could it be that it utilizes, say, ES5 getters/setters making it unusable in IE<9?
+
+I always wished there was a way to <b>quickly check</b> how well a certain script behaves. Not like we did [back in the days](https://groups.google.com/forum/?hl=en#!msg/comp.lang.javascript/PZDouKgwFGI/XKd8LYURyzcJ).
+
+The best thing for a code quality test like this is undoubtedly through JSHint <sup><a href="#jshint">[1]</a></sup>. It can answer most of those questions and many more. Unfortunately, "many more" part is a bit of a problem. Plugging a script code into <a href="http://jshint.com">jshint.com</a> usually yields tons of issues, not just with browser compatibility or global variables but also code style. These checks are a must for your own scripts, but for 3rd party code, I don't really care about missing semicolons (despite my love of them), whether constructors begin with uppercase, or if assignments happen in conditional statements. I only wish to know how well a script behaves <b>on the outside</b>. Now, a sloppy code style can certain be an indication of a bad quality of script overall. But more often than not it's a preference not a problem.
+
+Few days ago, I decided to hack something together; something simple, that would allow me to quickly plug the script and see a big picture.
+
+So I made <a href="http://jscritic.com">JSCritic</a>.
+
+Plug in script code and it answers some of the more pressing questions.
+
+<a href="http://jscritic.com">
+ <img src="/images/jscritic.png" style="width: 850px">
+</a>
+
+I tried using <a href="http://esprima.org">Esprima</a> at first, but quickly realized that most of the checks I care about are already in JSHint. So why not piggy back on that? <a href="http://github.com/kangax/jscritic">JSCritic</a> turned out to be a simple wrapper on top of it. I originally wrote it in Node, to be able to pass it filename and quickly see the results, then ported it to run in a browser.
+
+<!-- <pre lang="shell"><code>
+> node jscritic.js fabric.js
+
+- Does it browser sniff? Nope
+
+- Does it extend native objects? Yep (String)
+
+- Does it use `document.write`? Nope
+
+- Does it use eval? Yep
+ eval("var callback =" + js);
+
+- Does it use ES6 features? Nope
+
+- Does it use Mozilla-only features? Nope
+
+- Does it have IE incompatibilities? Yep (Extra comma, get/set are ES5 features)
+
+- How many global variables? 9 (line, column, GSS, GSS_CONFIG, selector, type, callback, c, ShadowDOMPolyfill)
+
+- How many unused variables? 47 (require, exports, escape, idPrefix, offset, flatten, _varsCache, statements, result, id, _id1, _id2, s, connector, module, props, vflFooter, col, heights, k, h, io, c, setVariable, coeff, expr, medium, strong, required, match, e, vars, tracker, exp, op, w, root, e2, e1, namesssss, self, _this, bridgessssss, names, trackersss, ifffff, node)
+
+Total size: 872.99KB
+Minified size: 250.75KB
+</code></pre> -->
+
+{% gist kangax/26e20fb726cbcbd27087 %}
+
+You can still run it in both.
+
+Another thing I wanted to see is <b>minified script size</b>. Some plugins have minified versions, some don't, some use better minifiers, some worse. I decided to minify content through <a href="https://github.com/mishoo/UglifyJS2">UglifyJS</a> — a de facto standard of minification at the moment — to get an objective overview of code size. Unfortunately, browser version of UglifyJS seems to be choking more often than Node one, so it might be safer to use the latter.
+
+I have to say that JSCritic is more of a prototype at the moment. Static analysis has its limitations, as well as JSHint. I haven't had much time to polish it, but hoping to improve in the near future or with the help of ever-awesome contributors. One thing to emphasize is that for best results you should <b>use non-minified source code</b> (you'll see exactly why below)!
+
+If you want to know more about tests, implementation details, and drawbacks, read on. Otherwise, hope you find it as useful as I do.
+
+### Global variables
+
+Let's first take a look at global variables detection. Unfortunately, it seems to be very simplistic in JSHint, failing to catch cases other than plain variable/function declarations in top level code.
+
+<!-- <pre lang="javascript"><code>
+var foo = 1;
+
+function bar() {
+ function baz () { }
+ qux = 123;
+}
+</code></pre> -->
+{% gist kangax/b7ee9179f1648a2282a2 %}
+
+Here it catches <code>foo</code>, <code>bar</code>, and <code>qux</code> as expected, but fails with all of these:
+
+<!-- <pre lang="javascript"><code>
+(function(){ window.foo = 1; })();
+(function(){ this.foo = 1; })();
+(function(){ self.foo = 1; })();
+(function(){ var global = this; global.foo = 1; })();
+(function(){ var global = this; global.foo = 1; }).call(this);
+</code></pre> -->
+
+{% gist kangax/18e89bb4cba0b86e6409 %}
+
+Granted, detecting globals via static analysis is hard. A more robust solution would be to actually execute code and compare global object "signature", just like I did in [detect-global bookmarklet](http://perfectionkills.com/detecting-global-variable-leaks/) back in 2009 (based on a script by Remy Sharp). Unfortunately, executing script is also not always easy, and global properties could be exported from various places (e.g. methods that need to be called explicitly); we have no idea which places those are.
+
+Still, JSHint catches a good number of globals and accidental leaks like these:
+
+<!-- <pre lang="javascript"><code>
+var foo = 1;
+ bar = 2;
+</code></pre> -->
+
+{% gist kangax/bbb236b2fe1bc427d75b %}
+
+It gives a decent overview, but you should still look through variables carefully as some of them might be false positives. I'm hoping this will be made more robust in the future JSHint versions (or we could try using hybrid detection method — both via static analysis and through global object signature).
+
+### Extended natives
+
+Detecting native object extensions has few limitations as well. While it catches both Array and String in example like this:
+
+<!-- <pre lang="javascript"><code>
+(function(){
+ Array.prototype.foo = function(){ };
+ String.prototype.bar = 123;
+})();
+</code></pre> -->
+
+{% gist kangax/d272abd2b954d29e844b %}
+
+..it fails with all of these:
+
+<!-- <pre lang="javascript"><code>
+(function(s) {
+
+ Object.myKeys = function(){ };
+
+ var proto = String.prototype;
+ proto.bar = 123;
+
+ Array['prototype'].foo = 'xyz';
+
+ s.prototype.blah = 'blah';
+
+})(String);
+</code></pre> -->
+
+{% gist kangax/caa7e3cce423f958b1a2 %}
+
+As you can see, it's also simplistic and could have false negatives. There's an [open JSHint issue](https://github.com/jshint/jshint/issues/1316) for this.
+
+### eval &amp; document.write
+
+Just like with other checks, there are false positives and false negatives. Here's some of them, just to give an idea of what to expect:
+
+<!-- <pre lang="javascript"><code>
+/* false negative
+
+ Issues: https://github.com/jshint/jshint/issues/738
+ https://github.com/jshint/jshint/issues/1204
+
+*/
+schemaEvaluator.eval(experimentId, schema);
+</code></pre> -->
+
+{% gist kangax/6a60374ada10d4b95b0d %}
+
+and with <code>document.write</code>:
+
+<!-- <pre lang="javascript"><code>
+(function(d) {
+
+ // catches
+ document.write(1);
+
+ // doesn't catch
+ d.write(1);
+ document['write'](1);
+
+})(document);
+</code></pre> -->
+
+{% gist kangax/f7be5ebdbad1e1b1c409 %}
+
+### Browser compatibility
+
+I included 3 checks for browser/engine compatibility — Mozilla-only extensions (let expressions, [expression closures](/a-closer-look-at-expression-closures), multiple catch blocks, etc.), things IE chokes on (e.g. extra comma), and ES6 additions (array comprehensions, generators, imports, etc.). All of these things could affect cross-browser support.
+
+### Browser sniffing
+
+To detect browser sniffing, we first check statically for occurance of `navigator` implied global (via JSHint), then check source for occurance of `navigator.userAgent`. This covers a lot of cases, but obviously won't catch any obscurities, so be careful. To make things easier, a chunk of code surrounding `navigator.userAgent` is pasted for expection purposes. You can quickly check what it's there for (is it for non-critical enhancement purposes or could it cause subtle bugs and/or full breakage?)
+
+### Unused variables
+
+Finally, I included unused variables check from JSHint. While not exactly an indication of external script behavior, seeing lots of those could be an indication of sloppy (and potentially buggy) code. I put it all the way at the end, as this is the least important check.
+
+So there it is. The set of rules can definitely be made larger (does it use ES5 features? does it use browser-sniffing-like inference? does it extend the DOM?) and more accurate in the future. For now you can use JSCritic as a quick first look under the hood.
+
+<p class="footnote" id="jshint">
+ <sup>[1]</sup> and perhaps ESLint, but I haven't had a chance to look into it.
+</p>
View
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 5eac9cc

Please sign in to comment.