From 2e3f1f13b5e674e63846372770b340e8bdfaae2a Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet@google.com>
Date: Thu, 27 Sep 2018 12:12:42 +0000
Subject: [PATCH] Improve diagnostics range reporting.

Summary:
If we have some range information coming from clang diagnostic, promote
that one even if it doesn't contain diagnostic location inside.

Reviewers: sammccall, ioeric

Reviewed By: ioeric

Subscribers: ilya-biryukov, jkorous, arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D52544

llvm-svn: 343197
---
 clang-tools-extra/clangd/Diagnostics.cpp               | 11 +++++++++++
 clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp | 12 +++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
index 893854d..a2f86eb 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -52,17 +52,28 @@ Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
   auto &M = D.getSourceManager();
   auto Loc = M.getFileLoc(D.getLocation());
   // Accept the first range that contains the location.
+  llvm::Optional<Range> FallbackRange;
   for (const auto &CR : D.getRanges()) {
     auto R = Lexer::makeFileCharRange(CR, M, L);
     if (locationInRange(Loc, R, M))
       return halfOpenToRange(M, R);
+    // If there are no ranges that contain the location report the first range.
+    if (!FallbackRange)
+      FallbackRange = halfOpenToRange(M, R);
   }
   // The range may be given as a fixit hint instead.
   for (const auto &F : D.getFixItHints()) {
     auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L);
     if (locationInRange(Loc, R, M))
       return halfOpenToRange(M, R);
+    // If there's a fixit that performs insertion, it has zero-width. Therefore
+    // it can't contain the location of the diag, but it might be possible that
+    // this should be reported as range. For example missing semicolon.
+    if (!FallbackRange && R.getBegin() == R.getEnd())
+      FallbackRange = halfOpenToRange(M, R);
   }
+  if (FallbackRange)
+    return *FallbackRange;
   // If no suitable range is found, just use the token at the location.
   auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
   if (!R.isValid()) // Fall back to location only, let the editor deal with it.
diff --git a/clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp b/clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp
index 5e662cb..48d9366 100644
--- a/clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp
+++ b/clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp
@@ -79,8 +79,9 @@ TEST(DiagnosticsTest, DiagnosticRanges) {
     int main() {
       $typo[[go\
 o]]();
-      foo()$semicolon[[]]
+      foo()$semicolon[[]]//with comments
       $unk[[unknown]]();
+      double bar = $type[["foo"]];
     }
   )cpp");
   EXPECT_THAT(
@@ -93,11 +94,16 @@ o]]();
                     Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
                 // This is a pretty normal range.
                 WithNote(Diag(Test.range("decl"), "'foo' declared here"))),
-          // This range is zero-width, and at the end of a line.
+          // This range is zero-width and insertion. Therefore make sure we are
+          // not expanding it into other tokens. Since we are not going to
+          // replace those.
           AllOf(Diag(Test.range("semicolon"), "expected ';' after expression"),
                 WithFix(Fix(Test.range("semicolon"), ";", "insert ';'"))),
           // This range isn't provided by clang, we expand to the token.
-          Diag(Test.range("unk"), "use of undeclared identifier 'unknown'")));
+          Diag(Test.range("unk"), "use of undeclared identifier 'unknown'"),
+          Diag(Test.range("type"),
+               "cannot initialize a variable of type 'double' with an lvalue "
+               "of type 'const char [4]'")));
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
-- 
2.7.4