Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Refactor into classes and add faster caching with options.import #22

Merged
merged 2 commits into from

2 participants

@jgable
Owner

Closes #20

  • Move logic for parsing and linting into new LessFile class
  • Extend LessFile as LessCachedFile with caching specific implementation
  • Move non grunt task files to tasks/lib to prevent being read by grunt
  • Combine the imports contents with less file contents when calculating cache hash key
  • Add extra tests for imports invalidation in less-file-spec.coffee

I know this is a big refactor, but the less-lint-task.coffee file was getting a little cramped. I moved the parsing and linting logic into classes that make it a little easier to unit test without grunt, but still kept all the existing unit tests. I'm hoping a subsequent PR will move the rest of the disparate error outputting functions to another class/file, but I feel like this is probably big enough for now.

Would really appreciate a review from @cvn and @kevinsawicki before I merge this in.

@jgable
Owner

Gonna merge this today if no feedback. Have a plan to implement custom css lint rules today if possible.

@cvn
cvn commented

Hey @jgable I'm gonna check this out and test today, was too busy to do it during the week.

@cvn
cvn commented

I'm getting errors when caching is enabled. If I turn caching off, I don't get the errors, and the first time I ran it with caching on, it was fine too, but every time after that, I get the same output.

Running "lesslint:src" (lesslint) task
src/pages/404/404.less (0)
undefined
src/pages/conversion/style.less (0)
src/pages/home/home.less (0)
undefined
src/pages/my-account/create.less (0)
src/pages/my-account/my-account.less (0)
src/pages/my-services/my-services-subpage.less (0)
undefined
src/pages/payment/payment-wizard.less (0)
undefined
undefined
src/pages/search-results/search-results.less (0)
src/pages/security-suite/security-suite.less (0)
src/pages/support/article.less (0)
undefined
src/pages/support/overview.less (0)
src/pages/tv-asset-details/tv-asset-details.less (0)
src/pages/tv/tv-live.less (0)
src/pages/tv/tv.less (0)
src/pages/user-preferences/user-preferences.less (0)
src/less/x-net-pages.less (0)

>> 6 lint errors in 22 files.
Warning: Task "lesslint:src" failed. Use --force to continue.

Aborted due to warnings.

Here is the verbose version up to the first "undefined". The rest of the output looks very similar.

