Fix imports in a module containing __future__

[vthemelis]

Fixes: #385

[coveralls]

coverage: 97.811% (-0.008%) from 97.819% when pulling da99fe3 on vthemelis:injecting-code-before-future into 068edfd on agronholm:master.

[agronholm]

Are the test suites passing for you locally?

[vthemelis]

Are the test suites passing for you locally?

Yes, I can get a clean run with tox. I'm just having trouble with linters being applied to places where they shouldn't by pre-commit.ci though which leads to redness on the PR.

[vthemelis]

@agronholm All 💚 now!

[agronholm]

Yes, I can get a clean run with tox. I'm just having trouble with linters being applied to places where they shouldn't by pre-commit.ci though which leads to redness on the PR.

Installing pre-commit locally would have made that easier..

[agronholm]

The test for this really belongs in test_transformer.py, not in dummymodule.py.

[vthemelis]

Installing pre-commit locally would have made that easier..

How do you run it locally? I tried pre-commit run but it was no-op for me.

[vthemelis]

The test for this really belongs in test_transformer.py, not in dummymodule.py.

It seemed like dummymodule.py gave me pretty good integration coverage already so I went this way. Do you want me to add unit tests explicitly to test_transformer.py too?

[agronholm]

First of all, you should do pre-commit install which ensures that the checks are run when you do git commit. If you want to run the checks without the git hook, do pre-commit run -a (-a meaning "run for all files").

[agronholm]

The problem seems to stem from __post_init__() only setting the right injection index if self.node is set, which it isn't when a module is being transformed by the import hook.

[agronholm]

I'm likely to reject this PR in favor of an alternate approach and an alternate test.

[agronholm]

Here's a test that fails without any patch:

def test_respect_future_import() -> None:
    # Regression test for #385
    node = parse(
        dedent(
            '''
            from __future__ import annotations
            
            def foo() -> int:
                return 1
            '''
        )
    )
    TypeguardTransformer().visit(node)
    assert (
        unparse(node)
        == dedent(
            '''
            from __future__ import annotations
            from typeguard import TypeCheckMemo
            from typeguard._functions import check_return_type

            def foo() -> int:
                memo = TypeCheckMemo(globals(), locals())
                return check_return_type('foo', 1, int, memo)
            '''
        ).strip()
    )

[vthemelis]

I'm likely to reject this PR in favor of an alternate approach and an alternate test.

Fair enough! Let me know if you want me to help.

[vthemelis]

Here's a test that fails without any patch:

Thanks for this! Seems to pass with my changes. Shall I commit it?

[agronholm]

I'm still considering my options. If I figure out a better way, I'll just push that and close this PR. Either way I'll let you know.

[vthemelis]

I'm still considering my options. If I figure out a better way, I'll just push that and close this PR. Either way I'll let you know.

Sounds good. I added the test in case we merge this PR.

[agronholm]

Alright, I resolved this via c11057a. Thanks for your initiative on the matter.

[vthemelis]

Thanks! Would it be possible to release the latest changes?

[agronholm]

Thanks! Would it be possible to release the latest changes?

Done.