Offenders layout for various CSS metrics

lib/metadata/policies.js

@@ -38,7 +38,7 @@ var policies = {
         "isAbnormalThreshold": 10,
         "offendersTransformFn": function(offenders) {
             return offenders.map(function(offender) {
-                var parts = /^(.*): ?(\d) ?occurrences$/.exec(offender);
+                var parts = /^(.*): ?(\d+) ?occurrences$/.exec(offender);
 
                 if (!parts) {
                     debug('DOMidDuplicated offenders transform function error with "%s"', offender);
@@ -107,7 +107,7 @@ var policies = {
         "takeOffendersFrom": "DOMqueriesDuplicated",
         "offendersTransformFn": function(offenders) {
             return offenders.map(function(offender) {
-                var parts = /^.* "(.*)" ?with ?(.*) ?\(in ?context ?(.*)\): ?(.*)\s?queries$/.exec(offender);
+                var parts = /^[^"]* ?"(.*)" ?with ?(.*) ?\(in ?context ?(.*)\): ?(.*)\s?queries$/.exec(offender);
 
                 if (!parts) {
                     debug('DOMqueriesAvoidable offenders transform function error with "%s"', offender);
@@ -265,7 +265,34 @@ var policies = {
         "message": "<p>Yellow Lab Tools failed to parse a CSS file. I doubt the problem comes from the css parser.</p><p>Maybe a <a href=\"http://jigsaw.w3.org/css-validator\" target=\"_blank\">CSS validator</a> can help you.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 1,
-        "isAbnormalThreshold": 1
+        "isAbnormalThreshold": 1,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var parts = /^(([^ \(]*|<inline CSS>) ?)?(?:\((((?! @ ).)*)( @ (\d+):(\d+))?\))?$/.exec(offender);
+
+                if (!parts) {
+                    debug('cssParsingErrors offenders transform function error with "%s"', offender);
+                    return offender;
+                }
+
+                var html = (parts[3]) ? parts[3] : '';
+
+                if (parts[2]) {
+                    html += (parts[2] === '<inline CSS>') ? ' &lt;inline CSS&gt;' : ' ' + offendersHelpers.urlToLink(parts[2]);
+                }
+
+                if (parts[5]) {
+                    html += ' @ ' + parts[6] + ':' + parts[7];
+                }
+
+                if (parts[2] && parts[2] !== '<inline CSS>') {
+                    var w3cDirectUrl = 'http://jigsaw.w3.org/css-validator/validator?uri=' + encodeURIComponent(parts[2]) + '&profile=css3&usermedium=all&warning=no';
+                    html += ' (<a href="' + w3cDirectUrl + '" target="_blank">Check on the W3C validator</a>)';
+                }
+
+                return html;
+            });
+        }
     },
     "cssRules": {
         "tool": "phantomas",
@@ -281,7 +308,14 @@ var policies = {
         "message": "<p>Complex selectors are CSS selectors with 4 or more expressions, like \"#header ul li .foo\".</p><p>They are adding more work for the browser, and this could be avoided by simplifying selectors.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 500,
-        "isAbnormalThreshold": 2000
+        "isAbnormalThreshold": 2000,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+
+                return '<b>' + splittedOffender.offender + '</b> @ ' + splittedOffender.line + ':' + splittedOffender.character;
+            });
+        }
     },
     "cssComplexSelectorsByAttribute": {
         "tool": "phantomas",
@@ -289,7 +323,16 @@ var policies = {
         "message": "<p>Complex attributes selectors are one of these:<ul><li>.foo[type*=bar] (contains bar)</li><li>.foo[type^=bar] (starts with bar)</li><li>.foo[type|=bar] (starts with bar or bar-)</li><li>.foo[type$=bar] (ends with bar)</li><li>.foo[type~=bar baz] (bar or baz)</li></ul></p><p>Their matching process needs more CPU and it has a cost on performances.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 50,
-        "isAbnormalThreshold": 100
+        "isAbnormalThreshold": 100,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+
+                var boldedOffender = splittedOffender.offender.replace(/(\[[^ ]+[~\|\^\$\*]="[^"]+"\])/g, '<b>$1</b>');
+
+                return boldedOffender + ' @ ' + splittedOffender.line + ':' + splittedOffender.character;
+            });
+        }
     },
     "cssImports": {
         "tool": "phantomas",
@@ -297,7 +340,14 @@ var policies = {
         "message": "<p>It’s bad for performance to use @import because CSS files don't get downloaded in parallel.</p><p>You should use &lt;link rel='stylesheet' href='a.css'&gt; instead.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 1,
-        "isAbnormalThreshold": 1
+        "isAbnormalThreshold": 1,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+
+                return '<b>' + splittedOffender.offender + '</b> @ ' + splittedOffender.line + ':' + splittedOffender.character;
+            });
+        }
     },
     "cssDuplicatedSelectors": {
         "tool": "phantomas",
@@ -305,7 +355,19 @@ var policies = {
         "message": "<p>This is when two or more selectors are strictly identical and should be merged.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 40,
-        "isAbnormalThreshold": 80
+        "isAbnormalThreshold": 80,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var parts = /^(.*) \((\d+) times\)$/.exec(offender);
+
+                if (!parts) {
+                    debug('cssDuplicatedSelectors offenders transform function error with "%s"', offender);
+                    return offender;
+                }
+
+                return parts[1] + ' (<b>x' + parts[2] + '</b>)';
+            });
+        }
     },
     "cssDuplicatedProperties": {
         "tool": "phantomas",
@@ -313,7 +375,21 @@ var policies = {
         "message": "<p>This is the number of property definitions duplicated within a selector.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 50,
-        "isAbnormalThreshold": 100
+        "isAbnormalThreshold": 100,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+
+                var parts = /^([^{]+) {([^ ]+): (.+)}$/.exec(splittedOffender.offender);
+
+                if (!parts) {
+                    debug('cssDuplicatedProperties offenders transform function error with "%s"', offender);
+                    return offender;
+                }
+
+                return 'Property <b>' + parts[2] + '</b> duplicated in <b>' + parts[1] + ' { }</b> @ ' + splittedOffender.line + ':' + splittedOffender.character;
+            });
+        }
     },
     "cssEmptyRules": {
         "tool": "phantomas",
@@ -321,7 +397,13 @@ var policies = {
         "message": "<p>Very easy to fix: remove all empty rules.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 40,
-        "isAbnormalThreshold": 100
+        "isAbnormalThreshold": 100,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+                return '<b>' + splittedOffender.offender + ' { }</b> @ ' + splittedOffender.line + ':' + splittedOffender.character;
+            });
+        }
     },
     "cssExpressions": {
         "tool": "phantomas",
@@ -329,7 +411,21 @@ var policies = {
         "message": "<p>Such as: expression( document.body.clientWidth > 600 ? \"600px\" : \"auto\" )</p><p>This is a bad practice as it slows down browsers. There are some simpler CSS3 methods for doing this.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 1,
-        "isAbnormalThreshold": 20
+        "isAbnormalThreshold": 20,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+
+                var parts = /^(.*) {([^ ]+): expression\((.*)\)}$/.exec(splittedOffender.offender);
+
+                if (!parts) {
+                    debug('cssExpressions offenders transform function error with "%s"', offender);
+                    return offender;
+                }
+
+                return parts[1] + ' {' + parts[2] + ': <b>expression(</b>' + parts[3] + '<b>)</b>} @ ' + splittedOffender.line + ':' + splittedOffender.character;
+            });
+        }
     },
     "cssImportants": {
         "tool": "phantomas",
@@ -337,7 +433,21 @@ var policies = {
         "message": "<p>It can be useful, but only as a last resort. It is a bad practice because it overrides the normal cascading logic. The more you use !important, the more you need it again to over-override. This conducts to a poor maintainability.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 50,
-        "isAbnormalThreshold": 150
+        "isAbnormalThreshold": 150,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+
+                var parts = /^(.*) {([^ ]+): (.*) ?\!important}$/.exec(splittedOffender.offender);
+
+                if (!parts) {
+                    debug('cssImportants offenders transform function error with "%s"', offender);
+                    return offender;
+                }
+
+                return parts[1] + ' {' + parts[2] + ': ' + parts[3] + ' <b>!important</b>} @ ' + splittedOffender.line + ':' + splittedOffender.character;
+            });
+        }
     },
     "cssOldIEFixes": {
         "tool": "phantomas",
@@ -345,7 +455,49 @@ var policies = {
         "message": "<p>What browser do you need to support? Once you've got the answer, take a look at these old rules that pollute your CSS code and remove them.</p><p>IE6:<ul><li>* html</li><li>html > body (everything but IE6)</li></ul><p><p>IE7:<ul><li><b>*</b>height: 123px;</li><li>height: 123px <b>!ie</b>;</li></ul><p><p>IE9:<ul><li>-ms-filter</li><li>progid:DXImageTransform.Microsoft</li></ul></p>",
         "isOkThreshold": 0,
         "isBadThreshold": 50,
-        "isAbnormalThreshold": 300
+        "isAbnormalThreshold": 300,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+
+                var parts = /^([^{]*)( {([^ ]+): (.*)})?$/.exec(splittedOffender.offender);
+
+                if (!parts) {
+                    debug('cssOldIEFixes offenders transform function error with "%s"', offender);
+                    return offender;
+                }
+
+                var rule = parts[1];
+                var property = parts[3];
+                var value = parts[4];
+                var browser = null;
+
+                if (rule.indexOf('* html') === 0) {
+                    rule = rule.replace(/^\* html/, '<b>* html</b>');
+                    browser = 'IE6';
+                } else if (rule.indexOf('html>body') === 0) {
+                    rule = rule.replace(/^html>body/, '<b>html>body</b>');
+                    browser = 'IE6';
+                } else if (property.indexOf('*') === 0) {
+                    property = '<b>' + property + '</b>';
+                    browser = 'IE7';
+                } else if (value.match(/\!ie$/)) {
+                    value = value.replace(/\!ie$/, '<b>!ie</b>');
+                    browser = 'IE7';
+                } else if (property === '-ms-filter') {
+                    property = '<b>-ms-filter</b>';
+                    browser = 'IE9';
+                } else if (value.indexOf('progid:DXImageTransform.Microsoft') === 0) {
+                    value = value.replace(/^progid:DXImageTransform\.Microsoft/, '<b>progid:DXImageTransform.Microsoft</b>');
+                    browser = 'IE9';
+                }
+
+                browser = browser ? '<b>' + browser + ' fix:</b> ' : '';
+                var propertyAndValue = (property && value) ? ' {' + property + ': ' + value + '}' : '';
+
+                return browser + rule + propertyAndValue + ' @ ' + splittedOffender.line + ':' + splittedOffender.character;
+            });
+        }
     },
     "cssOldPropertyPrefixes": {
         "tool": "phantomas",
@@ -353,7 +505,21 @@ var policies = {
         "message": "<p>Many property prefixes such as -moz- or -webkit- are not needed anymore, or by very few people. You can remove them or replace them with the non-prefixed version. This will help reducing your stylesheets weight.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 50,
-        "isAbnormalThreshold": 300
+        "isAbnormalThreshold": 300,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+
+                var parts = /^([^{]*)(?: { ([^ ]+): (.*) }) \/\/ (.*)$/.exec(splittedOffender.offender);
+
+                if (!parts) {
+                    debug('cssOldPropertyPrefixes offenders transform function error with "%s"', offender);
+                    return offender;
+                }
+
+                return '<div>' + parts[1] + ' {<b>' + parts[2] + '</b>: ' + parts[3] + '} @ ' + splittedOffender.line + ':' + splittedOffender.character + '</div><div><b>' + parts[4] + '</b></div>';
+            });
+        }
     },
     "cssUniversalSelectors": {
         "tool": "phantomas",

lib/offendersHelpers.js

@@ -136,9 +136,10 @@ var OffendersHelpers = function() {
         }
 
         var html = '<div class="offenderButton opens">backtrace<div class="backtrace">';
+        var that = this;
         backtraceArray.forEach(function(backtraceObj) {
             var functionName = (backtraceObj.functionName) ? backtraceObj.functionName + '() ' : '';
-            html += '<div>' + functionName + '<a href="' + backtraceObj.file + '" target="_blank">' + backtraceObj.file + '</a> line ' + backtraceObj.line + '</div>';
+            html += '<div>' + functionName + that.urlToLink(backtraceObj.file) + ' line ' + backtraceObj.line + '</div>';
         });
         return html + '</div></div>';
     };
