Skip to content

Commit bffa253

Browse files
committed
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
1 parent 05dc7a4 commit bffa253

File tree

2 files changed

+243
-17
lines changed

2 files changed

+243
-17
lines changed

‎src/rules/indentation.coffee‎

Lines changed: 116 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ module.exports = class Indentation
2424

2525
tokens: ['INDENT', '[', ']', '.']
2626

27+
keywords: [
28+
'->', '=>', '@', 'CATCH', 'CLASS', 'ELSE', 'FINALLY', 'FOR',
29+
'FORIN', 'FOROF', 'IDENTIFIER', 'IF', 'LEADING_WHEN', 'LOOP',
30+
'RETURN', 'SWITCH', 'THROW', 'TRY', 'UNTIL', 'WHEN', 'WHILE',
31+
'YIELD'
32+
]
33+
2734
constructor: ->
2835
@arrayTokens = [] # A stack tracking the array token pairs.
2936

@@ -82,8 +89,8 @@ module.exports = class Indentation
8289
numIndents = @getCorrectIndent(tokenApi)
8390

8491
# Now check the indentation.
85-
if not ignoreIndent and numIndents isnt expected
86-
return { context: "Expected #{expected} got #{numIndents}" }
92+
if not ignoreIndent and not (expected in numIndents)
93+
return { context: "Expected #{expected} got #{numIndents[0]}" }
8794

8895
# Return true if the current token is inside of an array.
8996
inArray: () ->
@@ -111,8 +118,10 @@ module.exports = class Indentation
111118
# Traverse up the token list until we see a CALL_START token.
112119
# Don't scan above this line
113120
findCallStart = tokenApi.peek(-callStart)
114-
while (findCallStart and findCallStart[0] isnt 'TERMINATOR')
121+
while (findCallStart and (findCallStart[0] isnt 'TERMINATOR' or
122+
not findCallStart.newLine?))
115123
{ first_line: lastCheck } = findCallStart[2]
124+
116125
callStart += 1
117126
findCallStart = tokenApi.peek(-callStart)
118127

@@ -147,20 +156,117 @@ module.exports = class Indentation
147156
if numIndents % expected isnt 0
148157
return { context: "Expected #{expected} got #{numIndents}" }
149158

159+
grabLineTokens: (tokenApi, lineNumber, all = false) ->
160+
{ tokensByLine } = tokenApi
161+
lineNumber-- until tokensByLine[lineNumber]? or lineNumber is 0
162+
163+
if all
164+
(tok for tok in tokensByLine[lineNumber])
165+
else
166+
(tok for tok in tokensByLine[lineNumber] when not
167+
tok.generated? and tok[0] isnt 'OUTDENT')
168+
150169
# Returns a corrected INDENT value if the current line is part of
151170
# a chained call. Otherwise returns original INDENT value.
152171
getCorrectIndent: (tokenApi) ->
153-
{ lineNumber, lines, tokens, i } = tokenApi
172+
{ lineNumber, lines, tokens } = tokenApi
154173

155174
curIndent = lines[lineNumber].match(/\S/)?.index
156175

