E731: Introduction of invalid indentation

[addisoncrump]

Python 3 allows for the use of different indentation styles, provided that they remain consistent within a single block and do not add tabs after spaces.

E731 fix may introduce invalid indentation if the indentation choice in the first block does not match the indentation of the block in which the fix is applied. As an example:

def a():
	pass

def b():
  c = lambda x: print(x)
  c("hi")

a uses tabs, b uses spaces. Ruff infers the indentation from the first observed, so the replacement suggested is:

def a():
	pass

def b():
  def c(x):
  	return print(x)
  c("hi")

The inner function c uses tabs, which triggers a tab inconsistency syntax error.

This was discovered by #4822.

[dhruvmanila]

This is occurring because the detect_indentation function will use the first Indent token to determine the indentation which in this case is the indented body of function a. I'm not sure what the correct solution would look like. If each block can have it's own indentation, a possible solution would be to keep track of indentation per-block and resolve it as per the location of the code generation.

[charliermarsh]

Yeah it might be tough right now. We have to find the indentation of the containing block. (This is an unlikely bug in practice but it should definitely be fixed when possible.)

[addisoncrump]

Er, so this no longer seems to trigger a tab inconsistency error. Local python accepts it no problem, and ruff no longer emits a panic about generating invalid content.

Solved? 😝

[T-256]

It's still issue if you've converted between a and b indentations:
After unsafe fix:

[dylwil3]

Just to clarify the state of this issue:

• The fix does not cause a syntax error
• The fix may cause a violation of E101 (which unfortunately does not have an autofix)
• The issue vanishes in the presence of an auto-formatter (either the original code would never have been in that state or, if somehow it was, formatting afterwards would fix the discrepancy)

It's hard for me to imagine a situation where this would arise in "real world" code.

It may be that other issues arise where it becomes clear that we want the generator to know about "local" styles, in which case this problem would be solved - but this particular case doesn't quite seem like motivation enough to introduce that functionality.

I would vote to close this as unplanned. What do others think?

[dhruvmanila]

It may be that other issues arise where it becomes clear that we want the generator to know about "local" styles, in which case this problem would be solved - but this particular case doesn't quite seem like motivation enough to introduce that functionality.

I would vote to close this as unplanned. What do others think?

Yeah, it'll also make the Generator a bit complex because it would need to track state on a per-block basis. I'd be fine in closing it but would check-in with @addisoncrump as the issue author.