Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Run linting in a Task #88

Merged
merged 11 commits into from
Aug 16, 2017
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
node_modules
.github_changelog_generator
/node_modules
.github_changelog_generator
55 changes: 55 additions & 0 deletions lib/Linter.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
{Task} = require('atom')

workers = new Set
workerFile = require.resolve('./core.coffee')

module.exports =
class Linter

name: 'CoffeeLint'
grammarScopes: [
'source.coffee'
'source.litcoffee'
'source.coffee.jsx'
'source.coffee.angular'
]
scope: 'file'
lintsOnChange: true

destroy: ->
worker?.terminate() for worker in workers

lint: (textEditor) ->
filePath = textEditor.getPath()
if filePath
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need an else here returning [] (to clear messages) or null (to tell Linter not to update any messages it might have for the file), otherwise you will return undefined and trigger a Linter message to the user that an invalid result was returned.

source = textEditor.getText()

isLiterate = textEditor.getGrammar().scopeName is 'source.litcoffee'


transform = ({level, message, rule, lineNumber, context}) ->
message = "#{message}. #{context}" if context
message = "#{message}. (#{rule})";

# Calculate range to make the error whole line
# without the indentation at begining of line
indentLevel = textEditor.indentationForBufferRow(lineNumber - 1)

startCol = (textEditor.getTabLength() * indentLevel)
endCol = textEditor.getBuffer().lineLengthForRow(lineNumber - 1)

