From b71b2df2b22e052f8540a23051b589c6ef870d0a Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 1 Aug 2023 11:11:07 -0700 Subject: [PATCH] Trim and dedent all Starlark doc strings following PEP-257 Documentation processors often require doc strings to be dedented. For example, Markdown interprets indented blocks as code. This means before handing doc strings to any form of API doc processor (and ideally - when storing the doc string), we want to dedent them to a minimal indentation level and trim blank lines. In the Python world, the standard algorithm for doing so is specified in PEP-257. Until now, we dedented multiline function doc strings in DocstringUtils using an algorithm which differed from PEP-257 in a number of corner cases. Meanwhile, all other docs (doc strings for rules, attributes, providers etc.) were not trimmed/dedented at all, despite often containing multiple lines. To fix, we introduce Starlark.trimDocString and use it comprehensively on all doc strings. Whenever possible, we store the docstring in its trimmed form. The one exception is function docstrings, because they are stored at parse time, not eval time; we have to trim them in the accessor method. This change allows us to massively simplify the DocstringUtils parser, since it no longer needs to mix low-level string munging for dedenting/trimming with the task of finding args, returns, and deprecation stanzas. A more comprehensive alternative to #13403 Fixes #13402 PiperOrigin-RevId: 552859517 Change-Id: I225f064c7b38f2fdbf78242d5b4597ec545518d4 --- .../analysis/starlark/StarlarkAttrModule.java | 2 +- .../starlark/StarlarkRuleClassFunctions.java | 13 +- .../starlark/StarlarkRepositoryModule.java | 8 +- .../FakeStarlarkAttrModuleApi.java | 67 +++----- .../FakeStarlarkRuleFunctionsApi.java | 11 +- .../repository/FakeRepositoryModule.java | 4 +- .../java/net/starlark/java/eval/Starlark.java | 116 ++++++++++++- .../starlark/java/eval/StarlarkFunction.java | 8 +- .../net/starlark/java/eval/StringModule.java | 4 +- .../StarlarkDocExtractTest.java | 19 ++- .../StarlarkRuleClassFunctionsTest.java | 38 ++++- .../golden.textproto | 2 +- .../testdata/misc_apis_test/golden.textproto | 2 +- .../provider_basic_test/golden.textproto | 2 +- .../unknown_predefines_test/golden.textproto | 2 +- src/test/java/net/starlark/java/eval/BUILD | 1 + .../net/starlark/java/eval/EvalTests.java | 1 + .../starlark/java/eval/EvaluationTest.java | 5 +- .../starlark/java/eval/StarlarkClassTest.java | 93 +++++++++++ .../starlark/common/DocstringUtils.java | 152 +++++------------- 20 files changed, 357 insertions(+), 193 deletions(-) create mode 100644 src/test/java/net/starlark/java/eval/StarlarkClassTest.java diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java index 72520c89a31830..2ca2e7f32589d6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java @@ -120,7 +120,7 @@ private static Attribute.Builder createAttribute( String name) throws EvalException { Attribute.Builder builder = Attribute.attr(name, type).starlarkDefined(); - doc.ifPresent(builder::setDoc); + doc.map(Starlark::trimDocString).ifPresent(builder::setDoc); Object defaultValue = arguments.get(DEFAULT_ARG); if (!Starlark.isNullOrNone(defaultValue)) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index 84ba8ba2b715b0..2f22c95c5d67f4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java @@ -34,6 +34,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; import com.google.devtools.build.lib.analysis.Allowlist; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; @@ -252,11 +253,15 @@ public static RuleClass getTestBaseRule(RuleDefinitionEnvironment env) { public Object provider(Object doc, Object fields, Object init, StarlarkThread thread) throws EvalException { StarlarkProvider.Builder builder = StarlarkProvider.builder(thread.getCallerLocation()); - Starlark.toJavaOptional(doc, String.class).ifPresent(builder::setDocumentation); + Starlark.toJavaOptional(doc, String.class) + .map(Starlark::trimDocString) + .ifPresent(builder::setDocumentation); if (fields instanceof Sequence) { builder.setSchema(Sequence.cast(fields, String.class, "fields")); } else if (fields instanceof Dict) { - builder.setSchema(Dict.cast(fields, String.class, String.class, "fields")); + builder.setSchema( + Maps.transformValues( + Dict.cast(fields, String.class, String.class, "fields"), Starlark::trimDocString)); } if (init == Starlark.NONE) { return builder.build(); @@ -495,7 +500,7 @@ public static StarlarkRuleFunction createRule( type, attributes, thread.getCallerLocation(), - Starlark.toJavaOptional(doc, String.class)); + Starlark.toJavaOptional(doc, String.class).map(Starlark::trimDocString)); } private static void checkAttributeName(String name) throws EvalException { @@ -673,7 +678,7 @@ public StarlarkAspect aspect( return new StarlarkDefinedAspect( implementation, - Starlark.toJavaOptional(doc, String.class), + Starlark.toJavaOptional(doc, String.class).map(Starlark::trimDocString), attrAspects.build(), attributes.build(), StarlarkAttrModule.buildProviderPredicate(requiredProvidersArg, "required_providers"), diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java index 91e2ecd56ae145..1bfdd5245e4f98 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java @@ -116,7 +116,9 @@ public StarlarkCallable repositoryRule( bzlModule.label(), bzlModule.bzlTransitiveDigest()); builder.setWorkspaceOnly(); return new RepositoryRuleFunction( - builder, implementation, Starlark.toJavaOptional(doc, String.class)); + builder, + implementation, + Starlark.toJavaOptional(doc, String.class).map(Starlark::trimDocString)); } /** @@ -298,7 +300,7 @@ public Object moduleExtension( .setImplementation(implementation) .setTagClasses( ImmutableMap.copyOf(Dict.cast(tagClasses, String.class, TagClass.class, "tag_classes"))) - .setDoc(Starlark.toJavaOptional(doc, String.class)) + .setDoc(Starlark.toJavaOptional(doc, String.class).map(Starlark::trimDocString)) .setDefiningBzlFileLabel( BzlInitThreadContext.fromOrFailFunction(thread, "module_extension").getBzlFile()) .setEnvVariables(ImmutableList.copyOf(Sequence.cast(environ, String.class, "environ"))) @@ -324,7 +326,7 @@ public TagClass tagClass( } return TagClass.create( attrBuilder.build(), - Starlark.toJavaOptional(doc, String.class), + Starlark.toJavaOptional(doc, String.class).map(Starlark::trimDocString), thread.getCallerLocation()); } } diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java index 76ac8c3ea1930c..e1f94ce67c9cf3 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Module; @@ -30,9 +31,7 @@ import net.starlark.java.eval.StarlarkInt; import net.starlark.java.eval.StarlarkThread; -/** - * Fake implementation of {@link StarlarkAttrModuleApi}. - */ +/** Fake implementation of {@link StarlarkAttrModuleApi}. */ public class FakeStarlarkAttrModuleApi implements StarlarkAttrModuleApi { @Override @@ -44,11 +43,7 @@ public Descriptor intAttribute( StarlarkThread thread) throws EvalException { return new FakeDescriptor( - AttributeType.INT, - Starlark.toJavaOptional(doc, String.class), - mandatory, - ImmutableList.of(), - defaultInt); + AttributeType.INT, toTrimmedString(doc), mandatory, ImmutableList.of(), defaultInt); } @Override @@ -61,7 +56,7 @@ public Descriptor stringAttribute( throws EvalException { return new FakeDescriptor( AttributeType.STRING, - Starlark.toJavaOptional(doc, String.class), + toTrimmedString(doc), mandatory, ImmutableList.of(), defaultString != null ? "\"" + defaultString + "\"" : null); @@ -87,11 +82,7 @@ public Descriptor labelAttribute( allNameGroups = allProviderNameGroups(providers, thread); } return new FakeDescriptor( - AttributeType.LABEL, - Starlark.toJavaOptional(doc, String.class), - mandatory, - allNameGroups, - defaultO); + AttributeType.LABEL, toTrimmedString(doc), mandatory, allNameGroups, defaultO); } @Override @@ -100,7 +91,7 @@ public Descriptor stringListAttribute( throws EvalException { return new FakeDescriptor( AttributeType.STRING_LIST, - Starlark.toJavaOptional(doc, String.class), + toTrimmedString(doc), mandatory, ImmutableList.of(), defaultList); @@ -115,11 +106,7 @@ public Descriptor intListAttribute( StarlarkThread thread) throws EvalException { return new FakeDescriptor( - AttributeType.INT_LIST, - Starlark.toJavaOptional(doc, String.class), - mandatory, - ImmutableList.of(), - defaultList); + AttributeType.INT_LIST, toTrimmedString(doc), mandatory, ImmutableList.of(), defaultList); } @Override @@ -141,11 +128,7 @@ public Descriptor labelListAttribute( allNameGroups = allProviderNameGroups(providers, thread); } return new FakeDescriptor( - AttributeType.LABEL_LIST, - Starlark.toJavaOptional(doc, String.class), - mandatory, - allNameGroups, - defaultList); + AttributeType.LABEL_LIST, toTrimmedString(doc), mandatory, allNameGroups, defaultList); } @Override @@ -168,7 +151,7 @@ public Descriptor labelKeyedStringDictAttribute( } return new FakeDescriptor( AttributeType.LABEL_STRING_DICT, - Starlark.toJavaOptional(doc, String.class), + toTrimmedString(doc), mandatory, allNameGroups, defaultList); @@ -179,7 +162,7 @@ public Descriptor boolAttribute( Boolean defaultO, Object doc, Boolean mandatory, StarlarkThread thread) throws EvalException { return new FakeDescriptor( AttributeType.BOOLEAN, - Starlark.toJavaOptional(doc, String.class), + toTrimmedString(doc), mandatory, ImmutableList.of(), Boolean.TRUE.equals(defaultO) ? "True" : "False"); @@ -189,11 +172,7 @@ public Descriptor boolAttribute( public Descriptor outputAttribute(Object doc, Boolean mandatory, StarlarkThread thread) throws EvalException { return new FakeDescriptor( - AttributeType.OUTPUT, - Starlark.toJavaOptional(doc, String.class), - mandatory, - ImmutableList.of(), - ""); + AttributeType.OUTPUT, toTrimmedString(doc), mandatory, ImmutableList.of(), ""); } @Override @@ -201,11 +180,7 @@ public Descriptor outputListAttribute( Boolean allowEmpty, Object doc, Boolean mandatory, StarlarkThread thread) throws EvalException { return new FakeDescriptor( - AttributeType.OUTPUT_LIST, - Starlark.toJavaOptional(doc, String.class), - mandatory, - ImmutableList.of(), - ""); + AttributeType.OUTPUT_LIST, toTrimmedString(doc), mandatory, ImmutableList.of(), ""); } @Override @@ -213,11 +188,7 @@ public Descriptor stringDictAttribute( Boolean allowEmpty, Dict defaultO, Object doc, Boolean mandatory, StarlarkThread thread) throws EvalException { return new FakeDescriptor( - AttributeType.STRING_DICT, - Starlark.toJavaOptional(doc, String.class), - mandatory, - ImmutableList.of(), - defaultO); + AttributeType.STRING_DICT, toTrimmedString(doc), mandatory, ImmutableList.of(), defaultO); } @Override @@ -226,7 +197,7 @@ public Descriptor stringListDictAttribute( throws EvalException { return new FakeDescriptor( AttributeType.STRING_LIST_DICT, - Starlark.toJavaOptional(doc, String.class), + toTrimmedString(doc), mandatory, ImmutableList.of(), defaultO); @@ -236,11 +207,11 @@ public Descriptor stringListDictAttribute( public Descriptor licenseAttribute( Object defaultO, Object doc, Boolean mandatory, StarlarkThread thread) throws EvalException { return new FakeDescriptor( - AttributeType.STRING_LIST, - Starlark.toJavaOptional(doc, String.class), - mandatory, - ImmutableList.of(), - defaultO); + AttributeType.STRING_LIST, toTrimmedString(doc), mandatory, ImmutableList.of(), defaultO); + } + + private static Optional toTrimmedString(Object doc) { + return Starlark.toJavaOptional(doc, String.class).map(Starlark::trimDocString); } @Override diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java index 1b8176e11711de..f3c3696e9bf34f 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java @@ -106,8 +106,7 @@ public Object provider(Object doc, Object fields, Object init, StarlarkThread th // fields is NONE, so there is no field information to add. } providerInfoList.add( - forProviderInfo( - fakeProvider, Starlark.toJavaOptional(doc, String.class), providerFieldInfos.build())); + forProviderInfo(fakeProvider, toTrimmedString(doc), providerFieldInfos.build())); if (init == Starlark.NONE) { return fakeProvider; } else { @@ -167,7 +166,7 @@ public StarlarkCallable rule( // Only the Builder is passed to RuleInfoWrapper as the rule name is not yet available. RuleInfo.Builder ruleInfo = RuleInfo.newBuilder().addAllAttribute(attrInfos); - Starlark.toJavaOptional(doc, String.class).ifPresent(ruleInfo::setDocString); + toTrimmedString(doc).ifPresent(ruleInfo::setDocString); Location loc = thread.getCallerLocation(); ruleInfoList.add(new RuleInfoWrapper(functionIdentifier, loc, ruleInfo)); @@ -231,7 +230,7 @@ public StarlarkAspectApi aspect( // Only the Builder is passed to AspectInfoWrapper as the aspect name is not yet available. AspectInfo.Builder aspectInfo = AspectInfo.newBuilder().addAllAttribute(attrInfos).addAllAspectAttribute(aspectAttrs); - Starlark.toJavaOptional(doc, String.class).ifPresent(aspectInfo::setDocString); + toTrimmedString(doc).ifPresent(aspectInfo::setDocString); aspectInfoList.add(new AspectInfoWrapper(fakeAspect, thread.getCallerLocation(), aspectInfo)); @@ -279,4 +278,8 @@ public int compare(AttributeInfo o1, AttributeInfo o2) { } } } + + private static Optional toTrimmedString(Object doc) { + return Starlark.toJavaOptional(doc, String.class).map(Starlark::trimDocString); + } } diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java index 668d9adfb14c67..3ab502b98fd455 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java @@ -97,7 +97,9 @@ public StarlarkCallable repositoryRule( // Only the Builder is passed to RuleInfoWrapper as the rule name is not yet available. RuleInfo.Builder ruleInfo = RuleInfo.newBuilder().addAllAttribute(attrInfos); - Starlark.toJavaOptional(doc, String.class).ifPresent(ruleInfo::setDocString); + Starlark.toJavaOptional(doc, String.class) + .map(Starlark::trimDocString) + .ifPresent(ruleInfo::setDocString); Location loc = thread.getCallerLocation(); ruleInfoList.add(new RuleInfoWrapper(functionIdentifier, loc, ruleInfo)); return functionIdentifier; diff --git a/src/main/java/net/starlark/java/eval/Starlark.java b/src/main/java/net/starlark/java/eval/Starlark.java index 545fb2816da974..722f37df51d43e 100644 --- a/src/main/java/net/starlark/java/eval/Starlark.java +++ b/src/main/java/net/starlark/java/eval/Starlark.java @@ -13,8 +13,12 @@ // limitations under the License. package net.starlark.java.eval; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Strings.isNullOrEmpty; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static java.lang.Math.min; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -540,6 +544,113 @@ public static String formatWithList( return pr.toString(); } + /** + * Returns a Starlark doc string with each line trimmed and dedented to the minimal common + * indentation level (except for the first line, which is always fully trimmed), and with leading + * and trailing empty lines removed, following the PEP-257 algorithm. See + * https://peps.python.org/pep-0257/#handling-docstring-indentation + * + *

