indent: Fix several issues related to indentation

swang · swang · commit bffa2532efd8 · 2015-08-20T22:55:07.000-07:00
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
diff --git a/src/rules/indentation.coffee b/src/rules/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]
diff --git a/test/test_indentation.coffee b/test/test_indentation.coffee
@@ -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)