range = [[lineNumber - 1, startCol], [lineNumber - 1, endCol]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just bring in atom-linter and use generateRange here like almost every other provider. This function has a much smarter version of this logic in place 😉.


return {
severity: if level is 'error' then 'error' else 'warning'
excerpt: message
location:
file: filePath
position: range
}

return new Promise (resolve) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same story with the if above, you need to handle the Promise rejection and return a valid result ([] or null) to Linter.

task = Task.once workerFile, filePath, source, isLiterate, (results) ->
workers.delete(task)
resolve(results.map(transform))
workers.add(task)
152 changes: 70 additions & 82 deletions lib/core.coffee
Original file line number Diff line number Diff line change
@@ -1,102 +1,90 @@
# This file is a Task file that will run in a separate process
# https://atom.io/docs/api/v1.19.0/Task

# I can't just map over parseInt because it needs the 2nd parameter and map
# passes the current index as the 2nd parameter
toInt = (str) -> parseInt(str, 10)
resolve = require 'resolve'
path = require 'path'

module.exports =

# The syntax that the linter handles. May be a string or
# list/tuple of strings. Names should be all lowercase.
#
# If you have the react plugin it switches source.coffee to source.coffee.jsx
# even if you aren't actually using jsx in that file.
scopes: [
'source.coffee'
'source.litcoffee'
'source.coffee.jsx'
'source.coffee.angular'
]
_resolveCoffeeLint = (filePath) ->
try
return path.dirname(resolve.sync('coffeelint/package.json', {
basedir: path.dirname(filePath)
}))
catch e
expected = "Cannot find module 'coffeelint/package.json'"
if e.message[...expected.length] is expected
return 'coffeelint'
throw e

_resolveCoffeeLint: (filePath) ->
try
return path.dirname(resolve.sync('coffeelint/package.json', {
basedir: path.dirname(filePath)
}))
catch e
expected = "Cannot find module 'coffeelint/package.json'"
if e.message[...expected.length] is expected
return 'coffeelint'
throw e
configImportsModules = (config) ->
return true for ruleName, rconfig of config when rconfig.module?
return userConfig?.coffeelint?.transforms?

configImportsModules: (config) ->
return true for ruleName, rconfig of config when rconfig.module?
return userConfig?.coffeelint?.transforms?
canImportModules = (coffeelint) ->
[major, minor, patch] = coffeelint.VERSION.split('.').map(toInt)

canImportModules: (coffeelint) ->
[major, minor, patch] = coffeelint.VERSION.split('.').map(toInt)
if major > 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any objection to just bringing in semver and using it here?

return true
if major is 1 and minor > 9
return true
if major is 1 and minor is 9 and patch >= 5
return true
false

if major > 1
return true
if major is 1 and minor > 9
return true
if major is 1 and minor is 9 and patch >= 5
return true
false
isCompatibleWithAtom = (coffeelint) ->
[major, minor, patch] = coffeelint.VERSION.split('.').map(toInt)

isCompatibleWithAtom: (coffeelint) ->
[major, minor, patch] = coffeelint.VERSION.split('.').map(toInt)
if major > 1
return true
if major is 1 and minor > 9
return true
if major is 1 and minor is 9 and patch >= 1
return true
false

if major > 1
return true
if major is 1 and minor > 9
return true
if major is 1 and minor is 9 and patch >= 1
return true
false
module.exports = (filePath, source, isLiterate) ->
showUpgradeError = false

lint: (filePath, source, scopeName) ->
isLiterate = scopeName is 'source.litcoffee'
showUpgradeError = false
coffeeLintPath = _resolveCoffeeLint(filePath)
coffeelint = require(coffeeLintPath)

coffeeLintPath = @_resolveCoffeeLint(filePath)
# Versions before 1.9.1 don't work with atom because of an assumption that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'atom' -> 'Atom'

# if window is defined, then it must be running in a browser. Atom breaks
# this assumption, so CoffeeLint < 1.9.1 will fail to find CoffeeScript.
# See https://github.com/clutchski/coffeelint/pull/383
if not isCompatibleWithAtom(coffeelint)
coffeeLintPath = 'coffeelint'
coffeelint = require(coffeeLintPath)
showUpgradeError = true

# Versions before 1.9.1 don't work with atom because of an assumption that
# if window is defined, then it must be running in a browser. Atom breaks
# this assumption, so CoffeeLint < 1.9.1 will fail to find CoffeeScript.
# See https://github.com/clutchski/coffeelint/pull/383
[major, minor, patch] = coffeelint.VERSION.split('.').map(toInt)
if not @isCompatibleWithAtom(coffeelint)
coffeeLintPath = 'coffeelint'
coffeelint = require(coffeeLintPath)
showUpgradeError = true

configFinder = require("#{coffeeLintPath}/lib/configfinder")
configFinder = require("#{coffeeLintPath}/lib/configfinder")

result = []
try
config = configFinder.getConfig(filePath)
if @configImportsModules(config) and not @canImportModules(coffeelint)
showUpgradeError = true
else
result = coffeelint.lint(source, config, isLiterate)
catch e
console.log(e.message)
console.log(e.stack)
result.push({
lineNumber: 1
level: 'error'
message: "CoffeeLint crashed, see console for error details."
rule: 'none'
})
result = []
try
config = configFinder.getConfig(filePath)
if configImportsModules(config) and not canImportModules(coffeelint)
showUpgradeError = true
else
result = coffeelint.lint(source, config, isLiterate)
catch e
console.log(e.message)
console.log(e.stack)
result.push({
lineNumber: 1
level: 'error'
message: "CoffeeLint crashed, see console for error details."
rule: 'none'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should just let this throw a error instead of showing this on line 1

})

if showUpgradeError
result = [{
lineNumber: 1
level: 'error'
message: "http://git.io/local_upgrade upgrade your project's CoffeeLint"
rule: 'none'
}]
if showUpgradeError
result = [{
lineNumber: 1
level: 'error'
message: "http://git.io/local_upgrade upgrade your project's CoffeeLint"
rule: 'none'
}]

return result
return result
6 changes: 5 additions & 1 deletion lib/init.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,9 @@ module.exports =
activate: ->
require("atom-package-deps").install()

deactivate: ->
@linter?.destroy()

provideLinter: ->
return require('./plus-linter.coffee')
Linter = require('./Linter.coffee')
@linter = new Linter
43 changes: 0 additions & 43 deletions lib/plus-linter.coffee

This file was deleted.

Loading