B904: ensure the raise is in the same context with the except

[isidentical]

Resolves #190. Introduces a simple version of context management, though more detailed analysis might need something a little bit specialized in the future (e.g stuff like this).

[cooperlees]

@Zac-HD - Thoughts?

@isidentical - Sorry - we run black formatting with an experimental feature:

• To fix: black --check --experimental-string-processing .

[isidentical]

@cooperlees ah, my bad. By the way, is there a particular reason why this project doesn't use pre-commit (or some other script) to do these stuff? If there is none, I can create a patch about it.

[Zac-HD]

The concept sounds good to me, though the diff looks like it's patching B902 instead of B904?

(this also seems pretty rare to me, but as a general rule "if someone provides a patch, it matters enough to merge")

[isidentical]

No actually not, I do not need to change anything on the B904's implementation, since it just checks if there are any except handlers within the current node_stack. What I did was to dynamically determine the current node stack from the context (through a property). I also needed the alter the B902 to work with the new implementation but no changes on it's behavior.

[cooperlees]

@cooperlees ah, my bad. By the way, is there a particular reason why this project doesn't use pre-commit (or some other script) to do these stuff? If there is none, I can create a patch about it.

There is no real reason other than I dislike pre-commit - But I deal with it since it seems the norm in the open source world. My editor keeps me black formatted and running the tests here are easy so I just do it manually. I would accept a pre-commit or something to make it easier for contributors tho.

[cooperlees]

LGTM - Just wondering if we could simplify the integer comparison I commented on with a suggestion - Just tell me if I am missing something from that logic.

[cooperlees]

Cleaner, but it's not really used anywhere else, but I do agree this makes it easier to know what it is, if you know what cls generally means ...

[isidentical]

I think cls makes a bit more sense than something[-2] (which is a lot less clear).

[davidism]

Very occasionally, a more nested pattern comes up, such as trying a backport import as well. I don't think I actually have any code that does this, but would this change also avoid the lint message in this case?

try:
    from preferred_library import Thing
except ImportError:
    try:
        from fallback_library import Thing
    except ImportError:
        class Thing:
            def __getattr__(self, name):
                raise AttributeError

[isidentical]

Very occasionally, a more nested pattern comes up, such as trying a backport import as well. I don't think I actually have any code that does this, but would this change also avoid the lint message in this case?

It should work for that case as well, will add it to the tests.

[cooperlees]

I think this all makes sense to me. I'd love a second maintainer to pass this over (not than many are active) as this confuses me in some parts. But I don't know how to articulate it. Will merge soon if no one objects. Thanks!

[isidentical]

Thanks @cooperlees! I'll try to address your review.

I'd love a second maintainer to pass this over (not than many are active) as this confuses me in some parts.

I assume this was not targeted at me, but I'm kind of interested in contributing more. Would love to help with other duties as well (PR reviews etc.).

[Zac-HD]

Looks good to me - thanks, @isidentical!

[cooperlees]

This is only failing on black. I'll fix before merging. If you get time, please reformat. Otherwise I'll take care of it.

[cooperlees]

@isidentical - Sorry it's taken me so long but now this with a rebase to master is unhappy. Do you have any cycles to clean this up? Was looking to release today. I'll see if I can understand too now or I might release without this and we just cut another release when you get time.

[cooperlees]

Is this all that is needed? How did this ever work if so :|

Suggested change

	context = attr.ib(default=attr.Factory(list))
	contexts = attr.ib(default=attr.Factory(list))

[cooperlees]

Ok - That makes sense to me. Will give @isidentical a while to respond. No idea how that self.contexts got changed to self.context ... Unless it was a refactor / push error.