"Remove all unused imports" should not remove implicitly used pytest fixture imports

[tl24]

Does the source.unusedImports action support keeping an unused import tagged with a comment? If so what does it support? For example we have pytest fixtures that we import which are flagged as unused

from .mocks import unstub_and_verify_fixture  # pylint: disable=unused-import

If I do remove all unused imports that import gets removed. I also tried using # noqa that some other linters support.

[erictraut]

Pylance isn't a linter (like pylint or ruff). Pylance is built on a type checker called pyright. The types of conditions that linters and type checkers detect are mostly distinct, but there is some overlap, such as with unused imports.

For type checkers, there is a standard way (defined in PEP 484) to suppress an error on a line. You use the comment # type: ignore. Pyright also supports # pyright: ignore. And you can use the latter with a specific error code as well (# pyright: ignore[reportUnusedImport]).

[debonte]

# type: ignore ... # pyright: ignore ...# pyright: ignore[reportUnusedImport]

Of these three, only # pyright: ignore worked for me in this scenario.

If I'm in strict mode I get a diagnostic and they all work to suppress the diagnostic. But I believe the "unused imports" action looks for DiagnosticCategory.UnusedCode hints rather than "true" diagnostics, and I'm guessing that's the reason for the difference.

[erictraut]

The reportUnusedImport diagnostic check is enabled by default only in strict type checking mode. If type checking is disabled or in "basic" mode, you should never see a diagnostic for this condition.

You will see the symbol "grayed out", but that's expected and should occur regardless of the reportUnusedImport setting. If the grayed-out text really bothers you, you could add an explicit (dummy) reference to the symbol or use a redundant import form.

[erictraut]

Ah, there was a bug in the diagnostic filtering code where it incorrectly filtered "UnusedCode" diagnostics when a bare # pyright: ignore was used on that line. That was unintentional and will be fixed in the next release.

[debonte]

If the grayed-out text really bothers you, you could add an explicit (dummy) reference to the symbol or use a redundant import form.

The user's request was some way to prevent the "Remove all unused imports" action from removing certain imports that are used but not in a way that Pylance recognizes. Specifically, in pytest if a test function has an argument with the same name as a fixture, that fixture will be automatically passed in at test-time, but we don't see it as a reference.

Using # pyright: ignore was actually suggested as a solution for this in #4083.

@rchiodo, both this issue and #4083 are about inadvertently removing pytest fixture imports. Maybe the pytest support in Pylance could be extended to prevent "Remove all unused imports" from removing these imports?

[rchiodo]

Maybe? That would likely require special code in the Remove unused imports action to special case this scenario.

[tl24]

The # pyright : ignore solution is fine with me

[erictraut]

The # pyright : ignore solution is fine with me

This behavior is unintended and due to a bug that will be fixed in the next release. It's not a solution to this problem.

[debonte]

Moving this issue to discussion as an enhancement request for comments and upvotes.

[luabud]

Created this discussion item related to this, to track the idea of supporting #noqa for skipping unused imports diagnostics: #5080