From fd2271f6d6c7a24369a65ce042e7b0893a01384f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Mon, 19 Jan 2015 20:31:06 +0100 Subject: [PATCH] Offenders layout for various CSS metrics --- lib/metadata/policies.js | 192 ++++++++++++++++++++++++++++-- lib/offendersHelpers.js | 24 +++- test/core/offendersHelpersTest.js | 41 ++++++- 3 files changed, 242 insertions(+), 15 deletions(-) diff --git a/lib/metadata/policies.js b/lib/metadata/policies.js index f1c9d7d..82ce5b7 100644 --- a/lib/metadata/policies.js +++ b/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": "

Yellow Lab Tools failed to parse a CSS file. I doubt the problem comes from the css parser.

Maybe a CSS validator can help you.

", "isOkThreshold": 0, "isBadThreshold": 1, - "isAbnormalThreshold": 1 + "isAbnormalThreshold": 1, + "offendersTransformFn": function(offenders) { + return offenders.map(function(offender) { + var parts = /^(([^ \(]*|) ?)?(?:\((((?! @ ).)*)( @ (\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>' : ' ' + offendersHelpers.urlToLink(parts[2]); + } + + if (parts[5]) { + html += ' @ ' + parts[6] + ':' + parts[7]; + } + + if (parts[2] && parts[2] !== '') { + var w3cDirectUrl = 'http://jigsaw.w3.org/css-validator/validator?uri=' + encodeURIComponent(parts[2]) + '&profile=css3&usermedium=all&warning=no'; + html += ' (Check on the W3C validator)'; + } + + return html; + }); + } }, "cssRules": { "tool": "phantomas", @@ -281,7 +308,14 @@ var policies = { "message": "

Complex selectors are CSS selectors with 4 or more expressions, like \"#header ul li .foo\".

They are adding more work for the browser, and this could be avoided by simplifying selectors.

", "isOkThreshold": 0, "isBadThreshold": 500, - "isAbnormalThreshold": 2000 + "isAbnormalThreshold": 2000, + "offendersTransformFn": function(offenders) { + return offenders.map(function(offender) { + var splittedOffender = offendersHelpers.cssOffenderPattern(offender); + + return '' + splittedOffender.offender + ' @ ' + splittedOffender.line + ':' + splittedOffender.character; + }); + } }, "cssComplexSelectorsByAttribute": { "tool": "phantomas", @@ -289,7 +323,16 @@ var policies = { "message": "

Complex attributes selectors are one of these:

  • .foo[type*=bar] (contains bar)
  • .foo[type^=bar] (starts with bar)
  • .foo[type|=bar] (starts with bar or bar-)
  • .foo[type$=bar] (ends with bar)
  • .foo[type~=bar baz] (bar or baz)

Their matching process needs more CPU and it has a cost on performances.

