From 89d0ce71a34d35a74f0aa4f4b63be5ab81264623 Mon Sep 17 00:00:00 2001 From: Andrey Vlasovskikh Date: Tue, 15 Dec 2015 20:06:31 +0300 Subject: [PATCH] Use smart PSI pointers in Python LookupElements to avoid invalid elements in completion (PY-17801) During completion while the user is typing the PSI structure of the current file may change significantly. The main reason is that Python doesn't have any braces around syntactic blocks, only indents, and some syntactically incorrect lines of partially typed code may break the block structure thus invalidating the completion results. --- .../plugins/ipnb/psi/IpnbPyReference.java | 16 +----------- .../codeInsight/PyDunderAllReference.java | 4 +-- .../completion/PyClassInsertHandler.java | 5 ++-- .../PyClassNameCompletionContributor.java | 15 ++++++----- .../completion/PyFunctionInsertHandler.java | 21 +++++++++++----- ...rClassAttributesCompletionContributor.java | 2 +- .../PyIterableVariableMacro.java | 5 +++- .../python/psi/impl/PyBoundFunction.java | 5 ++++ .../impl/references/PyImportReference.java | 5 ++-- .../impl/references/PyQualifiedReference.java | 4 +-- .../psi/impl/references/PyReferenceImpl.java | 23 +++++++++++------ .../resolve/CompletionVariantsProcessor.java | 25 +++++++++---------- 12 files changed, 72 insertions(+), 58 deletions(-) diff --git a/python/ipnb/src/org/jetbrains/plugins/ipnb/psi/IpnbPyReference.java b/python/ipnb/src/org/jetbrains/plugins/ipnb/psi/IpnbPyReference.java index 57db5088ed4d..6188dcc88f3e 100644 --- a/python/ipnb/src/org/jetbrains/plugins/ipnb/psi/IpnbPyReference.java +++ b/python/ipnb/src/org/jetbrains/plugins/ipnb/psi/IpnbPyReference.java @@ -1,12 +1,9 @@ package org.jetbrains.plugins.ipnb.psi; import com.google.common.collect.Lists; -import com.intellij.codeInsight.completion.CompletionUtil; -import com.intellij.codeInsight.lookup.LookupElement; import com.intellij.lang.annotation.HighlightSeverity; import com.intellij.openapi.editor.Editor; import com.intellij.psi.PsiDocumentManager; -import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; import com.intellij.psi.ResolveResult; import com.jetbrains.python.psi.PyQualifiedExpression; @@ -48,18 +45,7 @@ public class IpnbPyReference extends PyReferenceImpl { if (psiFile == null) continue; final CompletionVariantsProcessor processor = new CompletionVariantsProcessor(myElement); PyResolveUtil.scopeCrawlUp(processor, psiFile, null, null); - - for (LookupElement e : processor.getResultList()) { - final Object o = e.getObject(); - if (o instanceof PsiElement) { - final PsiElement original = CompletionUtil.getOriginalElement((PsiElement)o); - if (original == null) { - continue; - } - } - variants.add(e); - } - + variants.addAll(getOriginalElements(processor)); } } return variants.toArray(); diff --git a/python/src/com/jetbrains/python/codeInsight/PyDunderAllReference.java b/python/src/com/jetbrains/python/codeInsight/PyDunderAllReference.java index 8aaaea3e718a..f9e9a41ba72d 100644 --- a/python/src/com/jetbrains/python/codeInsight/PyDunderAllReference.java +++ b/python/src/com/jetbrains/python/codeInsight/PyDunderAllReference.java @@ -75,14 +75,14 @@ public class PyDunderAllReference extends PsiReferenceBase { final int offset = context.getTailOffset(); document.insertString(offset, "()"); - PyClass pyClass = (PyClass) item.getObject(); - PyFunction init = pyClass.findInitOrNew(true, null); + PyClass pyClass = PyUtil.as(item.getPsiElement(), PyClass.class); + PyFunction init = pyClass != null ? pyClass.findInitOrNew(true, null) : null; if (init != null && PyFunctionInsertHandler.hasParams(context, init)) { editor.getCaretModel().moveToOffset(offset+1); AutoPopupController.getInstance(context.getProject()).autoPopupParameterInfo(context.getEditor(), init); diff --git a/python/src/com/jetbrains/python/codeInsight/completion/PyClassNameCompletionContributor.java b/python/src/com/jetbrains/python/codeInsight/completion/PyClassNameCompletionContributor.java index 06bbce0c4472..92d62a0a4d53 100644 --- a/python/src/com/jetbrains/python/codeInsight/completion/PyClassNameCompletionContributor.java +++ b/python/src/com/jetbrains/python/codeInsight/completion/PyClassNameCompletionContributor.java @@ -126,9 +126,12 @@ public class PyClassNameCompletionContributor extends CompletionContributor { for (final String elementName : CompletionUtil.sortMatching(resultSet.getPrefixMatcher(), keys)) { for (T element : StubIndex.getElements(key, elementName, project, scope, elementClass)) { if (condition.value(element)) { - resultSet.addElement(LookupElementBuilder.createWithIcon(element) - .withTailText(" " + ((NavigationItem)element).getPresentation().getLocationString(), true) - .withInsertHandler(insertHandler)); + final String name = element.getName(); + if (name != null) { + resultSet.addElement(LookupElementBuilder.createWithSmartPointer(name, element).withIcon(element.getIcon(0)) + .withTailText(" " + ((NavigationItem)element).getPresentation().getLocationString(), true) + .withInsertHandler(insertHandler)); + } } } } @@ -142,7 +145,7 @@ public class PyClassNameCompletionContributor extends CompletionContributor { private static final InsertHandler FUNCTION_INSERT_HANDLER = new PyFunctionInsertHandler() { - public void handleInsert(final InsertionContext context, final LookupElement item) { + public void handleInsert(@NotNull final InsertionContext context, @NotNull final LookupElement item) { int tailOffset = context.getTailOffset()-1; super.handleInsert(context, item); // adds parentheses, modifies tail offset context.commitDocument(); @@ -172,14 +175,14 @@ public class PyClassNameCompletionContributor extends CompletionContributor { manager.commitDocument(document); } final PsiReference ref = context.getFile().findReferenceAt(tailOffset); - if (ref == null || ref.resolve() == item.getObject()) { + if (ref == null || ref.resolve() == item.getPsiElement()) { // no import statement needed return; } new WriteCommandAction(context.getProject(), context.getFile()) { @Override protected void run(@NotNull Result result) throws Throwable { - AddImportHelper.addImport((PsiNamedElement)item.getObject(), context.getFile(), (PyElement)ref.getElement()); + AddImportHelper.addImport(PyUtil.as(item.getPsiElement(), PsiNamedElement.class), context.getFile(), (PyElement)ref.getElement()); } }.execute(); } diff --git a/python/src/com/jetbrains/python/codeInsight/completion/PyFunctionInsertHandler.java b/python/src/com/jetbrains/python/codeInsight/completion/PyFunctionInsertHandler.java index 5007003eb138..d6f0ad295451 100644 --- a/python/src/com/jetbrains/python/codeInsight/completion/PyFunctionInsertHandler.java +++ b/python/src/com/jetbrains/python/codeInsight/completion/PyFunctionInsertHandler.java @@ -23,8 +23,11 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.util.PsiTreeUtil; import com.jetbrains.python.psi.PyFunction; import com.jetbrains.python.psi.PyReferenceExpression; +import com.jetbrains.python.psi.PyUtil; import com.jetbrains.python.psi.impl.PyCallExpressionHelper; import com.jetbrains.python.psi.resolve.PyResolveContext; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * @author yole @@ -33,23 +36,24 @@ public class PyFunctionInsertHandler extends ParenthesesInsertHandler implicitArgsCount; } + + @Nullable + private static PyFunction getFunction(@NotNull LookupElement item) { + return PyUtil.as(item.getPsiElement(), PyFunction.class); + } } diff --git a/python/src/com/jetbrains/python/codeInsight/completion/PySuperClassAttributesCompletionContributor.java b/python/src/com/jetbrains/python/codeInsight/completion/PySuperClassAttributesCompletionContributor.java index c2a6f19695a7..beed815066e6 100644 --- a/python/src/com/jetbrains/python/codeInsight/completion/PySuperClassAttributesCompletionContributor.java +++ b/python/src/com/jetbrains/python/codeInsight/completion/PySuperClassAttributesCompletionContributor.java @@ -47,7 +47,7 @@ public class PySuperClassAttributesCompletionContributor extends CompletionContr return; } for (PyTargetExpression expr : getSuperClassAttributes(containingClass)) { - result.addElement(LookupElementBuilder.create(expr, expr.getName() + " = ")); + result.addElement(LookupElementBuilder.createWithSmartPointer(expr.getName() + " = ", expr)); } } } diff --git a/python/src/com/jetbrains/python/codeInsight/liveTemplates/PyIterableVariableMacro.java b/python/src/com/jetbrains/python/codeInsight/liveTemplates/PyIterableVariableMacro.java index d3039e1eab22..1220b097bce5 100644 --- a/python/src/com/jetbrains/python/codeInsight/liveTemplates/PyIterableVariableMacro.java +++ b/python/src/com/jetbrains/python/codeInsight/liveTemplates/PyIterableVariableMacro.java @@ -71,7 +71,10 @@ public class PyIterableVariableMacro extends Macro { final PsiElement element = context.getPsiElementAtStartOffset(); if (element != null) { for (PsiNamedElement iterableElement : getIterableElements(element)) { - results.add(LookupElementBuilder.create(iterableElement)); + final String name = iterableElement.getName(); + if (name != null) { + results.add(LookupElementBuilder.createWithSmartPointer(name, iterableElement)); + } } } return results.toArray(new LookupElement[results.size()]); diff --git a/python/src/com/jetbrains/python/psi/impl/PyBoundFunction.java b/python/src/com/jetbrains/python/psi/impl/PyBoundFunction.java index 386d1bd60c3a..8eb0acaca8e5 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyBoundFunction.java +++ b/python/src/com/jetbrains/python/psi/impl/PyBoundFunction.java @@ -24,4 +24,9 @@ public class PyBoundFunction extends PyFunctionImpl { public PyBoundFunction(PyFunction function) { super(function.getNode()); } + + @Override + public boolean isPhysical() { + return false; + } } diff --git a/python/src/com/jetbrains/python/psi/impl/references/PyImportReference.java b/python/src/com/jetbrains/python/psi/impl/references/PyImportReference.java index df15e9219b1e..84a9b98c92ac 100644 --- a/python/src/com/jetbrains/python/psi/impl/references/PyImportReference.java +++ b/python/src/com/jetbrains/python/psi/impl/references/PyImportReference.java @@ -146,8 +146,9 @@ public class PyImportReference extends PyReferenceImpl { } else if (item instanceof LookupElement) { LookupElement lookupElement = (LookupElement) item; - if (lookupElement.getObject() instanceof PsiElement) { - itemElement = (PsiElement) lookupElement.getObject(); + final PsiElement element = lookupElement.getPsiElement(); + if (element != null) { + itemElement = element; } } return !(itemElement instanceof PsiFile); // TODO deeper check? diff --git a/python/src/com/jetbrains/python/psi/impl/references/PyQualifiedReference.java b/python/src/com/jetbrains/python/psi/impl/references/PyQualifiedReference.java index 7adcf80554b1..766cdafc3256 100644 --- a/python/src/com/jetbrains/python/psi/impl/references/PyQualifiedReference.java +++ b/python/src/com/jetbrains/python/psi/impl/references/PyQualifiedReference.java @@ -282,8 +282,8 @@ public class PyQualifiedReference extends PyReferenceImpl { if (name != null && name.endsWith(CompletionUtil.DUMMY_IDENTIFIER_TRIMMED)) { continue; } - if (ex instanceof PsiNamedElement && qualifierType instanceof PyClassType) { - variants.add(LookupElementBuilder.create((PsiNamedElement)ex) + if (ex instanceof PsiNamedElement && qualifierType instanceof PyClassType && name != null) { + variants.add(LookupElementBuilder.createWithSmartPointer(name, ex) .withTypeText(qualifierType.getName()) .withIcon(PlatformIcons.FIELD_ICON)); } diff --git a/python/src/com/jetbrains/python/psi/impl/references/PyReferenceImpl.java b/python/src/com/jetbrains/python/psi/impl/references/PyReferenceImpl.java index bee30ef84990..bfb5e81e5262 100644 --- a/python/src/com/jetbrains/python/psi/impl/references/PyReferenceImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/references/PyReferenceImpl.java @@ -597,19 +597,26 @@ public class PyReferenceImpl implements PsiReferenceEx, PsiPolyVariantReference } } - // Throw away fake elements used for completion internally - for (LookupElement e : processor.getResultList()) { - final Object o = e.getObject(); - if (o instanceof PsiElement) { - final PsiElement original = CompletionUtil.getOriginalElement((PsiElement)o); + ret.addAll(getOriginalElements(processor)); + return ret.toArray(); + } + + /** + * Throws away fake elements used for completion internally. + */ + protected List getOriginalElements(@NotNull CompletionVariantsProcessor processor) { + final List ret = Lists.newArrayList(); + for (LookupElement item : processor.getResultList()) { + final PsiElement e = item.getPsiElement(); + if (e != null) { + final PsiElement original = CompletionUtil.getOriginalElement(e); if (original == null) { continue; } } - ret.add(e); + ret.add(item); } - - return ret.toArray(); + return ret; } @Override diff --git a/python/src/com/jetbrains/python/psi/resolve/CompletionVariantsProcessor.java b/python/src/com/jetbrains/python/psi/resolve/CompletionVariantsProcessor.java index 7f8799fde597..98a8d38cb658 100644 --- a/python/src/com/jetbrains/python/psi/resolve/CompletionVariantsProcessor.java +++ b/python/src/com/jetbrains/python/psi/resolve/CompletionVariantsProcessor.java @@ -22,6 +22,7 @@ import com.intellij.openapi.util.Condition; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.PsiElement; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.QualifiedName; import com.intellij.util.Function; import com.intellij.util.PlatformIcons; import com.jetbrains.python.PyNames; @@ -31,7 +32,6 @@ import com.jetbrains.python.codeInsight.controlflow.ScopeOwner; import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil; import com.jetbrains.python.psi.*; import com.jetbrains.python.psi.impl.PyBuiltinCache; -import com.intellij.psi.util.QualifiedName; import com.jetbrains.python.psi.types.TypeEvalContext; import org.jetbrains.annotations.Nullable; @@ -59,17 +59,17 @@ public class CompletionVariantsProcessor extends VariantsProcessor { mySuppressParentheses = true; } - protected LookupElementBuilder setupItem(LookupElementBuilder item) { - final Object object = item.getObject(); + private LookupElementBuilder setupItem(LookupElementBuilder item) { + final PsiElement element = item.getPsiElement(); if (!myPlainNamesOnly) { if (!mySuppressParentheses && - object instanceof PyFunction && ((PyFunction)object).getProperty() == null && - !PyUtil.hasCustomDecorators((PyFunction)object) && - !isSingleArgDecoratorCall(myContext, (PyFunction)object)) { - final Project project = ((PyFunction)object).getProject(); + element instanceof PyFunction && ((PyFunction)element).getProperty() == null && + !PyUtil.hasCustomDecorators((PyFunction)element) && + !isSingleArgDecoratorCall(myContext, (PyFunction)element)) { + final Project project = element.getProject(); item = item.withInsertHandler(PyFunctionInsertHandler.INSTANCE); final TypeEvalContext context = TypeEvalContext.codeCompletion(project, myContext != null ? myContext.getContainingFile() : null); - final List parameters = PyUtil.getParameters((PyFunction)object, context); + final List parameters = PyUtil.getParameters((PyFunction)element, context); final String params = StringUtil.join(parameters, new Function() { @Override public String fun(PyParameter pyParameter) { @@ -78,13 +78,12 @@ public class CompletionVariantsProcessor extends VariantsProcessor { }, ", "); item = item.withTailText("(" + params + ")"); } - else if (object instanceof PyClass) { + else if (element instanceof PyClass) { item = item.withInsertHandler(PyClassInsertHandler.INSTANCE); } } String source = null; - if (object instanceof PsiElement) { - final PsiElement element = (PsiElement)object; + if (element != null) { PyClass cls = null; if (element instanceof PyFunction) { @@ -158,14 +157,14 @@ public class CompletionVariantsProcessor extends VariantsProcessor { if (PyUtil.isClassPrivateName(name) && !PyUtil.inSameFile(element, myContext)) { return; } - myVariants.put(name, setupItem(LookupElementBuilder.create(element, name).withIcon(element.getIcon(0)))); + myVariants.put(name, setupItem(LookupElementBuilder.createWithSmartPointer(name, element).withIcon(element.getIcon(0)))); } protected void addImportedElement(String referencedName, NameDefiner definer, PyElement expr) { Icon icon = expr.getIcon(0); // things like PyTargetExpression cannot have a general icon, but here we only have variables if (icon == null) icon = PlatformIcons.VARIABLE_ICON; - LookupElementBuilder lookupItem = setupItem(LookupElementBuilder.create(expr, referencedName).withIcon(icon)); + LookupElementBuilder lookupItem = setupItem(LookupElementBuilder.createWithSmartPointer(referencedName, expr).withIcon(icon)); myVariants.put(referencedName, lookupItem); } }