[clangd] Fixed clangd diagnostics priority

[SureYeaah]

Summary

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

clang-tools-extra/clangd/Diagnostics.cpp

 #include "Protocol.h"
 #include "SourceCode.h"
 #include "clang/Basic/AllDiagnostics.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #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"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
for (const auto &F : D.getFixItHints()) {
     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.
+  // If the token at location is not a comment, use the token.
+  // Otherwise use zero width insertion range
   auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
-  if (!R.isValid()) // Fall back to location only, let the editor deal with it.
+  if (R.isValid()) {
+    // check if token at location is a priority i.e. not a comment
+    Token token;
+    bool isTokenPriority = false;
+    if(!Lexer::getRawToken(Loc, token, M, L, true))
+      isTokenPriority = token.getKind() != tok::comment;
+    if (isTokenPriority) {}
+    else if (!isTokenPriority && FallbackRange)
+      return *FallbackRange;
+  }
+  else if (FallbackRange)
+    return *FallbackRange;
+  else // Fall back to location only, let the editor deal with it.
     R = CharSourceRange::getCharRange(Loc);
   return halfOpenToRange(M, R);
 }
 
 void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
                           const LangOptions &LangOpts) {
   const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();

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

+
 //===--- DiagnosticsTests.cpp ------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
Position pos(int line, int character) {
   return Res;
 }
 
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
     namespace test{};
     void $decl[[foo]]();
+    class T{$explicit[[]]$constructor[[T]](int a);};
     int main() {
       $typo[[go\
 o]]();
       foo()$semicolon[[]]//with comments
       $unk[[unknown]]();
       double $type[[bar]] = "foo";
       struct Foo { int x; }; Foo a;
       a.$nomember[[y]];
       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"),
                      "use of undeclared identifier 'goo'; did you mean 'foo'?"),
                 DiagSource(Diag::Clang), DiagName("undeclared_var_use_suggest"),
                 WithFix(
                     Fix(Test.range("typo"), "foo", "change 'go\\…' to 'foo'")),
                 // This is a pretty normal range.
                 WithNote(Diag(Test.range("decl"), "'foo' declared here"))),
           // 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("type"),
                "cannot initialize a variable of type 'double' with an lvalue "
                "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) {
   Annotations Test("[[void]] main() {}");
   auto TU = TestTU::withCode(Test.code());
   EXPECT_THAT(TU.build().getDiagnostics(),
               ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
                                 WithFix(Fix(Test.range(), "int",