-
Notifications
You must be signed in to change notification settings - Fork 171
Several enhancements, including CJSX fixes #632
Changes from all commits
019bd2b
5d5da21
9c65f29
65b3b83
e10b4e0
0f32767
e370247
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,9 @@ | |
"space_operators": { | ||
"level": "error" | ||
}, | ||
"max_line_length": { | ||
"value": "100" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 80 is just wayyy too low, it was causing issues in the test files. Easier to just up the limit than to adhere to a dogmatic 80. |
||
}, | ||
"cyclomatic_complexity": { | ||
"value": 11, | ||
"level": "warn" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,8 +32,8 @@ | |
"strip-json-comments": "^1.0.2" | ||
}, | ||
"devDependencies": { | ||
"vows": ">=0.8.1", | ||
"underscore": ">=1.4.4" | ||
"underscore": ">=1.4.4", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. npm automatically alphabeticalizes now. |
||
"vows": ">=0.8.1" | ||
}, | ||
"license": "MIT", | ||
"scripts": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -222,13 +222,12 @@ coffeelint.registerRule require './rules/no_this.coffee' | |
coffeelint.registerRule require './rules/eol_last.coffee' | ||
coffeelint.registerRule require './rules/no_private_function_fat_arrows.coffee' | ||
|
||
hasSyntaxError = (source) -> | ||
getTokens = (source) -> | ||
try | ||
# If there are syntax errors this will abort the lexical and line | ||
# linters. | ||
CoffeeScript.tokens(source) | ||
return false | ||
return true | ||
return CoffeeScript.tokens(source) | ||
return null | ||
|
||
ErrorReport = require('./error_report.coffee') | ||
coffeelint.getErrorReport = -> | ||
|
@@ -310,9 +309,10 @@ coffeelint.lint = (source, userConfig = {}, literate = false) -> | |
|
||
# only do further checks if the syntax is okay, otherwise they just fail | ||
# with syntax error exceptions | ||
unless hasSyntaxError(source) | ||
tokens = getTokens(source) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we were passing the source to Coffeescript before twice, which basically means we're doing twice as much work as needed. |
||
if tokens | ||
# Do lexical linting. | ||
lexicalLinter = new LexicalLinter(source, config, _rules, CoffeeScript) | ||
lexicalLinter = new LexicalLinter(source, config, _rules, CoffeeScript, tokens) | ||
lexErrors = lexicalLinter.lint() | ||
errors = errors.concat(lexErrors) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,12 +28,18 @@ findFile = (name, dir) -> | |
# Possibly find CoffeeLint configuration within a package.json file. | ||
loadNpmConfig = (dir) -> | ||
fp = findFile('package.json', dir) | ||
loadJSON(fp)?.coffeelintConfig if fp | ||
loadJSON(fp, 'coffeelintConfig') if fp | ||
|
||
# Parse a JSON file gracefully. | ||
loadJSON = (filename) -> | ||
loadJSON = (filename, attr = null) -> | ||
try | ||
JSON.parse(stripComments(fs.readFileSync(filename).toString())) | ||
config = JSON.parse(stripComments(fs.readFileSync(filename).toString())) | ||
if attr | ||
return null if not config[attr]? | ||
config = config[attr] | ||
|
||
config.__location__ = filename | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is so we can remember where the config file is loaded from, so we can resolve the "extends" property relative to the location of the configfile (as opposed to the cwd) |
||
return config | ||
catch e | ||
process.stderr.write "Could not load JSON file '#{filename}': #{e}" | ||
null | ||
|
@@ -48,7 +54,7 @@ getConfig = (dir) -> | |
return loadJSON(process.env.COFFEELINT_CONFIG) | ||
|
||
npmConfig = loadNpmConfig(dir) | ||
return npmConfig if npmConfig | ||
return npmConfig if npmConfig | ||
projConfig = findFile('coffeelint.json', dir) | ||
return loadJSON(projConfig) if projConfig | ||
|
||
|
@@ -86,11 +92,16 @@ expandModuleNames = (dir, config) -> | |
|
||
config | ||
|
||
extendConfig = (config) -> | ||
extendConfig = (dir, config) -> | ||
unless config.extends | ||
return config | ||
try | ||
parentConfig = require resolve config.extends, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we cant resolve the path relative to our present directory, then we can look in node_modules |
||
basedir: dir | ||
extensions: ['.js', '.coffee', '.json'] | ||
catch | ||
parentConfig = require config.extends | ||
|
||
parentConfig = require config.extends | ||
extendedConfig = {} | ||
|
||
for ruleName, rule of config | ||
|
@@ -109,8 +120,9 @@ exports.getConfig = (filename = null) -> | |
|
||
config = getConfig(dir) | ||
|
||
dir = path.dirname(config.__location__) | ||
if config | ||
config = extendConfig(config) | ||
config = extendConfig(dir, config) | ||
config = expandModuleNames(dir, config) | ||
|
||
config |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
class TokenApi | ||
|
||
constructor: (CoffeeScript, source, @config, @tokensByLine) -> | ||
@tokens = CoffeeScript.tokens(source) | ||
constructor: (CoffeeScript, source, @config, @tokensByLine, @tokens) -> | ||
@tokens ?= CoffeeScript.tokens(source) | ||
@lines = source.split('\n') | ||
@tokensByLine = {} # A map of tokens by line. | ||
|
||
|
@@ -18,10 +18,10 @@ BaseLinter = require './base_linter.coffee' | |
# | ||
module.exports = class LexicalLinter extends BaseLinter | ||
|
||
constructor: (source, config, rules, CoffeeScript) -> | ||
constructor: (source, config, rules, CoffeeScript, tokens) -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is so we can pass in the tokens from coffeelint.coffee (so we're not running |
||
super source, config, rules | ||
|
||
@tokenApi = new TokenApi CoffeeScript, source, @config, @tokensByLine | ||
@tokenApi = new TokenApi CoffeeScript, source, @config, @tokensByLine, tokens | ||
# This needs to be available on the LexicalLinter so it can be passed | ||
# to the LineLinter when this finishes running. | ||
@tokensByLine = @tokenApi.tokensByLine | ||
|
@@ -64,4 +64,13 @@ module.exports = class LexicalLinter extends BaseLinter | |
attrs.lineNumber ?= @lineNumber | ||
attrs.lineNumber += 1 | ||
attrs.line = @tokenApi.lines[attrs.lineNumber - 1] | ||
if attrs.token | ||
token = attrs.token | ||
attrs.lineNumber = token[2].first_line + 1 | ||
attrs.columnNumber = token[2].first_column + 1 | ||
if token[2].last_line | ||
attrs.lineNumberEnd = token[2].last_line + 1 | ||
if token[2].last_column | ||
attrs.columnNumberEnd = token[2].last_column + 1 | ||
|
||
super ruleName, attrs |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,10 @@ module.exports = class ColonAssignmentSpacing | |
when 'left' | ||
token[2].first_column - previousToken[2].last_column - 1 | ||
when 'right' | ||
nextToken[2].first_column - token[2].first_column - 1 | ||
# csx tags 'column' resolves to the beginning of the tag definition, rather | ||
# than the '<' | ||
offset = if nextToken[0] != 'CSX_TAG' then -1 else -2 | ||
nextToken[2].first_column - token[2].first_column + offset | ||
|
||
checkSpacing = (direction) -> | ||
spacing = getSpaceFromToken direction | ||
|
@@ -57,7 +60,8 @@ module.exports = class ColonAssignmentSpacing | |
[isLeftSpaced, leftSpacing] = checkSpacing 'left' | ||
[isRightSpaced, rightSpacing] = checkSpacing 'right' | ||
|
||
if isLeftSpaced and isRightSpaced | ||
if token.csxColon or isLeftSpaced and isRightSpaced | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have no control over spacing of "csxColons" (these are autogenerated by the compiler) |
||
null | ||
else | ||
token: token | ||
context: "Incorrect spacing around column #{token[2].first_column}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed so we can export the default rules for use in eslint.