PY-9795 Properly handle insertion of multiple parameters at once

diff --git a/python/src/com/jetbrains/python/documentation/PyDocstringGenerator.java b/python/src/com/jetbrains/python/documentation/PyDocstringGenerator.java
index c363adcfe9cd72acf8899c755f4f93f4532a52f8..e61c3ef58de0f77316945f0c5b1be1a9d2fad248 100644 (file)
--- a/python/src/com/jetbrains/python/documentation/PyDocstringGenerator.java
+++ b/python/src/com/jetbrains/python/documentation/PyDocstringGenerator.java
@@ -130,7 +130,7 @@ public class PyDocstringGenerator {
         for (PyParameter param : ((PyFunction)myDocStringOwner).getParameterList().getParameters()) {
           final String paramName = param.getName();
           final StructuredDocString docString = getStructuredDocString();
-          if (StringUtil.isEmpty(paramName) || param.isSelf() || docString != null && docString.getParamTypeSubstring(paramName) != null) {
+          if (StringUtil.isEmpty(paramName) || param.isSelf() || docString != null && docString.getParameters().contains(paramName)) {
             continue;
           }
           final String signatureType = signature != null ? signature.getArgTypeQualifiedName(paramName) : null;

diff --git a/python/src/com/jetbrains/python/documentation/SectionBasedDocString.java b/python/src/com/jetbrains/python/documentation/SectionBasedDocString.java
index 9dce45d4cd4b1f8a232c147878b42fb3760c9691..93da871b7a5c5461bff7f74727c37826763c53e2 100644 (file)
--- a/python/src/com/jetbrains/python/documentation/SectionBasedDocString.java
+++ b/python/src/com/jetbrains/python/documentation/SectionBasedDocString.java
@@ -309,7 +309,12 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
 
   @Override
   public List<String> getParameters() {
-    return null;
+    return ContainerUtil.map(getParameterSubstrings(), new Function<Substring, String>() {
+      @Override
+      public String fun(Substring substring) {
+        return substring.toString();
+      }
+    });
   }
 
   @Override

diff --git a/python/src/com/jetbrains/python/documentation/docstrings/DocStringUpdater.java b/python/src/com/jetbrains/python/documentation/docstrings/DocStringUpdater.java
index 9b4dd65da8c430ffe44dad85ffe176fd1a92e374..26d61b6cb6a28d08d43e8db9b52f63d5c00ae9b0 100644 (file)
--- a/python/src/com/jetbrains/python/documentation/docstrings/DocStringUpdater.java
+++ b/python/src/com/jetbrains/python/documentation/docstrings/DocStringUpdater.java
@@ -81,6 +81,7 @@ public abstract class DocStringUpdater<T extends DocStringLineParser> {
 
   @NotNull
   public final String getDocStringText() {
+    beforeApplyingModifications();
     // Move closing quotes to the next line, if new lines are going to be inserted
     if (myOriginalDocString.getLineCount() == 1 && !myUpdates.isEmpty()) {
       insertAfterLine(0, myMinContentIndent);
@@ -101,6 +102,10 @@ public abstract class DocStringUpdater<T extends DocStringLineParser> {
     return myBuilder.toString();
   }
 
+  protected void beforeApplyingModifications() {
+    
+  }
+
   @NotNull
   public T getOriginalDocString() {
     return myOriginalDocString;

diff --git a/python/src/com/jetbrains/python/documentation/docstrings/SectionBasedDocStringUpdater.java b/python/src/com/jetbrains/python/documentation/docstrings/SectionBasedDocStringUpdater.java
index 177abc74379b1d73da12b0b5d590bea8feb5c61a..cc90406a297924f2621af688ad1e71ec2745be23 100644 (file)
--- a/python/src/com/jetbrains/python/documentation/docstrings/SectionBasedDocStringUpdater.java
+++ b/python/src/com/jetbrains/python/documentation/docstrings/SectionBasedDocStringUpdater.java
@@ -27,58 +27,24 @@ import com.jetbrains.python.toolbox.Substring;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.ArrayList;
 import java.util.List;
 
 /**
  * @author Mikhail Golubev
  */
 public abstract class SectionBasedDocStringUpdater extends DocStringUpdater<SectionBasedDocString> {
+  private final List<AddParameter> myAddParameterRequests = new ArrayList<AddParameter>();
+
   public SectionBasedDocStringUpdater(@NotNull SectionBasedDocString docString, @NotNull String minContentIndent) {
     super(docString, minContentIndent);
   }
 
   @Override
-  public final void addParameter(@NotNull final String name, @Nullable String type) {
-    if (type != null) {
-      final Substring typeSub = myOriginalDocString.getParamTypeSubstring(name);
-      if (typeSub != null) {
-        replace(typeSub.getTextRange(), type);
-        return;
-      }
-      final Substring nameSub = ContainerUtil.find(myOriginalDocString.getParameterSubstrings(), new Condition<Substring>() {
-        @Override
-        public boolean value(Substring substring) {
-          return substring.toString().equals(name);
-        }
-      });
-      if (nameSub != null) {
-        updateParamDeclarationWithType(nameSub, type);
-        return;
-      }
-    }
-    final Section paramSection = findFirstParametersSection();
-    if (paramSection != null) {
-      final List<SectionField> fields = paramSection.getFields();
-      if (!fields.isEmpty()) {
-        final SectionField firstField = fields.get(0);
-        final String newLine = createParamLine(name, type, getSectionIndent(paramSection), getFieldIndent(paramSection, firstField));
-        insertBeforeLine(getFieldStartLine(firstField), newLine);
-      }
-      else {
-        final String newLine = createParamLine(name, type, getSectionIndent(paramSection), getExpectedFieldIndent());
-        insertAfterLine(getSectionLastTitleLine(paramSection), newLine);
-      }
-    }
-    else {
-      final int line = findLastNonEmptyLine();
-      final String newSection = createBuilder()
-        .withSectionIndent(getExpectedFieldIndent())
-        .addEmptyLine()
-        .startParametersSection()
-        .addParameter(name, type, "")
-        .buildContent(getExpectedSectionIndent(), true);
-      insertAfterLine(line, newSection);
-    }
+  public final void addParameter(@NotNull String name, @Nullable String type) {
+    // because any of requests to add new parameter can lead to creation of a new parameter section
+    // it's not safe to process them independently 
+    myAddParameterRequests.add(new AddParameter(name, type));
   }
 
   @Override
@@ -122,7 +88,7 @@ public abstract class SectionBasedDocStringUpdater extends DocStringUpdater<Sect
       for (SectionField param : section.getFields()) {
         if (param.getName().equals(name)) {
           if (section.getFields().size() == 1) {
-            removeLines(getSectionStartLine(section), getFieldEndLine(param));            
+            removeLines(getSectionStartLine(section), getFieldEndLine(param));
           }
           else {
             removeLines(getFieldStartLine(param), getFieldEndLine(param));
@@ -133,6 +99,76 @@ public abstract class SectionBasedDocStringUpdater extends DocStringUpdater<Sect
     }
   }
 
+  @Override
+  protected void beforeApplyingModifications() {
+    final List<AddParameter> newParams = new ArrayList<AddParameter>();
+    for (AddParameter param : myAddParameterRequests) {
+      if (param.type != null) {
+        final Substring typeSub = myOriginalDocString.getParamTypeSubstring(param.name);
+        if (typeSub != null) {
+          replace(typeSub.getTextRange(), param.type);
+          continue;
+        }
+        final Substring nameSub = findParamNameSubstring(param.name);
+        if (nameSub != null) {
+          updateParamDeclarationWithType(nameSub, param.type);
+          continue;
+        }
+      }
+      newParams.add(param);
+    }
+    if (!newParams.isEmpty()) {
+      final SectionBasedDocStringBuilder paramBlockBuilder = createBuilder();
+      final Section firstParamSection = findFirstParametersSection();
+      // Insert whole new parameter block
+      if (firstParamSection == null) {
+        paramBlockBuilder
+          .addEmptyLine()
+          .startParametersSection();
+        final String blockText = buildBlock(paramBlockBuilder, newParams, getExpectedFieldIndent(), getExpectedSectionIndent());
+        insertAfterLine(findLastNonEmptyLine(), blockText);
+      }
+      // Update existing parameter block
+      else {
+        final SectionField firstParamField = ContainerUtil.getFirstItem(firstParamSection.getFields());
+        // Section exist, but empty
+        if (firstParamField == null) {
+          final String blockText = buildBlock(paramBlockBuilder, newParams, getExpectedFieldIndent(), getSectionIndent(firstParamSection));
+          insertAfterLine(getSectionLastTitleLine(firstParamSection), blockText);
+        }
+        else {
+          // Section contain other parameter declarations
+          final String blockText = buildBlock(paramBlockBuilder, newParams,
+                                              getFieldIndent(firstParamSection, firstParamField),
+                                              getSectionIndent(firstParamSection));
+          insertBeforeLine(getFieldStartLine(firstParamField), blockText);
+        }
+      }
+    }
+  }
+
+  @NotNull
+  private static String buildBlock(@NotNull SectionBasedDocStringBuilder builder,
+                                   @NotNull List<AddParameter> params,
+                                   @NotNull String sectionIndent,
+                                   @NotNull String indent) {
+    builder.withSectionIndent(sectionIndent);
+    for (AddParameter param : params) {
+      builder.addParameter(param.name, param.type, "");
+    }
+    return builder.buildContent(indent, true);
+  }
+
+
+  private Substring findParamNameSubstring(@NotNull final String name) {
+    return ContainerUtil.find(myOriginalDocString.getParameterSubstrings(), new Condition<Substring>() {
+      @Override
+      public boolean value(Substring substring) {
+        return substring.toString().equals(name);
+      }
+    });
+  }
+
   abstract void updateParamDeclarationWithType(@NotNull Substring nameSubstring, @NotNull String type);
 
   protected int getSectionLastTitleLine(@NotNull Section paramSection) {
@@ -141,16 +177,6 @@ public abstract class SectionBasedDocStringUpdater extends DocStringUpdater<Sect
 
   protected abstract SectionBasedDocStringBuilder createBuilder();
 
-  protected String createParamLine(@NotNull String name,
-                                   @Nullable String type,
-                                   @NotNull String docStringIndent,
-                                   @NotNull String sectionIndent) {
-    return createBuilder()
-      .withSectionIndent(sectionIndent)
-      .addParameter(name, type, "")
-      .buildContent(docStringIndent, true);
-  }
-
   protected String createReturnLine(@NotNull String type,
                                     @NotNull String docStringIndent,
                                     @NotNull String sectionIndent) {
@@ -231,6 +257,16 @@ public abstract class SectionBasedDocStringUpdater extends DocStringUpdater<Sect
     else {
       //noinspection ConstantConditions
       return field.getNameAsSubstring().getEndLine();
-    } 
+    }
+  }
+
+  private static class AddParameter {
+    @NotNull final String name;
+    @Nullable final String type;
+
+    public AddParameter(@NotNull String name, @Nullable String type) {
+      this.name = name;
+      this.type = type;
+    }
   }
 }

diff --git a/python/testData/intentions/afterAddMissingParamsInGoogleDocString.py b/python/testData/intentions/afterAddMissingParamsInGoogleDocString.py
new file mode 100644 (file)
index 0000000..a1e1855
--- /dev/null
+++ b/python/testData/intentions/afterAddMissingParamsInGoogleDocString.py
@@ -0,0 +1,8 @@
+def f(a, b, c, d):
+    """
+    Parameters:
+      b: 
+      d: 
+      a : foo
+      c : bar
+    """
\ No newline at end of file

diff --git a/python/testData/intentions/afterAddMissingParamsInGoogleDocStringEmptyParamSection.py b/python/testData/intentions/afterAddMissingParamsInGoogleDocStringEmptyParamSection.py
new file mode 100644 (file)
index 0000000..ed4172f
--- /dev/null
+++ b/python/testData/intentions/afterAddMissingParamsInGoogleDocStringEmptyParamSection.py
@@ -0,0 +1,7 @@
+def f(x, y, z):
+    """
+    Parameters:
+        x: 
+        y: 
+        z: 
+    """
\ No newline at end of file

diff --git a/python/testData/intentions/afterAddMissingParamsInGoogleDocStringNoParamSection.py b/python/testData/intentions/afterAddMissingParamsInGoogleDocStringNoParamSection.py
new file mode 100644 (file)
index 0000000..e9214a3
--- /dev/null
+++ b/python/testData/intentions/afterAddMissingParamsInGoogleDocStringNoParamSection.py
@@ -0,0 +1,8 @@
+def f(x, y, z):
+    """
+
+    Parameters:
+        x: 
+        y: 
+        z: 
+    """
\ No newline at end of file

diff --git a/python/testData/intentions/beforeAddMissingParamsInGoogleDocString.py b/python/testData/intentions/beforeAddMissingParamsInGoogleDocString.py
new file mode 100644 (file)
index 0000000..02344a1
--- /dev/null
+++ b/python/testData/intentions/beforeAddMissingParamsInGoogleDocString.py
@@ -0,0 +1,6 @@
+def <caret>f(a, b, c, d):
+    """
+    Parameters:
+      a : foo
+      c : bar
+    """
\ No newline at end of file

diff --git a/python/testData/intentions/beforeAddMissingParamsInGoogleDocStringEmptyParamSection.py b/python/testData/intentions/beforeAddMissingParamsInGoogleDocStringEmptyParamSection.py
new file mode 100644 (file)
index 0000000..e3f4e4a
--- /dev/null
+++ b/python/testData/intentions/beforeAddMissingParamsInGoogleDocStringEmptyParamSection.py
@@ -0,0 +1,4 @@
+def <caret>f(x, y, z):
+    """
+    Parameters:
+    """
\ No newline at end of file

diff --git a/python/testData/intentions/beforeAddMissingParamsInGoogleDocStringNoParamSection.py b/python/testData/intentions/beforeAddMissingParamsInGoogleDocStringNoParamSection.py
new file mode 100644 (file)
index 0000000..6f3030a
--- /dev/null
+++ b/python/testData/intentions/beforeAddMissingParamsInGoogleDocStringNoParamSection.py
@@ -0,0 +1,3 @@
+def <caret>f(x, y, z):
+    """
+    """
\ No newline at end of file

diff --git a/python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java b/python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java
index 10ab4e6cf0ba7835afd4dfcb579882608a9e1832..e0898ab983d0f4c7794f8c5a529c7eb41b4f2ae4 100644 (file)
--- a/python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java
+++ b/python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java
@@ -288,24 +288,24 @@ public class  PyIntentionTest extends PyTestCase {
 
   public void testTypeInDocstring() {
     getCommonCodeStyleSettings().getIndentOptions().INDENT_SIZE = 2;
-    doDocReferenceTest(DocStringFormat.REST);
+    doDocParamTypeTest(DocStringFormat.REST);
   }
 
   public void testTypeInDocstring3() {
-    doDocReferenceTest(DocStringFormat.REST);
+    doDocParamTypeTest(DocStringFormat.REST);
   }
 
   public void testTypeInDocstring4() {
-    doDocReferenceTest(DocStringFormat.REST);
+    doDocParamTypeTest(DocStringFormat.REST);
   }
 
   public void testTypeInDocstringParameterInCallable() {
-    doDocReferenceTest(DocStringFormat.REST);
+    doDocParamTypeTest(DocStringFormat.REST);
   }
 
   public void testTypeInDocstring5() {
     getCommonCodeStyleSettings().getIndentOptions().INDENT_SIZE = 2;
-    doDocReferenceTest(DocStringFormat.REST);
+    doDocParamTypeTest(DocStringFormat.REST);
   }
 
   public void testTypeInDocstringAtTheEndOfFunction() {
@@ -317,12 +317,12 @@ public class  PyIntentionTest extends PyTestCase {
   }
 
   public void testTypeInDocstring7() {         //PY-8930
-    doDocReferenceTest(DocStringFormat.REST);
+    doDocParamTypeTest(DocStringFormat.REST);
   }
 
   // PY-16456
   public void testTypeInDocStringDifferentIndentationSize() {
-    doDocReferenceTest(DocStringFormat.REST);
+    doDocParamTypeTest(DocStringFormat.REST);
   }
 
   // PY-16456
@@ -446,47 +446,47 @@ public class  PyIntentionTest extends PyTestCase {
 
   // PY-9795
   public void testParamTypeInNewGoogleDocString() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);
+    doDocParamTypeTest(DocStringFormat.GOOGLE);
   }
 
   // PY-9795
   public void testParamTypeInEmptyGoogleDocString() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);  
+    doDocParamTypeTest(DocStringFormat.GOOGLE);  
   }
   
   // PY-9795
   public void testParamTypeInGoogleDocStringOnlySummaryOneLine() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);  
+    doDocParamTypeTest(DocStringFormat.GOOGLE);  
   }
 
   // PY-9795
   public void testParamTypeInGoogleDocStringOnlySummary() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);
+    doDocParamTypeTest(DocStringFormat.GOOGLE);
   }
   
   // PY-9795
   public void testParamTypeInGoogleDocStringEmptyParamSection() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);
+    doDocParamTypeTest(DocStringFormat.GOOGLE);
   }
   
   // PY-9795
   public void testParamTypeInGoogleDocStringParamDeclaredNoParenthesis() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);
+    doDocParamTypeTest(DocStringFormat.GOOGLE);
   }
 
   // PY-9795
   public void testParamTypeInGoogleDocStringParamDeclaredEmptyParenthesis() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);