For whitespace trimming, we use the same definition of whitespace as the Starlark {@code + * string.strip} method. + * + *

Following PEP-257, we expand tabs in the doc string with tab size 8 before dedenting. + * Starlark does not use tabs for indentation, but Starlark string values may contain tabs, so we + * choose to expand them for consistency with Python. + * + *

The intent is to turn documentation strings like + * + * 

+   *     """Heading
+   *
+   *     Details paragraph
+   *     """
+   *
+ * + * and + * + * 
+   *     """
+   *     Heading
+   *
+   *     Details paragraph
+   *     """
+   *
+ * + * into the desired "Heading\n\nDetails paragraph" form, and avoid the risk of documentation + * processors interpreting indented parts of the original string as special formatting (e.g. code + * blocks in the case of Markdown). + */ + public static String trimDocString(String docString) { + ImmutableList lines = expandTabs(docString, 8).lines().collect(toImmutableList()); + if (lines.isEmpty()) { + return ""; + } + // First line is special: we fully strip it and ignore it for leading spaces calculation + String firstLineTrimmed = StringModule.INSTANCE.strip(lines.get(0), NONE); + Iterable subsequentLines = Iterables.skip(lines, 1); + int minLeadingSpaces = Integer.MAX_VALUE; + for (String line : subsequentLines) { + String strippedLeading = StringModule.INSTANCE.lstrip(line, NONE); + if (!strippedLeading.isEmpty()) { + int leadingSpaces = line.length() - strippedLeading.length(); + minLeadingSpaces = min(leadingSpaces, minLeadingSpaces); + } + } + if (minLeadingSpaces == Integer.MAX_VALUE) { + minLeadingSpaces = 0; + } + + StringBuilder result = new StringBuilder(); + result.append(firstLineTrimmed); + for (String line : subsequentLines) { + // Length check ensures we ignore leading empty lines + if (result.length() > 0) { + result.append("\n"); + } + if (line.length() > minLeadingSpaces) { + result.append(StringModule.INSTANCE.rstrip(line.substring(minLeadingSpaces), NONE)); + } + } + // Remove trailing empty lines + return StringModule.INSTANCE.rstrip(result.toString(), NONE); + } + + /** + * Expands tab characters to one or more spaces, producing the same indentation level at any given + * point on any given line as would be expected when rendering the string with a given tab size; a + * Java port of Python's {@code str.expandtabs}. + */ + static String expandTabs(String line, int tabSize) { + if (!line.contains("\t")) { + // Don't alloc in the fast case. + return line; + } + checkArgument(tabSize > 0); + StringBuilder result = new StringBuilder(); + int col = 0; + for (int i = 0; i < line.length(); i++) { + char c = line.charAt(i); + switch (c) { + case '\n': + case '\r': + result.append(c); + col = 0; + break; + case '\t': + int spaces = tabSize - col % tabSize; + for (int j = 0; j < spaces; j++) { + result.append(' '); + } + col += spaces; + break; + default: + result.append(c); + col++; + } + } + return result.toString(); + } + /** Returns a slice of a sequence as if by the Starlark operation {@code x[start:stop:step]}. */ public static Object slice( Mutability mu, Object x, Object startObj, Object stopObj, Object stepObj) @@ -972,7 +1083,10 @@ public static Object execFileProgram(Program prog, Module module, StarlarkThread int[] globalIndex = module.getIndicesOfGlobals(rfn.getGlobals()); if (module.getDocumentation() == null) { - module.setDocumentation(rfn.getDocumentation()); + String documentation = rfn.getDocumentation(); + if (documentation != null) { + module.setDocumentation(Starlark.trimDocString(documentation)); + } } StarlarkFunction toplevel = diff --git a/src/main/java/net/starlark/java/eval/StarlarkFunction.java b/src/main/java/net/starlark/java/eval/StarlarkFunction.java index 60f0af44390461..3136a3bc3e6de9 100644 --- a/src/main/java/net/starlark/java/eval/StarlarkFunction.java +++ b/src/main/java/net/starlark/java/eval/StarlarkFunction.java @@ -145,10 +145,14 @@ public String getName() { return rfn.getName(); } - /** Returns the value denoted by the function's doc string literal, or null if absent. */ + /** + * Returns the value denoted by the function's doc string literal (trimmed if necessary), or null + * if absent. + */ @Nullable public String getDocumentation() { - return rfn.getDocumentation(); + String documentation = rfn.getDocumentation(); + return documentation != null ? Starlark.trimDocString(documentation) : null; } public Module getModule() { diff --git a/src/main/java/net/starlark/java/eval/StringModule.java b/src/main/java/net/starlark/java/eval/StringModule.java index b4cdc137fd9a09..302dbc1312b50c 100644 --- a/src/main/java/net/starlark/java/eval/StringModule.java +++ b/src/main/java/net/starlark/java/eval/StringModule.java @@ -184,9 +184,11 @@ public String upper(String self) { *

Note that this differs from Python 2.7, which uses ctype.h#isspace(), and from * java.lang.Character#isWhitespace(), which does not recognize U+00A0. */ + // TODO(https://github.com/bazelbuild/starlark/issues/112): use the Unicode definition of + // whitespace, matching Python 3. private static final String LATIN1_WHITESPACE = ("\u0009" + "\n" + "\u000B" + "\u000C" + "\r" + "\u001C" + "\u001D" + "\u001E" + "\u001F" - + "\u0020" + "\u0085" + "\u00A0"); + + " " + "\u0085" + "\u00A0"); private static String stringLStrip(String self, String chars) { CharMatcher matcher = CharMatcher.anyOf(chars); diff --git a/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/StarlarkDocExtractTest.java b/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/StarlarkDocExtractTest.java index 607ee5c3db8fd0..4c54059625c77c 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/StarlarkDocExtractTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/StarlarkDocExtractTest.java @@ -824,7 +824,10 @@ public void repositoryRule() throws Exception { "", "my_repo_rule = repository_rule(", " implementation = _impl,", - " doc = 'My repository rule',", + " doc = '''My repository rule", + "", + " With details", + " ''',", " attrs = {", " 'a': attr.string(doc = 'My doc', default = 'foo'),", " 'b': attr.string(mandatory = True),", @@ -855,7 +858,7 @@ public void repositoryRule() throws Exception { .setRuleName("foo.repo_rule") .setOriginKey( OriginKey.newBuilder().setName("my_repo_rule").setFile("//:dep.bzl").build()) - .setDocString("My repository rule") + .setDocString("My repository rule\n\nWith details") .addAllAttribute(ModuleInfoExtractor.IMPLICIT_REPOSITORY_RULE_ATTRIBUTES) .addAttribute( AttributeInfo.newBuilder() @@ -878,7 +881,9 @@ public void moduleExtension() throws Exception { scratch.file( "dep.bzl", "_install = tag_class(", - " doc = 'Install',", + " doc = '''Install", + " ", + " With details''',", " attrs = {", " 'artifacts': attr.string_list(doc = 'Artifacts'),", " '_hidden': attr.bool(),", @@ -896,7 +901,9 @@ public void moduleExtension() throws Exception { " pass", "", "my_ext = module_extension(", - " doc = 'My extension',", + " doc = '''My extension", + "", + " With details''',", " tag_classes = {", " 'install': _install,", " 'artifact': _artifact,", @@ -925,12 +932,12 @@ public void moduleExtension() throws Exception { .containsExactly( ModuleExtensionInfo.newBuilder() .setExtensionName("foo.ext") - .setDocString("My extension") + .setDocString("My extension\n\nWith details") .setOriginKey(OriginKey.newBuilder().setFile("//:dep.bzl").build()) .addTagClass( ModuleExtensionTagClassInfo.newBuilder() .setTagName("install") - .setDocString("Install") + .setDocString("Install\n\nWith details") .addAttribute( AttributeInfo.newBuilder() .setName("artifacts") diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java index 67e17504c6c1b3..dbbfc96f4b5e73 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java @@ -870,6 +870,11 @@ public void testAttrDoc( Attribute documented = buildAttribute("documented", String.format("attr.%s(doc='foo')", attrType)); assertThat(documented.getDoc()).isEqualTo("foo"); + Attribute documentedNeedingDedent = + buildAttribute( + "documented", + String.format("attr.%s(doc='''foo\n\n More details.\n ''')", attrType)); + assertThat(documentedNeedingDedent.getDoc()).isEqualTo("foo\n\nMore details."); Attribute undocumented = buildAttribute("undocumented", String.format("attr.%s()", attrType)); assertThat(undocumented.getDoc()).isNull(); } @@ -899,11 +904,21 @@ public void testRuleDoc() throws Exception { ev, "def impl(ctx):", " return None", - "documented_rule = rule(impl, doc='My doc string')", + "documented_rule = rule(impl, doc = 'My doc string')", + "long_documented_rule = rule(", + " impl,", + " doc = '''Long doc", + "", + " With details", + "''',", + ")", "undocumented_rule = rule(impl)"); StarlarkRuleFunction documentedRule = (StarlarkRuleFunction) ev.lookup("documented_rule"); + StarlarkRuleFunction longDocumentedRule = + (StarlarkRuleFunction) ev.lookup("long_documented_rule"); StarlarkRuleFunction undocumentedRule = (StarlarkRuleFunction) ev.lookup("undocumented_rule"); assertThat(documentedRule.getDocumentation()).hasValue("My doc string"); + assertThat(longDocumentedRule.getDocumentation()).hasValue("Long doc\n\nWith details"); assertThat(undocumentedRule.getDocumentation()).isEmpty(); } @@ -1720,10 +1735,13 @@ public void declaredProviderDocumentation() throws Exception { evalAndExport( ev, "UndocumentedInfo = provider()", - "DocumentedInfo = provider(doc = 'My documented provider')", + "DocumentedInfo = provider(doc = '''", + " My documented provider", + "", + " Details''')", // Note fields below are not alphabetized "SchemafulWithoutDocsInfo = provider(fields = ['b', 'a'])", - "SchemafulWithDocsInfo = provider(fields = {'b': 'Field b', 'a': 'Field a'})"); + "SchemafulWithDocsInfo = provider(fields = {'b': 'Field b', 'a': 'Field\\n a'})"); StarlarkProvider undocumentedInfo = (StarlarkProvider) ev.lookup("UndocumentedInfo"); StarlarkProvider documentedInfo = (StarlarkProvider) ev.lookup("DocumentedInfo"); @@ -1732,11 +1750,11 @@ public void declaredProviderDocumentation() throws Exception { StarlarkProvider schemafulWithDocsInfo = (StarlarkProvider) ev.lookup("SchemafulWithDocsInfo"); assertThat(undocumentedInfo.getDocumentation()).isEmpty(); - assertThat(documentedInfo.getDocumentation()).hasValue("My documented provider"); + assertThat(documentedInfo.getDocumentation()).hasValue("My documented provider\n\nDetails"); assertThat(schemafulWithoutDocsInfo.getSchema()) .containsExactly("b", Optional.empty(), "a", Optional.empty()); assertThat(schemafulWithDocsInfo.getSchema()) - .containsExactly("b", Optional.of("Field b"), "a", Optional.of("Field a")); + .containsExactly("b", Optional.of("Field b"), "a", Optional.of("Field\na")); } @Test @@ -2199,10 +2217,20 @@ public void aspectDoc() throws Exception { "def _impl(target, ctx):", // " pass", "documented_aspect = aspect(_impl, doc='My doc string')", + "long_documented_aspect = aspect(", + " _impl,", + " doc='''", + " My doc string", + " ", + " With details''',", + ")", "undocumented_aspect = aspect(_impl)"); StarlarkDefinedAspect documentedAspect = (StarlarkDefinedAspect) ev.lookup("documented_aspect"); assertThat(documentedAspect.getDocumentation()).hasValue("My doc string"); + StarlarkDefinedAspect longDocumentedAspect = + (StarlarkDefinedAspect) ev.lookup("long_documented_aspect"); + assertThat(longDocumentedAspect.getDocumentation()).hasValue("My doc string\n\nWith details"); StarlarkDefinedAspect undocumentedAspect = (StarlarkDefinedAspect) ev.lookup("undocumented_aspect"); assertThat(undocumentedAspect.getDocumentation()).isEmpty(); diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/function_wrap_multiple_lines_test/golden.textproto b/src/test/java/com/google/devtools/build/skydoc/testdata/function_wrap_multiple_lines_test/golden.textproto index 20846fe4f06857..e3255e3c534f38 100644 --- a/src/test/java/com/google/devtools/build/skydoc/testdata/function_wrap_multiple_lines_test/golden.textproto +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/function_wrap_multiple_lines_test/golden.textproto @@ -123,7 +123,7 @@ rule_info { } attribute { name: "deps" - doc_string: "\nThe dependencies to use. Defaults to the most recent ANTLR 3 release,\nbut if you need to use a different version, you can specify the\ndependencies here.\n" + doc_string: "The dependencies to use. Defaults to the most recent ANTLR 3 release,\nbut if you need to use a different version, you can specify the\ndependencies here." type: LABEL_LIST default_value: "[Label(\"@antlr3_runtimes//:tool\")]" } diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/misc_apis_test/golden.textproto b/src/test/java/com/google/devtools/build/skydoc/testdata/misc_apis_test/golden.textproto index ff7dbce0f141f4..3a7f865d521330 100644 --- a/src/test/java/com/google/devtools/build/skydoc/testdata/misc_apis_test/golden.textproto +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/misc_apis_test/golden.textproto @@ -9,7 +9,7 @@ rule_info { } attribute { name: "deps" - doc_string: "\nA list of dependencies.\n" + doc_string: "A list of dependencies." type: LABEL_LIST provider_name_group { provider_name: "MyInfo" diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/provider_basic_test/golden.textproto b/src/test/java/com/google/devtools/build/skydoc/testdata/provider_basic_test/golden.textproto index 14c7b778a64460..0f82fdc11fb27a 100644 --- a/src/test/java/com/google/devtools/build/skydoc/testdata/provider_basic_test/golden.textproto +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/provider_basic_test/golden.textproto @@ -15,7 +15,7 @@ provider_info { } provider_info { provider_name: "MyVeryDocumentedInfo" - doc_string: "\nA provider with some really neat documentation.\nLook on my works, ye mighty, and despair!\n" + doc_string: "A provider with some really neat documentation.\nLook on my works, ye mighty, and despair!" field_info { name: "favorite_food" doc_string: "A string representing my favorite food" diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/unknown_predefines_test/golden.textproto b/src/test/java/com/google/devtools/build/skydoc/testdata/unknown_predefines_test/golden.textproto index 00e84866d2bbb7..6936c857c0484a 100644 --- a/src/test/java/com/google/devtools/build/skydoc/testdata/unknown_predefines_test/golden.textproto +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/unknown_predefines_test/golden.textproto @@ -38,4 +38,4 @@ func_info { mandatory: true } } -module_docstring: "\nTests that the documentation is generated even when unknown modules, calls, or providers are used.\n" +module_docstring: "Tests that the documentation is generated even when unknown modules, calls, or providers are used." diff --git a/src/test/java/net/starlark/java/eval/BUILD b/src/test/java/net/starlark/java/eval/BUILD index 82386955da80e7..3249dc9e1dea0b 100644 --- a/src/test/java/net/starlark/java/eval/BUILD +++ b/src/test/java/net/starlark/java/eval/BUILD @@ -24,6 +24,7 @@ java_test( "MutabilityTest.java", "PrinterTest.java", "StarlarkAnnotationsTest.java", + "StarlarkClassTest.java", "StarlarkEvaluationTest.java", "StarlarkFlagGuardingTest.java", "StarlarkListTest.java", diff --git a/src/test/java/net/starlark/java/eval/EvalTests.java b/src/test/java/net/starlark/java/eval/EvalTests.java index ab364c13d6e83a..63105613cf7b09 100644 --- a/src/test/java/net/starlark/java/eval/EvalTests.java +++ b/src/test/java/net/starlark/java/eval/EvalTests.java @@ -26,6 +26,7 @@ MethodLibraryTest.class, MutabilityTest.class, PrinterTest.class, + StarlarkClassTest.class, StarlarkEvaluationTest.class, StarlarkFlagGuardingTest.class, StarlarkAnnotationsTest.class, diff --git a/src/test/java/net/starlark/java/eval/EvaluationTest.java b/src/test/java/net/starlark/java/eval/EvaluationTest.java index 2f4c37fc8c8b7f..241110df864cf4 100644 --- a/src/test/java/net/starlark/java/eval/EvaluationTest.java +++ b/src/test/java/net/starlark/java/eval/EvaluationTest.java @@ -791,7 +791,8 @@ public void moduleWithDocString() throws Exception { assertThat(module.getDocumentation()).isNull(); ParserInput input = ParserInput.fromLines( - "\"\"\"Module doc header", // + "\"\"\"", + "Module doc header", // "", "Module doc details", "\"\"\"", @@ -805,7 +806,7 @@ public void moduleWithDocString() throws Exception { StarlarkThread thread = new StarlarkThread(mu, StarlarkSemantics.DEFAULT); Starlark.execFile(input, FileOptions.DEFAULT, module, thread); } - assertThat(module.getDocumentation()).isEqualTo("Module doc header\n\nModule doc details\n"); + assertThat(module.getDocumentation()).isEqualTo("Module doc header\n\nModule doc details"); } @Test diff --git a/src/test/java/net/starlark/java/eval/StarlarkClassTest.java b/src/test/java/net/starlark/java/eval/StarlarkClassTest.java new file mode 100644 index 00000000000000..6acb968a1cdeb9 --- /dev/null +++ b/src/test/java/net/starlark/java/eval/StarlarkClassTest.java @@ -0,0 +1,93 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package net.starlark.java.eval; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for static utility methods in the {@link Starlark} class. */ +@RunWith(JUnit4.class) +public final class StarlarkClassTest { + + @Test + public void trimDocString() { + // See https://peps.python.org/pep-0257/#handling-docstring-indentation + // Single line + assertThat(Starlark.trimDocString("")).isEmpty(); + assertThat(Starlark.trimDocString(" ")).isEmpty(); + assertThat(Starlark.trimDocString("\t\t\t")).isEmpty(); + assertThat(Starlark.trimDocString("Hello world")).isEqualTo("Hello world"); + assertThat(Starlark.trimDocString(" Hello world ")).isEqualTo("Hello world"); + // First line is always fully trimmed, regardless of subsequent indentation levels. + assertThat(Starlark.trimDocString(" Hello\t\nworld")).isEqualTo("Hello\nworld"); + assertThat(Starlark.trimDocString(" Hello \n world")).isEqualTo("Hello\nworld"); + assertThat(Starlark.trimDocString(" Hello \n world")).isEqualTo("Hello\nworld"); + // Subsequent lines are dedented to their minimal indentation level and fully right-trimmed + assertThat(Starlark.trimDocString(" Hello \n world \n and \n good-bye ")) + .isEqualTo("Hello\n world\nand\n good-bye"); + // ... and the first line's indentation does not affect minimal indentation level computation. + assertThat(Starlark.trimDocString(" Hello\n world\n and\n good-bye")) + .isEqualTo("Hello\n world\n and\ngood-bye"); + // Blank lines are trimmed and do not affect indentation level computation + assertThat( + Starlark.trimDocString( + " Hello \n\n world \n\n\n \n and \n \n good-bye ")) + .isEqualTo("Hello\n\n world\n\n\n\nand\n\n good-bye"); + // Windows-style \r\n is simplified to \n + assertThat(Starlark.trimDocString("Hello\r\nworld")).isEqualTo("Hello\nworld"); + assertThat(Starlark.trimDocString("Hello\r\n\r\nworld")).isEqualTo("Hello\n\nworld"); + // Leading and trailing blank lines are removed + assertThat(Starlark.trimDocString("\n\n\n")).isEmpty(); + assertThat(Starlark.trimDocString("\r\n\r\n\r\n")).isEmpty(); + assertThat(Starlark.trimDocString("\n \n \n ")).isEmpty(); + assertThat(Starlark.trimDocString("\n \r\n \r\n ")).isEmpty(); + assertThat(Starlark.trimDocString("\n\r\nHello world\n\r\n")).isEqualTo("Hello world"); + assertThat(Starlark.trimDocString("\n\n \nHello\nworld\n\n \n\t")).isEqualTo("Hello\nworld"); + assertThat(Starlark.trimDocString("\n\n \t\nHello\n world\n \n")).isEqualTo("Hello\n world"); + // Tabs are expanded to size 8 (following Python convention) + assertThat(Starlark.trimDocString("Hello\tworld")).isEqualTo("Hello world"); + assertThat(Starlark.trimDocString("\n\tHello\n\t\tworld")).isEqualTo("Hello\n world"); + assertThat(Starlark.trimDocString("\n \tHello\n\t\tworld")).isEqualTo("Hello\n world"); + assertThat(Starlark.trimDocString("\n Hello\n\tworld")).isEqualTo("Hello\n world"); + } + + @Test + public void expandTabs() { + assertThat(Starlark.expandTabs("", 8)).isEmpty(); + assertThat(Starlark.expandTabs("Hello\nworld", 8)).isEqualTo("Hello\nworld"); + + assertThat(Starlark.expandTabs("\t", 1)).isEqualTo(" "); + assertThat(Starlark.expandTabs("\t", 2)).isEqualTo(" "); + assertThat(Starlark.expandTabs(" \t", 2)).isEqualTo(" "); + assertThat(Starlark.expandTabs("\t", 8)).isEqualTo(" "); + assertThat(Starlark.expandTabs(" \t", 8)).isEqualTo(" "); + + assertThat(Starlark.expandTabs("01\t012\t0123\t01234", 4)).isEqualTo("01 012 0123 01234"); + assertThat(Starlark.expandTabs("01\t012\t0123\t01234", 8)) + .isEqualTo("01 012 0123 01234"); + + assertThat(Starlark.expandTabs("01\t\n\t012\t0123\t01234", 4)) + .isEqualTo("01 \n 012 0123 01234"); + assertThat(Starlark.expandTabs("\r01\r\n\t012\r\n\t0123\t01234\n", 8)) + .isEqualTo("\r01\r\n 012\r\n 0123 01234\n"); + + assertThrows(IllegalArgumentException.class, () -> Starlark.expandTabs("\t", 0)); + assertThrows(IllegalArgumentException.class, () -> Starlark.expandTabs("\t", -1)); + } +} diff --git a/src/tools/starlark/java/com/google/devtools/starlark/common/DocstringUtils.java b/src/tools/starlark/java/com/google/devtools/starlark/common/DocstringUtils.java index 222276ad929945..791ad657afc789 100644 --- a/src/tools/starlark/java/com/google/devtools/starlark/common/DocstringUtils.java +++ b/src/tools/starlark/java/com/google/devtools/starlark/common/DocstringUtils.java @@ -20,7 +20,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -30,7 +32,7 @@ public final class DocstringUtils { private DocstringUtils() {} // uninstantiable /** - * Parses a docstring. + * Parses a trimmed docstring. * *

The format of the docstring is as follows * @@ -58,7 +60,12 @@ private DocstringUtils() {} // uninstantiable * """ * } * - * @param docstring a docstring of the format described above + *

We expect the docstring to already be trimmed and dedented to a minimal common indentation + * level by {@link Starlark#trimDocString} or an equivalent PEP-257 style trim() implementation; + * note that {@link StarlarkFunction#getDocumentation} returns a correctly trimmed and dedented + * doc string. + * + * @param doc a docstring of the format described above * @param parseErrors a list to which parsing error messages are written * @return the parsed docstring information */ @@ -166,64 +173,35 @@ public String getDescription() { } private static class DocstringParser { - private final String docstring; - /** Start offset of the current line. */ - private int startOfLineOffset = 0; - /** End offset of the current line. */ - private int endOfLineOffset = 0; - /** Current line number within the docstring. */ - private int lineNumber = 0; /** - * The indentation of the docstring literal in the source file. - * - *

Every line except the first one must be indented by at least that many spaces. + * Current line number within the docstring, 1-based; 0 indicates that parsing has not started; + * {@code lineNumber > lines.size()} indicates EOF. */ - private int baselineIndentation = 0; + private int lineNumber = 0; + /** Whether there was a blank line before the current line. */ private boolean blankLineBefore = false; + /** Whether we've seen a special section, e.g. 'Args:', already. */ private boolean specialSectionsStarted = false; - /** List of all parsed lines in the docstring so far, including all indentation. */ - private ArrayList originalLines = new ArrayList<>(); - /** - * The current line in the docstring with the baseline indentation removed. - * - *

If the indentation of a docstring line is less than the expected {@link - * #baselineIndentation}, only the existing indentation is stripped; none of the remaining - * characters are cut off. - */ + + /** List of all parsed lines in the docstring. */ + private final ImmutableList lines; + + /** Iterator over lines. */ + private final Iterator linesIter; + + /** The current line in the docstring. */ private String line = ""; + /** Errors that occurred so far. */ private final List errors = new ArrayList<>(); private DocstringParser(String docstring) { - this.docstring = docstring; - - // Infer the indentation level: - // the smallest amount of leading whitespace - // common to all non-blank lines except the first. - int indentation = Integer.MAX_VALUE; - boolean first = true; - for (String line : Splitter.on("\n").split(docstring)) { - // ignore first line - if (first) { - first = false; - continue; - } - // count leading spaces - int i; - for (i = 0; i < line.length() && line.charAt(i) == ' '; i++) {} - if (i != line.length()) { - indentation = Math.min(indentation, i); - } - } - if (indentation == Integer.MAX_VALUE) { - indentation = 0; - } - + this.lines = ImmutableList.copyOf(Splitter.on("\n").split(docstring)); + this.linesIter = lines.iterator(); + // Load the summary line nextLine(); - // the indentation is only relevant for the following lines, not the first one: - this.baselineIndentation = indentation; } /** @@ -232,69 +210,20 @@ private DocstringParser(String docstring) { * @return whether there are lines remaining to be parsed */ private boolean nextLine() { - if (startOfLineOffset >= docstring.length()) { - return false; - } - blankLineBefore = line.trim().isEmpty(); - startOfLineOffset = endOfLineOffset; - if (startOfLineOffset >= docstring.length()) { - // Previous line was the last; previous line had no trailing newline character. + if (linesIter.hasNext()) { + blankLineBefore = line.trim().isEmpty(); + line = linesIter.next(); + lineNumber++; + return true; + } else { line = ""; + lineNumber = lines.size() + 1; return false; } - // If not the first line, advance start past the newline character. In the case where there is - // no more content, then the previous line was the second-to-last line and this last line is - // empty. - if (docstring.charAt(startOfLineOffset) == '\n') { - startOfLineOffset += 1; - } - lineNumber++; - endOfLineOffset = docstring.indexOf('\n', startOfLineOffset); - if (endOfLineOffset < 0) { - endOfLineOffset = docstring.length(); - } - String originalLine = docstring.substring(startOfLineOffset, endOfLineOffset); - originalLines.add(originalLine); - int indentation = getIndentation(originalLine); - if (endOfLineOffset == docstring.length() && startOfLineOffset != 0) { - if (!originalLine.trim().isEmpty()) { - error("closing docstring quote should be on its own line, indented the same as the " - + "opening quote"); - } else if (indentation != baselineIndentation) { - error("closing docstring quote should be indented the same as the opening quote"); - } - } - if (originalLine.trim().isEmpty()) { - line = ""; - } else { - if (indentation < baselineIndentation) { - error( - "line indented too little (here: " - + indentation - + " spaces; expected: " - + baselineIndentation - + " spaces)"); - startOfLineOffset += indentation; - } else { - startOfLineOffset += baselineIndentation; - } - line = docstring.substring(startOfLineOffset, endOfLineOffset); - } - return true; - } - - /** - * Returns whether the current line is the last one in the docstring. - * - *

It is possible for both this function and {@link #eof} to return true if all content has - * been exhausted, or if the last line is empty. - */ - private boolean onLastLine() { - return endOfLineOffset >= docstring.length(); } private boolean eof() { - return startOfLineOffset >= docstring.length(); + return lineNumber > lines.size(); } private static int getIndentation(String line) { @@ -310,7 +239,7 @@ private void error(String message) { } private void error(int lineNumber, String message) { - errors.add(new DocstringParseError(message, lineNumber, originalLines.get(lineNumber - 1))); + errors.add(new DocstringParseError(message, lineNumber, lines.get(lineNumber - 1))); } private void parseArgumentSection( @@ -375,9 +304,7 @@ DocstringInfo parse() { + " \n\n" + "For more details, please have a look at the documentation."); } - if (!(onLastLine() && line.trim().isEmpty())) { - longDescriptionLines.add(line); - } + longDescriptionLines.add(line); nextLine(); } } @@ -399,7 +326,7 @@ private void checkSectionStart(boolean duplicateSection) { } private String checkForNonStandardDeprecation(String line) { - if (line.toLowerCase().startsWith("deprecated:") || line.contains("DEPRECATED")) { + if (line.toLowerCase(Locale.ROOT).startsWith("deprecated:") || line.contains("DEPRECATED")) { error( "use a 'Deprecated:' section for deprecations, similar to a 'Returns:' section:\n\n" + "Deprecated:\n" @@ -562,7 +489,10 @@ public String getMessage() { return message; } - /** Returns the line number in the containing Starlark file which contains this error. */ + /** + * Returns the line number (skipping leading blank lines, if any) in the original doc string + * which contains this error. + */ public int getLineNumber() { return lineNumber; }