[clangd] Revert back to previous heuristic for diagnostic range extraction.

[kadircet]

Summary

Also add a few new test cases and a special case into handling of empty fixit ranges that collides with location of a diag.

[kadircet]

• Revert to previous heuristic with special handling of empty fixit range.

clangd/Diagnostics.cpp

bool locationInRange(SourceLocation L, CharSourceRange R,
   return L != R.getEnd() && M.isPointWithin(L, R.getBegin(), R.getEnd());
 }
 
 // Clang diags have a location (shown as ^) and 0 or more ranges (~~~~).
 // LSP needs a single range.
 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);
   }
+  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 (!FallbackRange && R.getBegin() == R.getEnd())
+    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);

unittests/clangd/ClangdUnitTests.cpp

Position pos(int line, int character) {
   Res.line = line;
   Res.character = character;
   return Res;
 }
 
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
+    namespace test{};
     void $decl[[foo]]();
     int main() {
       $typo[[go\
 o]]();
       foo()$semicolon[[]]//with comments
       $unk[[unknown]]();
-      double bar = $type[["foo"]];
+      double $type[[bar]] = "foo";
+      struct Foo { int x; }; Foo a;
+      a.$nomember[[y]];
+      test::$nomembernamespace[[test]];
     }
   )cpp");
   EXPECT_THAT(
       TestTU::withCode(Test.code()).build().getDiagnostics(),
       ElementsAre(
           // This range spans lines.
           AllOf(Diag(Test.range("typo"),
                      "use of undeclared identifier 'goo'; did you mean 'foo'?"),
                 WithFix(
                     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 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]'")));
+               "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'")));
 }
 
 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",