From 4646a9ab2602e62aa8a18c25beda2dac12cf5320 Mon Sep 17 00:00:00 2001 From: fzaiser Date: Thu, 12 Oct 2017 12:54:20 +0200 Subject: Skylint: set module docstring location to just the first line Previously, the location of the warning was the whole file. Now it is 1:1-2:1. This doesn't make sense if the file is empty but I think it's good enough since that's an edge case. RELNOTES: none PiperOrigin-RevId: 171942703 --- .../java/com/google/devtools/skylark/skylint/DocstringChecker.java | 3 ++- .../com/google/devtools/skylark/skylint/DocstringCheckerTests.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'src') 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 48ba98acf5..c19a68b58f 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 @@ -48,7 +48,8 @@ public class DocstringChecker extends SyntaxTreeVisitor { if (moduleDocstring == null) { // The reported location starts on the first line since that's where the docstring is expected Location start = new Location(1, 1); - Location end = Location.from(node.getLocation().getEndLineAndColumn()); + // This location is invalid if the file is empty but this edge case is not worth the trouble. + Location end = new Location(2, 1); LocationRange range = new LocationRange(start, end); issues.add(new Issue("file has no module docstring", range)); } else { 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 3f28066876..8b399ccd34 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 @@ -40,7 +40,7 @@ public class DocstringCheckerTests { String errorMessage = findIssues("# no module docstring", "def function():", " pass # no function docstring") .toString(); - Truth.assertThat(errorMessage).contains("1:1-3:30: file has no module docstring"); + Truth.assertThat(errorMessage).contains("1:1-2:1: file has no module docstring"); Truth.assertThat(errorMessage).contains("2:1-3:30: function 'function' has no docstring"); } -- cgit v1.2.3