SpaceConsistencyBear only catches spaces that match indent_size when use_spaces = False

[WillowP]

Overview Description

Hello!

I have a Python project that is written using tabs. Without setting indent_size, I would expect that files written with 2 spaces or 4 spaces for indentation would be caught by SpaceConsistencyBear, but only files with 4 spaces are caught.

Steps To Reproduce

1. Create .coafile like this:

#.coafile
[all]
bears = SpaceConsistencyBear
use_spaces = False
files = my_file.py

2. Create my_file.py file like this (notice only 2 space for indent):

#my_file.py
def my_func():
  print("Hello Coala!")

3. Run $ coala, receive no suggested patches.
4. Modify my_file.py like this (with 4 spaces for indent):

#my_file.py
def my_func():
    print("Hello Coala!")

5. Run $ coala, receive suggested patches.
6. Add indent_size = 2 to the .coafile:

#.coafile
[all]
bears = SpaceConsistencyBear
use_spaces = False
files = my_file.py
indent_size = 2

7. Run $ coala on both versions of my_file.py - both are caught.

Actual Results

SpaceConsistencyBear only catches spaces that match indent_size when use_spaces = False

Expected Results

SpaceConsistencyBear catches all files indented with spaces when use_spaces = False

Reproducibility

Every time.

Additional Information:

Hope this helps :)

[moksh-pathak]

Hi, can I work on this issue?

[abhishalya]

@Heisenberg403 Sure, feel free to work on the issue. Do have a look if there's a PR already created or for this issue so that the efforts are not duplicated :)

[thehanimo]

I did some digging and the code responsible for this is:

coala/coalib/bearlib/spacing/SpacingHelper.py

Lines 77 to 120 in 7d4247b

	@enforce_signature
	def replace_spaces_with_tabs(self, line: str):
	"""
	Replaces spaces with tabs where possible. However in no case only one
	space will be replaced by a tab.
	Example: " \t a_text another" will be converted to
	"\t a_text\tanother", assuming the tab_width is set to 4.
	:param line: The string with spaces to replace.
	:return: The converted string.
	"""
	currspaces = 0
	result = ''
	# Tracking the index of the string isnt enough because tabs are
	# spanning over multiple columns
	tabless_position = 0
	for char in line:
	if char == ' ':
	currspaces += 1
	tabless_position += 1
	elif char == '\t':
	space_count = (self.tab_width - tabless_position
	% self.tab_width)
	currspaces += space_count
	tabless_position += space_count
	else:
	result += currspaces*' ' + char
	currspaces = 0
	tabless_position += 1
	# tabless_position is now incremented to point _after_ the current
	# char
	if tabless_position % self.tab_width == 0 and currspaces:
	if currspaces == 1 and char == ' ':
	result += ' '
	else:
	result += '\t'
	currspaces = 0
	result += currspaces*' '
	return result

The main reason this happens is because SpaceConsistencyBear does not calculate indentation sizing on the fly. Not specifying indent_size = 2 is equivalent to setting indent_size = 4. And when indent_size is set, SpaceConsistencyBear does not consider spaces with a lower width. So when you don't specify it in your first example, the double-spaced indent is ignored. In your second example where you used indent_size = 2, look carefully at what is suggested and you'll see that the two-space indent is replaced with one tab and the four-space indent is replaced with two tabs! Not sure if that is expected behaviour.

@abhishalya I've not thought this through but will modifying/creating a function to detect indentation on the fly work? All we need is the leading whitespace for a line and it's predecessor's. The current replace_spaces_with_tabs function does not seem to have been built to fix indentation.

EDIT:
There's a get_indentation function in SpacingHelper.py which can help. We will need to account for space widths from 1 to indent-size. Maybe scan through the file to get min space-width and suggest changes accordingly?