Running "lesslint:src" (lesslint) task
Verifying property lesslint.src exists in config...OK
Files: src/pages/404/404.less, src/pages/billing-and-transactions/billing-and-transactions.less, src/pages/conversion/style.less, src/pages/home/home.less, src/pages/login/login.less, src/pages/my-account/create.less, src/pages/my-account/my-account.less, src/pages/my-services/my-services-subpage.less, src/pages/my-services/my-services.less, src/pages/payment/payment-wizard.less, src/pages/recover-id/recover-id.less, src/pages/reset-password/reset-password.less, src/pages/search-results/search-results.less, src/pages/security-suite/security-suite.less, src/pages/support/article.less, src/pages/support/locations.less, src/pages/support/overview.less, src/pages/tv-asset-details/tv-asset-details.less, src/pages/tv/tv-live.less, src/pages/tv/tv.less, src/pages/user-preferences/user-preferences.less, src/less/x-net-pages.less -> src
Options: less=undefined, csslint={"csslintrc":".csslintrc"}, imports=["src/less/**/*.less","src/pages/**/*.less","src/features/**/*.less"], cache
Linting 'src/pages/404/404.less'Reading src/pages/404/404.less...OK
Reading src/less/_util.less...OK
Reading src/less/x-net-pages.less...OK
Reading src/pages/404/404.less...OK
Reading src/pages/billing-and-transactions/billing-and-transactions.less...OK
Reading src/pages/conversion/style.less...OK
Reading src/pages/home/home.less...OK
Reading src/pages/home/homepage-customize/homepage-customize.less...OK
Reading src/pages/home/tv-widgets/primetime-tonight/primetime-tonight.less...OK
Reading src/pages/home/tv-widgets/tv-everywhere/tv-everywhere.less...OK
Reading src/pages/home/widget-shelf/billing-widget/billing-widget.less...OK
Reading src/pages/home/widget-shelf/email-widget/email-widget.less...OK
Reading src/pages/home/widget-shelf/voicemail-widget/voicemail-widget.less...OK
Reading src/pages/home/widget-shelf/widget-shelf.less...OK
Reading src/pages/login/login.less...OK
Reading src/pages/my-account/create.less...OK
Reading src/pages/my-account/less/_billing-transactions.less...OK
Reading src/pages/my-account/less/_login.less...OK
Reading src/pages/my-account/less/_page.less...OK
Reading src/pages/my-account/less/_services-equipment.less...OK
Reading src/pages/my-account/less/_user-preferences-overlay.less...OK
Reading src/pages/my-account/less/_user-preferences.less...OK
Reading src/pages/my-account/my-account.less...OK
Reading src/pages/my-services/less/_internet.less...OK
Reading src/pages/my-services/less/_my-services-shelf.less...OK
Reading src/pages/my-services/less/_phone.less...OK
Reading src/pages/my-services/less/_tv.less...OK
Reading src/pages/my-services/my-services-subpage.less...OK
Reading src/pages/my-services/my-services.less...OK
Reading src/pages/payment/payment-wizard.less...OK
Reading src/pages/recover-id/recover-id.less...OK
Reading src/pages/reset-password/reset-password.less...OK
Reading src/pages/search-results/search-results.less...OK
Reading src/pages/security-suite/less/_cta.less...OK
Reading src/pages/security-suite/less/_devices.less...OK
Reading src/pages/security-suite/less/_features.less...OK
Reading src/pages/security-suite/less/_info.less...OK
Reading src/pages/security-suite/less/_page.less...OK
Reading src/pages/security-suite/less/_requirements.less...OK
Reading src/pages/security-suite/modals/_modals.less...OK
Reading src/pages/security-suite/security-suite.less...OK
Reading src/pages/security-suite/shelf/_shelf.less...OK
Reading src/pages/support/article.less...OK
Reading src/pages/support/less/_support-video.less...OK
Reading src/pages/support/less/_support-videos-carousel.less...OK
Reading src/pages/support/locations.less...OK
Reading src/pages/support/overview.less...OK
Reading src/pages/tv-asset-details/less/primary-content.less...OK
Reading src/pages/tv-asset-details/tv-asset-details.less...OK
Reading src/pages/tv/less/guide-filter.less...OK
Reading src/pages/tv/less/guide-grid.less...OK
Reading src/pages/tv/less/guide-navigation.less...OK
Reading src/pages/tv/less/guide-program-details.less...OK
Reading src/pages/tv/live/less/base.less...OK
Reading src/pages/tv/live/less/grid-view.less...OK
Reading src/pages/tv/live/less/list-view.less...OK
Reading src/pages/tv/live/live.less...OK
Reading src/pages/tv/live/modal-parental-restriction/_modal-parental-restriction.less...OK
Reading src/pages/tv/tv-live.less...OK
Reading src/pages/tv/tv.less...OK
Reading src/pages/user-preferences/user-preferences.less...OK
Reading src/features/cast-details-modal/cast-details-modal.less...OK
Reading src/features/download-manager/download-manager-modal.less...OK
Reading src/features/episodes-carousel/episodes-carousel.less...OK
Reading src/features/expanded-alert/expanded-alert.less...OK
Reading src/features/hero/hero.less...OK
Reading src/features/image-slideshow-modal/image-slideshow-modal.less...OK
Reading src/features/login-modal/login-modal.less...OK
Reading src/features/login/login.less...OK
Reading src/features/modal/modal.less...OK
Reading src/features/more-like-this/more-like-this.less...OK
Reading src/features/paperless-billing/paperless-billing.less...OK
Reading src/features/promotions/promotions.less...OK
Reading src/features/recaptcha/recaptcha.less...OK
Reading src/features/record-asset-modal/record-asset-modal.less...OK
Reading src/features/recovery/recovery.less...OK
Reading src/features/send-asset-modal/send-asset-modal.less...OK
Reading src/features/simple-example-modal/simple-example-modal.less...OK
Reading src/features/tv-listings-modal/tv-listings-modal.less...OK
Reading src/features/video-player-modal/video-player-modal.less...OK
Reading src/features/watch-list/watch-list.less...OK
Reading .csslintrc...OK
Parsing .csslintrc...OK
src/pages/404/404.less (0)
Linting 'src/pages/billing-and-transactions/billing-and-transactions.less'Reading src/pages/billing-and-transactions/billing-and-transactions.less...OK
Reading src/less/_util.less...OK
Reading src/less/x-net-pages.less...OK
Reading src/pages/404/404.less...OK
Reading src/pages/billing-and-transactions/billing-and-transactions.less...OK
Reading src/pages/conversion/style.less...OK
Reading src/pages/home/home.less...OK
Reading src/pages/home/homepage-customize/homepage-customize.less...OK
Reading src/pages/home/tv-widgets/primetime-tonight/primetime-tonight.less...OK
Reading src/pages/home/tv-widgets/tv-everywhere/tv-everywhere.less...OK
Reading src/pages/home/widget-shelf/billing-widget/billing-widget.less...OK
Reading src/pages/home/widget-shelf/email-widget/email-widget.less...OK
Reading src/pages/home/widget-shelf/voicemail-widget/voicemail-widget.less...OK
Reading src/pages/home/widget-shelf/widget-shelf.less...OK
Reading src/pages/login/login.less...OK
Reading src/pages/my-account/create.less...OK
Reading src/pages/my-account/less/_billing-transactions.less...OK
Reading src/pages/my-account/less/_login.less...OK
Reading src/pages/my-account/less/_page.less...OK
Reading src/pages/my-account/less/_services-equipment.less...OK
Reading src/pages/my-account/less/_user-preferences-overlay.less...OK
Reading src/pages/my-account/less/_user-preferences.less...OK
Reading src/pages/my-account/my-account.less...OK
Reading src/pages/my-services/less/_internet.less...OK
Reading src/pages/my-services/less/_my-services-shelf.less...OK
Reading src/pages/my-services/less/_phone.less...OK
Reading src/pages/my-services/less/_tv.less...OK
Reading src/pages/my-services/my-services-subpage.less...OK
Reading src/pages/my-services/my-services.less...OK
Reading src/pages/payment/payment-wizard.less...OK
Reading src/pages/recover-id/recover-id.less...OK
Reading src/pages/reset-password/reset-password.less...OK
Reading src/pages/search-results/search-results.less...OK
Reading src/pages/security-suite/less/_cta.less...OK
Reading src/pages/security-suite/less/_devices.less...OK
Reading src/pages/security-suite/less/_features.less...OK
Reading src/pages/security-suite/less/_info.less...OK
Reading src/pages/security-suite/less/_page.less...OK
Reading src/pages/security-suite/less/_requirements.less...OK
Reading src/pages/security-suite/modals/_modals.less...OK
Reading src/pages/security-suite/security-suite.less...OK
Reading src/pages/security-suite/shelf/_shelf.less...OK
Reading src/pages/support/article.less...OK
Reading src/pages/support/less/_support-video.less...OK
Reading src/pages/support/less/_support-videos-carousel.less...OK
Reading src/pages/support/locations.less...OK
Reading src/pages/support/overview.less...OK
Reading src/pages/tv-asset-details/less/primary-content.less...OK
Reading src/pages/tv-asset-details/tv-asset-details.less...OK
Reading src/pages/tv/less/guide-filter.less...OK
Reading src/pages/tv/less/guide-grid.less...OK
Reading src/pages/tv/less/guide-navigation.less...OK
Reading src/pages/tv/less/guide-program-details.less...OK
Reading src/pages/tv/live/less/base.less...OK
Reading src/pages/tv/live/less/grid-view.less...OK
Reading src/pages/tv/live/less/list-view.less...OK
Reading src/pages/tv/live/live.less...OK
Reading src/pages/tv/live/modal-parental-restriction/_modal-parental-restriction.less...OK
Reading src/pages/tv/tv-live.less...OK
Reading src/pages/tv/tv.less...OK
Reading src/pages/user-preferences/user-preferences.less...OK
Reading src/features/cast-details-modal/cast-details-modal.less...OK
Reading src/features/download-manager/download-manager-modal.less...OK
Reading src/features/episodes-carousel/episodes-carousel.less...OK
Reading src/features/expanded-alert/expanded-alert.less...OK
Reading src/features/hero/hero.less...OK
Reading src/features/image-slideshow-modal/image-slideshow-modal.less...OK
Reading src/features/login-modal/login-modal.less...OK
Reading src/features/login/login.less...OK
Reading src/features/modal/modal.less...OK
Reading src/features/more-like-this/more-like-this.less...OK
Reading src/features/paperless-billing/paperless-billing.less...OK
Reading src/features/promotions/promotions.less...OK
Reading src/features/recaptcha/recaptcha.less...OK
Reading src/features/record-asset-modal/record-asset-modal.less...OK
Reading src/features/recovery/recovery.less...OK
Reading src/features/send-asset-modal/send-asset-modal.less...OK
Reading src/features/simple-example-modal/simple-example-modal.less...OK
Reading src/features/tv-listings-modal/tv-listings-modal.less...OK
Reading src/features/video-player-modal/video-player-modal.less...OK
Reading src/features/watch-list/watch-list.less...OK
undefined
@jgable Refactor into classes
- Move logic for parsing and linting into new LessFile class
- Extend LessFile as LessCachedFile with caching specific implementation
- Move non grunt task files to tasks/lib to prevent being read by grunt
- Add extra tests for imports invalidation in less-file-spec.coffee
30e4162
@jgable
Owner

