diff options
-rw-r--r-- | src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java | 14 | ||||
-rw-r--r-- | src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java | 9 |
2 files changed, 19 insertions, 4 deletions
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java index bc9edc37de..66b36a245f 100644 --- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java +++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java @@ -16,6 +16,7 @@ package com.google.devtools.skylark.skylint; import static com.google.devtools.skylark.skylint.DocstringUtils.extractDocstring; +import com.google.devtools.build.lib.events.Location.LineAndColumn; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.Expression; import com.google.devtools.build.lib.syntax.FunctionDefStatement; @@ -79,11 +80,20 @@ public class DocstringChecker extends SyntaxTreeVisitor { super.visit(node); StringLiteral functionDocstring = extractDocstring(node.getStatements()); if (functionDocstring == null && !node.getIdentifier().getName().startsWith("_")) { + Location start = Location.from(node.getLocation().getStartLineAndColumn()); + Location end; + if (node.getStatements().isEmpty()) { + // The function body can be empty because the parser discards `pass` statements: + end = Location.from(node.getLocation().getEndLineAndColumn()); + } else { + LineAndColumn lac = node.getStatements().get(0).getLocation().getStartLineAndColumn(); + end = new Location(lac.getLine(), lac.getColumn() - 1); // right before the first statement + } issues.add( - Issue.create( + new Issue( MISSING_DOCSTRING_CATEGORY, "function '" + node.getIdentifier().getName() + "' has no docstring", - node.getLocation())); + new LocationRange(start, end))); } if (functionDocstring == null) { return; diff --git a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java index 377f0eed7c..876ce24688 100644 --- a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java +++ b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java @@ -38,12 +38,17 @@ public class DocstringCheckerTests { @Test public void reportMissingDocString() throws Exception { String errorMessage = - findIssues("# no module docstring", "def function():", " pass # no function docstring") + findIssues("# no module docstring", "def function():", " return # no function docstring") .toString(); Truth.assertThat(errorMessage) .contains("1:1-2:1: file has no module docstring [missing-docstring]"); Truth.assertThat(errorMessage) - .contains("2:1-3:30: function 'function' has no docstring [missing-docstring]"); + .contains("2:1-3:2: function 'function' has no docstring [missing-docstring]"); + // The following function has zero statements since the parser throws `pass` statements away. + // Hence we have to check this case to make sure the end location is set correctly. + errorMessage = findIssues("def function():", " pass # no function docstring").toString(); + Truth.assertThat(errorMessage) + .contains("1:1-2:30: function 'function' has no docstring [missing-docstring]"); } @Test |