", "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, '$1'); + + return boldedOffender + ' @ ' + splittedOffender.line + ':' + splittedOffender.character; + }); + } }, "cssImports": { "tool": "phantomas", @@ -297,7 +340,14 @@ var policies = { "message": "

It’s bad for performance to use @import because CSS files don't get downloaded in parallel.

You should use <link rel='stylesheet' href='a.css'> instead.

", "isOkThreshold": 0, "isBadThreshold": 1, - "isAbnormalThreshold": 1 + "isAbnormalThreshold": 1, + "offendersTransformFn": function(offenders) { + return offenders.map(function(offender) { + var splittedOffender = offendersHelpers.cssOffenderPattern(offender); + + return '' + splittedOffender.offender + ' @ ' + splittedOffender.line + ':' + splittedOffender.character; + }); + } }, "cssDuplicatedSelectors": { "tool": "phantomas", @@ -305,7 +355,19 @@ var policies = { "message": "

This is when two or more selectors are strictly identical and should be merged.

", "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] + ' (x' + parts[2] + ')'; + }); + } }, "cssDuplicatedProperties": { "tool": "phantomas", @@ -313,7 +375,21 @@ var policies = { "message": "

This is the number of property definitions duplicated within a selector.

", "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 ' + parts[2] + ' duplicated in ' + parts[1] + ' { } @ ' + splittedOffender.line + ':' + splittedOffender.character; + }); + } }, "cssEmptyRules": { "tool": "phantomas", @@ -321,7 +397,13 @@ var policies = { "message": "

Very easy to fix: remove all empty rules.

", "isOkThreshold": 0, "isBadThreshold": 40, - "isAbnormalThreshold": 100 + "isAbnormalThreshold": 100, + "offendersTransformFn": function(offenders) { + return offenders.map(function(offender) { + var splittedOffender = offendersHelpers.cssOffenderPattern(offender); + return '' + splittedOffender.offender + ' { } @ ' + splittedOffender.line + ':' + splittedOffender.character; + }); + } }, "cssExpressions": { "tool": "phantomas", @@ -329,7 +411,21 @@ var policies = { "message": "

Such as: expression( document.body.clientWidth > 600 ? \"600px\" : \"auto\" )

This is a bad practice as it slows down browsers. There are some simpler CSS3 methods for doing this.

", "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] + ': expression(' + parts[3] + ')} @ ' + splittedOffender.line + ':' + splittedOffender.character; + }); + } }, "cssImportants": { "tool": "phantomas", @@ -337,7 +433,21 @@ var policies = { "message": "

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.

", "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] + ' !important} @ ' + splittedOffender.line + ':' + splittedOffender.character; + }); + } }, "cssOldIEFixes": { "tool": "phantomas", @@ -345,7 +455,49 @@ var policies = { "message": "

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.

IE6:

  • * html
  • html > body (everything but IE6)

IE7:

  • *height: 123px;
  • height: 123px !ie;

IE9:

  • -ms-filter
  • progid:DXImageTransform.Microsoft

", "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/, '* html'); + browser = 'IE6'; + } else if (rule.indexOf('html>body') === 0) { + rule = rule.replace(/^html>body/, 'html>body'); + browser = 'IE6'; + } else if (property.indexOf('*') === 0) { + property = '' + property + ''; + browser = 'IE7'; + } else if (value.match(/\!ie$/)) { + value = value.replace(/\!ie$/, '!ie'); + browser = 'IE7'; + } else if (property === '-ms-filter') { + property = '-ms-filter'; + browser = 'IE9'; + } else if (value.indexOf('progid:DXImageTransform.Microsoft') === 0) { + value = value.replace(/^progid:DXImageTransform\.Microsoft/, 'progid:DXImageTransform.Microsoft'); + browser = 'IE9'; + } + + browser = browser ? '' + browser + ' fix: ' : ''; + var propertyAndValue = (property && value) ? ' {' + property + ': ' + value + '}' : ''; + + return browser + rule + propertyAndValue + ' @ ' + splittedOffender.line + ':' + splittedOffender.character; + }); + } }, "cssOldPropertyPrefixes": { "tool": "phantomas", @@ -353,7 +505,21 @@ var policies = { "message": "

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.

", "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 '
' + parts[1] + ' {' + parts[2] + ': ' + parts[3] + '} @ ' + splittedOffender.line + ':' + splittedOffender.character + '
' + parts[4] + '
'; + }); + } }, "cssUniversalSelectors": { "tool": "phantomas", diff --git a/lib/offendersHelpers.js b/lib/offendersHelpers.js index 39a048f..66b11f3 100644 --- a/lib/offendersHelpers.js +++ b/lib/offendersHelpers.js @@ -136,9 +136,10 @@ var OffendersHelpers = function() { } var html = '
backtrace
'; + var that = this; backtraceArray.forEach(function(backtraceObj) { var functionName = (backtraceObj.functionName) ? backtraceObj.functionName + '() ' : ''; - html += '
' + functionName + '' + backtraceObj.file + ' line ' + backtraceObj.line + '
'; + html += '
' + functionName + that.urlToLink(backtraceObj.file) + ' line ' + backtraceObj.line + '
'; }); return html + '
'; }; @@ -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 '' + shortUrl + ''; + }; + + 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(); \ No newline at end of file diff --git a/test/core/offendersHelpersTest.js b/test/core/offendersHelpersTest.js index ce23d07..072d2d1 100644 --- a/test/core/offendersHelpersTest.js +++ b/test/core/offendersHelpersTest.js @@ -175,7 +175,7 @@ describe('offendersHelpers', function() { } ]); - result.should.equal('
backtrace
callback() http://pouet.com/js/main.js line 1
'); + result.should.equal('
backtrace
callback() http://pouet.com/js/main.js line 1
'); }); 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('http://www.google.com/js/main.js'); + }); + + it('should ellypsis the url if too long', function() { + var result = offendersHelpers.urlToLink('http://www.google.com/js/longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong/main.js'); + + result.should.equal('http://www.google.com/js/longlonglonglonglonglo ... longlonglonglonglonglonglonglonglonglong/main.js'); + }); + + }); + + + 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' + }); + }); + + }); + });