Ahh... I found the issue, hasCached doesn't pass back an error (it's just an fs.exists). I updated this PR with the new code, might have to use a git reset --hard or something since I force pushed up to update this PR without an extra commit.

@cvn
cvn commented

OK, it works now. The task runs fine with caching on now, and it successfully detects changes to files in imports: []. In my environment, enabling caching actually slows down lesslint by about .1s (on a ~6.5s task) on average. This is the case even when I have changed nothing at all in between runs.

@jgable Add caching to import file lookups
- Create new LessImportFile class
  - Overrides getContents and getDigest to look in a shared contents
  lookup for contents and digests done in previous runs
26e802f
@jgable
Owner

Ahh, yeah we are reading and hashing each import every time for every file. We could cache the contents for later files and speed it up a little.

I've updated with another commit (26e802f) that will keep the subsequent lookups from reading from disk and calculating the hash all over again.

Thanks for testing everything out! I think this is almost ready to merge.

@jgable
Owner

Just tested this out with about 77 files in our project. Changing any files in the imports: [] causes relying files to lint without cache (around 5s) and subsequent runs with no changes are sub second again.

@jgable jgable merged commit b84960a into from
@jgable
Owner

Now published as version 1.0.0 on npm. I figured the large amount of changes was worth a major update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 2, 2014
  1. Refactor into classes

    authored
    - Move logic for parsing and linting into new LessFile class
    - Extend LessFile as LessCachedFile with caching specific implementation
    - Move non grunt task files to tasks/lib to prevent being read by grunt
    - Add extra tests for imports invalidation in less-file-spec.coffee
  2. Add caching to import file lookups

    authored
    - Create new LessImportFile class
      - Overrides getContents and getDigest to look in a shared contents
      lookup for contents and digests done in previous runs
