regression in 2.0.0: add test for false warning about unused exception when using locals()

[dirk-thomas]

This patch adds a test for a case which worked with the previous release (1.6.0, no warning) but fails with the latest release (2.0.0, false warning about unused e).

Since I don't know where / when between the releases the behavior regressed I created this test to cover that use case. Once the bug has been fixed the test should pass again.

[sigmavirus24]

This has nothing to do with locals(). Prior to 2.0 we didn't have the check for unused exception bindings. I don't think this will ever be "fixed"

[asmeurer]

Using locals() is kind of like import *, which pyflakes does warn about. I'd personally rather have pyflakes warn and give false positives for locals() meta-programming than give up and stop warning at all. At least then it might still catch unrelated bugs.

[dirk-thomas]

This has nothing to do with locals(). Prior to 2.0 we didn't have the check for unused exception bindings.

Thanks for clarifying why it has changed.

I don't think this will ever be "fixed"

Can you elaborate why that is the case? Outside of exception handling locals() is specially handled exactly for this case - to avoid false warnings about unused variables. I understand why it might be something which doesn't get addressed right now but why "never"?

[sigmavirus24]

Outside of exception handling locals() is specially handled exactly for this case

I'm not confident that's accurate, but I'm also not certain that's inaccurate. Can you share your evidence for this assertion?

[sloretz]

@sigmavirus24 I think @dirk-thomas might be referring to this test.

pyflakes/pyflakes/test/test_other.py

Lines 1178 to 1186 in a13dbe6

	def test_unusedVariableAsLocals(self):
	"""
	Using locals() it is perfectly valid to have unused variables
	"""
	self.flakes('''
	def a():
	b = 1
	return locals()
	''')

[jayvdb]

Using locals() is kind of like import *

The approach that I used for import * was to create a special class of binding StarImportation, which allowed for a different error to be reported, which could then be ignored if desired.

The same could be done with locals() , to mark all names as used by the locals() , but that usage would be replaced by any other more specific usage of the name.

Then the report would show which names in the scope are only used by locals() , and that more specific error could then be ignored when appropriate.

#343 fixes this bug, and provides the basis for reporting unused variables differently if they were only used by locals()

[dirk-thomas]

#343 fixes this bug

With the alternative pull request up (though still not being merge) I will close this PR and remove my fork.