PY-31025 Properly extract description of the return value in Google docstrings

when both its type and optional name are omitted. Just as the reference implementation
in sphinxcontrib.napoleon we now require a trailing colon on the first line of a field
to treat this text as the return type, otherwise it's considered the return value description.

Also, I dropped support for named return and yield values in Google Code Style docstrings,
as Napoleon doesn't feature them as well.
This commit is contained in:
Mikhail Golubev
2018-08-02 20:36:00 +03:00
parent a8d1c3b958
commit a41986b1b4
13 changed files with 192 additions and 83 deletions

View File

@@ -335,7 +335,7 @@ public class PyDocumentationBuilder {
docStringExpression = addInheritedDocString(pyFunction, pyClass);
}
if (docStringExpression != null) {
formatParametersAndReturnValue(docStringExpression, pyFunction);
addFunctionSpecificSections(docStringExpression, pyFunction);
}
}
else if (elementDefinition instanceof PyFile) {
@@ -356,7 +356,7 @@ public class PyDocumentationBuilder {
}
}
private void formatParametersAndReturnValue(@NotNull PyStringLiteralExpression docstring, @NotNull PyFunction function) {
private void addFunctionSpecificSections(@NotNull PyStringLiteralExpression docstring, @NotNull PyFunction function) {
final StructuredDocString structured = DocStringUtil.parseDocString(docstring);
final List<PyCallableParameter> parameters = function.getParameters(myContext);

View File

@@ -16,10 +16,12 @@
package com.jetbrains.python.documentation.docstrings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.intellij.openapi.util.Pair;
import com.intellij.util.containers.ContainerUtil;
import com.jetbrains.python.toolbox.Substring;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.List;
import java.util.regex.Matcher;
@@ -47,36 +49,54 @@ public class GoogleCodeStyleDocString extends SectionBasedDocString {
"Notes",
"Warnings");
private static final ImmutableMap<String, FieldType> ourSectionFieldMapping =
ImmutableMap.<String, FieldType>builder()
.put(RETURNS_SECTION, FieldType.OPTIONAL_TYPE)
.put(YIELDS_SECTION, FieldType.OPTIONAL_TYPE)
.put(RAISES_SECTION, FieldType.ONLY_TYPE)
.put(METHODS_SECTION, FieldType.ONLY_NAME)
.put(KEYWORD_ARGUMENTS_SECTION, FieldType.NAME_WITH_OPTIONAL_TYPE)
.put(PARAMETERS_SECTION, FieldType.NAME_WITH_OPTIONAL_TYPE)
.put(ATTRIBUTES_SECTION, FieldType.NAME_WITH_OPTIONAL_TYPE)
.put(OTHER_PARAMETERS_SECTION, FieldType.NAME_WITH_OPTIONAL_TYPE)
.build();
public GoogleCodeStyleDocString(@NotNull Substring text) {
super(text);
}
@Nullable
@Override
protected FieldType getFieldType(@NotNull String title) {
return ourSectionFieldMapping.get(title);
}
/**
* <h3>Examples</h3>
* <ol>
* <li>
* mayHaveType=true, preferType=false
* canHaveBothNameAndType=true, preferType=false
* <pre>{@code
* Attributes:
* arg1 (int): description; `(int)` is optional
* }</pre>
* </li>
* <li>
* mayHaveType=true, preferType=true
* canHaveBothNameAndType=true, preferType=true
* <pre>{@code
* Returns:
* code (int): description; `(int)` is optional
* }</pre>
* </li>
* <li>
* mayHaveType=false, preferType=false
* canHaveBothNameAndType=false, preferType=false
* <pre>{@code
* Methods:
* my_method() : description
* }</pre>
* </li>
* <li>
* mayHaveType=false, preferType=true
* canHaveBothNameAndType=false, preferType=true
* <pre>{@code
* Raises:
* Exception : description
@@ -86,41 +106,52 @@ public class GoogleCodeStyleDocString extends SectionBasedDocString {
* </ol>
*/
@Override
protected Pair<SectionField, Integer> parseSectionField(int lineNum,
int sectionIndent,
boolean mayHaveType,
boolean preferType) {
protected Pair<SectionField, Integer> parseSectionField(int lineNum, int sectionIndent, @NotNull FieldType fieldType) {
final Substring line = getLine(lineNum);
Substring name, type = null;
Substring name = null;
Substring type = null;
Substring description;
// Napoleon requires that each parameter line contains a colon - we don't because
// we need to parse and complete parameter names before colon is typed
final List<Substring> colonSeparatedParts = splitByFirstColon(line);
assert colonSeparatedParts.size() <= 2;
final Substring textBeforeColon = colonSeparatedParts.get(0);
name = textBeforeColon.trim();
if (mayHaveType) {
final Matcher matcher = FIELD_NAME_AND_TYPE.matcher(textBeforeColon);
if (matcher.matches()) {
name = textBeforeColon.getMatcherGroup(matcher, 1).trim();
type = textBeforeColon.getMatcherGroup(matcher, 2).trim();
// In cases like the following:
//
// Returns:
// Foo
//
// Napoleon treats "Foo" as the return value description, not type, since there is no subsequent colon.
if (colonSeparatedParts.size() == 2 || !fieldType.canHaveOnlyDescription) {
description = colonSeparatedParts.size() == 2 ? colonSeparatedParts.get(1) : null;
name = textBeforeColon.trim();
if (fieldType.canHaveBothNameAndType) {
final Matcher matcher = FIELD_NAME_AND_TYPE.matcher(textBeforeColon);
if (matcher.matches()) {
name = textBeforeColon.getMatcherGroup(matcher, 1).trim();
type = textBeforeColon.getMatcherGroup(matcher, 2).trim();
}
}
if (fieldType.preferType && type == null) {
type = name;
name = null;
}
if (name != null) {
name = cleanUpName(name);
}
if (name != null ? !isValidName(name.toString()) : !isValidType(type.toString())) {
return Pair.create(null, lineNum);
}
}
else {
description = textBeforeColon;
}
if (preferType && type == null) {
type = name;
name = null;
}
if (name != null) {
name = cleanUpName(name);
}
if (name != null ? !isValidName(name.toString()) : !isValidType(type.toString())) {
return Pair.create(null, lineNum);
}
final Pair<List<Substring>, Integer> pair;
if (colonSeparatedParts.size() == 2) {
Substring description = colonSeparatedParts.get(1);
if (description != null) {
// parse line with indentation at least one space greater than indentation of the field
pair = parseIndentedBlock(lineNum + 1, getLineIndentSize(lineNum));
final Pair<List<Substring>, Integer> pair = parseIndentedBlock(lineNum + 1, getLineIndentSize(lineNum));
final List<Substring> nestedBlock = pair.getFirst();
if (!nestedBlock.isEmpty()) {
//noinspection ConstantConditions

View File

@@ -16,10 +16,12 @@
package com.jetbrains.python.documentation.docstrings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.intellij.openapi.util.Pair;
import com.jetbrains.python.toolbox.Substring;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.List;
@@ -46,7 +48,20 @@ public class NumpyDocString extends SectionBasedDocString {
"References",
"Examples",
"Notes",
"Warnings");
"Warnings");
private static final ImmutableMap<String, FieldType> ourSectionFieldMapping =
ImmutableMap.<String, FieldType>builder()
.put(RETURNS_SECTION, FieldType.TYPE_WITH_OPTIONAL_NAME)
.put(YIELDS_SECTION, FieldType.TYPE_WITH_OPTIONAL_NAME)
.put(RAISES_SECTION, FieldType.ONLY_TYPE)
.put(METHODS_SECTION, FieldType.ONLY_NAME)
.put(KEYWORD_ARGUMENTS_SECTION, FieldType.NAME_WITH_OPTIONAL_TYPE)
.put(PARAMETERS_SECTION, FieldType.NAME_WITH_OPTIONAL_TYPE)
.put(ATTRIBUTES_SECTION, FieldType.NAME_WITH_OPTIONAL_TYPE)
.put(OTHER_PARAMETERS_SECTION, FieldType.NAME_WITH_OPTIONAL_TYPE)
.build();
private Substring mySignature;
@@ -65,6 +80,12 @@ public class NumpyDocString extends SectionBasedDocString {
return nextNonEmptyLineNum;
}
@Nullable
@Override
protected FieldType getFieldType(@NotNull String title) {
return ourSectionFieldMapping.get(title);
}
@NotNull
@Override
protected Pair<Substring, Integer> parseSectionHeader(int lineNum) {
@@ -85,13 +106,10 @@ public class NumpyDocString extends SectionBasedDocString {
}
@Override
protected Pair<SectionField, Integer> parseSectionField(int lineNum,
int sectionIndent,
boolean mayHaveType,
boolean preferType) {
protected Pair<SectionField, Integer> parseSectionField(int lineNum, int sectionIndent, @NotNull FieldType kind) {
final Substring line = getLine(lineNum);
Substring namesPart, type = null, description = null;
if (mayHaveType) {
if (kind.canHaveBothNameAndType) {
final List<Substring> colonSeparatedParts = splitByFirstColon(line);
namesPart = colonSeparatedParts.get(0).trim();
if (colonSeparatedParts.size() == 2) {
@@ -101,7 +119,7 @@ public class NumpyDocString extends SectionBasedDocString {
else {
namesPart = line.trim();
}
if (preferType && type == null) {
if (kind.preferType && type == null) {
type = namesPart;
namesPart = null;
}

View File

@@ -16,7 +16,6 @@
package com.jetbrains.python.documentation.docstrings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.util.containers.ContainerUtil;
@@ -50,7 +49,7 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
@NonNls public static final String METHODS_SECTION = "methods";
@NonNls public static final String OTHER_PARAMETERS_SECTION = "other parameters";
@NonNls public static final String YIELDS_SECTION = "yields";
private static final Pattern PLAIN_TEXT = Pattern.compile("\\w+(\\s+\\w+){2}"); // dumb heuristic - consecutive words
protected static final Map<String, String> SECTION_ALIASES =
@@ -80,20 +79,31 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
.build();
private static final Pattern SPHINX_REFERENCE_RE = Pattern.compile("(:\\w+:\\S+:`.+?`|:\\S+:`.+?`|`.+?`)");
public static Set<String> SECTION_NAMES = SECTION_ALIASES.keySet();
private static final ImmutableSet<String> SECTIONS_WITH_NAME_AND_OPTIONAL_TYPE = ImmutableSet.of(ATTRIBUTES_SECTION,
PARAMETERS_SECTION,
KEYWORD_ARGUMENTS_SECTION,
OTHER_PARAMETERS_SECTION);
private static final ImmutableSet<String> SECTIONS_WITH_TYPE_AND_OPTIONAL_NAME = ImmutableSet.of(RETURNS_SECTION, YIELDS_SECTION);
private static final ImmutableSet<String> SECTIONS_WITH_TYPE = ImmutableSet.of(RAISES_SECTION);
private static final ImmutableSet<String> SECTIONS_WITH_NAME = ImmutableSet.of(METHODS_SECTION);
public static final Set<String> SECTION_NAMES = SECTION_ALIASES.keySet();
protected enum FieldType {
NAME_WITH_OPTIONAL_TYPE(true, false, false),
TYPE_WITH_OPTIONAL_NAME(true, true, false),
ONLY_TYPE(false, true, false),
OPTIONAL_TYPE(false, true, true),
ONLY_NAME(false, false, false);
public final boolean canHaveBothNameAndType;
public final boolean preferType;
public final boolean canHaveOnlyDescription;
FieldType(boolean canHaveType, boolean preferType, boolean canHaveOnlyDescription) {
this.canHaveBothNameAndType = canHaveType;
this.canHaveOnlyDescription = canHaveOnlyDescription;
this.preferType = preferType;
}
}
@Nullable
public static String getNormalizedSectionTitle(@NotNull @NonNls String title) {
return SECTION_ALIASES.get(title.toLowerCase());
}
public static boolean isValidSectionTitle(@NotNull @NonNls String title) {
return StringUtil.isCapitalized(title) && getNormalizedSectionTitle(title) != null;
}
@@ -179,25 +189,17 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
@NotNull
protected Pair<SectionField, Integer> parseSectionField(int lineNum, @NotNull String normalizedSectionTitle, int sectionIndent) {
if (SECTIONS_WITH_NAME_AND_OPTIONAL_TYPE.contains(normalizedSectionTitle)) {
return parseSectionField(lineNum, sectionIndent, true, false);
}
if (SECTIONS_WITH_TYPE_AND_OPTIONAL_NAME.contains(normalizedSectionTitle)) {
return parseSectionField(lineNum, sectionIndent, true, true);
}
if (SECTIONS_WITH_NAME.contains(normalizedSectionTitle)) {
return parseSectionField(lineNum, sectionIndent, false, false);
}
if (SECTIONS_WITH_TYPE.contains(normalizedSectionTitle)) {
return parseSectionField(lineNum, sectionIndent, false, true);
final FieldType fieldType = getFieldType(normalizedSectionTitle);
if (fieldType != null) {
return parseSectionField(lineNum, sectionIndent, fieldType);
}
return parseGenericField(lineNum, sectionIndent);
}
protected abstract Pair<SectionField, Integer> parseSectionField(int lineNum,
int sectionIndent,
boolean mayHaveType,
boolean preferType);
@Nullable
protected abstract FieldType getFieldType(@NotNull String title);
protected abstract Pair<SectionField, Integer> parseSectionField(int lineNum, int sectionIndent, @NotNull FieldType kind);
@NotNull
protected Pair<SectionField, Integer> parseGenericField(int lineNum, int sectionIndent) {
@@ -219,9 +221,9 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
}
protected boolean isSectionBreak(int lineNum, int curSectionIndent) {
return lineNum >= getLineCount() ||
return lineNum >= getLineCount() ||
// note that field may have the same indent as its containing section
(!isEmpty(lineNum) && getLineIndentSize(lineNum) <= getSectionIndentationThreshold(curSectionIndent)) ||
(!isEmpty(lineNum) && getLineIndentSize(lineNum) <= getSectionIndentationThreshold(curSectionIndent)) ||
isSectionStart(lineNum);
}
@@ -254,7 +256,11 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
}
protected boolean isValidType(@NotNull String type) {
return !type.isEmpty() && !PLAIN_TEXT.matcher(type).find();
return !type.isEmpty() && !isPlainText(type);
}
protected static boolean isPlainText(@NotNull String type) {
return PLAIN_TEXT.matcher(type).find();
}
protected boolean isValidName(@NotNull String name) {
@@ -387,7 +393,7 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
}
return result;
}
@Nullable
@Override
public String getKeywordArgumentDescription(@Nullable String paramName) {
@@ -444,7 +450,7 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
}
return result;
}
@Nullable
private SectionField getFirstReturnField() {
return ContainerUtil.getFirstItem(getReturnFields());
@@ -535,7 +541,7 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
public String getTitle() {
return myTitle.toString();
}
@NotNull
public String getNormalizedTitle() {
//noinspection ConstantConditions
@@ -613,7 +619,7 @@ public abstract class SectionBasedDocString extends DocStringLineParser implemen
return myType;
}
@Nullable
@Nullable
public String getDescription() {
return myDescription == null ? null : PyIndentUtil.removeCommonIndent(myDescription.getValue(), true);
}

View File

@@ -0,0 +1,6 @@
def func():
"""
Returns:
int:
return value description
"""

View File

@@ -0,0 +1,5 @@
def f(x):
"""
Returns:
return value description
"""

View File

@@ -0,0 +1,6 @@
def f(x):
"""
Returns:
return value description
with continuation
"""

View File

@@ -1,10 +0,0 @@
def func():
"""
Return:
status_code (int): HTTP status code
template (str) : path to template in template roots
Yields:
progress (float): floating point value in range [0, 1) indicating progress
of the task
"""

View File

@@ -0,0 +1,15 @@
def func():
"""
Return
------
status_code: int
HTTP status code
template: str
path to template in template roots
Yields
------
progress: float
floating point value in range [0, 1) indicating progress
of the task
"""

View File

@@ -0,0 +1 @@
<html><body><div class='definition'><pre><a href="psi_element://#module#GoogleDocstringWithReturnValueDescriptionWithoutType">GoogleDocstringWithReturnValueDescriptionWithoutType</a><br>def <b>func</b>() -&gt; None</pre></div><div class='content'>Unittest placehoder</div><table class='sections'><tr><td valign='top' class='section'><p>Returns:</td><td valign='top'>Nothing</td></table></body></html>

View File

@@ -0,0 +1,7 @@
def fu<the_ref>nc():
"""Dummy method does nothing
Returns:
Nothing
"""
pass

View File

@@ -556,6 +556,11 @@ public class PyQuickDocTest extends LightMarkedTestCase {
doMultiFileCheckByHTML("numpy.py");
}
// PY-31025
public void testGoogleDocstringWithReturnValueDescriptionWithoutType() {
checkHTMLOnly();
}
@Override
protected String getTestDataPath() {
return super.getTestDataPath() + "/quickdoc/";

View File

@@ -187,8 +187,8 @@ public class PySectionBasedDocStringTest extends PyTestCase {
"Third line", docString.getSummary());
}
public void testNamedReturnsAndYields() {
final GoogleCodeStyleDocString docString = findAndParseGoogleStyleDocString();
public void testNumpyNamedReturnsAndYields() {
final NumpyDocString docString = findAndParseNumpyStyleDocString();
assertEmpty(docString.getSummary());
assertSize(2, docString.getSections());
@@ -402,6 +402,25 @@ public class PySectionBasedDocStringTest extends PyTestCase {
}
// PY-31025
public void testGoogleDescriptionOfReturnValueWithoutType() {
final GoogleCodeStyleDocString docString = findAndParseGoogleStyleDocString();
assertEquals("return value description", docString.getReturnDescription());
}
// PY-31025
public void testGoogleMultilineDescriptionOfReturnValueWithoutType() {
final GoogleCodeStyleDocString docString = findAndParseGoogleStyleDocString();
assertEquals("return value description\n" +
"with continuation", docString.getReturnDescription());
}
// PY-31025
public void testGoogleDescriptionOfReturnValueOnNextLine() {
final GoogleCodeStyleDocString docString = findAndParseGoogleStyleDocString();
assertEquals("return value description", docString.getReturnDescription());
}
@Override
protected String getTestDataPath() {
return super.getTestDataPath() + "/docstrings";