ignore-pass-statements introduces unnecessary pass statements

[yarnabrina]

If "--ignore-pass-statements" is used, removed imports are being replaced with pass.

>>> import autoflake
>>> 
>>> print(autoflake.__version__)
2.0.0
>>> 
>>> code = """
... import os
... 
... print("hello")
... """
>>> 
>>> fixed_code = autoflake.fix_code(code, ignore_pass_statements=True)
>>> print(fixed_code)

pass

print("hello")

>>> 

This looks like a bug.

[fsouza]

Yeah, removed imports are always replaced with pass and then the pass statements are cleared, it's an unfortunate combination. We should definitely do better.

[wlrd]

@fsouza do we know why this is happening and is anyone taking a look? If not, I can try to dig deeper into figuring out what is going on here.

[fsouza]

Oh, it's just how it works, it's an accident of the design: to guarantee that the code is valid after the removal of the import, autoflake plays safe and puts a pass statement (trusting that it can be removed later, but then if ignore-pass-statement is set, it's never removed), you can see the comment in the code and where it happens:

autoflake/autoflake.py

Lines 657 to 661 in 60eeb31

	# We need to replace import with "pass" in case the import is the
	# only line inside a block. For example,
	# "if True:\n import os". In such cases, if the import is
	# removed, the block will be left hanging with no body.
	return get_indentation(line) + "pass" + get_line_ending(line)

We can reuse the logic to determine whether a pass can be removed to safely remove the import, or have a way to identify which passes were introduced by the removal of the import.

No one is currently working on this, feel free to take a stab!

[KevinMGranger]

I intend to take a swing at this.

My approach:

1. Generate a random id (just a random string) for the whole execution
2. Replace unused imports instead with pass # AUTOFLAKE:$RANDOM_ID
3. Remove pass lines matching the above, even if ignore-pass-statement is set.

I think this is good enough for now. If we want to address fancier features like #234: removing newly-empty TYPE_CHECKING blocks in the future, it might be better to express each line as either a string or some sort of special RemovedImport() sentinel.

(Side note: I think #234 as mentioned above is in the same boat as #52).

[keriksson-rosenqvist]

Is there any update on this?