From c36a6673c8d4ee744d329cf118b0458cc85427c7 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Mon, 7 Sep 2015 13:43:43 +0300 Subject: [PATCH] PY-16761 Don't expose details of parameter naming, PyDocStringGenerator operates directly on PyParameter elements --- .../SpecifyTypeInDocstringIntention.java | 7 ++- .../codeInsight/intentions/TypeIntention.java | 8 ++-- .../python/documentation/DocStringUtil.java | 8 ---- .../documentation/PyDocstringGenerator.java | 35 ++++++++++---- .../inspections/PyDocstringInspection.java | 17 +++---- .../quickfix/DocstringQuickFix.java | 48 ++++++++++++------- .../com/jetbrains/python/PyQuickFixTest.java | 4 +- 7 files changed, 73 insertions(+), 54 deletions(-) diff --git a/python/src/com/jetbrains/python/codeInsight/intentions/SpecifyTypeInDocstringIntention.java b/python/src/com/jetbrains/python/codeInsight/intentions/SpecifyTypeInDocstringIntention.java index fc3b191a1fd0..bc645609897e 100644 --- a/python/src/com/jetbrains/python/codeInsight/intentions/SpecifyTypeInDocstringIntention.java +++ b/python/src/com/jetbrains/python/codeInsight/intentions/SpecifyTypeInDocstringIntention.java @@ -58,7 +58,7 @@ public class SpecifyTypeInDocstringIntention extends TypeIntention { PsiReference reference = problemElement == null ? null : problemElement.getReference(); final PsiElement resolved = reference != null ? reference.resolve() : null; - PyParameter parameter = getParameter(problemElement, resolved); + PyNamedParameter parameter = getParameter(problemElement, resolved); final PyCallable callable; if (parameter != null) { @@ -72,7 +72,7 @@ public class SpecifyTypeInDocstringIntention extends TypeIntention { } } - private static void generateDocstring(@Nullable PyParameter param, PyFunction pyFunction) { + private static void generateDocstring(@Nullable PyNamedParameter param, PyFunction pyFunction) { if (!DocStringUtil.ensureNotPlainDocstringFormat(pyFunction)) { return; } @@ -85,8 +85,7 @@ public class SpecifyTypeInDocstringIntention extends TypeIntention { if (signature != null) { type = ObjectUtils.chooseNotNull(signature.getArgTypeQualifiedName(paramName), type); } - final String docParamName = DocStringUtil.getPreferredParameterName(docstringGenerator.getDocStringFormat(), param); - docstringGenerator.withParamTypedByName(docParamName, type); + docstringGenerator.withParamTypedByName(param, type); } else { docstringGenerator.withReturnValue(type); diff --git a/python/src/com/jetbrains/python/codeInsight/intentions/TypeIntention.java b/python/src/com/jetbrains/python/codeInsight/intentions/TypeIntention.java index 78679a96f21f..effca1d4e9b9 100644 --- a/python/src/com/jetbrains/python/codeInsight/intentions/TypeIntention.java +++ b/python/src/com/jetbrains/python/codeInsight/intentions/TypeIntention.java @@ -99,10 +99,10 @@ public abstract class TypeIntention implements IntentionAction { } @Nullable - protected static PyParameter getParameter(PyExpression problemElement, PsiElement resolved) { - PyParameter parameter = as(problemElement, PyParameter.class); - if (resolved instanceof PyParameter) { - parameter = (PyParameter)resolved; + protected static PyNamedParameter getParameter(PyExpression problemElement, PsiElement resolved) { + PyNamedParameter parameter = as(problemElement, PyNamedParameter.class); + if (resolved instanceof PyNamedParameter) { + parameter = (PyNamedParameter)resolved; } return parameter == null || parameter.isSelf() ? null : parameter; } diff --git a/python/src/com/jetbrains/python/documentation/DocStringUtil.java b/python/src/com/jetbrains/python/documentation/DocStringUtil.java index 6ebaf6a7328b..6fa1a3c415cb 100644 --- a/python/src/com/jetbrains/python/documentation/DocStringUtil.java +++ b/python/src/com/jetbrains/python/documentation/DocStringUtil.java @@ -306,12 +306,4 @@ public class DocStringUtil { } return true; } - - @NotNull - public static String getPreferredParameterName(@NotNull DocStringFormat format, @NotNull PyParameter parameter) { - if (format == DocStringFormat.GOOGLE && parameter.getAsNamed() != null) { - return parameter.getAsNamed().getRepr(false); - } - return StringUtil.notNullize(parameter.getName()); - } } diff --git a/python/src/com/jetbrains/python/documentation/PyDocstringGenerator.java b/python/src/com/jetbrains/python/documentation/PyDocstringGenerator.java index 6f47959e40c2..365c4044b1eb 100644 --- a/python/src/com/jetbrains/python/documentation/PyDocstringGenerator.java +++ b/python/src/com/jetbrains/python/documentation/PyDocstringGenerator.java @@ -112,16 +112,20 @@ public class PyDocstringGenerator { @NotNull String text) { return new PyDocstringGenerator(null, text, format, indentation); } - + @NotNull public PyDocstringGenerator withParam(@NotNull String name) { return withParamTypedByName(name, null); } + @NotNull + public PyDocstringGenerator withParam(@NotNull PyNamedParameter param) { + return withParam(getPreferredParameterName(param)); + } + @NotNull public PyDocstringGenerator withoutParam(@NotNull String name) { - myRemovedParams.add(new DocstringParam(name, null, false)); - return this; + return withParamTypedByName(name, null); } @NotNull @@ -129,6 +133,11 @@ public class PyDocstringGenerator { myAddedParams.add(new DocstringParam(name, type, false)); return this; } + + @NotNull + public PyDocstringGenerator withParamTypedByName(@NotNull PyNamedParameter name, @Nullable String type) { + return withParamTypedByName(getPreferredParameterName(name), type); + } @NotNull public PyDocstringGenerator withReturnValue(@Nullable String type) { @@ -148,13 +157,13 @@ public class PyDocstringGenerator { return this; } - @NotNull public PyDocstringGenerator addFirstEmptyLine() { myAddFirstEmptyLine = true; return this; } + @NotNull public PyDocstringGenerator forceNewMode() { myNewMode = true; @@ -178,7 +187,7 @@ public class PyDocstringGenerator { if (StringUtil.isEmpty(paramName) || param.isSelf() || docString != null && docString.getParameters().contains(paramName)) { continue; } - withParam(DocStringUtil.getPreferredParameterName(getDocStringFormat(), param)); + withParam((PyNamedParameter)param); } final RaiseVisitor visitor = new RaiseVisitor(); final PyStatementList statementList = ((PyFunction)myDocStringOwner).getStatementList(); @@ -344,6 +353,15 @@ public class PyDocstringGenerator { } } + @NotNull + public String getPreferredParameterName(@NotNull PyNamedParameter parameter) { + if (getDocStringFormat() == DocStringFormat.GOOGLE && parameter.getAsNamed() != null) { + return parameter.getAsNamed().getRepr(false); + } + return StringUtil.notNullize(parameter.getName()); + } + + @NotNull private static String getDefaultType(@NotNull DocstringParam param) { if (StringUtil.isEmpty(param.getType())) { @@ -551,10 +569,10 @@ public class PyDocstringGenerator { } public static class DocstringParam { + private final String myName; private final String myType; private final boolean myReturnValue; - private DocstringParam(@NotNull String name, @Nullable String type, boolean isReturn) { myName = name; myType = type; @@ -605,13 +623,13 @@ public class PyDocstringGenerator { ", myReturnValue=" + myReturnValue + '}'; } - } + } private static class RaiseVisitor extends PyRecursiveElementVisitor { + private boolean myHasRaise = false; private boolean myHasReturn = false; @Nullable private PyExpression myRaiseTarget = null; - @Override public void visitPyRaiseStatement(@NotNull PyRaiseStatement node) { myHasRaise = true; @@ -640,6 +658,7 @@ public class PyDocstringGenerator { } return ""; } + } } diff --git a/python/src/com/jetbrains/python/inspections/PyDocstringInspection.java b/python/src/com/jetbrains/python/inspections/PyDocstringInspection.java index 07ea5e06938f..bff48f346748 100644 --- a/python/src/com/jetbrains/python/inspections/PyDocstringInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyDocstringInspection.java @@ -26,7 +26,6 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementVisitor; import com.jetbrains.python.PyBundle; -import com.jetbrains.python.documentation.DocStringFormat; import com.jetbrains.python.documentation.DocStringUtil; import com.jetbrains.python.documentation.PlainDocString; import com.jetbrains.python.inspections.quickfix.DocstringQuickFix; @@ -150,13 +149,11 @@ public class PyDocstringInspection extends PyInspection { if (pyDocStringOwner instanceof PyFunction) { PyParameter[] realParams = ((PyFunction)pyDocStringOwner).getParameterList().getParameters(); - List missingParams = getMissingParams(docString, realParams); + List missingParams = getMissingParams(docString, realParams); boolean registered = false; if (!missingParams.isEmpty()) { - for (PyParameter param : missingParams) { - final DocStringFormat format = DocStringUtil.getConfiguredDocStringFormat(pyDocStringOwner); - final String docParamName = DocStringUtil.getPreferredParameterName(format, param); - registerProblem(param, "Missing parameter " + param.getName() + " in docstring", new DocstringQuickFix(docParamName, null)); + for (PyNamedParameter param : missingParams) { + registerProblem(param, "Missing parameter " + param.getName() + " in docstring", new DocstringQuickFix(param, null)); } registered = true; } @@ -193,16 +190,16 @@ public class PyDocstringInspection extends PyInspection { return Lists.newArrayList(unexpected.values()); } - private static List getMissingParams(StructuredDocString docString, PyParameter[] realParams) { - List missing = new ArrayList(); + private static List getMissingParams(StructuredDocString docString, PyParameter[] realParams) { + List missing = new ArrayList(); final List docStringParameters = docString.getParameters(); for (PyParameter p : realParams) { - if (p.isSelf() || p instanceof PySingleStarParameter || p instanceof PyTupleParameter) { + if (p.isSelf() || !(p instanceof PyNamedParameter)) { continue; } //noinspection ConstantConditions if (!docStringParameters.contains(p.getName())) { - missing.add(p); + missing.add((PyNamedParameter)p); } } return missing; diff --git a/python/src/com/jetbrains/python/inspections/quickfix/DocstringQuickFix.java b/python/src/com/jetbrains/python/inspections/quickfix/DocstringQuickFix.java index ed6bba3dfaa6..294b592048e3 100644 --- a/python/src/com/jetbrains/python/inspections/quickfix/DocstringQuickFix.java +++ b/python/src/com/jetbrains/python/inspections/quickfix/DocstringQuickFix.java @@ -23,14 +23,14 @@ import com.intellij.openapi.editor.EditorFactory; import com.intellij.openapi.project.Project; import com.intellij.psi.PsiDocumentManager; import com.intellij.psi.PsiElement; +import com.intellij.psi.SmartPointerManager; +import com.intellij.psi.SmartPsiElementPointer; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.IncorrectOperationException; import com.jetbrains.python.PyBundle; import com.jetbrains.python.codeInsight.intentions.PyGenerateDocstringIntention; import com.jetbrains.python.documentation.PyDocstringGenerator; -import com.jetbrains.python.psi.PyClass; -import com.jetbrains.python.psi.PyDocStringOwner; -import com.jetbrains.python.psi.PyFunction; -import com.jetbrains.python.psi.PyStringLiteralExpression; +import com.jetbrains.python.psi.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -38,21 +38,30 @@ import org.jetbrains.annotations.Nullable; * User : catherine */ public class DocstringQuickFix implements LocalQuickFix { - String myMissingText; - String myUnexpected; + private final SmartPsiElementPointer myMissingParam; + private final String myUnexpectedParamName; - public DocstringQuickFix(String missing, String unexpected) { - myMissingText = missing; - myUnexpected = unexpected; + public DocstringQuickFix(@Nullable PyNamedParameter missing, @Nullable String unexpectedParamName) { + if (missing != null) { + myMissingParam = SmartPointerManager.getInstance(missing.getProject()).createSmartPsiElementPointer(missing); + } + else { + myMissingParam = null; + } + myUnexpectedParamName = unexpectedParamName; } @NotNull public String getName() { - if (myMissingText != null) { - return PyBundle.message("QFIX.docstring.add.$0", myMissingText); + if (myMissingParam != null) { + final PyNamedParameter param = myMissingParam.getElement(); + if (param == null) { + throw new IncorrectOperationException("Parameter was invalidates before quickfix is called"); + } + return PyBundle.message("QFIX.docstring.add.$0", param.getName()); } - else if (myUnexpected != null) { - return PyBundle.message("QFIX.docstring.remove.$0", myUnexpected); + else if (myUnexpectedParamName != null) { + return PyBundle.message("QFIX.docstring.remove.$0", myUnexpectedParamName); } else { return PyBundle.message("QFIX.docstring.insert.stub"); @@ -82,17 +91,20 @@ public class DocstringQuickFix implements LocalQuickFix { PyDocStringOwner docStringOwner = PsiTreeUtil.getParentOfType(descriptor.getPsiElement(), PyDocStringOwner.class); if (docStringOwner == null) return; PyStringLiteralExpression docStringExpression = docStringOwner.getDocStringExpression(); - if (docStringExpression == null && myMissingText == null && myUnexpected == null) { + if (docStringExpression == null && myMissingParam == null && myUnexpectedParamName == null) { addEmptyDocstring(docStringOwner); return; } if (docStringExpression != null) { final PyDocstringGenerator generator = PyDocstringGenerator.forDocStringOwner(docStringOwner); - if (myMissingText != null) { - generator.withParam(myMissingText); + if (myMissingParam != null) { + final PyNamedParameter param = myMissingParam.getElement(); + if (param != null) { + generator.withParam(param); + } } - else if (myUnexpected != null) { - generator.withoutParam(myUnexpected.trim()); + else if (myUnexpectedParamName != null) { + generator.withoutParam(myUnexpectedParamName.trim()); } generator.buildAndInsert(); } diff --git a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java index 73e48ee48d1c..27f9d2175883 100644 --- a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java +++ b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java @@ -519,7 +519,7 @@ public class PyQuickFixTest extends PyTestCase { runWithDocStringFormat(DocStringFormat.GOOGLE, new Runnable() { @Override public void run() { - doInspectionTest(PyDocstringInspection.class, PyBundle.message("QFIX.docstring.add.$0", "*args"), true, true); + doInspectionTest(PyDocstringInspection.class, PyBundle.message("QFIX.docstring.add.$0", "args"), true, true); } }); } @@ -529,7 +529,7 @@ public class PyQuickFixTest extends PyTestCase { runWithDocStringFormat(DocStringFormat.GOOGLE, new Runnable() { @Override public void run() { - doInspectionTest(PyDocstringInspection.class, PyBundle.message("QFIX.docstring.add.$0", "**kwargs"), true, true); + doInspectionTest(PyDocstringInspection.class, PyBundle.message("QFIX.docstring.add.$0", "kwargs"), true, true); } }); }