+    doDocParamTypeTest(DocStringFormat.GOOGLE);
   }
   
   // PY-9795
   public void testParamTypeInGoogleDocStringOtherParamDeclared() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);
+    doDocParamTypeTest(DocStringFormat.GOOGLE);
   }
   
   // PY-9795
   public void testParamTypeInGoogleDocStringOtherSectionExists() {
-    doDocReferenceTest(DocStringFormat.GOOGLE);
+    doDocParamTypeTest(DocStringFormat.GOOGLE);
   }
   
   // PY-9795
@@ -529,7 +529,22 @@ public class  PyIntentionTest extends PyTestCase {
       codeInsightSettings.INSERT_TYPE_DOCSTUB = oldInsertTypeDocStub;
     }
   }
+
+  // PY-9795
+  public void testAddMissingParamsInGoogleDocString() {
+    doDocAddMissingParamsTest(DocStringFormat.GOOGLE);
+  }
+
+  // PY-9795
+  public void testAddMissingParamsInGoogleDocStringNoParamSection() {
+    doDocAddMissingParamsTest(DocStringFormat.GOOGLE);
+  }
   
+  // PY-9795
+  public void testAddMissingParamsInGoogleDocStringEmptyParamSection() {
+    doDocAddMissingParamsTest(DocStringFormat.GOOGLE);
+  }
+
   // PY-4717
   public void testReturnTypeInNewNumpyDocString() {
     doDocReturnTypeTest(DocStringFormat.NUMPY);
@@ -537,47 +552,47 @@ public class  PyIntentionTest extends PyTestCase {
 
   // PY-4717
   public void testParamTypeInNewNumpyDocString() {
-    doDocReferenceTest(DocStringFormat.NUMPY);
+    doDocParamTypeTest(DocStringFormat.NUMPY);
   }
   
    // PY-4717
   public void testParamTypeInEmptyNumpyDocString() {
-    doDocReferenceTest(DocStringFormat.NUMPY);  
+    doDocParamTypeTest(DocStringFormat.NUMPY);  
   }
   
   // PY-4717
   public void testParamTypeInNumpyDocStringOnlySummaryOneLine() {
-    doDocReferenceTest(DocStringFormat.NUMPY);  
+    doDocParamTypeTest(DocStringFormat.NUMPY);  
   }
 
   // PY-4717
   public void testParamTypeInNumpyDocStringOnlySummary() {
-    doDocReferenceTest(DocStringFormat.NUMPY);
+    doDocParamTypeTest(DocStringFormat.NUMPY);
   }
   
   // PY-4717
   public void testParamTypeInNumpyDocStringEmptyParamSection() {
-    doDocReferenceTest(DocStringFormat.NUMPY);
+    doDocParamTypeTest(DocStringFormat.NUMPY);
   }
   
   // PY-4717
   public void testParamTypeInNumpyDocStringParamDeclaredNoColon() {
-    doDocReferenceTest(DocStringFormat.NUMPY);
+    doDocParamTypeTest(DocStringFormat.NUMPY);
   }
 
   // PY-4717
   public void testParamTypeInNumpyDocStringParamDeclaredColon() {
-    doDocReferenceTest(DocStringFormat.NUMPY);
+    doDocParamTypeTest(DocStringFormat.NUMPY);
   }
   
   // PY-4717
   public void testParamTypeInNumpyDocStringOtherParamDeclared() {
-    doDocReferenceTest(DocStringFormat.NUMPY);
+    doDocParamTypeTest(DocStringFormat.NUMPY);
   }
   
   // PY-4717
   public void testParamTypeInNumpyDocStringOtherSectionExists() {
-    doDocReferenceTest(DocStringFormat.NUMPY);
+    doDocParamTypeTest(DocStringFormat.NUMPY);
   }
   
   // PY-4717
@@ -613,7 +628,7 @@ public class  PyIntentionTest extends PyTestCase {
     });
   }
 
-  private void doDocReferenceTest(@NotNull DocStringFormat format) {
+  private void doDocParamTypeTest(@NotNull DocStringFormat format) {
     runWithDocStringFormat(format, new Runnable() {
       public void run() {
         doTest(PyBundle.message("INTN.specify.type"));
@@ -630,4 +645,13 @@ public class  PyIntentionTest extends PyTestCase {
 
   }
 
+  public void doDocAddMissingParamsTest(@NotNull DocStringFormat format) {
+    runWithDocStringFormat(format, new Runnable() {
+      @Override
+      public void run() {
+        doTest(PyBundle.message("INTN.add.parameters.to.docstring"));
+      }
+    });
+
+  }
 }