Pylance-release: Don't offer completions / auto-imports for class/function/parameter/import alias names

Created on 23 Jul 2020  路  12Comments  路  Source: microsoft/pylance-release

Describe the bug
For tests automatically adds import for fixture. Pytest know where find out fixture(conftest.py), not required make auto import for it.

To Reproduce
1) install pytest
2) create conftest.py with that example code:

import pytest

@pytest.fixture()
def ExampleFixture():
    return ['This', 'is', 'example', 'fixture']

3) create test_example.py with that code:

import pytest

def test_is_instance_list(ExampleFixture):
    assert is instance(ExampleFixture, list)

Expected behavior
Import for ExampleFixture is not added

VS Code extension or command-line
VS Code extension
Version: v2020.7.3

enhancement fixed in next version

Most helpful comment

Before we add any settings, we need to fix this completion context issue and not offer completions in conditions where we can't actually know a name. I wouldn't want to add a setting to work around an issue that we're going to end up fixing anyway (and that setting in particular is not as simple as you might think).

All 12 comments

Hey @anatoly-kor!

Just to be clear, what you're describing is that you are seeing a completion that auto-imports the fixture and you wouldn't expect this behaviour?

Hi @savannahostrowski. Yes! I mean when test(with fixture) is wrote, auto-import add imports, but doesn't must because for pytest not required import fixture modules.
Quote from docs:

You don鈥檛 need to import the fixture you want to use in a test, it automatically gets discovered by pytest.

Thanks for confirming. In this case, the problem is that completions are offered in some contexts where they don't make sense (parameter, function, and class names). If you're defining a function, we shouldn't try to autocomplete a parameter name (because we would never know what the right names are for a brand new function).

Seeing the same issue here. Maybe a context could be added to the auto-import feature to disable it when a conftest.py file is in the same directory.

Overall I must say I've found the auto-import feature to be more of a hurdle than anything else. When an auto-import gets added is usually a duplicate or something I didn't really want, and in most case the module from which I'm adding a class or function is not discovered. It would be nice if the feature could be disable completely.

If you have feedback about what could be improved with auto-imports in general, we'd appreciate those in a separate issue. If we are offering something that's a duplicate or something you don't want, then that's something to solve. This specific issue is about offering completions for names we can't actually know (brand new funcs/classes/parameters) and would be hit regardless of the auto-import setting.

It would be nice if the feature could be disable completely.

That is #64, which we plan to do.

Right, I didn't want to confuse the issue and #64 sounds good.

To avoid auto-importing fixtures, something that could be checked is if the object has the attribute _pytestfixturefunction and if so not offer auto-import.

I like the solution proposed in #178. Today, the setting python.analysis.autoImportCompletions allows either true or false, but instead it could be always, ask or never. That way we don't need special rules for specific test libraries such as pytest.

That level of control over completions is not possible in VS Code as it stands now. We return a list of completions given a location in the code, and VS Code displays it. We can't defer this or ask for user input if they want to expand some extra stuff, because this is effectively a request for state where there's only one answer given the current document and location.

The auto import feature is really useful in most scenarios, however as stated in this issue, it can be a little inconvenient with pytest when you're entering a fixture into your test function's signature and it auto-imports. Pytest does not require you to import its fixtures from a conftest.py file since its test discovery process looks for that filename.

By the design of the pytest library, users use a filename that is always called conftest.py and can be at different directory levels within a project directory structure to indicate the scope of the contained fixtures.

Is it possible to create an array or object setting to be used in the workspace's settings.json that takes a glob pattern to ignore a certain directory and/or filename?

For example, VS Code's seach.exclude setting is one of many examples using this.

The setting might look something like:

    "python.analysis.autoImportExclusions": {
        "conftest.py": true,
        "**/conftest.py": true,
        ...
    },

Before we add any settings, we need to fix this completion context issue and not offer completions in conditions where we can't actually know a name. I wouldn't want to add a setting to work around an issue that we're going to end up fixing anyway (and that setting in particular is not as simple as you might think).

OK, if PyLance doesn't add auto-imports for pytest fixtures once you've put in the fix, then I'm not worried how that's achieved. If a setting isn't required, then even better!

This issue has been fixed in version 2020.11.1, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#2020111-11-november-2020

Was this page helpful?
0 / 5 - 0 ratings