Permalink
Browse files

indent: Fix several issues related to indentation

This addresses several issues regarding indentation.

1. Being able to indent conditional if/else assignments to make them
look more pleasing.

Most users wanted to do something like this

```cs
y = if a
      true
    else
      false
```

This now won't return an error for the user.

Fixes #468, #345

2. Fixed some bugs around chaining (.) operations

There were some cases of chaining operators not being evaluated
correctly. One was due to not checking for end tokens with `newLine`
properties.

Fixes #469, #348

3. Better cleanup of indented multi-line conditional statements.

Just some cleanup of the logic for the conditional in an if statement.

4. Empty if conditionals now do not generate indent message

While I think more often than not you should never need to have an empty
if conditional in your code. I do recognize it probably will happen, and
someone who chooses to use one should not be punished for it with an
indentation issue.

Closes #312
  • Loading branch information...
1 parent 05dc7a4 commit bffa2532efd8bcca5d9c1b3a9d3b5914e882dd5f @swang swang committed Aug 21, 2015
Showing with 243 additions and 17 deletions.
  1. +116 −10 src/rules/indentation.coffee
  2. +127 −7 test/test_indentation.coffee
@@ -24,6 +24,13 @@ module.exports = class Indentation
tokens: ['INDENT', '[', ']', '.']
+ keywords: [
+ '->', '=>', '@', 'CATCH', 'CLASS', 'ELSE', 'FINALLY', 'FOR',
+ 'FORIN', 'FOROF', 'IDENTIFIER', 'IF', 'LEADING_WHEN', 'LOOP',
+ 'RETURN', 'SWITCH', 'THROW', 'TRY', 'UNTIL', 'WHEN', 'WHILE',
+ 'YIELD'
+ ]
+
constructor: ->
@arrayTokens = [] # A stack tracking the array token pairs.
@@ -82,8 +89,8 @@ module.exports = class Indentation
numIndents = @getCorrectIndent(tokenApi)
# Now check the indentation.
- if not ignoreIndent and numIndents isnt expected
- return { context: "Expected #{expected} got #{numIndents}" }
+ if not ignoreIndent and not (expected in numIndents)
+ return { context: "Expected #{expected} got #{numIndents[0]}" }
# Return true if the current token is inside of an array.
inArray: () ->
@@ -111,8 +118,10 @@ module.exports = class Indentation
# Traverse up the token list until we see a CALL_START token.
# Don't scan above this line
findCallStart = tokenApi.peek(-callStart)
- while (findCallStart and findCallStart[0] isnt 'TERMINATOR')
+ while (findCallStart and (findCallStart[0] isnt 'TERMINATOR' or
+ not findCallStart.newLine?))
{ first_line: lastCheck } = findCallStart[2]
+
callStart += 1
findCallStart = tokenApi.peek(-callStart)
@@ -147,20 +156,117 @@ module.exports = class Indentation
if numIndents % expected isnt 0
return { context: "Expected #{expected} got #{numIndents}" }
+ grabLineTokens: (tokenApi, lineNumber, all = false) ->
+ { tokensByLine } = tokenApi
+ lineNumber-- until tokensByLine[lineNumber]? or lineNumber is 0
+
+ if all
+ (tok for tok in tokensByLine[lineNumber])
+ else
+ (tok for tok in tokensByLine[lineNumber] when not
+ tok.generated? and tok[0] isnt 'OUTDENT')
+
# Returns a corrected INDENT value if the current line is part of
# a chained call. Otherwise returns original INDENT value.
getCorrectIndent: (tokenApi) ->
- { lineNumber, lines, tokens, i } = tokenApi
+ { lineNumber, lines, tokens } = tokenApi
curIndent = lines[lineNumber].match(/\S/)?.index
prevNum = 1
prevNum += 1 while (/^\s*(#|$)/.test(lines[lineNumber - prevNum]))
- prevLine = lines[lineNumber - prevNum]
- prevIndent = prevLine.match(/^(\s*)\./)?[1].length
-
- if prevIndent > 0
- return curIndent - prevLine.match(/\S/)?.index
+ prevTokens = @grabLineTokens tokenApi, lineNumber - prevNum
+
+ if prevTokens[0]?[0] is 'INDENT'
+ # Pass both the INDENT value and the location of the first token
+ # after the INDENT because sometimes CoffeeScript doesn't return
+ # the correct INDENT if there is something like an if/else
+ # inside an if/else inside of a -> function definition: e.g.
+ #
+ # ->
+ # r = if a
+ # if b
+ # 2
+ # else
+ # 3
+ # else
+ # 4
+ #
+ # will error without: curIndent - prevTokens[1]?[2].first_column
+
+ return [curIndent - prevTokens[1]?[2].first_column,
+ curIndent - prevTokens[0][1]]
else
- return tokens[i][1]
+ prevIndent = prevTokens[0]?[2].first_column
+ # This is a scan to handle extra indentation from if/else
+ # statements to make them look nicer: e.g.
+ #
+ # r = if a
+ # true
+ # else
+ # false
+ #
+ # is valid.
+ #
+ # r = if a
+ # true
+ # else
+ # false
+ #
+ # is also valid.
+ for _, j in prevTokens when prevTokens[j][0] is '=' and
+ prevTokens[j + 1]?[0] is 'IF'
+ skipAssign = curIndent - prevTokens[j + 1][2].first_column
+ ret = curIndent - prevIndent
+ return [ret] if skipAssign < 0
+ return [skipAssign, ret]
+
+ # This happens when there is an extra indent to maintain long
+ # conditional statements (IF/UNLESS): e.g.
+ #
+ # ->
+ # if a is c and
+ # (false or
+ # long.expression.that.necessitates(linebreak))
+ # @foo()
+ #
+ # is valid (note that there an only an extra indent in the last
+ # statement is required and not the line above it
+ #
+ # ->
+ # if a is c and
+ # (false or
+ # long.expression.that.necessitates(linebreak))
+ # @foo()
+ # is also OK.
+ while prevIndent > curIndent
+ tryLine = lineNumber - prevNum
+ prevTokens = @grabLineTokens tokenApi, tryLine, true
+
+ # This is to handle weird object/string indentation.
+ # See: 'Handle edge-case weirdness with strings in objects'
+ # test case in test_indentation.coffee or in the file,
+ # test_no_empty_functions.coffee, which is why/how I
+ # caught this.
+ if prevTokens[0]?[0] is 'INDENT'
+ prevIndent = prevTokens[0][1]
+ prevTokens = prevTokens[1..]
+
+ t = 0
+ # keep looping prevTokens until we find a token in @keywords
+ # or we just run out of tokens in prevTokens
+ until not prevTokens[t]? or prevTokens[t][0] in @keywords
+ t++
+
+ # slice off everything before 't'
+ prevTokens = prevTokens[t..]
+ prevNum++
+
+ # if there isn't a valid token, restart the while loop
+ continue unless prevTokens[0]?
+
+ # set new "prevIndent"
+ prevIndent = prevTokens[0]?[2].first_column
+
+ return [curIndent - prevIndent]
@@ -128,13 +128,7 @@ vows.describe('indent').addBatch({
'is ignored. Issue #4' : (source) ->
errors = coffeelint.lint(source)
- errors = coffeelint.lint(source)
- assert.equal(errors.length, 1)
- error = errors[0]
-
- assert.equal(error.rule, 'indentation')
- assert.equal(error.lineNumber, 7)
- assert.equal(error.context, 'Expected 2 got 10')
+ assert.isEmpty(errors)
'Consecutive indented chained invocations' :
@@ -403,6 +397,7 @@ vows.describe('indent').addBatch({
'is permitted': (source) ->
errors = coffeelint.lint(source)
assert.isEmpty(errors)
+
'Make sure indentation check is not affected outside proper scope' :
topic : """
a
@@ -421,4 +416,129 @@ vows.describe('indent').addBatch({
errors = coffeelint.lint(source)
assert.isEmpty(errors)
+ 'Handle edge-case weirdness with strings in objects':
+ # see test_no_empty_functions to understand why i needed to add this
+ # and to add the code to handle it
+ topic:
+ '''
+ call(
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbb" : first(
+ 'x = (@y) ->')
+ )
+ '''
+
+ 'is permitted': (source) ->
+ errors = coffeelint.lint(source)
+ assert.isEmpty(errors)
+
+ # Fixes people wanted to heavily indent if statements attached to assignment
+ # See: #468, #345
+ 'Handle different if statements indentations' :
+ topic : '''
+ r = unless p1
+ if p2
+ 1
+ else
+ 2
+ else
+ 3
+
+ s = unless p1
+ if p2
+ 1
+ else
+ 2
+ else
+ 3
+
+ t =
+ if z
+ true
+ else
+ true
+
+ u = if p1
+ if p2
+ 1
+ else
+ 2
+ else
+ 3
+
+ ->
+ x = unless y1
+ if y2
+ 1
+ else
+ y2
+ else
+ 3
+ '''
+
+ 'is permitted': (source) ->
+ errors = coffeelint.lint(source)
+ assert.isEmpty(errors)
+
+ # See #469
+ 'Handle paren alignment issues' :
+ topic:
+ '''
+ foo
+ .bar (
+ baz
+ ) ->
+ return
+
+ foo
+ .bar (baz) ->
+ return
+
+ bar (
+ baz
+ ) ->
+ return
+ '''
+
+ 'is permitted': (source) ->
+ errors = coffeelint.lint(source)
+ assert.isEmpty(errors)
+
+ # Fixes #312
+ 'Handles empty if statements':
+ topic:
+ '''
+ x = ->
+ for a in tokens
+ if a.type is "image"
+ "image"
+ else if a.type is "video" # ignore video for now!
+ else
+ "unknown"
+ '''
+
+ 'is permitted': (source) ->
+ errors = coffeelint.lint(source)
+ assert.isEmpty(errors)
+
+ # Fix #348
+ 'Handle off-indentation bug from arguments that should be ignored':
+ topic:
+ '''
+ angular.module('app', ['abc']).run([
+ '$a'
+ '$b'
+ ($xyz
+ $tuv
+ $qrs) ->
+ $http
+ .get '/'
+ .respond -> []
+ ])
+ '''
+
+ 'is permitted': (source) ->
+ errors = coffeelint.lint(source)
+ assert.isEmpty(errors)
+
}).export(module)

0 comments on commit bffa253

Please sign in to comment.