From ebba681c856d08a70bb8a75990506da7d72505d4 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Wed, 12 Jun 2024 13:32:31 +0500 Subject: [PATCH] PY-62208 Don't suggest names shorter than five characters unless it's an extended completion Otherwise, we end up with dozens of unintentionally public names such as "s", "i", "k" even in the standard library (e.g. `this.s` or `pickletools.i`). Ideally, we should rely on .pyi stubs and the content of `__all__` to offer only explicitly exposed API, but not every module has any of those two, and it's not clear how to match .py files and the corresponding .pyi stubs fast enough for completion. GitOrigin-RevId: 163c472654e60ae63ff893142b8ddb9accc56393 --- .../python/PythonCommonCompletionTest.java | 11 ++++++++-- .../PyClassNameCompletionContributor.java | 22 ++++++++++++------- .../a.after.py | 1 - .../tooCommonImportableNamesNotSuggested/a.py | 1 - .../mod.py | 2 -- .../a.py | 1 + .../mod.py | 6 +++++ 7 files changed, 30 insertions(+), 14 deletions(-) delete mode 100644 python/testData/completion/tooCommonImportableNamesNotSuggested/a.after.py delete mode 100644 python/testData/completion/tooCommonImportableNamesNotSuggested/a.py delete mode 100644 python/testData/completion/tooCommonImportableNamesNotSuggested/mod.py create mode 100644 python/testData/completion/tooShortImportableNamesSuggestedOnlyInExtendedCompletion/a.py create mode 100644 python/testData/completion/tooShortImportableNamesSuggestedOnlyInExtendedCompletion/mod.py diff --git a/python/python-common-tests/com/jetbrains/python/PythonCommonCompletionTest.java b/python/python-common-tests/com/jetbrains/python/PythonCommonCompletionTest.java index 026402021d26..a9156bd36630 100644 --- a/python/python-common-tests/com/jetbrains/python/PythonCommonCompletionTest.java +++ b/python/python-common-tests/com/jetbrains/python/PythonCommonCompletionTest.java @@ -2250,8 +2250,15 @@ public abstract class PythonCommonCompletionTest extends PythonCommonTestCase { } // PY-62208 - public void testTooCommonImportableNamesNotSuggested() { - doMultiFileTest(); + public void testTooShortImportableNamesSuggestedOnlyInExtendedCompletion() { + myFixture.copyDirectoryToProject(getTestName(true), ""); + myFixture.configureByFile("a.py"); + myFixture.complete(CompletionType.BASIC, 1); + List basicCompletionVariants = myFixture.getLookupElementStrings(); + assertDoesntContain(basicCompletionVariants, "c1", "c2"); + myFixture.complete(CompletionType.BASIC, 2); + List extendedCompletionVariants = myFixture.getLookupElementStrings(); + assertContainsElements(extendedCompletionVariants, "c1", "c2"); } private static void runWithImportableNamesInBasicCompletionDisabled(@NotNull Runnable action) { diff --git a/python/python-psi-impl/src/com/jetbrains/python/codeInsight/completion/PyClassNameCompletionContributor.java b/python/python-psi-impl/src/com/jetbrains/python/codeInsight/completion/PyClassNameCompletionContributor.java index 84cd501903fa..f7314f5ecc49 100644 --- a/python/python-psi-impl/src/com/jetbrains/python/codeInsight/completion/PyClassNameCompletionContributor.java +++ b/python/python-psi-impl/src/com/jetbrains/python/codeInsight/completion/PyClassNameCompletionContributor.java @@ -52,7 +52,7 @@ public final class PyClassNameCompletionContributor extends PyImportableNameComp // See https://plugins.jetbrains.com/plugin/18465-sputnik private static final boolean TRACING_WITH_SPUTNIK_ENABLED = false; private static final Logger LOG = Logger.getInstance(PyClassNameCompletionContributor.class); - private static final Set TOO_COMMON_NAMES = Set.of("main", "test"); + private static final int NAME_TOO_SHORT_FOR_BASIC_COMPLETION_THRESHOLD = 5; public PyClassNameCompletionContributor() { if (TRACING_WITH_SPUTNIK_ENABLED) { @@ -63,7 +63,8 @@ public final class PyClassNameCompletionContributor extends PyImportableNameComp @Override protected void doFillCompletionVariants(@NotNull CompletionParameters parameters, @NotNull CompletionResultSet result) { - if (!PyCodeInsightSettings.getInstance().INCLUDE_IMPORTABLE_NAMES_IN_BASIC_COMPLETION && !parameters.isExtendedCompletion()) { + boolean isExtendedCompletion = parameters.isExtendedCompletion(); + if (!PyCodeInsightSettings.getInstance().INCLUDE_IMPORTABLE_NAMES_IN_BASIC_COMPLETION && !isExtendedCompletion) { return; } PsiFile originalFile = parameters.getOriginalFile(); @@ -72,7 +73,7 @@ public final class PyClassNameCompletionContributor extends PyImportableNameComp PyTargetExpression targetExpr = as(position.getParent(), PyTargetExpression.class); boolean insideUnqualifiedReference = refExpr != null && !refExpr.isQualified(); boolean insidePattern = targetExpr != null && position.getParent().getParent() instanceof PyCapturePattern; - boolean insideStringLiteralInExtendedCompletion = position instanceof PyStringElement && parameters.isExtendedCompletion(); + boolean insideStringLiteralInExtendedCompletion = position instanceof PyStringElement && isExtendedCompletion; if (!(insideUnqualifiedReference || insidePattern || insideStringLiteralInExtendedCompletion)) { return; } @@ -103,7 +104,10 @@ public final class PyClassNameCompletionContributor extends PyImportableNameComp StubIndex.getInstance().processAllKeys(PyExportedModuleAttributeIndex.KEY, elementName -> { ProgressManager.checkCanceled(); counters.scannedNames++; - if (TOO_COMMON_NAMES.contains(elementName)) return true; + if (elementName.length() < NAME_TOO_SHORT_FOR_BASIC_COMPLETION_THRESHOLD && !isExtendedCompletion) { + counters.tooShortNames++; + return true; + } if (!result.getPrefixMatcher().isStartMatch(elementName)) return true; return stubIndex.processElements(PyExportedModuleAttributeIndex.KEY, elementName, project, scope, PyElement.class, exported -> { ProgressManager.checkCanceled(); @@ -111,7 +115,7 @@ public final class PyClassNameCompletionContributor extends PyImportableNameComp if (name == null || namesInScope.contains(name)) return true; QualifiedName fqn = getFullyQualifiedName(exported); if (!isApplicableInInsertionContext(exported, fqn, position, typeEvalContext)) { - counters.notApplicableInContext++; + counters.notApplicableInContextNames++; return true; } if (alreadySuggested.add(fqn)) { @@ -141,7 +145,7 @@ public final class PyClassNameCompletionContributor extends PyImportableNameComp }); }, scope); }, duration -> { - LOG.debug(counters + " computed in " + duration + " ms"); + LOG.debug(counters + " computed for prefix '" + result.getPrefixMatcher().getPrefix() + "' in " + duration + " ms"); if (TRACING_WITH_SPUTNIK_ENABLED) { //noinspection UseOfSystemOutOrSystemErr System.out.println("\1h('Importable names completion','%d')".formatted((duration / 10) * 10)); @@ -229,16 +233,18 @@ public final class PyClassNameCompletionContributor extends PyImportableNameComp private static class Counters { int scannedNames; int privateNames; + int tooShortNames; + int notApplicableInContextNames; int totalVariants; - int notApplicableInContext; @Override public String toString() { return "Counters{" + "scannedNames=" + scannedNames + ", privateNames=" + privateNames + + ", tooShortNames=" + tooShortNames + + ", notApplicableInContextNames=" + notApplicableInContextNames + ", totalVariants=" + totalVariants + - ", notApplicableInContext=" + notApplicableInContext + '}'; } } diff --git a/python/testData/completion/tooCommonImportableNamesNotSuggested/a.after.py b/python/testData/completion/tooCommonImportableNamesNotSuggested/a.after.py deleted file mode 100644 index 6fcc24c9280a..000000000000 --- a/python/testData/completion/tooCommonImportableNamesNotSuggested/a.after.py +++ /dev/null @@ -1 +0,0 @@ -mai \ No newline at end of file diff --git a/python/testData/completion/tooCommonImportableNamesNotSuggested/a.py b/python/testData/completion/tooCommonImportableNamesNotSuggested/a.py deleted file mode 100644 index 6fcc24c9280a..000000000000 --- a/python/testData/completion/tooCommonImportableNamesNotSuggested/a.py +++ /dev/null @@ -1 +0,0 @@ -mai \ No newline at end of file diff --git a/python/testData/completion/tooCommonImportableNamesNotSuggested/mod.py b/python/testData/completion/tooCommonImportableNamesNotSuggested/mod.py deleted file mode 100644 index 336e825c89f5..000000000000 --- a/python/testData/completion/tooCommonImportableNamesNotSuggested/mod.py +++ /dev/null @@ -1,2 +0,0 @@ -def main(): - pass diff --git a/python/testData/completion/tooShortImportableNamesSuggestedOnlyInExtendedCompletion/a.py b/python/testData/completion/tooShortImportableNamesSuggestedOnlyInExtendedCompletion/a.py new file mode 100644 index 000000000000..37118d6a3f04 --- /dev/null +++ b/python/testData/completion/tooShortImportableNamesSuggestedOnlyInExtendedCompletion/a.py @@ -0,0 +1 @@ +c \ No newline at end of file diff --git a/python/testData/completion/tooShortImportableNamesSuggestedOnlyInExtendedCompletion/mod.py b/python/testData/completion/tooShortImportableNamesSuggestedOnlyInExtendedCompletion/mod.py new file mode 100644 index 000000000000..19a9e895583e --- /dev/null +++ b/python/testData/completion/tooShortImportableNamesSuggestedOnlyInExtendedCompletion/mod.py @@ -0,0 +1,6 @@ +s = "foo" +for c1, c2 in zip(s, s[1:]): + pass + + +