[clangd] Assert that the testcases in LocateSymbol.All have no diagnostics

[nridge]

Summary

Also fix some bugs in the testcases which this exposed.

[merge_guards_bot]

Unit tests: pass. 61162 tests passed, 0 failed and 728 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[kadircet]

thanks for taking a look at this!

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

432

nit: I would rather turn this into a declaration.

517

could we rather move these cases into a new test case?

in order to prevent accidental reliance on these flags when adding new tests.

521

use ADD_FAILURE instead.

[nridge]

Address review comments

[merge_guards_bot]

Unit tests: pass. 61163 tests passed, 0 failed and 728 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[kadircet]

Thanks!

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

519

nit: I would rather keep the loop and just use ADD_FAILURE() instead of llvm::errs().

573

nit: this one can be dropped.

577

nit: this looks like an overkill to me, in the end we are trying to test hover behavior, not diagnostics. I would drop the whole loop here.

[nridge]

Address nits

[merge_guards_bot]

Unit tests: pass. 61301 tests passed, 0 failed and 736 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

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

const char *Tests[] = {
         struct [[MyClass]] {};
         } // namespace ns1
         int main() {
           ns1::My^Class* Params;
         }
       )cpp",
 
       R"cpp(// Function definition via pointer
-        int [[foo]](int) {}
+        void [[foo]](int) {}
         int main() {
           auto *X = &^foo;
         }
       )cpp",
 
       R"cpp(// Function declaration via call
         int $decl[[foo]](int);
         int main() {
           return ^foo(42);
         }
       )cpp",
 
       R"cpp(// Field
         struct Foo { int [[x]]; };
         int main() {
           Foo bar;
-          bar.^x;
+          (void)bar.^x;
         }
       )cpp",
 
       R"cpp(// Field, member initializer
         struct Foo {
           int [[x]];
           Foo() : ^x(0) {}
         };
       )cpp",
 
-      R"cpp(// Field, GNU old-style field designator
-        struct Foo { int [[x]]; };
-        int main() {
-          Foo bar = { ^x : 1 };
-        }
-      )cpp",
-
       R"cpp(// Field, field designator
         struct Foo { int [[x]]; };
         int main() {
           Foo bar = { .^x = 2 };
         }
       )cpp",
 
       R"cpp(// Method call
const char *Tests[] = {
 
       R"cpp(// Template template type parameter
         template <template<typename> class [[T]]>
         void foo() { ^T<int> t; }
       )cpp",
 
       R"cpp(// Namespace
         namespace $decl[[ns]] {
-        struct Foo { static void bar(); }
+        struct Foo { static void bar(); };
         } // namespace ns
         int main() { ^ns::Foo::bar(); }
       )cpp",
 
       R"cpp(// Macro
-        #define MACRO 0
-        #define [[MACRO]] 1
-        int main() { return ^MACRO; }
-        #define MACRO 2
-        #undef macro
-      )cpp",
-
-      R"cpp(// Macro
        class TTT { public: int a; };
        #define [[FF]](S) if (int b = S.a) {}
        void f() {
          TTT t;
          F^F(t);
        }
       )cpp",
 
       R"cpp(// Macro argument
        int [[i]];
        #define ADDRESSOF(X) &X;
        int *j = ADDRESSOF(^i);
       )cpp",
 
       R"cpp(// Symbol concatenated inside macro (not supported)
        int *pi;
-       #define POINTER(X) p # X;
+       #define POINTER(X) p ## X;
        int i = *POINTER(^i);
       )cpp",
 
       R"cpp(// Forward class declaration
         class Foo;
         class [[Foo]] {};
         F^oo* foo();
       )cpp",
const char *Tests[] = {
 
         template <>
         double [[var]]<int> = 10;
 
         double y = va^r<int>;
       )cpp",
 
       R"cpp(// No implicit constructors
-        class X {
+        struct X {
           X(X&& x) = default;
         };
-        X [[makeX]]() {}
+        X $decl[[makeX]]();
         void foo() {
           auto x = m^akeX();
         }
       )cpp",
 
       R"cpp(
         struct X {
-          X& [[operator]]++() {}
+          X& $decl[[operator]]++();
         };
         void foo(X& x) {
           +^+x;
         }
       )cpp",
 
       R"cpp(// Declaration of explicit template specialization
         template <typename T>
for (const char *Test : Tests) {
 
     TestTU TU;
     TU.Code = T.code();
 
     // FIXME: Auto-completion in a template requires disabling delayed template
     // parsing.
     TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
 
     auto AST = TU.build();
+    for (auto &D : AST.getDiagnostics())
+      ADD_FAILURE() << D;
+    ASSERT_TRUE(AST.getDiagnostics().empty()) << Test;
+
+    auto Results = locateSymbolAt(AST, T.point());
+
+    if (!WantDecl) {
+      EXPECT_THAT(Results, IsEmpty()) << Test;
+    } else {
+      ASSERT_THAT(Results, ::testing::SizeIs(1)) << Test;
+      EXPECT_EQ(Results[0].PreferredDeclaration.range, *WantDecl) << Test;
+      llvm::Optional<Range> GotDef;
+      if (Results[0].Definition)
+        GotDef = Results[0].Definition->range;
+      EXPECT_EQ(WantDef, GotDef) << Test;
+    }
+  }
+}
+
+// LocateSymbol test cases that produce warnings.
+// These are separated out from All so that in All we can assert
+// that there are no diagnostics.
+TEST(LocateSymbol, Warnings) {
+  const char *Tests[] = {
+      R"cpp(// Field, GNU old-style field designator
+        struct Foo { int [[x]]; };
+        int main() {
+          Foo bar = { ^x : 1 };
+        }
+      )cpp",
+
+      R"cpp(// Macro
+        #define MACRO 0
+        #define [[MACRO]] 1
+        int main() { return ^MACRO; }
+        #define MACRO 2
+        #undef macro
+      )cpp",
+  };
+
+  for (const char *Test : Tests) {
+    Annotations T(Test);
+    llvm::Optional<Range> WantDecl;
+    llvm::Optional<Range> WantDef;
+    if (!T.ranges().empty())
+      WantDecl = WantDef = T.range();
+    if (!T.ranges("decl").empty())
+      WantDecl = T.range("decl");
+    if (!T.ranges("def").empty())
+      WantDef = T.range("def");
+
+    TestTU TU;
+    TU.Code = T.code();
+
+    auto AST = TU.build();
     auto Results = locateSymbolAt(AST, T.point());
 
     if (!WantDecl) {
       EXPECT_THAT(Results, IsEmpty()) << Test;
     } else {
       ASSERT_THAT(Results, ::testing::SizeIs(1)) << Test;
       EXPECT_EQ(Results[0].PreferredDeclaration.range, *WantDecl) << Test;
       llvm::Optional<Range> GotDef;
       if (Results[0].Definition)
         GotDef = Results[0].Definition->range;
       EXPECT_EQ(WantDef, GotDef) << Test;
     }
   }