Skip to content

Commit

Permalink
Refactor implementation
Browse files Browse the repository at this point in the history
* Change name, when providing linter
* Resolve only ignoreGlobs from the package.json options
* Rename linter key in options to linterName
* Simply pass linter object to lint(), so it doesn't need to return a
function
* Make getting a linter synchronous
* Implement naive error serialization (assumes errors have a message)
* In the main process, change the linter's lintText() to return a
promise
* Reject the aforementioned promise with a proper error if the worker
responds with a serialized error object
* Simplify worker implementation and response object
* Attempt to simplify range conversion
  • Loading branch information
novemberborn committed Jan 9, 2017
1 parent f024ed1 commit c988c64
Show file tree
Hide file tree
Showing 16 changed files with 294 additions and 336 deletions.
83 changes: 39 additions & 44 deletions init.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const findOptions = require('./lib/findOptions')
const getLinter = require('./lib/getLinter')
const lint = require('./lib/lint')
const perform = require('./lib/lint')
const cleanLinters = require('./lib/getLinter').cleanLinters
const minimatch = require('minimatch')
const path = require('path')
const { relative } = require('path')

function suppressError (err) {
return [
Expand All @@ -21,49 +21,44 @@ function suppressError (err) {
})
}

module.exports = {
deactivate () {
cleanLinters()
},
provideLinter () {
return {
name: 'lint',
grammarScopes: ['source.js', 'source.js.jsx'],
scope: 'file',
lintOnFly: true,
lint (textEditor) {
const fileContent = textEditor.getText()
const filePath = textEditor.getPath()
const fileScope = textEditor.getGrammar().scopeName
function lint (textEditor) {
const { scopeName } = textEditor.getGrammar()
if (!this.grammarScopes.includes(scopeName)) return Promise.resolve([])

if (!this.grammarScopes.includes(fileScope)) {
return Promise.resolve([])
}
// Get text at the time the linter was invoked.
const text = textEditor.getText()
const path = textEditor.getPath()

return findOptions(filePath)
.then(options => {
const ignoreGlobs = options.options && options.options.ignore || []
const fileIsIgnored = ignoreGlobs.some(pattern => {
const relativePath = path.relative(options.projectRoot, filePath)
return minimatch(relativePath, pattern)
})
if (fileIsIgnored) {
return [] // No errors
}
return getLinter(options.linter, options.projectRoot)
.then(lint(filePath, fileContent))
})
.catch(err => {
if (!suppressError(err)) {
atom.notifications.addError(err.message || 'Something bad happened', {
error: err,
detail: err.stack,
dismissable: true
})
}
return []
})
return findOptions(path)
.then(({ linterName, projectRoot, ignoreGlobs }) => {
const relativePath = relative(projectRoot, path)
const fileIsIgnored = ignoreGlobs.some(pattern => minimatch(relativePath, pattern))
if (fileIsIgnored) return [] // No errors

const linter = getLinter(linterName, projectRoot)
return perform(linter, path, text)
})
.catch(err => {
if (!suppressError(err)) {
atom.notifications.addError(err.message || 'Something bad happened', {
error: err,
detail: err.stack,
dismissable: true
})
}
}
}

return []
})
}

exports.deactivate = () => {
cleanLinters()
}

exports.provideLinter = () => ({
name: 'standard-engine',
grammarScopes: ['source.js', 'source.js.jsx'],
scope: 'file',
lintOnFly: true,
lint
})
80 changes: 40 additions & 40 deletions lib/findOptions.js
Original file line number Diff line number Diff line change
@@ -1,46 +1,46 @@
const path = require('path')
const { dirname } = require('path')
const readPkgUp = require('read-pkg-up')
const supportedLinters = require('./supportedLinters')

function findOptions (filePath) {
return readPkgUp({ cwd: path.dirname(filePath) }).then(result => {
const packageJson = result.pkg
if (!packageJson) {
throw new Error('no package.json found')
}

let linters = []

if (packageJson.devDependencies) {
const lintersFromDeps = Object.keys(packageJson.devDependencies)
.filter(key => supportedLinters.includes(key))
linters = linters.concat(lintersFromDeps)
}

if (packageJson['standard-engine']) {
linters.unshift(packageJson['standard-engine'])
}

if (linters.length < 1) {
throw new Error('no supported linter found')
}

const linter = linters[0]
const pathToProject = path.dirname(result.path)

// Support scoped packages. Assume their `cmd` (and thus options key) is
// configured as the package name *without* the scope prefix.
let optionsKey = linter
if (optionsKey.includes('/')) {
optionsKey = optionsKey.split('/')[1]
}

return {
linter,
projectRoot: pathToProject,
options: packageJson[optionsKey] || {}
}
})
function findOptions (file) {
return readPkgUp({ cwd: dirname(file) })
.then(({ path, pkg }) => {
if (!pkg) throw new Error('no package.json found')

let linters = []
if (pkg.devDependencies) {
for (const dep of Object.keys(pkg.devDependencies)) {
if (supportedLinters.includes(dep)) {
linters.push(dep)
}
}
}
if (pkg['standard-engine']) {
linters.unshift(pkg['standard-engine'])
}

if (linters.length === 0) {
throw new Error('no supported linter found')
}

const linterName = linters[0]

// Support scoped packages. Assume their `cmd` (and thus options key) is
// configured as the package name *without* the scope prefix.
let optionsKey = linterName
if (optionsKey.includes('/')) {
optionsKey = optionsKey.split('/')[1]
}

const options = pkg[optionsKey] || {}
const { ignore: ignoreGlobs = [] } = options

return {
ignoreGlobs,
linterName,
projectRoot: dirname(path)
}
})
}

module.exports = findOptions
79 changes: 37 additions & 42 deletions lib/getLinter.js
Original file line number Diff line number Diff line change
@@ -1,86 +1,81 @@
const { fork } = require('child_process')
const path = require('path')
const { resolve: resolvePath } = require('path')
const lruCache = require('lru-cache')

const linters = lruCache({
max: 2,
dispose (cacheKey, linter) {
dispose (_, linter) {
linter.shutdown()
}
})

const lintWorkerPath = path.resolve(__dirname, 'lintWorker.js')
const lintWorkerPath = resolvePath(__dirname, 'lintWorker.js')

function getCallbackOnce (pending, id) {
const callback = pending.get(id)
function getResolversOnce (pending, id) {
const resolvers = pending.get(id)
pending.delete(id)
return callback
return resolvers
}

const createLinter = (linterName, projectRoot) => {
const child = fork(lintWorkerPath, [ linterName ], {
cwd: projectRoot
})

let sequenceNumber = 0
const pendingCallbacks = new Map()
const pendingResolvers = new Map()

child.on('message', data => {
const callback = getCallbackOnce(pendingCallbacks, data.id)
if (callback) {
if (data.err) {
return callback(data.err)
child.on('message', ({ id, error, results }) => {
const resolvers = getResolversOnce(pendingResolvers, id)
if (resolvers) {
if (error) {
resolvers.reject(new Error(error.message))
} else {
return callback(null, data.result)
resolvers.resolve(results)
}
}
})

return {
lintText (filePath, fileContent, cb) {
sequenceNumber++
const id = sequenceNumber
pendingCallbacks.set(id, cb)

child.send({
filename: filePath,
id: id,
source: fileContent
lintText (filename, source) {
const id = ++sequenceNumber
return new Promise((resolve, reject) => {
pendingResolvers.set(id, { resolve, reject })
child.send({ filename, id, source })
})
},

shutdown () {
// Shutdown is optimistic, but may be called when the linter is
// intentionally removed from the cache. Ignore any errors thrown.
try {
child.disconnect()
} catch (err) {}
},
exited: new Promise(resolve => {
child.once('exit', resolve)
})

exited: new Promise(resolve => child.once('exit', resolve))
}
}

module.exports = function getLinter (linterName, projectRoot) {
return new Promise((resolve, reject) => {
const cacheKey = linterName + '\n' + projectRoot
let linter = linters.get(cacheKey)
function getLinter (linterName, projectRoot) {
const cacheKey = linterName + '\n' + projectRoot
if (linters.has(cacheKey)) return linters.get(cacheKey)

if (!linter) {
linter = createLinter(linterName, projectRoot)
linters.set(cacheKey, linter)
linter.exited.then(() => {
// A new linter may have been created in the meantime, make sure not to
// delete that one.
if (linters.peek(cacheKey) === linter) {
linters.del(cacheKey)
}
})
const linter = createLinter(linterName, projectRoot)
linters.set(cacheKey, linter)
linter.exited.then(() => {
// A new linter may have been created in the meantime, make sure not to
// delete that one.
if (linters.peek(cacheKey) === linter) {
linters.del(cacheKey)
}

resolve(linter)
})

return linter
}

module.exports.cleanLinters = () => {
exports = module.exports = getLinter

exports.cleanLinters = () => {
linters.reset()
}
43 changes: 15 additions & 28 deletions lib/lint.js
Original file line number Diff line number Diff line change
@@ -1,35 +1,22 @@
function getRange (line, col, src) {
line = typeof line !== 'undefined' ? parseInt((line - 1), 10) : 0
col = typeof col !== 'undefined' ? parseInt(col - 1, 10) : 0
src = src || ''
src = src.substring(0, col)
function getRange (line = 1, column = 1, source = '') {
const line0 = line - 1
const column0 = column - 1

return [[line, col - src.trim().length], [line, col]]
const end = [line0, column0]
const start = [line0, column0 - source.slice(0, column0).trim().length]
return [start, end]
}

module.exports = function lint (filePath, fileContent) {
return linter => new Promise((resolve, reject) => {
linter.lintText(filePath, fileContent, (err, output) => {
if (err) {
if (typeof err === 'string') {
err = new Error(err)
}
return reject(err)
}
module.exports = function lint (linter, filePath, fileContent) {
return linter.lintText(filePath, fileContent)
.then(([{ messages } = {}]) => {
if (!Array.isArray(messages)) throw new Error('invalid lint report')

const results = output && output.results && output.results[0]
const messages = results && results.messages

if (!Array.isArray(messages)) {
return reject(new Error('invalid lint report'))
}

resolve(messages.map(msg => ({
type: msg.fatal ? 'Error' : 'Warning',
text: msg.message,
return messages.map(({ fatal, message: text, line, column, source }) => ({
type: fatal ? 'Error' : 'Warning',
text,
filePath,
range: getRange(msg.line, msg.column, msg.source)
})))
range: getRange(line, column, source)
}))
})
})
}
Loading

0 comments on commit c988c64

Please sign in to comment.