From f4c155e073b80e3ba722b7587ade592f5fef3889 Mon Sep 17 00:00:00 2001 From: Ekaterina Tuzova Date: Sun, 6 Sep 2015 17:51:46 +0300 Subject: [PATCH] code cleanup (names, nullability) + extracted function for building documentation for properties + removed duplicated PyUtil.isPythonIdentifier --- .../CollectionElementNameMacro.java | 4 +- .../documentation/PyDocumentationBuilder.java | 98 ++++++++++--------- .../src/com/jetbrains/python/psi/PyUtil.java | 4 - .../KeywordArgumentCompletionUtil.java | 2 +- .../com/jetbrains/python/PyQuickDocTest.java | 85 ++++++++-------- 5 files changed, 97 insertions(+), 96 deletions(-) diff --git a/python/src/com/jetbrains/python/codeInsight/liveTemplates/CollectionElementNameMacro.java b/python/src/com/jetbrains/python/codeInsight/liveTemplates/CollectionElementNameMacro.java index a9bcd7e78580..34e0add06255 100644 --- a/python/src/com/jetbrains/python/codeInsight/liveTemplates/CollectionElementNameMacro.java +++ b/python/src/com/jetbrains/python/codeInsight/liveTemplates/CollectionElementNameMacro.java @@ -19,7 +19,7 @@ import com.intellij.codeInsight.lookup.LookupElement; import com.intellij.codeInsight.lookup.LookupElementBuilder; import com.intellij.codeInsight.template.*; import com.intellij.openapi.util.text.StringUtil; -import com.jetbrains.python.psi.PyUtil; +import com.jetbrains.python.PyNames; import org.jetbrains.annotations.NotNull; import java.util.ArrayList; @@ -62,7 +62,7 @@ public class CollectionElementNameMacro extends Macro { } } final String result = smartUnPluralize(param); - return result != null && PyUtil.isPythonIdentifier(result) ? new TextResult(result) : null; + return result != null && PyNames.isIdentifier(result) ? new TextResult(result) : null; } private static String smartUnPluralize(String param) { diff --git a/python/src/com/jetbrains/python/documentation/PyDocumentationBuilder.java b/python/src/com/jetbrains/python/documentation/PyDocumentationBuilder.java index f3a52224d03b..6355ffe29c44 100644 --- a/python/src/com/jetbrains/python/documentation/PyDocumentationBuilder.java +++ b/python/src/com/jetbrains/python/documentation/PyDocumentationBuilder.java @@ -73,32 +73,25 @@ class PyDocumentationBuilder { myResult = wrapInTag("html", wrapInTag("body", myResult)); } - public String build() { - final ChainIterable reassignCat = new ChainIterable(); // sequence for reassignment info, etc - PsiElement followed = resolveToDocStringOwner(reassignCat); - - // check if we got a property ref. - // if so, element is an accessor, and originalElement if an identifier - // TODO: use messages from resources! + private boolean buildForProperty(PsiElement followed, PsiElement outerElement) { PyClass cls; - PsiElement outer = null; boolean isProperty = false; - String accessorKind = "None"; + final TypeEvalContext context = TypeEvalContext.userInitiated(myElement.getProject(), myElement.getContainingFile()); if (myOriginalElement != null) { String elementName = myOriginalElement.getText(); - if (PyUtil.isPythonIdentifier(elementName)) { - outer = myOriginalElement.getParent(); - if (outer instanceof PyQualifiedExpression) { - PyExpression qualifier = ((PyQualifiedExpression)outer).getQualifier(); + if (PyNames.isIdentifier(elementName)) { + + if (outerElement instanceof PyQualifiedExpression) { + final PyExpression qualifier = ((PyQualifiedExpression)outerElement).getQualifier(); if (qualifier != null) { - PyType type = context.getType(qualifier); + final PyType type = context.getType(qualifier); if (type instanceof PyClassType) { cls = ((PyClassType)type).getPyClass(); Property property = cls.findProperty(elementName, true, null); if (property != null) { isProperty = true; - final AccessDirection dir = AccessDirection.of((PyElement)outer); + final AccessDirection dir = AccessDirection.of((PyElement)outerElement); Maybe accessor = property.getByDirection(dir); myProlog .addItem("property ").addWith(TagBold, $().addWith(TagCode, $(elementName))) @@ -123,6 +116,7 @@ class PyDocumentationBuilder { } myBody.addItem(BR); if (accessor.isDefined() && accessor.value() == null) followed = null; + String accessorKind; if (dir == AccessDirection.READ) { accessorKind = "Getter"; } @@ -133,19 +127,44 @@ class PyDocumentationBuilder { accessorKind = "Deleter"; } if (followed != null) myEpilog.addWith(TagSmall, $(BR, BR, accessorKind, " of property")).addItem(BR); + + if (!(followed instanceof PyDocStringOwner)) { + String accessorMessage; + if (followed != null) { + accessorMessage = "Declaration: "; + } + else { + accessorMessage = accessorKind + " is not defined."; + } + myBody.addWith(TagItalic, $(accessorMessage)).addItem(BR); + if (followed != null) myBody.addItem(combUp(PyUtil.getReadableRepr(followed, false))); + } } } } } } } + return isProperty; + } + + + public String build() { + final ChainIterable reassignmentChain = new ChainIterable(); + final TypeEvalContext context = TypeEvalContext.userInitiated(myElement.getProject(), myElement.getContainingFile()); + PsiElement outerElement = myOriginalElement != null ? myOriginalElement.getParent() : null; + + PsiElement followed = resolveToDocStringOwner(reassignmentChain); + final boolean isProperty = buildForProperty(followed, outerElement); + if (myProlog.isEmpty() && !isProperty && !isAttribute()) { - myProlog.add(reassignCat); + myProlog.add(reassignmentChain); } // now followed may contain a doc string if (followed instanceof PyDocStringOwner) { + PyClass cls; String docString = null; PyStringLiteralExpression docStringExpression = ((PyDocStringOwner)followed).getDocStringExpression(); if (docStringExpression != null) docString = docStringExpression.getStringValue(); @@ -181,26 +200,14 @@ class PyDocumentationBuilder { addFormattedDocString(myElement, docString, myBody, myEpilog); } } - else if (isProperty) { - // if it was a normal accessor, ti would be a function, handled by previous branch - String accessorMessage; - if (followed != null) { - accessorMessage = "Declaration: "; - } - else { - accessorMessage = accessorKind + " is not defined."; - } - myBody.addWith(TagItalic, $(accessorMessage)).addItem(BR); - if (followed != null) myBody.addItem(combUp(PyUtil.getReadableRepr(followed, false))); - } else if (isAttribute()) { addAttributeDoc(); } else if (followed instanceof PyNamedParameter) { myBody.addItem(combUp("Parameter " + PyUtil.getReadableRepr(followed, false))); boolean typeFromDocstringAdded = addTypeAndDescriptionFromDocstring((PyNamedParameter)followed); - if (outer instanceof PyExpression) { - PyType type = context.getType((PyExpression)outer); + if (outerElement instanceof PyExpression) { + PyType type = context.getType((PyExpression)outerElement); if (type != null) { String typeString = null; if (type instanceof PyDynamicallyEvaluatedType) { @@ -210,8 +217,8 @@ class PyDocumentationBuilder { } } else { - if (outer.getReference() != null) { - PsiElement target = outer.getReference().resolve(); + if (outerElement.getReference() != null) { + PsiElement target = outerElement.getReference().resolve(); if (target instanceof PyTargetExpression) { final String targetName = ((PyTargetExpression)target).getName(); @@ -232,9 +239,9 @@ class PyDocumentationBuilder { } } } - else if (followed != null && outer instanceof PyReferenceExpression) { + else if (followed != null && outerElement instanceof PyReferenceExpression) { myBody.addItem(combUp("\nInferred type: ")); - PythonDocumentationProvider.describeExpressionTypeWithLinks(myBody, (PyReferenceExpression)outer, context); + PythonDocumentationProvider.describeExpressionTypeWithLinks(myBody, (PyReferenceExpression)outerElement, context); } if (myBody.isEmpty() && myEpilog.isEmpty()) { return null; // got nothing substantial to say! @@ -249,12 +256,11 @@ class PyDocumentationBuilder { } @Nullable - private PsiElement resolveToDocStringOwner(ChainIterable prolog_cat) { + private PsiElement resolveToDocStringOwner(ChainIterable reassignmentChain) { // here the ^Q target is already resolved; the resolved element may point to intermediate assignments if (myElement instanceof PyTargetExpression) { - final String target_name = myElement.getText(); - //prolog_cat.add(TagSmall.apply($("Assigned to ", element.getText(), BR))); - prolog_cat.addWith(TagSmall, $(PyBundle.message("QDOC.assigned.to.$0", target_name)).addItem(BR)); + final String targetName = myElement.getText(); + reassignmentChain.addWith(TagSmall, $(PyBundle.message("QDOC.assigned.to.$0", targetName)).addItem(BR)); final PyExpression assignedValue = ((PyTargetExpression)myElement).findAssignedValue(); if (assignedValue instanceof PyReferenceExpression) { final PsiElement resolved = resolveWithoutImplicits((PyReferenceExpression)assignedValue); @@ -265,20 +271,18 @@ class PyDocumentationBuilder { return assignedValue; } if (myElement instanceof PyReferenceExpression) { - //prolog_cat.add(TagSmall.apply($("Assigned to ", element.getText(), BR))); - prolog_cat.addWith(TagSmall, $(PyBundle.message("QDOC.assigned.to.$0", myElement.getText())).addItem(BR)); + reassignmentChain.addWith(TagSmall, $(PyBundle.message("QDOC.assigned.to.$0", myElement.getText())).addItem(BR)); return resolveWithoutImplicits((PyReferenceExpression)myElement); } // it may be a call to a standard wrapper if (myElement instanceof PyCallExpression) { final PyCallExpression call = (PyCallExpression)myElement; - Pair wrap_info = PyCallExpressionHelper.interpretAsModifierWrappingCall(call, myOriginalElement); - if (wrap_info != null) { - String wrapper_name = wrap_info.getFirst(); - PyFunction wrapped_func = wrap_info.getSecond(); - //prolog_cat.addWith(TagSmall, $("Wrapped in ").addWith(TagCode, $(wrapper_name)).add(BR)); - prolog_cat.addWith(TagSmall, $(PyBundle.message("QDOC.wrapped.in.$0", wrapper_name)).addItem(BR)); - return wrapped_func; + Pair wrapInfo = PyCallExpressionHelper.interpretAsModifierWrappingCall(call, myOriginalElement); + if (wrapInfo != null) { + String wrapperName = wrapInfo.getFirst(); + PyFunction wrappedFunction = wrapInfo.getSecond(); + reassignmentChain.addWith(TagSmall, $(PyBundle.message("QDOC.wrapped.in.$0", wrapperName)).addItem(BR)); + return wrappedFunction; } } return myElement; diff --git a/python/src/com/jetbrains/python/psi/PyUtil.java b/python/src/com/jetbrains/python/psi/PyUtil.java index 75fe387be9ae..727d4c686e6c 100644 --- a/python/src/com/jetbrains/python/psi/PyUtil.java +++ b/python/src/com/jetbrains/python/psi/PyUtil.java @@ -1082,10 +1082,6 @@ public class PyUtil { return name.length() > 4 && name.startsWith("__") && name.endsWith("__"); } - public static boolean isPythonIdentifier(@NotNull String name) { - return PyNames.isIdentifier(name); - } - /** * Constructs new lookup element for completion of keyword argument with equals sign appended. * diff --git a/python/src/com/jetbrains/python/psi/impl/references/KeywordArgumentCompletionUtil.java b/python/src/com/jetbrains/python/psi/impl/references/KeywordArgumentCompletionUtil.java index 63df3d5e3c01..26d2c73f3f78 100644 --- a/python/src/com/jetbrains/python/psi/impl/references/KeywordArgumentCompletionUtil.java +++ b/python/src/com/jetbrains/python/psi/impl/references/KeywordArgumentCompletionUtil.java @@ -202,7 +202,7 @@ public class KeywordArgumentCompletionUtil { if (Comparing.equal(myKwArgs.getName(), operandName) && argument instanceof PyStringLiteralExpression) { String name = ((PyStringLiteralExpression)argument).getStringValue(); - if (PyUtil.isPythonIdentifier(name)) { + if (PyNames.isIdentifier(name)) { myRet.add(PyUtil.createNamedParameterLookup(name, argument.getProject())); } } diff --git a/python/testSrc/com/jetbrains/python/PyQuickDocTest.java b/python/testSrc/com/jetbrains/python/PyQuickDocTest.java index 38539be50cb7..e940befd7ef2 100644 --- a/python/testSrc/com/jetbrains/python/PyQuickDocTest.java +++ b/python/testSrc/com/jetbrains/python/PyQuickDocTest.java @@ -16,7 +16,7 @@ package com.jetbrains.python; import com.intellij.openapi.util.text.StringUtil; -import com.intellij.openapi.vfs.VfsUtil; +import com.intellij.openapi.vfs.VfsUtilCore; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiElement; import com.intellij.testFramework.TestDataFile; @@ -27,7 +27,6 @@ import com.jetbrains.python.fixtures.LightMarkedTestCase; import com.jetbrains.python.fixtures.PyTestCase; import com.jetbrains.python.psi.*; import com.jetbrains.python.psi.impl.PythonLanguageLevelPusher; -import junit.framework.Assert; import java.io.IOException; import java.util.Map; @@ -57,24 +56,24 @@ public class PyQuickDocTest extends LightMarkedTestCase { } private void checkByHTML(String text) { - Assert.assertNotNull(text); + assertNotNull(text); checkByHTML(text, "/quickdoc/" + getTestName(false) + ".html"); } private void checkByHTML(String text, @TestDataFile String filePath) { final String fullPath = getTestDataPath() + filePath; - final VirtualFile vFile = PyTestCase.getVirtualFileByName(fullPath); - Assert.assertNotNull("file " + fullPath + " not found", vFile); + final VirtualFile virtualFile = PyTestCase.getVirtualFileByName(fullPath); + assertNotNull("file " + fullPath + " not found", virtualFile); String loadedText; try { - loadedText = VfsUtil.loadText(vFile); + loadedText = VfsUtilCore.loadText(virtualFile); } catch (IOException e) { throw new RuntimeException(e); } String fileText = StringUtil.convertLineSeparators(loadedText, "\n"); - Assert.assertEquals(fileText.trim(), text.trim()); + assertEquals(fileText.trim(), text.trim()); } @Override @@ -84,34 +83,35 @@ public class PyQuickDocTest extends LightMarkedTestCase { private void checkRefDocPair() { Map marks = loadTest(); - Assert.assertEquals(2, marks.size()); - final PsiElement original_elt = marks.get(""); - PsiElement doc_elt = original_elt.getParent(); // ident -> expr - Assert.assertTrue(doc_elt instanceof PyStringLiteralExpression); - String doc_text = ((PyStringLiteralExpression)doc_elt).getStringValue(); - Assert.assertNotNull(doc_text); + assertEquals(2, marks.size()); + final PsiElement originalElement = marks.get(""); + PsiElement docElement = originalElement.getParent(); // ident -> expr + assertTrue(docElement instanceof PyStringLiteralExpression); + String stringValue = ((PyStringLiteralExpression)docElement).getStringValue(); + assertNotNull(stringValue); - PsiElement ref_elt = marks.get("").getParent(); // ident -> expr - final PyDocStringOwner doc_owner = (PyDocStringOwner)((PyReferenceExpression)ref_elt).getReference().resolve(); - Assert.assertEquals(doc_elt, doc_owner.getDocStringExpression()); + PsiElement referenceElement = marks.get("").getParent(); // ident -> expr + final PyDocStringOwner docOwner = (PyDocStringOwner)((PyReferenceExpression)referenceElement).getReference().resolve(); + assertNotNull(docOwner); + assertEquals(docElement, docOwner.getDocStringExpression()); - checkByHTML(myProvider.generateDoc(doc_owner, original_elt)); + checkByHTML(myProvider.generateDoc(docOwner, originalElement)); } private void checkHTMLOnly() { Map marks = loadTest(); - final PsiElement original_elt = marks.get(""); - PsiElement ref_elt = original_elt.getParent(); // ident -> expr - final PsiElement doc_owner = ((PyReferenceExpression)ref_elt).getReference().resolve(); - checkByHTML(myProvider.generateDoc(doc_owner, original_elt)); + final PsiElement originalElement = marks.get(""); + PsiElement referenceElement = originalElement.getParent(); // ident -> expr + final PsiElement docOwner = ((PyReferenceExpression)referenceElement).getReference().resolve(); + checkByHTML(myProvider.generateDoc(docOwner, originalElement)); } private void checkHover() { Map marks = loadTest(); - final PsiElement original_elt = marks.get(""); - PsiElement ref_elt = original_elt.getParent(); // ident -> expr - final PsiElement docOwner = ((PyReferenceExpression)ref_elt).getReference().resolve(); - checkByHTML(myProvider.getQuickNavigateInfo(docOwner, ref_elt)); + final PsiElement originalElement = marks.get(""); + PsiElement referenceElement = originalElement.getParent(); // ident -> expr + final PsiElement docOwner = ((PyReferenceExpression)referenceElement).getReference().resolve(); + checkByHTML(myProvider.getQuickNavigateInfo(docOwner, referenceElement)); } public void testDirectFunc() { @@ -157,17 +157,18 @@ public class PyQuickDocTest extends LightMarkedTestCase { public void testInheritedMethod() { Map marks = loadTest(); - Assert.assertEquals(2, marks.size()); - PsiElement doc_elt = marks.get("").getParent(); // ident -> expr - Assert.assertTrue(doc_elt instanceof PyStringLiteralExpression); - String doc_text = ((PyStringLiteralExpression)doc_elt).getStringValue(); - Assert.assertNotNull(doc_text); + assertEquals(2, marks.size()); + PsiElement docElement = marks.get("").getParent(); // ident -> expr + assertTrue(docElement instanceof PyStringLiteralExpression); + String docText = ((PyStringLiteralExpression)docElement).getStringValue(); + assertNotNull(docText); PsiElement ref_elt = marks.get("").getParent(); // ident -> expr - final PyDocStringOwner doc_owner = (PyDocStringOwner)((PyReferenceExpression)ref_elt).getReference().resolve(); - Assert.assertNull(doc_owner.getDocStringExpression()); // no direct doc! + final PyDocStringOwner docOwner = (PyDocStringOwner)((PyReferenceExpression)ref_elt).getReference().resolve(); + assertNotNull(docOwner); + assertNull(docOwner.getDocStringExpression()); // no direct doc! - checkByHTML(myProvider.generateDoc(doc_owner, null)); + checkByHTML(myProvider.generateDoc(docOwner, null)); } public void testPropNewGetter() { @@ -177,10 +178,10 @@ public class PyQuickDocTest extends LightMarkedTestCase { public void testPropNewSetter() { PythonLanguageLevelPusher.setForcedLanguageLevel(myFixture.getProject(), LanguageLevel.PYTHON26); Map marks = loadTest(); - PsiElement ref_elt = marks.get(""); + PsiElement referenceElement = marks.get(""); try { - final PyDocStringOwner doc_owner = (PyDocStringOwner)((PyTargetExpression)(ref_elt.getParent())).getReference().resolve(); - checkByHTML(myProvider.generateDoc(doc_owner, ref_elt)); + final PyDocStringOwner docStringOwner = (PyDocStringOwner)((PyTargetExpression)(referenceElement.getParent())).getReference().resolve(); + checkByHTML(myProvider.generateDoc(docStringOwner, referenceElement)); } finally { PythonLanguageLevelPusher.setForcedLanguageLevel(myFixture.getProject(), null); @@ -190,10 +191,10 @@ public class PyQuickDocTest extends LightMarkedTestCase { public void testPropNewDeleter() { PythonLanguageLevelPusher.setForcedLanguageLevel(myFixture.getProject(), LanguageLevel.PYTHON26); Map marks = loadTest(); - PsiElement ref_elt = marks.get(""); + PsiElement referenceElement = marks.get(""); try { - final PyDocStringOwner doc_owner = (PyDocStringOwner)((PyReferenceExpression)(ref_elt.getParent())).getReference().resolve(); - checkByHTML(myProvider.generateDoc(doc_owner, ref_elt)); + final PyDocStringOwner docStringOwner = (PyDocStringOwner)((PyReferenceExpression)(referenceElement.getParent())).getReference().resolve(); + checkByHTML(myProvider.generateDoc(docStringOwner, referenceElement)); } finally { PythonLanguageLevelPusher.setForcedLanguageLevel(myFixture.getProject(), null); @@ -207,9 +208,9 @@ public class PyQuickDocTest extends LightMarkedTestCase { public void testPropOldSetter() { Map marks = loadTest(); - PsiElement ref_elt = marks.get(""); - final PyDocStringOwner doc_owner = (PyDocStringOwner)((PyTargetExpression)(ref_elt.getParent())).getReference().resolve(); - checkByHTML(myProvider.generateDoc(doc_owner, ref_elt)); + PsiElement referenceElement = marks.get(""); + final PyDocStringOwner docStringOwner = (PyDocStringOwner)((PyTargetExpression)(referenceElement.getParent())).getReference().resolve(); + checkByHTML(myProvider.generateDoc(docStringOwner, referenceElement)); } public void testPropOldDeleter() {