@@ -150,6 +151,27 @@ var OffendersHelpers = function() {
         });
     };
 
+    this.urlToLink = function(url) {
+        var shortUrl = (url.length > 110) ? url.substr(0, 47) + ' ... ' + url.substr(-48) : url;
+        return '<a href="' + url + '" target="_blank" title="' + url + '">' + shortUrl + '</a>';
+    };
+
+    this.cssOffenderPattern = function(offender) {
+        var parts = /^(.*) @ (\d+):(\d+)$/.exec(offender);
+        
+        if (!parts) {
+            return {
+                offender: offender
+            };
+        } else {
+            return {
+                offender: parts[1],
+                line: parseInt(parts[2], 10),
+                character: parseInt(parts[3], 10)
+            };
+        }
+    };
+
 };
 
 module.exports = new OffendersHelpers();

test/core/offendersHelpersTest.js

@@ -175,7 +175,7 @@ describe('offendersHelpers', function() {
                 }
             ]);
 
-            result.should.equal('<div class="offenderButton opens">backtrace<div class="backtrace"><div><a href="http://pouet.com/js/jquery.footer-transverse-min-v1.0.20.js" target="_blank">http://pouet.com/js/jquery.footer-transverse-min-v1.0.20.js</a> line 1</div><div>callback() <a href="http://pouet.com/js/main.js" target="_blank">http://pouet.com/js/main.js</a> line 1</div></div></div>');
+            result.should.equal('<div class="offenderButton opens">backtrace<div class="backtrace"><div><a href="http://pouet.com/js/jquery.footer-transverse-min-v1.0.20.js" target="_blank" title="http://pouet.com/js/jquery.footer-transverse-min-v1.0.20.js">http://pouet.com/js/jquery.footer-transverse-min-v1.0.20.js</a> line 1</div><div>callback() <a href="http://pouet.com/js/main.js" target="_blank" title="http://pouet.com/js/main.js">http://pouet.com/js/main.js</a> line 1</div></div></div>');
         });
 
         it('should display "no backtrace"', function() {
@@ -228,4 +228,43 @@ describe('offendersHelpers', function() {
 
     });
 
+    describe('urlToLink', function() {
+
+        it('should transform an url into an html link', function() {
+            var result = offendersHelpers.urlToLink('http://www.google.com/js/main.js');
+
+            result.should.equal('<a href="http://www.google.com/js/main.js" target="_blank" title="http://www.google.com/js/main.js">http://www.google.com/js/main.js</a>');
+        });
+
+        it('should ellypsis the url if too long', function() {
+            var result = offendersHelpers.urlToLink('http://www.google.com/js/longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong/main.js');
+
+            result.should.equal('<a href="http://www.google.com/js/longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong/main.js" target="_blank" title="http://www.google.com/js/longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong/main.js">http://www.google.com/js/longlonglonglonglonglo ... longlonglonglonglonglonglonglonglonglong/main.js</a>');
+        });
+
+    });
+
+
+    describe('cssOffenderPattern', function() {
+
+        it('should transform a css offender into an object', function() {
+            var result = offendersHelpers.cssOffenderPattern('.pagination .plus ul li @ 30:31862');
+
+            result.should.deep.equal({
+                offender: '.pagination .plus ul li',
+                line: 30,
+                character: 31862
+            });
+        });
+
+        it('should handle the case where line and char are not here', function() {
+            var result = offendersHelpers.cssOffenderPattern('.pagination .plus ul li');
+
+            result.should.deep.equal({
+                offender: '.pagination .plus ul li'
+            });
+        });
+
+    });
+
 });