[Clangd] Fixed clangd diagnostics priority

Summary:
- Fixed diagnostics where zero width inserted ranges were being used instead of the whole token
- Added unit tests

Patch by @sureyeaah !

Reviewers: sammccall, kadircet

Reviewed By: kadircet

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

Tags: #clang-tools-extra, #clang

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

llvm-svn: 363253

clang-tools-extra/clangd/Diagnostics.cpp

@@ -18,6 +18,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -90,24 +91,19 @@ Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
     if (locationInRange(Loc, R, M))
       return halfOpenToRange(M, R);
   }
-  llvm::Optional<Range> FallbackRange;
   // 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 (R.getBegin() == R.getEnd() && Loc == R.getBegin())
-      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.
-    R = CharSourceRange::getCharRange(Loc);
+  // If the token at the location is not a comment, we use the token.
+  // If we can't get the token at the location, fall back to using the location
+  auto R = CharSourceRange::getCharRange(Loc);
+  Token Tok;
+  if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment)) {
+    R = CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getEndLoc());
+  }
   return halfOpenToRange(M, R);
 }
 

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

@@ -101,6 +101,7 @@ TEST(DiagnosticsTest, DiagnosticRanges) {
   Annotations Test(R"cpp(
     namespace test{};
     void $decl[[foo]]();
+    class T{$explicit[[]]$constructor[[T]](int a);};
     int main() {
       $typo[[go\
 o]]();
@@ -112,8 +113,10 @@ o]]();
       test::$nomembernamespace[[test]];
     }
   )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  TU.ClangTidyChecks = "-*,google-explicit-constructor";
   EXPECT_THAT(
-      TestTU::withCode(Test.code()).build().getDiagnostics(),
+      TU.build().getDiagnostics(),
       ElementsAre(
           // This range spans lines.
           AllOf(Diag(Test.range("typo"),
@@ -135,7 +138,13 @@ o]]();
                "of type 'const char [4]'"),
           Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
           Diag(Test.range("nomembernamespace"),
-               "no member named 'test' in namespace 'test'")));
+               "no member named 'test' in namespace 'test'"),
+          // We make sure here that the entire token is highlighted
+          AllOf(Diag(Test.range("constructor"),
+                     "single-argument constructors must be marked explicit to "
+                     "avoid unintentional implicit conversions"),
+                WithFix(Fix(Test.range("explicit"), "explicit ",
+                            "insert 'explicit '")))));
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {