Add an option to force future annotations imports on all files #5

Rogdham · 2022-05-04T20:30:32Z

Hello,

This PR adds an optional flag --force-future-annotations (or config parameter).

If provided, the plugin also reports files missing the from __future__ import annotations import even if PEP 563 would not have allowed to rewrite anything. In that case, code FA101 is used instead of FA100.

This allows to make sure that all linted files include the future import, which one may want for consistency.

Of course, feel free to suggest any changes 🙂

codecov · 2022-05-04T20:49:45Z

Codecov Report

Merging #5 (a8dc356) into main (2ffb76e) will decrease coverage by 3.44%.
The diff coverage is 88.88%.

@@             Coverage Diff             @@
##              main       #5      +/-   ##
===========================================
- Coverage   100.00%   96.55%   -3.45%     
===========================================
  Files            1        1              
  Lines           46       58      +12     
===========================================
+ Hits            46       56      +10     
- Misses           0        2       +2

Impacted Files	Coverage Δ
flake8_future_annotations/checker.py	`96.55% <88.88%> (-3.45%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ffb76e...a8dc356. Read the comment docs.

codecov · 2022-05-04T20:49:57Z

Codecov Report

Merging #5 (e06c4d5) into main (2ffb76e) will decrease coverage by 3.44%.
The diff coverage is 88.88%.

@@             Coverage Diff             @@
##              main       #5      +/-   ##
===========================================
- Coverage   100.00%   96.55%   -3.45%     
===========================================
  Files            1        1              
  Lines           46       58      +12     
===========================================
+ Hits            46       56      +10     
- Misses           0        2       +2

Impacted Files	Coverage Δ
flake8_future_annotations/checker.py	`96.55% <88.88%> (-3.45%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ffb76e...e06c4d5. Read the comment docs.

TylerYep · 2022-05-04T20:56:22Z

flake8_future_annotations/checker.py

@@ -78,6 +79,7 @@ def visit_Attribute(self, node: ast.Attribute) -> None:
 class FutureAnnotationsChecker:
    name = "flake8-future-annotations"
    version = "0.0.4"
+    force_future_annotations = False


Why make this a class-level attribute? Seems like we could just define this variable in __init__ instead?

It seems that parse_options must be a classmethod else it does not work with flake8. This is why I put the attribute on the class itself and not the instance.

Tell me if I'm missing something.

TylerYep · 2022-05-04T21:00:06Z

tests/test_checker.py

-
-        assert len(errors) == 0, (str(filepath), errors)
-
+@pytest.mark.parametrize("force_future_annotations", (False, True))


Can you add separate test cases for this feature? I don't think we need to touch the existing test cases.

Having all test cases combined into a mega test makes it difficult to read.

Tell me if that's better now

TylerYep · 2022-05-04T21:00:31Z

flake8_future_annotations/checker.py

+            "--force-future-annotations",
+            action="store_true",
+            parse_from_config=True,
+            help="Force the use of future annotations imports",


help="Force the use of from __future__ import annotations in all files.",

TylerYep · 2022-05-04T21:00:56Z

flake8_future_annotations/checker.py

+            yield lineno, char_offset, message, type(self)
+
+    @staticmethod
+    def add_options(option_manager: Any) -> None:


Can we use OptionManager instead of Any?
from flake8.options.manager import OptionManager

I tried but mypy complains that there are no types exposed for flake8 module.

I found this issue on the flake8 repo which seems relevant.

If you prefer I can use OptionManager anyways and telling mypy to ignore it (making it typechecked as if it was Any).

TylerYep · 2022-05-04T21:05:24Z

Thanks for the contribution! I left some comments of things to fix, but overall it looks like a good change.

TylerYep · 2022-05-12T14:33:47Z

I'm having trouble testing this via pip install -e . because I keep getting the error:

Traceback (most recent call last):
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/bin/flake8", line 8, in <module>
    sys.exit(main())
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/main/cli.py", line 22, in main
    app.run(argv)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/main/application.py", line 375, in run
    self._run(argv)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/main/application.py", line 363, in _run
    self.initialize(argv)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/main/application.py", line 344, in initialize
    self.register_plugin_options()
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/main/application.py", line 169, in register_plugin_options
    self.check_plugins.register_options(self.option_manager)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/plugins/manager.py", line 500, in register_options
    list(self.manager.map(register_and_enable))
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/plugins/manager.py", line 309, in map
    yield func(self.plugins[name], *args, **kwargs)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/plugins/manager.py", line 496, in register_and_enable
    call_register_options(plugin)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/plugins/manager.py", line 409, in generated_function
    return method(optmanager, *args, **kwargs)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/plugins/manager.py", line 220, in register_options
    add_options(optmanager)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8_future_annotations/checker.py", line 106, in add_options
    option_manager.add_option(
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/site-packages/flake8/options/manager.py", line 415, in add_option
    self._current_group.add_argument(*option_args, **option_kwargs)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/argparse.py", line 1436, in add_argument
    return self._add_action(action)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/argparse.py", line 1638, in _add_action
    action = super(_ArgumentGroup, self)._add_action(action)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/argparse.py", line 1450, in _add_action
    self._check_conflict(action)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/argparse.py", line 1587, in _check_conflict
    conflict_handler(action, confl_optionals)
  File "/Users/tyler.yep/.pyenv/versions/3.10.0/lib/python3.10/argparse.py", line 1596, in _handle_conflict_error
    raise ArgumentError(action, message % conflict_string)
argparse.ArgumentError: argument --force-future-annotations: conflicting option string: --force-future-annotations

This error is blocking the merge + release of this PR. If you can help me debug this (I suspect it might be a local issue?), that would help me a lot

Rogdham · 2022-05-12T18:18:01Z

Hello @TylerYep and sorry for the issue, I had an old virtualenv which was preventing me from catching it.

After some non-trivial debugging, I added a commit to this PR to fix the problem. It seems that add_options was called several times by flake8, which was trying to add the option twice (hence the error message). It seems that the F prefix was already used by some other plugins, which was the issue. By specifying FA instead, the issue seems to go away.

Note that you may need to delete your virtualenv and create a new one from scratch (in which you run pip install -e . again), otherwise the change may not catch up.

Tell me if that works for you!

TylerYep · 2022-05-13T05:07:47Z

Thank you for fixing the error! I have tested+merged the change and will put out a new release within the next few days.

Rogdham · 2022-05-13T07:05:55Z

Fantastic! Thank you 🥳

TylerYep reviewed May 4, 2022

View reviewed changes

Add option to always report missing FA imports

d989260

Fix multiple calls to add_options

98eb16a

reorder functions

e06c4d5

TylerYep approved these changes May 13, 2022

View reviewed changes

TylerYep merged commit 8732fa2 into TylerYep:main May 13, 2022

Rogdham deleted the force-future-annotations branch May 13, 2022 07:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an option to force future annotations imports on all files #5

Add an option to force future annotations imports on all files #5

Rogdham commented May 4, 2022

codecov bot commented May 4, 2022

codecov bot commented May 4, 2022 •

edited

Loading

TylerYep May 4, 2022

Rogdham May 5, 2022

TylerYep May 4, 2022

Rogdham May 5, 2022

TylerYep May 4, 2022

Rogdham May 5, 2022

TylerYep May 4, 2022

Rogdham May 5, 2022

TylerYep commented May 4, 2022

TylerYep commented May 12, 2022

Rogdham commented May 12, 2022

TylerYep commented May 13, 2022

Rogdham commented May 13, 2022


		assert len(errors) == 0, (str(filepath), errors)

		@pytest.mark.parametrize("force_future_annotations", (False, True))

Add an option to force future annotations imports on all files #5

Add an option to force future annotations imports on all files #5

Conversation

Rogdham commented May 4, 2022

codecov bot commented May 4, 2022

Codecov Report

codecov bot commented May 4, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerYep commented May 4, 2022

TylerYep commented May 12, 2022

Rogdham commented May 12, 2022

TylerYep commented May 13, 2022

Rogdham commented May 13, 2022

codecov bot commented May 4, 2022 •

edited

Loading