157176
prevNum = 1
158177
prevNum += 1 while (/^\s*(#|$)/.test(lines[lineNumber - prevNum]))
159178

160-
prevLine = lines[lineNumber - prevNum]
161-
prevIndent = prevLine.match(/^(\s*)\./)?[1].length
162-
163-
if prevIndent > 0
164-
return curIndent - prevLine.match(/\S/)?.index
179+
prevTokens = @grabLineTokens tokenApi, lineNumber - prevNum
180+
181+
if prevTokens[0]?[0] is 'INDENT'
182+
# Pass both the INDENT value and the location of the first token
183+
# after the INDENT because sometimes CoffeeScript doesn't return
184+
# the correct INDENT if there is something like an if/else
185+
# inside an if/else inside of a -> function definition: e.g.
186+
#
187+
# ->
188+
# r = if a
189+
# if b
190+
# 2
191+
# else
192+
# 3
193+
# else
194+
# 4
195+
#
196+
# will error without: curIndent - prevTokens[1]?[2].first_column
197+
198+
return [curIndent - prevTokens[1]?[2].first_column,
199+
curIndent - prevTokens[0][1]]
165200
else
166-
return tokens[i][1]
201+
prevIndent = prevTokens[0]?[2].first_column
202+
# This is a scan to handle extra indentation from if/else
203+
# statements to make them look nicer: e.g.
204+
#
205+
# r = if a
206+
# true
207+
# else
208+
# false
209+
#
210+
# is valid.
211+
#
212+
# r = if a
213+
# true
214+
# else
215+
# false
216+
#
217+
# is also valid.
218+
for _, j in prevTokens when prevTokens[j][0] is '=' and
219+
prevTokens[j + 1]?[0] is 'IF'
220+
skipAssign = curIndent - prevTokens[j + 1][2].first_column
221+
ret = curIndent - prevIndent
222+
return [ret] if skipAssign < 0
223+
return [skipAssign, ret]
224+
225+
# This happens when there is an extra indent to maintain long
226+
# conditional statements (IF/UNLESS): e.g.
227+
#
228+
# ->
229+
# if a is c and
230+
# (false or
231+
# long.expression.that.necessitates(linebreak))
232+
# @foo()
233+
#
234+
# is valid (note that there an only an extra indent in the last
235+
# statement is required and not the line above it
236+
#
237+
# ->
238+
# if a is c and
239+
# (false or
240+
# long.expression.that.necessitates(linebreak))
241+
# @foo()
242+
# is also OK.
243+
while prevIndent > curIndent
244+
tryLine = lineNumber - prevNum
245+
prevTokens = @grabLineTokens tokenApi, tryLine, true
246+
247+
# This is to handle weird object/string indentation.
248+
# See: 'Handle edge-case weirdness with strings in objects'
249+
# test case in test_indentation.coffee or in the file,
250+
# test_no_empty_functions.coffee, which is why/how I
251+
# caught this.
252+
if prevTokens[0]?[0] is 'INDENT'
253+
prevIndent = prevTokens[0][1]
254+
prevTokens = prevTokens[1..]
255+
256+
t = 0
257+
# keep looping prevTokens until we find a token in @keywords
258+
# or we just run out of tokens in prevTokens
259+
until not prevTokens[t]? or prevTokens[t][0] in @keywords
260+
t++
261+
262+
# slice off everything before 't'
263+
prevTokens = prevTokens[t..]
264+
prevNum++
265+
266+
# if there isn't a valid token, restart the while loop
267+
continue unless prevTokens[0]?
268+
269+
# set new "prevIndent"
270+
prevIndent = prevTokens[0]?[2].first_column
271+
272+
return [curIndent - prevIndent]

‎test/test_indentation.coffee‎

Lines changed: 127 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,7 @@ vows.describe('indent').addBatch({
128128

129129
'is ignored. Issue #4' : (source) ->
130130
errors = coffeelint.lint(source)
131-
errors = coffeelint.lint(source)
132-
assert.equal(errors.length, 1)
133-
error = errors[0]
134-
135-
assert.equal(error.rule, 'indentation')
136-
assert.equal(error.lineNumber, 7)
137-
assert.equal(error.context, 'Expected 2 got 10')
131+
assert.isEmpty(errors)
138132

139133
'Consecutive indented chained invocations' :
140134

@@ -403,6 +397,7 @@ vows.describe('indent').addBatch({
403397
'is permitted': (source) ->
404398
errors = coffeelint.lint(source)
405399
assert.isEmpty(errors)
400+
406401
'Make sure indentation check is not affected outside proper scope' :
407402
topic : """
408403
a
@@ -421,4 +416,129 @@ vows.describe('indent').addBatch({
421416
errors = coffeelint.lint(source)
422417
assert.isEmpty(errors)
423418

419+
'Handle edge-case weirdness with strings in objects':
420+
# see test_no_empty_functions to understand why i needed to add this
421+
# and to add the code to handle it
422+
topic:
423+
'''
424+
call(
425+
"aaaaaaaaaaaaaaaaaaaaaaaaaaa
426+
bbbbbbbbbbbbbbbbbbbbbbb" : first(
427+
'x = (@y) ->')
428+
)
429+
'''
430+
431+
'is permitted': (source) ->
432+
errors = coffeelint.lint(source)
433+
assert.isEmpty(errors)
434+
435+
# Fixes people wanted to heavily indent if statements attached to assignment
436+
# See: #468, #345
437+
'Handle different if statements indentations' :
438+
topic : '''
439+
r = unless p1
440+
if p2
441+
1
442+
else
443+
2
444+
else
445+
3
446+
447+
s = unless p1
448+
if p2
449+
1
450+
else
451+
2
452+
else
453+
3
454+
455+
t =
456+
if z
457+
true
458+
else
459+
true
460+
461+
u = if p1
462+
if p2
463+
1
464+
else
465+
2
466+
else
467+
3
468+
469+
->
470+
x = unless y1
471+
if y2
472+
1
473+
else
474+
y2
475+
else
476+
3
477+
'''
478+
479+
'is permitted': (source) ->
480+
errors = coffeelint.lint(source)
481+
assert.isEmpty(errors)
482+
483+
# See #469
484+
'Handle paren alignment issues' :
485+
topic:
486+
'''
487+
foo
488+
.bar (
489+
baz
490+
) ->
491+
return
492+
493+
foo
494+
.bar (baz) ->
495+
return
496+
497+
bar (
498+
baz
499+
) ->
500+
return
501+
'''
502+
503+
'is permitted': (source) ->
504+
errors = coffeelint.lint(source)
505+
assert.isEmpty(errors)
506+
507+
# Fixes #312
508+
'Handles empty if statements':
509+
topic:
510+
'''
511+
x = ->
512+
for a in tokens
513+
if a.type is "image"
514+
"image"
515+
else if a.type is "video" # ignore video for now!
516+
else
517+
"unknown"
518+
'''
519+
520+
'is permitted': (source) ->
521+
errors = coffeelint.lint(source)
522+
assert.isEmpty(errors)
523+
524+
# Fix #348
525+
'Handle off-indentation bug from arguments that should be ignored':
526+
topic:
527+
'''
528+
angular.module('app', ['abc']).run([
529+
'$a'
530+
'$b'
531+
($xyz
532+
$tuv
533+
$qrs) ->
534+
$http
535+
.get '/'
536+
.respond -> []
537+
])
538+
'''
539+
540+
'is permitted': (source) ->
541+
errors = coffeelint.lint(source)
542+
assert.isEmpty(errors)
543+
424544
}).export(module)

0 commit comments

Comments
 (0)