This page is out of date. Refresh to see the latest.
View
4 Gruntfile.coffee
@@ -6,7 +6,7 @@ module.exports = (grunt) ->
glob_to_multiple:
expand: true
cwd: 'src'
- src: ['*.coffee']
+ src: ['**/*.coffee']
dest: 'tasks'
ext: '.js'
@@ -34,5 +34,5 @@ module.exports = (grunt) ->
grunt.loadNpmTasks('grunt-shell')
grunt.registerTask 'clean', -> require('rimraf').sync('tasks')
grunt.registerTask('lint', ['coffeelint'])
- grunt.registerTask('test', ['default', 'shell:test'])
+ grunt.registerTask('test', ['default', 'lesslint:clearCache', 'shell:test'])
grunt.registerTask('default', ['lint', 'coffee'])
View
145 spec/less-file-spec.coffee
@@ -0,0 +1,145 @@
+path = require 'path'
+grunt = require 'grunt'
+{LessFile, LessImportFile, LessCachedFile} = require '../tasks/lib/less-file'
+
+describe 'less-file', ->
+ describe 'LessFile', ->
+ filePath = path.join(__dirname, 'fixtures', 'valid.less')
+ file = null
+
+ beforeEach ->
+ file = new LessFile(filePath, {}, grunt)
+
+ it 'can load contents', ->
+ contents = file.getContents()
+ expect(contents).toBe grunt.file.read(filePath)
+
+ it 'can get a hash', ->
+ hash = file.getDigest()
+ expect(hash).toNotBe null
+ expect(hash.length).toBeGreaterThan 0
+
+ it 'can lint a file', (done) ->
+ file.lint (err, result, less, css) ->
+ expect(err).toBe null
+ expect(result).toBe undefined
+ expect(less).toNotBe undefined
+ expect(less.length).toBeGreaterThan 0
+ expect(css).toNotBe undefined
+ expect(css.length).toBeGreaterThan 0
+
+ done()
+
+ describe 'LessImportFile', ->
+ filePath = path.join(__dirname, 'fixtures', 'valid.less')
+ file = null
+
+ beforeEach ->
+ file = new LessImportFile(filePath, {}, grunt)
+
+ it 'does not read from disk if already loaded before', ->
+
+ spyOn(grunt.file, 'read').andCallFake (filePath) -> 'some fake file content'
+
+ hash = file.getDigest()
+
+ expect(grunt.file.read).toHaveBeenCalled()
+
+ otherFile = new LessImportFile(filePath, {}, grunt)
+ otherHash = otherFile.getDigest()
+
+ # There is no toHaveBeenCalled 1, so we check the calls length here
+ expect(grunt.file.read.calls.length).toBe 1
+ expect(hash).toBe otherHash
+
+ describe 'LessCachedFile', ->
+ filePath = path.join(__dirname, 'fixtures', 'valid.less')
+ file = null
+
+ beforeEach ->
+ file = new LessCachedFile(filePath, {}, grunt)
+
+ it 'can load contents', ->
+ contents = file.getContents()
+ expect(contents).toBe grunt.file.read(filePath)
+
+ it 'can get a hash', ->
+ hash = file.getDigest()
+ expect(hash).toNotBe null
+ expect(hash.length).toBeGreaterThan 0
+
+ it 'can lint a file', (done) ->
+ # See if we read the file
+ spyOn(file, 'getCss').andCallThrough()
+ # Force no caching
+ spyOn(file.cache, 'hasCached').andCallFake (hash, cb) -> cb(false)
+ spyOn(file.cache, 'addCached').andCallFake (hash, cb) -> cb(null)
+
+ file.lint (err, result, less, css) ->
+ expect(file.getCss).toHaveBeenCalled()
+ expect(err).toBe null
+ expect(result).toBe undefined
+ expect(less).toNotBe undefined
+ expect(less.length).toBeGreaterThan 0
+ expect(css).toNotBe undefined
+ expect(css.length).toBeGreaterThan 0
+
+ done()
+
+ it 'does not parse less if previously cached successful run', (done) ->
+ # See if we read the file
+ spyOn(file, 'getCss').andCallThrough()
+ # Force cache hit
+ spyOn(file.cache, 'hasCached').andCallFake (hash, cb) -> cb(true)
+ spyOn(file.cache, 'addCached').andCallFake (hash, cb) -> cb(null)
+
+ file.lint (err, result, less, css) ->
+ expect(file.getCss).not.toHaveBeenCalled()
+
+ done()
+
+ it 'uses imports from config option as a cache key so changes in import files cause re-linting', (done) ->
+ file.options.imports = ['spec/fixtures/file.less']
+ # See if we read the file
+ spyOn(file, 'getCss').andCallThrough()
+
+ hashKey = null
+ spyOn(file.cache, 'hasCached').andCallFake (hash, cb) ->
+ hashKey = hash
+ cb false
+
+ spyOn(file.cache, 'addCached').andCallFake (hash, cb) -> cb(null)
+
+ # Stub the getContents so we can change it on subsequent runs through
+ contentsCalls = 0
+ spyOn(file, 'getImportsContents').andCallFake ->
+ contentsCalls += 1
+ ["body { margin: #{contentsCalls}px; }"]
+
+ file.lint (err, result, less, css) ->
+ expect(file.getCss).toHaveBeenCalled()
+ expect(contentsCalls).toBe 1
+ expect(err).toBe null
+ expect(result).toBe undefined
+
+ otherFile = new LessCachedFile(filePath, {}, grunt)
+ otherFile.options.imports = ['spec/fixtures/file.less']
+
+ spyOn(otherFile, 'getCss').andCallThrough()
+
+ otherHashKey = null
+ spyOn(otherFile.cache, 'hasCached').andCallFake (hash, cb) ->
+ otherHashKey = hash
+ cb false
+
+ spyOn(otherFile.cache, 'addCached').andCallFake (hash, cb) -> cb(null)
+ spyOn(otherFile, 'getImportsContents').andCallFake ->
+ contentsCalls += 1
+ ["body { margin: #{contentsCalls}px; }"]
+
+ otherFile.lint (err, otherResult, less, css) ->
+ expect(otherFile.getCss).toHaveBeenCalled()
+ expect(contentsCalls).toBe 2
+ expect(otherHashKey).not.toBe hashKey
+
+ done()
View
32 spec/less-lint-task-spec.coffee
@@ -5,7 +5,7 @@ crypto = require 'crypto'
grunt = require 'grunt'
tmp = require 'tmp'
{parseString} = require 'xml2js'
-{LintCache} = require '../tasks/lint-cache'
+{LintCache} = require '../tasks/lib/lint-cache'
_ = grunt.util._
@@ -273,7 +273,19 @@ describe 'LESS Lint task', ->
grunt.loadTasks(path.resolve(__dirname, '..', 'tasks'))
taskCount = 0
tasksDone = false
+
addCacheHash = null
+ cacheHits = 0
+ addCacheHit = (filePath, cachePath) ->
+ cacheHits += 1
+ grunt.event.on 'lesslint.cache.hit', addCacheHit
+
+ cacheAdds = 0
+ addCacheAdd = (filePath, cacheHash) ->
+ cacheAdds += 1
+ addCacheHash = cacheHash
+ grunt.event.on 'lesslint.cache.add', addCacheAdd
+
grunt.registerTask 'done', 'done', ->
taskCount++
@@ -284,22 +296,20 @@ describe 'LESS Lint task', ->
output = []
spyOn(process.stdout, 'write').andCallFake (data='') ->
output.push(data.toString())
- spyOn(LintCache.prototype, 'hasCached').andCallFake (hash, done) ->
- # only return false the first time
- done(taskCount == 1)
- spyOn(LintCache.prototype, 'addCached').andCallFake (hash, done) ->
- addCacheHash = hash
- done()
+
grunt.task.run(['lesslint', 'done']).start()
waitsFor -> tasksDone
runs ->
+ # Remove the event listeners
+ grunt.event.off 'lesslint.cache.hit', addCacheHit
+ grunt.event.off 'lesslint.cache.add', addCacheAdd
+
taskOutput = output.join('')
expect(taskOutput).toContain '1 file lint free'
# should be called both times
- expect(LintCache.prototype.hasCached.callCount).toBe(2)
+ expect(cacheHits).toBe(1)
# should only be called the first time
- expect(LintCache.prototype.addCached.callCount).toBe(1)
+ expect(cacheAdds).toBe(1)
- less = grunt.file.read(path.join(__dirname, 'fixtures', 'valid.less'))
expect(addCacheHash).toNotEqual null
- expect(addCacheHash).toNotEqual ''
+ expect(addCacheHash).toNotEqual ''
View
2  spec/lint-utils-spec.coffee
@@ -1,6 +1,6 @@
fs = require 'fs'
path = require 'path'
-linter = require '../tasks/lint-utils'
+linter = require '../tasks/lib/lint-utils'
describe 'lint-utils', ->
describe '.findLessMapping()', ->
View
202 src/less-lint-task.coffee
@@ -1,7 +1,8 @@
{CSSLint} = require 'csslint'
{Parser} = require 'less'
-{findLessMapping, findPropertyLineNumber, getPropertyName} = require './lint-utils'
-{LintCache} = require './lint-cache'
+{findLessMapping, findPropertyLineNumber, getPropertyName} = require './lib/lint-utils'
+{LintCache} = require './lib/lint-cache'
+{LessFile, LessCachedFile} = require './lib/less-file'
async = require 'async'
path = require 'path'
crypto = require 'crypto'
@@ -16,50 +17,7 @@ defaultLessOptions =
module.exports = (grunt) ->
{stripPath} = require('grunt-lib-contrib').init grunt
- parseLess = (file, less, options, callback) ->
- configLessOptions = options.less ? grunt.config.get('less.options')
- lessOptions = grunt.util._.extend({filename: file}, configLessOptions, defaultLessOptions)
-
- if less
- parser = new Parser(lessOptions)
- try
- parser.parse less, (error, tree) ->
- if error?
- callback(error)
- else
- callback(null, less, tree.toCSS())
- catch error
- callback(error)
- else
- callback(null, '', '')
-
- lintCss = (css, options, callback) ->
- unless css
- callback(null, [])
- return
-
- rules = {}
- externalOptions = {}
- CSSLint.getRules().forEach ({id}) -> rules[id] = 1
-
- cssLintOptions = options.csslint ? grunt.config.get('csslint.options')
- if cssLintOptions?.csslintrc
- externalOptions = grunt.file.readJSON cssLintOptions.csslintrc
- delete cssLintOptions.csslintrc
-
- grunt.util._.extend(cssLintOptions, externalOptions)
-
- for id, enabled of cssLintOptions
- if cssLintOptions[id]
- rules[id] = cssLintOptions[id]
- else
- delete rules[id]
-
- result = CSSLint.verify(css, rules)
- if result.messages?.length > 0
- callback(null, result)
- else
- callback()
+ _ = grunt.util._
originalPositionFor = (css, less, file, line) ->
cssLines = css.split('\n')
@@ -92,7 +50,7 @@ module.exports = (grunt) ->
writeToFormatters = (options, results) ->
formatters = options.formatters
- return unless grunt.util._.isArray(formatters)
+ return unless _.isArray(formatters)
formatters.forEach ({id, dest}) ->
return unless id and dest
@@ -106,12 +64,58 @@ module.exports = (grunt) ->
formatterOutput += formatter.endFormat()
grunt.file.write(dest, formatterOutput)
+ # TODO: Refactor to a class somewhere
+ processLintErrors = (file, importsToLint, result, less, css) ->
+ messages = result.messages ? []
+ messages = messages.filter (message) ->
+ isFileError(file, css, message.line - 1, importsToLint)
+
+ fileErrors = 0
+
+ grunt.log.writeln("#{file.yellow} (#{messages.length})")
+
+ messages = _.groupBy messages, ({message}) -> message
+ for ruleMessage, ruleMessages of messages
+ rule = ruleMessages[0].rule
+ fullRuleMessage = "#{ruleMessage} "
+ fullRuleMessage += "#{rule.desc} " if rule.desc and rule.desc isnt ruleMessage
+ grunt.log.writeln(fullRuleMessage + "(#{rule.id})".grey)
+
+ for message in ruleMessages
+ line = message.line
+ line--
+ fileErrors++
+ continue if line < 0
+
+ {lineNumber, filePath, less} = originalPositionFor(css, less, file, line)
+
+ if lineNumber >= 0
+ message.line = lineNumber
+ errorPrefix = "#{stripPath(filePath, process.cwd())} #{lineNumber + 1}:".yellow
+
+ grunt.log.error("#{errorPrefix} #{less.split('\n')[lineNumber].trim()}")
+ else
+ cssLine = css.split('\n')[line]
+ if cssLine?
+ errorPrefix = "#{line + 1}:".yellow
+ grunt.log.error("#{errorPrefix} #{cssLine.trim()}")
+
+ grunt.log.writeln("Failed to find map CSS line #{line + 1} to a LESS line.".yellow)
+
+ fileErrors
+
grunt.registerMultiTask 'lesslint', 'Validate LESS files with CSS Lint', ->
options = @options
+ # Default to the less task options
+ less: grunt.config.get('less.options')
+ # Default to csslint task options
+ csslint: grunt.config.get('csslint.options')
+ # Default to no imports
+ imports: []
+ # Default to no caching
cache: false
- importsToLint = options.imports ? []
fileCount = 0
errorCount = 0
results = {}
@@ -120,83 +124,25 @@ module.exports = (grunt) ->
grunt.verbose.write("Linting '#{file}'")
fileCount++
- less = grunt.file.read(file)
- # Bug out early if no less content
- return callback() unless less
-
- if options.cache
- cache = new LintCache(options.cache)
+ unless options.cache
+ lessFile = new LessFile(file, options, grunt)
+ else
+ lessFile = new LessCachedFile(file, options, grunt)
- # Parse the less always because imports could have changed
- parseLess file, less, options, (error, less, css) ->
- if error?
+ lessFile.lint (err, lintResult, less, css) ->
+ if err?
errorCount++
- grunt.log.writeln("Error parsing #{file.yellow}")
- grunt.log.writeln(error.message)
- callback()
- return
-
- # Takes the css and (optional) hash and processes them with CSSLint,
- # made into a function because we may need to make an async call to cache.hasCached
- processCss = (css, hash) ->
- lintCss css, options, (error, result={}) ->
- messages = result.messages ? []
- messages = messages.filter (message) ->
- isFileError(file, css, message.line - 1, importsToLint)
-
- if messages.length > 0
- results[file] = result
- grunt.log.writeln("#{file.yellow} (#{messages.length})")
-
- messages = grunt.util._.groupBy messages, ({message}) -> message
- for ruleMessage, ruleMessages of messages
- rule = ruleMessages[0].rule
- fullRuleMessage = "#{ruleMessage} "
- fullRuleMessage += "#{rule.desc} " if rule.desc and rule.desc isnt ruleMessage
- grunt.log.writeln(fullRuleMessage + "(#{rule.id})".grey)
-
- for message in ruleMessages
- line = message.line
- line--
- errorCount++
- continue if line < 0
-
- {lineNumber, filePath, less} = originalPositionFor(css, less, file, line)
-
- if lineNumber >= 0
- message.line = lineNumber
- errorPrefix = "#{stripPath(filePath, process.cwd())} #{lineNumber + 1}:".yellow
-
- grunt.log.error("#{errorPrefix} #{less.split('\n')[lineNumber].trim()}")
- else
- cssLine = css.split('\n')[line]
- if cssLine?
- errorPrefix = "#{line + 1}:".yellow
- grunt.log.error("#{errorPrefix} #{cssLine.trim()}")
-
- grunt.log.writeln("Failed to find map CSS line #{line + 1} to a LESS line.".yellow)
- else if options.cache
- # Add the originally hashed less file to the cached successes
- return cache.addCached hash, (error) ->
- if error?
- grunt.log.writeln "Error cacheing result: #{file.yellow}"
- callback()
-
- callback()
-
- if options.cache
- # Cache based on generated css instead of just less content
- hash = crypto.createHash('md5').update(css).digest('base64')
-
- cache.hasCached hash, (isCached) ->
- # Bug out early if we've already linted this file successfully before
- if isCached
- grunt.verbose.writeln "Skipping previously linted file: #{file.green}"
- return callback()
-
- processCss css, hash
- else
- processCss css
+ grunt.log.writeln(err.message)
+ return callback()
+
+ if lintResult
+ # Save for later
+ results[file] = lintResult
+ # Show error messages
+ fileLintErrors = processLintErrors(file, options.imports, lintResult, less, css)
+ errorCount += fileLintErrors
+
+ callback()
@filesSrc.forEach (file) -> queue.push(file)
@@ -211,3 +157,13 @@ module.exports = (grunt) ->
grunt.log.writeln()
grunt.log.error("#{errorCount} lint #{grunt.util.pluralize(errorCount, 'error/errors')} in #{fileCount} #{grunt.util.pluralize(fileCount, 'file/files')}.")
done(false)
+
+ grunt.registerTask 'lesslint:clearCache', ->
+ done = @async()
+
+ cache = new LintCache()
+
+ cache.clear (err) ->
+ grunt.log.error(err.message) if err
+
+ done()
View
36 src/lib/css-linter.coffee
@@ -0,0 +1,36 @@
+
+{CSSLint} = require 'csslint'
+_ = require 'underscore'
+
+module.exports = class CssLinter
+ constructor: (@options, @grunt) ->
+
+ lint: (css, callback) ->
+ unless css
+ return callback(null, [])
+
+ externalOptions = {}
+
+ rules = _.reduce CSSLint.getRules(), (memo, {id}) ->
+ memo[id] = 1
+ memo
+ , {}
+
+ cssLintOptions = @options.csslint
+ if cssLintOptions?.csslintrc
+ externalOptions = @grunt.file.readJSON cssLintOptions.csslintrc
+ delete cssLintOptions.csslintrc
+
+ _.extend(cssLintOptions, externalOptions)
+
+ for id, enabled of cssLintOptions
+ if cssLintOptions[id]
+ rules[id] = cssLintOptions[id]
+ else
+ delete rules[id]
+
+ result = CSSLint.verify(css, rules)
+ if result.messages?.length > 0
+ callback(null, result)
+ else
+ callback()
View
127 src/lib/less-file.coffee
@@ -0,0 +1,127 @@
+
+crypto = require 'crypto'
+LessParser = require './less-parser'
+CssLinter = require './css-linter'
+{LintCache} = require './lint-cache'
+
+# Base Class representing a file to be linted
+class LessFile
+ constructor: (@filePath, @options = {}, @grunt) ->
+
+ lint: (callback) ->
+ # Parse the LESS into CSS
+ @getCss (err, css) =>
+ return callback(new Error("Error parsing #{@filePath.yellow}: #{err.message}")) if err
+
+ # Lint the css
+ @lintCss css, (err, result) =>
+ return callback(new Error("Error linting #{@filePath.yellow}: #{err.message}")) if err
+
+ callback null, result, @getContents(), css
+
+ # Broken out for extension/stubbing
+ lintCss: (css, callback) ->
+ linter = new CssLinter(@options, @grunt)
+
+ # Lint the CSS
+ linter.lint css, callback
+
+ getContents: (forced) ->
+ return @contents if @contents? and not forced
+
+ @contents = @grunt.file.read @filePath
+
+ getDigest: ->
+ return @digest if @digest
+
+ @digest = crypto.createHash('sha256').update(@getContents()).digest('base64')
+
+ @digest
+
+ getCss: (callback) ->
+ @getTree (err, tree) ->
+ return callback(err) if err
+
+ callback null, tree.toCSS()
+
+ # Just in case someone needs just the tree for something later
+ getTree: (callback) ->
+ contents = @getContents()
+
+ # Bug out early if no LESS content
+ return callback null, '' unless contents
+
+ parser = new LessParser(@filePath, @options)
+
+ parser.parse contents, callback
+
+# An in-memory hold of import contents and hashes
+sharedImportsContents = {}
+
+# Extended LessFile that keeps an in memory lookup of contents and hashes
+# to speed of subsequent lookups.
+class LessImportFile extends LessFile
+ getContents: ->
+ if sharedImportsContents[@filePath]?.contents
+ return sharedImportsContents[@filePath].contents
+
+ contents = super()
+
+ sharedImportsContents[@filePath] ||= { }
+ sharedImportsContents[@filePath].contents = contents
+
+ getDigest: ->
+ if sharedImportsContents[@filePath]?.digest
+ return sharedImportsContents[@filePath].digest
+
+ digest = super()
+
+ sharedImportsContents[@filePath] ||= { }
+ sharedImportsContents[@filePath].digest = digest
+
+# Extended LessFile with some logic for caching
+class LessCachedFile extends LessFile
+ constructor: (@filePath, @options = {}, @grunt) ->
+ super
+
+ @cache = new LintCache(@options.cache)
+
+ lint: (callback) ->
+ hash = @getDigest()
+
+ @cache.hasCached hash, (isCached, cachedPath) =>
+ if isCached
+ @grunt.event.emit 'lesslint.cache.hit', @filePath, cachedPath, hash
+ return callback()
+
+ # Call the super, not sure if I can use super() here since I'm in a callback
+ LessFile.prototype.lint.call @, (err, result, less, css) =>
+ return callback(err) if err
+
+ # If there were errors found, pass them back and don't cache
+ return callback(null, result, less, css) if result?
+
+ # Otherwise, add to cache
+ @cache.addCached hash, (err, cachedAddPath) =>
+ return callback(err) if err
+
+ @grunt.event.emit 'lesslint.cache.add', @filePath, hash, cachedAddPath
+ callback(null, result, less, css)
+
+ # Hash the cached files based on import contents
+ getDigest: ->
+ myHash = super()
+
+ return myHash unless @options.imports?
+
+ importsContents = @getImportsContents()
+
+ crypto.createHash('sha256')
+ .update(myHash)
+ .update(importsContents.join(''))
+ .digest('base64')
+
+ getImportsContents: ->
+ (new LessImportFile(importFilePath, {}, @grunt).getDigest() for importFilePath in @grunt.file.expand(@options.imports))
+
+module.exports = { LessFile, LessImportFile, LessCachedFile }
View
26 src/lib/less-parser.coffee
@@ -0,0 +1,26 @@
+
+_ = require 'underscore'
+{Parser} = require 'less'
+
+defaultLessOptions =
+ cleancss: false
+ compress: false
+ dumpLineNumbers: 'comments'
+ optimization: null
+ syncImport: true
+
+# LessParser encapsulates the options and defaults used when parsing LESS files.
+module.exports = class LessParser
+ constructor: (fileName, opts) ->
+ # Make sure we have some default options if none passed
+ opts = _.defaults(opts || {}, defaultLessOptions)
+
+ # Set the options and create the parser
+ @opts = _.extend({ filename: fileName }, opts)
+ @parser = new Parser(@opts)
+
+ parse: (less, callback) ->
+ try
+ @parser.parse less, callback
+ catch err
+ callback err
View
2  src/lint-cache.coffee → src/lib/lint-cache.coffee
@@ -5,7 +5,7 @@ path = require 'path'
_ = grunt.util._
# Grab the package info only once instead of on every instantiation
-packageInfo = grunt.file.readJSON(path.resolve(path.join(__dirname, '..', 'package.json')))
+packageInfo = grunt.file.readJSON(path.resolve(path.join(__dirname, '..', '..', 'package.json')))
class LintCache extends CacheSwap
@category = 'lesshashed'
View
0  src/lint-utils.coffee → src/lib/lint-utils.coffee
File renamed without changes
Something went wrong with that request. Please try again.