OSDN Git Service

[clangd] Errors in TestTU cause test failures unless suppressed with error-ok.
authorSam McCall <sam.mccall@gmail.com>
Wed, 22 Jan 2020 15:38:41 +0000 (16:38 +0100)
committerSam McCall <sam.mccall@gmail.com>
Fri, 24 Jan 2020 10:16:27 +0000 (11:16 +0100)
Summary:
The historic behavior of TestTU is to gather diagnostics and otherwise ignore
them. So if a test has a syntax error, and doesn't assert diagnostics, it
silently misbehaves.
This can be annoying when developing tests, as evidenced by various tests
gaining "assert no diagnostics" where that's not really the point of the test.

This patch aims to make that default behavior. For the first error
(not warning), TestTU will call ADD_FAILURE().

This can be suppressed with a comment containing "error-ok". For now that will
suppress any errors in the TU. We can make this stricter later -verify style.
(-verify itself is hard to reuse because of DiagnosticConsumer interfaces...)
A magic-comment was chosen over a TestTU option because of table-driven tests.

In addition to the behavior change, this patch:
  - adds //error-ok where we're knowingly testing invalid code
    (e.g. for diagnostics, crash-resilience, or token-level tests)
  - fixes a bunch of errors in the checked-in tests, mostly trivial (missing ;)
  - removes a bunch of now-redundant instances of "assert no diagnostics"

Reviewers: kadircet

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

Tags: #clang

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

18 files changed:
clang-tools-extra/clangd/unittests/ASTTests.cpp
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/FileIndexTests.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/clangd/unittests/PrintASTTests.cpp
clang-tools-extra/clangd/unittests/QualityTests.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
clang-tools-extra/clangd/unittests/TweakTests.cpp
clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

index 8049588..21f70dc 100644 (file)
@@ -37,8 +37,6 @@ TEST(GetDeducedType, KwAutoExpansion) {
   for (Test T : Tests) {
     Annotations File(T.AnnotatedCode);
     auto AST = TestTU::withCode(File.code()).build();
-    ASSERT_TRUE(AST.getDiagnostics().empty())
-        << AST.getDiagnostics().begin()->Message;
     SourceManagerForFile SM("foo.cpp", File.code());
 
     for (Position Pos : File.points()) {
index 8eee755..5ecdbeb 100644 (file)
@@ -65,7 +65,7 @@ TEST(CollectMainFileMacros, SelectedMacros) {
         #define $2[[PREPEND]](X) MACRO##X()
         #define $3[[MACROA]]() 123
         int B = $1[[CONCAT]](MACRO);
-        int D = $2[[PREPEND]](A)
+        int D = $2[[PREPEND]](A);
       )cpp",
       R"cpp(
         // FIXME: Macro names in a definition are not detected.
index ef73519..79d51fa 100644 (file)
@@ -104,6 +104,7 @@ Position pos(int line, int character) {
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
+    // error-ok
     namespace test{};
     void $decl[[foo]]();
     class T{$explicit[[]]$constructor[[T]](int a);};
@@ -153,7 +154,7 @@ o]]();
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
-  Annotations Test("[[void]] main() {}");
+  Annotations Test("[[void]] main() {} // error-ok");
   auto TU = TestTU::withCode(Test.code());
   EXPECT_THAT(TU.build().getDiagnostics(),
               ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
@@ -170,7 +171,7 @@ TEST(DiagnosticsTest, FlagsMatter) {
 
 TEST(DiagnosticsTest, DiagnosticPreamble) {
   Annotations Test(R"cpp(
-    #include $[["not-found.h"]]
+    #include $[["not-found.h"]] // error-ok
   )cpp");
 
   auto TU = TestTU::withCode(Test.code());
@@ -290,7 +291,7 @@ TEST(DiagnosticTest, ClangTidyWarningAsError) {
   Annotations Main(R"cpp(
     int main() {
       int i = 3;
-      double f = [[8]] / i;
+      double f = [[8]] / i; // error-ok
     }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
@@ -309,6 +310,7 @@ TEST(DiagnosticTest, LongFixMessages) {
   // We limit the size of printed code.
   Annotations Source(R"cpp(
     int main() {
+      // error-ok
       int somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier;
       [[omereallyreallyreallyreallyreallyreallyreallyreallylongidentifier]]= 10;
     }
@@ -323,13 +325,14 @@ TEST(DiagnosticTest, LongFixMessages) {
           "'somereallyreallyreallyreallyreallyreallyreallyreal…'"))));
   // Only show changes up to a first newline.
   Source = Annotations(R"cpp(
+    // error-ok
     int main() {
       int ident;
       [[ide\
-n]] = 10;
+n]] = 10; // error-ok
     }
   )cpp");
-  TU = TestTU::withCode(Source.code());
+  TU.Code = Source.code();
   EXPECT_THAT(TU.build().getDiagnostics(),
               ElementsAre(WithFix(
                   Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'"))));
@@ -339,7 +342,7 @@ TEST(DiagnosticTest, ClangTidyWarningAsErrorTrumpsSuppressionComment) {
   Annotations Main(R"cpp(
     int main() {
       int i = 3;
-      double f = [[8]] / i;  // NOLINT
+      double f = [[8]] / i;  // NOLINT // error-ok
     }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
@@ -363,7 +366,7 @@ TEST(DiagnosticsTest, Preprocessor) {
   Annotations Test(R"cpp(
     #ifndef FOO
     #define FOO
-      int a = [[b]];
+      int a = [[b]]; // error-ok
     #else
       int x = y;
     #endif
@@ -379,7 +382,7 @@ TEST(DiagnosticsTest, InsideMacros) {
     #define RET(x) return x + 10
 
     int* foo() {
-      RET($foo[[0]]);
+      RET($foo[[0]]); // error-ok
     }
     int* bar() {
       return $bar[[TEN]];
@@ -398,7 +401,7 @@ TEST(DiagnosticsTest, NoFixItInMacro) {
   Annotations Test(R"cpp(
     #define Define(name) void name() {}
 
-    [[Define]](main)
+    [[Define]](main) // error-ok
   )cpp");
   auto TU = TestTU::withCode(Test.code());
   EXPECT_THAT(TU.build().getDiagnostics(),
@@ -540,7 +543,7 @@ buildIndexWithSymbol(llvm::ArrayRef<SymbolWithHeader> Syms) {
 }
 
 TEST(IncludeFixerTest, IncompleteType) {
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace ns {
   class X;
   $nested[[X::]]Nested n;
@@ -573,7 +576,7 @@ int main() {
 }
 
 TEST(IncludeFixerTest, NoSuggestIncludeWhenNoDefinitionInHeader) {
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace ns {
   class X;
 }
@@ -604,7 +607,7 @@ int main() {
 }
 
 TEST(IncludeFixerTest, Typo) {
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace ns {
 void foo() {
   $unqualified1[[X]] x;
@@ -648,7 +651,7 @@ void bar() {
 }
 
 TEST(IncludeFixerTest, MultipleMatchedSymbols) {
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace na {
 namespace nb {
 void foo() {
@@ -673,7 +676,7 @@ void foo() {
 }
 
 TEST(IncludeFixerTest, NoCrashMemebrAccess) {
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
     struct X { int  xyz; };
     void g() { X x; x.$[[xy]] }
   )cpp");
@@ -690,7 +693,7 @@ TEST(IncludeFixerTest, NoCrashMemebrAccess) {
 TEST(IncludeFixerTest, UseCachedIndexResults) {
   // As index results for the identical request are cached, more than 5 fixes
   // are generated.
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
 $insert[[]]void foo() {
   $x1[[X]] x;
   $x2[[X]] x;
@@ -729,7 +732,7 @@ void bar(X *x) {
 }
 
 TEST(IncludeFixerTest, UnresolvedNameAsSpecifier) {
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace ns {
 }
 void g() {  ns::$[[scope]]::X_Y();  }
@@ -751,7 +754,7 @@ void g() {  ns::$[[scope]]::X_Y();  }
 }
 
 TEST(IncludeFixerTest, UnresolvedSpecifierWithSemaCorrection) {
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace clang {
 void f() {
   // "clangd::" will be corrected to "clang::" by Sema.
@@ -797,7 +800,7 @@ void f() {
 }
 
 TEST(IncludeFixerTest, SpecifiedScopeIsNamespaceAlias) {
-  Annotations Test(R"cpp(
+  Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace a {}
 namespace b = a;
 namespace c {
@@ -825,7 +828,7 @@ TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) {
 
     struct A {
       Templ<char> s;
-      A() { [[a]]; } // this caused crashes if we compute scopes lazily.
+      A() { [[a]]; /*error-ok*/ } // crash if we compute scopes lazily.
     };
   )cpp");
 
@@ -841,7 +844,7 @@ TEST(DiagsInHeaders, DiagInsideHeader) {
   Annotations Main(R"cpp(
     #include [["a.h"]]
     void foo() {})cpp");
-  Annotations Header("[[no_type_spec]];");
+  Annotations Header("[[no_type_spec]]; // error-ok");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   EXPECT_THAT(TU.build().getDiagnostics(),
@@ -856,7 +859,8 @@ TEST(DiagsInHeaders, DiagInTransitiveInclude) {
     #include [["a.h"]]
     void foo() {})cpp");
   TestTU TU = TestTU::withCode(Main.code());
-  TU.AdditionalFiles = {{"a.h", "#include \"b.h\""}, {"b.h", "no_type_spec;"}};
+  TU.AdditionalFiles = {{"a.h", "#include \"b.h\""},
+                        {"b.h", "no_type_spec; // error-ok"}};
   EXPECT_THAT(TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(), "in included file: C++ requires a "
@@ -869,7 +873,8 @@ TEST(DiagsInHeaders, DiagInMultipleHeaders) {
     #include $b[["b.h"]]
     void foo() {})cpp");
   TestTU TU = TestTU::withCode(Main.code());
-  TU.AdditionalFiles = {{"a.h", "no_type_spec;"}, {"b.h", "no_type_spec;"}};
+  TU.AdditionalFiles = {{"a.h", "no_type_spec; // error-ok"},
+                        {"b.h", "no_type_spec; // error-ok"}};
   EXPECT_THAT(TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range("a"), "in included file: C++ requires a type "
@@ -884,8 +889,9 @@ TEST(DiagsInHeaders, PreferExpansionLocation) {
     #include "b.h"
     void foo() {})cpp");
   TestTU TU = TestTU::withCode(Main.code());
-  TU.AdditionalFiles = {{"a.h", "#include \"b.h\"\n"},
-                        {"b.h", "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+  TU.AdditionalFiles = {
+      {"a.h", "#include \"b.h\"\n"},
+      {"b.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}};
   EXPECT_THAT(TU.build().getDiagnostics(),
               UnorderedElementsAre(Diag(Main.range(),
                                         "in included file: C++ requires a type "
@@ -900,9 +906,10 @@ TEST(DiagsInHeaders, PreferExpansionLocationMacros) {
     #include [["b.h"]]
     void foo() {})cpp");
   TestTU TU = TestTU::withCode(Main.code());
-  TU.AdditionalFiles = {{"a.h", "#include \"c.h\"\n"},
-                        {"b.h", "#include \"c.h\"\n"},
-                        {"c.h", "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+  TU.AdditionalFiles = {
+      {"a.h", "#include \"c.h\"\n"},
+      {"b.h", "#include \"c.h\"\n"},
+      {"c.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}};
   EXPECT_THAT(TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(), "in included file: C++ requires a "
@@ -920,7 +927,7 @@ TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) {
                         {"c.h", R"cpp(
       #ifndef X
       #define X
-      no_type_spec_0;
+      no_type_spec_0; // error-ok
       no_type_spec_1;
       no_type_spec_2;
       no_type_spec_3;
@@ -943,7 +950,7 @@ TEST(DiagsInHeaders, OnlyErrorOrFatal) {
     #include [["a.h"]]
     void foo() {})cpp");
   Annotations Header(R"cpp(
-    [[no_type_spec]];
+    [[no_type_spec]]; // error-ok
     int x = 5/0;)cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
@@ -960,7 +967,7 @@ TEST(DiagsInHeaders, FromNonWrittenSources) {
     void foo() {})cpp");
   Annotations Header(R"cpp(
     int x = 5/0;
-    int b = [[FOO]];)cpp");
+    int b = [[FOO]]; // error-ok)cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
@@ -974,7 +981,7 @@ TEST(DiagsInHeaders, FromNonWrittenSources) {
 TEST(DiagsInHeaders, ErrorFromMacroExpansion) {
   Annotations Main(R"cpp(
   void bar() {
-    int fo;
+    int fo; // error-ok
     #include [["a.h"]]
   })cpp");
   Annotations Header(R"cpp(
@@ -991,7 +998,7 @@ TEST(DiagsInHeaders, ErrorFromMacroExpansion) {
 TEST(DiagsInHeaders, ErrorFromMacroArgument) {
   Annotations Main(R"cpp(
   void bar() {
-    int fo;
+    int fo; // error-ok
     #include [["a.h"]]
   })cpp");
   Annotations Header(R"cpp(
index 9418b68..a65352d 100644 (file)
@@ -407,7 +407,6 @@ TEST(FileIndexTest, ReferencesInMainFileWithPreamble) {
   TestTU TU;
   TU.HeaderCode = "class Foo{};";
   Annotations Main(R"cpp(
-    #include "foo.h"
     void f() {
       [[Foo]] foo;
     }
index 0f450e8..39ab889 100644 (file)
@@ -75,7 +75,6 @@ protected:
     auto TU = TestTU::withCode(A.code());
     TU.ExtraArgs = Flags;
     auto AST = TU.build();
-    EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()) << Code;
     llvm::Annotations::Range R = A.range();
     SelectionTree Selection(AST.getASTContext(), AST.getTokens(), R.Begin,
                             R.End);
@@ -592,11 +591,6 @@ protected:
     TU.ExtraArgs.push_back("-std=c++2a");
 
     auto AST = TU.build();
-    for (auto &D : AST.getDiagnostics()) {
-      if (D.Severity > DiagnosticsEngine::Warning)
-        ADD_FAILURE() << D << Code;
-    }
-
     auto *TestDecl = &findDecl(AST, "foo");
     if (auto *T = llvm::dyn_cast<FunctionTemplateDecl>(TestDecl))
       TestDecl = T->getTemplatedDecl();
index 67257ba..87dd056 100644 (file)
@@ -563,7 +563,6 @@ class Foo {})cpp";
     TU.ExtraArgs.push_back("-std=c++17");
     TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
     auto AST = TU.build();
-    ASSERT_TRUE(AST.getDiagnostics().empty());
 
     auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
     ASSERT_TRUE(H);
@@ -630,8 +629,6 @@ TEST(Hover, NoHover) {
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
     auto AST = TU.build();
-    ASSERT_TRUE(AST.getDiagnostics().empty());
-
     auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
     ASSERT_FALSE(H);
   }
@@ -1589,9 +1586,6 @@ TEST(Hover, All) {
     TU.ExtraArgs.push_back("-std=c++17");
     TU.ExtraArgs.push_back("-Wno-gnu-designator");
     auto AST = TU.build();
-    for (const auto &D : AST.getDiagnostics())
-      ADD_FAILURE() << D;
-    ASSERT_TRUE(AST.getDiagnostics().empty());
 
     auto H = getHover(AST, T.point(), format::getLLVMStyle(), Index.get());
     ASSERT_TRUE(H);
@@ -1626,10 +1620,6 @@ TEST(Hover, DocsFromIndex) {
 
   TestTU TU = TestTU::withCode(T.code());
   auto AST = TU.build();
-  for (const auto &D : AST.getDiagnostics())
-    ADD_FAILURE() << D;
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   Symbol IndexSym;
   IndexSym.ID = *getSymbolID(&findDecl(AST, "X"));
   IndexSym.Documentation = "comment from index";
@@ -1663,10 +1653,6 @@ TEST(Hover, DocsFromAST) {
 
   TestTU TU = TestTU::withCode(T.code());
   auto AST = TU.build();
-  for (const auto &D : AST.getDiagnostics())
-    ADD_FAILURE() << D;
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   for (const auto &P : T.points()) {
     auto H = getHover(AST, P, format::getLLVMStyle(), nullptr);
     ASSERT_TRUE(H);
@@ -1690,10 +1676,6 @@ TEST(Hover, DocsFromMostSpecial) {
 
   TestTU TU = TestTU::withCode(T.code());
   auto AST = TU.build();
-  for (const auto &D : AST.getDiagnostics())
-    ADD_FAILURE() << D;
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   for (auto Comment : {"doc1", "doc2", "doc3"}) {
     for (const auto &P : T.points(Comment)) {
       auto H = getHover(AST, P, format::getLLVMStyle(), nullptr);
@@ -1866,9 +1848,6 @@ TEST(Hover, ExprTests) {
     Annotations T(C.Code);
     TestTU TU = TestTU::withCode(T.code());
     auto AST = TU.build();
-    for (const auto &D : AST.getDiagnostics())
-      ADD_FAILURE() << D;
-
     auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
     ASSERT_TRUE(H);
     HoverInfo ExpectedHover;
index 85b8f43..a19a88b 100644 (file)
@@ -173,6 +173,7 @@ TEST(ParsedASTTest, TokensAfterPreamble) {
       #include "foo.h"
       first_token;
       void test() {
+        // error-ok: invalid syntax, just examining token stream
       }
       last_token
 )cpp";
@@ -236,24 +237,26 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
     // - preamble ends
     ^ID(int A);
     // Macro arguments included.
-    ^MACRO_ARGS(^MACRO_ARGS(^MACRO_EXP(int), A), ^ID(= 2));
+    ^MACRO_ARGS(^MACRO_ARGS(^MACRO_EXP(int), E), ^ID(= 2));
 
     // Macro names inside other macros not included.
     #define ^MACRO_ARGS2(X, Y) X Y
     #define ^FOO BAR
     #define ^BAR 1
-    int A = ^FOO;
+    int F = ^FOO;
 
     // Macros from token concatenations not included.
     #define ^CONCAT(X) X##A()
     #define ^PREPEND(X) MACRO##X()
     #define ^MACROA() 123
-    int B = ^CONCAT(MACRO);
-    int D = ^PREPEND(A)
+    int G = ^CONCAT(MACRO);
+    int H = ^PREPEND(A);
 
     // Macros included not from preamble not included.
     #include "foo.inc"
 
+    int printf(const char*, ...);
+    void exit(int);
     #define ^assert(COND) if (!(COND)) { printf("%s", #COND); exit(0); }
 
     void test() {
index 807b3de..9e4cc56 100644 (file)
@@ -81,7 +81,7 @@ INSTANTIATE_TEST_CASE_P(ASTUtilsTests, ASTUtils,
                                   template <template <class> class ...>
                                   class Aux {};
                                   template <> class ^Aux<Bar, Bar> {};
-                                  template <template <class> T>
+                                  template <template <class> class T>
                                   class ^Aux<T, T> {};)cpp",
                                 {"<Bar, Bar>", "<T, T>"}},
                             {
@@ -104,7 +104,7 @@ INSTANTIATE_TEST_CASE_P(ASTUtilsTests, ASTUtils,
                                   template <>
                                   int ^S<double> = 0;)cpp",
                                 {"<T *>", "<double>"}},
-                        })),);
+                        })), );
 } // namespace
 } // namespace clangd
 } // namespace clang
index 0c739d7..d2d0261 100644 (file)
@@ -112,7 +112,7 @@ TEST(QualityTests, SymbolRelevanceSignalExtraction) {
   int deprecated() { return 0; }
 
   namespace { struct X { void y() { int z; } }; }
-  struct S{}
+  struct S{};
   )cpp";
   auto AST = Test.build();
 
index 65a3d27..4f0054e 100644 (file)
@@ -294,7 +294,7 @@ TEST(RenameTest, WithinFileRename) {
       // Derived destructor explicit call.
       R"cpp(
         class [[Bas^e]] {};
-        class Derived : public [[Bas^e]] {}
+        class Derived : public [[Bas^e]] {};
 
         int main() {
           [[Bas^e]] *foo = new Derived();
index 581309b..4860127 100644 (file)
@@ -259,7 +259,7 @@ TEST(SelectionTest, CommonAncestor) {
       // Tricky case: CXXConstructExpr wants to claim the whole init range.
       {
           R"cpp(
-            class X { X(int); };
+            struct X { X(int); };
             class Y {
               X x;
               Y() : [[^x(4)]] {}
@@ -308,7 +308,7 @@ TEST(SelectionTest, CommonAncestor) {
             };
             Str makeStr(const char*);
             void loop() {
-              for (const char* C : [[mak^eStr("foo"^)]])
+              for (const char C : [[mak^eStr("foo"^)]])
                 ;
             }
           )cpp",
@@ -484,7 +484,7 @@ TEST(SelectionTest, MacroArgExpansion) {
   // (This is because we don't associate the stringified token with the arg).
   Case = R"cpp(
     void die(const char*);
-    #define assert(x) (x ? (void)0 : die(#x)
+    #define assert(x) (x ? (void)0 : die(#x))
     void foo() { assert(^42); }
   )cpp";
   Test = Annotations(Case);
index d51f41a..baffc6a 100644 (file)
@@ -271,7 +271,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
       R"cpp(
       struct $Class[[AA]] {
         int $Field[[A]];
-      }
+      };
       int $Variable[[B]];
       $Class[[AA]] $Variable[[A]]{$Variable[[B]]};
     )cpp",
@@ -355,6 +355,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
       };
       class $Class[[Foo]] {};
       class $Class[[Bar]] {
+      public:
         $Class[[Foo]] $Field[[Fo]];
         $Enum[[En]] $Field[[E]];
         int $Field[[I]];
@@ -432,6 +433,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
         $Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $LocalVariable[[GG]];
         $LocalVariable[[GG]].$Method[[foo]](&$LocalVariable[[FF]]);
         $Class[[A]]<$Function[[foo]]> $LocalVariable[[AA]];
+      }
     )cpp",
       // Tokens that share a source range but have conflicting Kinds are not
       // highlighted.
@@ -466,7 +468,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
         $Macro[[INC_VAR]]($LocalVariable[[variable]]);
       }
       void $Macro[[SOME_NAME]]();
-      $Macro[[DEF_VAR]]($Variable[[XYZ]], 567);
+      $Macro[[DEF_VAR]]($Variable[[MMMMM]], 567);
       $Macro[[DEF_VAR_REV]](756, $Variable[[AB]]);
 
       #define $Macro[[CALL_FN]](F) F();
@@ -599,7 +601,7 @@ $InactiveCode[[]]      #endif
       struct $Class[[Foo]] {
         $Class[[Foo]]<$TemplateParameter[[TT]], $TemplateParameter[[TTs]]...>
           *$Field[[t]];
-      }
+      };
     )cpp",
       // Inactive code highlighting
       R"cpp(
@@ -673,7 +675,6 @@ sizeof...($TemplateParameter[[Elements]]);
     class $Class[[A]] {
       #include "imp.h"
     };
-    #endif
   )cpp",
                      {{"imp.h", R"cpp(
     int someMethod();
index c25fbcd..023de02 100644 (file)
@@ -138,12 +138,15 @@ TEST(SymbolInfoTests, All) {
                                            "c:TestTU.cpp@38@F@bar#I#@aaa")}},
           {
               R"cpp( // Lambda capture
-          int ii;
-          auto lam = [ii]() {
-            return i^i;
-          };
+          void foo() {
+            int ii;
+            auto lam = [ii]() {
+              return i^i;
+            };
+          }
         )cpp",
-              {CreateExpectedSymbolDetails("ii", "", "c:@ii")}},
+              {CreateExpectedSymbolDetails("ii", "foo",
+                                           "c:TestTU.cpp@54@F@foo#@ii")}},
           {
               R"cpp( // Macro reference
           #define MACRO 5\nint i = MAC^RO;
index 75393f1..5471733 100644 (file)
@@ -13,6 +13,7 @@
 #include "index/FileIndex.h"
 #include "index/MemIndex.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/Utils.h"
 
@@ -75,6 +76,22 @@ ParsedAST TestTU::build() const {
     ADD_FAILURE() << "Failed to build code:\n" << Code;
     llvm_unreachable("Failed to build TestTU!");
   }
+  // Check for error diagnostics and report gtest failures (unless expected).
+  // This guards against accidental syntax errors silently subverting tests.
+  // error-ok is awfully primitive - using clang -verify would be nicer.
+  // Ownership and layering makes it pretty hard.
+  if (llvm::none_of(Files, [](const auto &KV) {
+        return llvm::StringRef(KV.second).contains("error-ok");
+      })) {
+    for (const auto &D : AST->getDiagnostics())
+      if (D.Severity >= DiagnosticsEngine::Error) {
+        ADD_FAILURE()
+            << "TestTU failed to build (suppress with /*error-ok*/): \n"
+            << D << "\n\nFor code:\n"
+            << Code;
+        break; // Just report first error for simplicity.
+      }
+  }
   return std::move(*AST);
 }
 
index 403f2a8..c535645 100644 (file)
@@ -64,6 +64,8 @@ struct TestTU {
   // Simulate a header guard of the header (using an #import directive).
   bool ImplicitHeaderGuard = true;
 
+  // By default, build() will report Error diagnostics as GTest errors.
+  // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
   SymbolSlab headerSymbols() const;
   std::unique_ptr<SymbolIndex> index() const;
index 38f958c..2a99521 100644 (file)
@@ -76,26 +76,23 @@ TEST(FileEdits, AbsolutePath) {
 TWEAK_TEST(SwapIfBranches);
 TEST_F(SwapIfBranchesTest, Test) {
   Context = Function;
-  EXPECT_EQ(apply("^if (true) {return 100;} else {continue;}"),
-            "if (true) {continue;} else {return 100;}");
-  EXPECT_EQ(apply("^if () {return 100;} else {continue;}"),
-            "if () {continue;} else {return 100;}")
+  EXPECT_EQ(apply("^if (true) {return;} else {(void)0;}"),
+            "if (true) {(void)0;} else {return;}");
+  EXPECT_EQ(apply("^if (/*error-ok*/) {return;} else {(void)0;}"),
+            "if (/*error-ok*/) {(void)0;} else {return;}")
       << "broken condition";
-  EXPECT_AVAILABLE("^i^f^^(^t^r^u^e^) { return 100; } ^e^l^s^e^ { continue; }");
-  EXPECT_UNAVAILABLE("if (true) {^return ^100;^ } else { ^continue^;^ }");
+  EXPECT_AVAILABLE("^i^f^^(^t^r^u^e^) { return; } ^e^l^s^e^ { return; }");
+  EXPECT_UNAVAILABLE("if (true) {^return ^;^ } else { ^return^;^ }");
   // Available in subexpressions of the condition;
-  EXPECT_THAT("if(2 + [[2]] + 2) { return 2 + 2 + 2; } else {continue;}",
-              isAvailable());
+  EXPECT_THAT("if(2 + [[2]] + 2) { return; } else {return;}", isAvailable());
   // But not as part of the branches.
-  EXPECT_THAT("if(2 + 2 + 2) { return 2 + [[2]] + 2; } else { continue; }",
+  EXPECT_THAT("if(2 + 2 + 2) { [[return]]; } else { return; }",
               Not(isAvailable()));
   // Range covers the "else" token, so available.
-  EXPECT_THAT("if(2 + 2 + 2) { return 2 + [[2 + 2; } else {continue;]]}",
-              isAvailable());
+  EXPECT_THAT("if(2 + 2 + 2) { return[[; } else {return;]]}", isAvailable());
   // Not available in compound statements in condition.
-  EXPECT_THAT(
-      "if([]{return [[true]];}()) { return 2 + 2 + 2; } else { continue; }",
-      Not(isAvailable()));
+  EXPECT_THAT("if([]{return [[true]];}()) { return; } else { return; }",
+              Not(isAvailable()));
   // Not available if both sides aren't braced.
   EXPECT_THAT("^if (1) return; else { return; }", Not(isAvailable()));
   // Only one if statement is supported!
@@ -144,7 +141,7 @@ TEST_F(ObjCLocalizeStringLiteralTest, Test) {
 TWEAK_TEST(DumpAST);
 TEST_F(DumpASTTest, Test) {
   EXPECT_AVAILABLE("^int f^oo() { re^turn 2 ^+ 2; }");
-  EXPECT_UNAVAILABLE("/*c^omment*/ int foo() return 2 ^ + 2; }");
+  EXPECT_UNAVAILABLE("/*c^omment*/ int foo() return 2 ^ + 2; }");
   EXPECT_THAT(apply("int x = 2 ^+ 2;"),
               AllOf(StartsWith("message:"), HasSubstr("BinaryOperator"),
                     HasSubstr("'+'"), HasSubstr("|-IntegerLiteral"),
@@ -164,7 +161,7 @@ TEST_F(DumpSymbolTest, Test) {
 TWEAK_TEST(ShowSelectionTree);
 TEST_F(ShowSelectionTreeTest, Test) {
   EXPECT_AVAILABLE("^int f^oo() { re^turn 2 ^+ 2; }");
-  EXPECT_AVAILABLE("/*c^omment*/ int foo() return 2 ^ + 2; }");
+  EXPECT_AVAILABLE("/*c^omment*/ int foo() return 2 ^ + 2; }");
 
   const char *Output = R"(message:
  TranslationUnitDecl 
@@ -186,7 +183,7 @@ TEST_F(DumpRecordLayoutTest, Test) {
   EXPECT_THAT("template <typename T> struct ^X { T t; };", Not(isAvailable()));
   EXPECT_THAT("enum ^X {};", Not(isAvailable()));
 
-  EXPECT_THAT(apply("struct ^X { int x; int y; }"),
+  EXPECT_THAT(apply("struct ^X { int x; int y; };"),
               AllOf(StartsWith("message:"), HasSubstr("0 |   int x")));
 }
 
@@ -234,6 +231,7 @@ TEST_F(ExtractVariableTest, Test) {
   EXPECT_AVAILABLE(AvailableCases);
 
   const char *NoCrashCases = R"cpp(
+    // error-ok: broken code, but shouldn't crash
     template<typename T, typename ...Args>
     struct Test<T, Args...> {
     Test(const T &v) :val[[(^]]) {}
@@ -269,7 +267,7 @@ TEST_F(ExtractVariableTest, Test) {
       for(int a = 1, b = 2, c = 3; a > [[b + c]]; [[a++]])
         a = [[a + 1]];
       // lambda
-      auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;}
+      auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;};
       // assignment
       xyz([[a = 5]]);
       xyz([[a *= 5]]);
@@ -477,6 +475,7 @@ TEST_F(AnnotateHighlightingsTest, Test) {
 TWEAK_TEST(ExpandMacro);
 TEST_F(ExpandMacroTest, Test) {
   Header = R"cpp(
+    // error-ok: not real c++, just token manipulation
     #define FOO 1 2 3
     #define FUNC(X) X+X+X
     #define EMPTY
@@ -513,7 +512,7 @@ TEST_F(ExpandAutoTypeTest, Test) {
     namespace ns {
       struct Class {
         struct Nested {};
-      }
+      };
       void Func();
     }
     inline namespace inl_ns {
@@ -536,7 +535,7 @@ TEST_F(ExpandAutoTypeTest, Test) {
   EXPECT_EQ(apply("namespace ns { void f() { ^auto C = Class(); } }"),
             "namespace ns { void f() { Class C = Class(); } }");
   // undefined functions should not be replaced
-  EXPECT_THAT(apply("au^to x = doesnt_exist();"),
+  EXPECT_THAT(apply("au^to x = doesnt_exist(); // error-ok"),
               StartsWith("fail: Could not deduce type for 'auto' type"));
   // function pointers should not be replaced
   EXPECT_THAT(apply("au^to x = &ns::Func;"),
@@ -551,8 +550,8 @@ TEST_F(ExpandAutoTypeTest, Test) {
   EXPECT_EQ(apply("namespace x { void y() { struct S{}; ^auto z = S(); } }"),
             "namespace x { void y() { struct S{}; S z = S(); } }");
   // replace array types
-  EXPECT_EQ(apply(R"cpp(au^to x = "test")cpp"),
-            R"cpp(const char * x = "test")cpp");
+  EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"),
+            R"cpp(const char * x = "test";)cpp");
 
   EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
 
@@ -977,7 +976,7 @@ TEST_F(DefineInlineTest, TriggersOnFunctionDecl) {
   }]]
 
   // Definition with no body.
-  class Bar { Bar() = def^ault; }
+  class Bar { Bar() = def^ault; };
   )cpp");
 }
 
@@ -1213,9 +1212,7 @@ TEST_F(DefineInlineTest, TransformDecls) {
       public:
         void foo();
         int x;
-        static int y;
       };
-      Foo::y = 0;
 
       enum En { Zero, One };
       En x = Zero;
@@ -1229,9 +1226,7 @@ TEST_F(DefineInlineTest, TransformDecls) {
       public:
         void foo();
         int x;
-        static int y;
       };
-      Foo::y = 0;
 
       enum En { Zero, One };
       En x = Zero;
@@ -1682,11 +1677,11 @@ TEST_F(DefineInlineTest, HandleMacros) {
           namespace a { class Foo{}; }
           void foo();
           using namespace a;
-          void f^oo(){BODY})cpp",
+          void f^oo(){BODY();})cpp",
        R"cpp(
           #define BODY Foo
           namespace a { class Foo{}; }
-          void foo(){BODY}
+          void foo(){BODY();}
           using namespace a;
           )cpp"},
 
@@ -1899,13 +1894,13 @@ TEST_F(DefineInlineTest, AddInline) {
                                testPath("a.h"), "constexpr void foo(){}")));
 
   // Class members don't need "inline".
-  ExtraFiles["a.h"] = "struct Foo { void foo(); }";
+  ExtraFiles["a.h"] = "struct Foo { void foo(); };";
   apply(R"cpp(#include "a.h"
               void Foo::fo^o() {})cpp",
         &EditedFiles);
   EXPECT_THAT(EditedFiles,
               testing::ElementsAre(FileWithContents(
-                  testPath("a.h"), "struct Foo { void foo(){} }")));
+                  testPath("a.h"), "struct Foo { void foo(){} };")));
 
   // Function template doesn't need to be "inline"d.
   ExtraFiles["a.h"] = "template <typename T> void foo();";
@@ -1990,7 +1985,7 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
   // out-of-line in such cases.
   EXPECT_UNAVAILABLE(R"cpp(
     template <typename> struct Foo { void fo^o(){} };
-    })cpp");
+    )cpp");
 }
 
 TEST_F(DefineOutlineTest, FailsWithoutSource) {
@@ -2134,14 +2129,14 @@ TEST_F(DefineOutlineTest, QualifyReturnValue) {
     llvm::StringRef ExpectedSource;
   } Cases[] = {
       {R"cpp(
-        namespace a { class Foo; }
+        namespace a { class Foo{}; }
         using namespace a;
-        Foo fo^o() { return; })cpp",
+        Foo fo^o() { return {}; })cpp",
        R"cpp(
-        namespace a { class Foo; }
+        namespace a { class Foo{}; }
         using namespace a;
         Foo foo() ;)cpp",
-       "a::Foo foo() { return; }"},
+       "a::Foo foo() { return {}; }"},
       {R"cpp(
         namespace a {
           class Foo {
@@ -2158,12 +2153,12 @@ TEST_F(DefineOutlineTest, QualifyReturnValue) {
         })cpp",
        "a::Foo::Bar a::Foo::foo() { return {}; }\n"},
       {R"cpp(
-        class Foo;
-        Foo fo^o() { return; })cpp",
+        class Foo {};
+        Foo fo^o() { return {}; })cpp",
        R"cpp(
-        class Foo;
+        class Foo {};
         Foo foo() ;)cpp",
-       "Foo foo() { return; }"},
+       "Foo foo() { return {}; }"},
   };
   llvm::StringMap<std::string> EditedFiles;
   for (auto &Case : Cases) {
index f365a14..6bbcfff 100644 (file)
@@ -71,8 +71,6 @@ int main() {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   for (Position Pt : Source.points()) {
     const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
     EXPECT_EQ(&findDecl(AST, "Child2"), static_cast<const NamedDecl *>(RD));
@@ -95,8 +93,6 @@ int main() {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   for (Position Pt : Source.points()) {
     const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
     EXPECT_EQ(&findDecl(AST, "Child2"), static_cast<const NamedDecl *>(RD));
@@ -118,8 +114,6 @@ int main() {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   for (Position Pt : Source.points()) {
     const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
     // A field does not unambiguously specify a record type
@@ -147,8 +141,6 @@ struct Child2 : Child1 {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   const CXXRecordDecl *Parent =
       dyn_cast<CXXRecordDecl>(&findDecl(AST, "Parent"));
   const CXXRecordDecl *Child1 =
@@ -183,8 +175,6 @@ struct Child : Parent1, Parent3 {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   const CXXRecordDecl *Parent1 =
       dyn_cast<CXXRecordDecl>(&findDecl(AST, "Parent1"));
   const CXXRecordDecl *Parent2 =
@@ -210,8 +200,6 @@ struct Child : Parent {};
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   const CXXRecordDecl *Parent =
       dyn_cast<CXXRecordDecl>(&findDecl(AST, "Parent"));
   const CXXRecordDecl *Child =
@@ -260,8 +248,6 @@ struct Child2 : Parent<int> {};
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   const CXXRecordDecl *Parent =
       dyn_cast<ClassTemplateDecl>(&findDecl(AST, "Parent"))->getTemplatedDecl();
   const CXXRecordDecl *ParentSpec =
@@ -289,8 +275,6 @@ struct Child<int> : Parent {};
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   const CXXRecordDecl *Parent =
       dyn_cast<CXXRecordDecl>(&findDecl(AST, "Parent"));
   const CXXRecordDecl *Child =
@@ -320,8 +304,6 @@ struct Child3 : T {};
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   const CXXRecordDecl *Parent =
       dyn_cast<ClassTemplateDecl>(&findDecl(AST, "Parent"))->getTemplatedDecl();
   const CXXRecordDecl *Child1 =
@@ -397,7 +379,7 @@ TEST(TypeHierarchy, RecursiveHierarchyUnbounded) {
   template <int N>
   struct $SDef[[S]] : S<N + 1> {};
 
-  S^<0> s;
+  S^<0> s; // error-ok
   )cpp");
 
   TestTU TU = TestTU::withCode(Source.code());
@@ -444,8 +426,6 @@ TEST(TypeHierarchy, RecursiveHierarchyBounded) {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion
   // for either a concrete starting point or a dependent starting point.
   llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
@@ -482,7 +462,6 @@ TEST(TypeHierarchy, DeriveFromImplicitSpec) {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
   auto Index = TU.index();
-  ASSERT_TRUE(AST.getDiagnostics().empty());
 
   llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
       AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
@@ -507,7 +486,6 @@ TEST(TypeHierarchy, DeriveFromPartialSpec) {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
   auto Index = TU.index();
-  ASSERT_TRUE(AST.getDiagnostics().empty());
 
   llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
       AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
@@ -531,7 +509,6 @@ TEST(TypeHierarchy, DeriveFromTemplate) {
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
   auto Index = TU.index();
-  ASSERT_TRUE(AST.getDiagnostics().empty());
 
   // FIXME: We'd like this to return the implicit specialization Child<int>,
   //        but currently libIndex does not expose relationships between
index 79d7090..b5c9803 100644 (file)
@@ -515,10 +515,6 @@ TEST(LocateSymbol, All) {
     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) {
@@ -868,7 +864,7 @@ TEST(FindReferences, WithinAST) {
 
       R"cpp(// Forward declaration
         class [[Foo]];
-        class [[Foo]] {}
+        class [[Foo]] {};
         int main() {
           [[Fo^o]] foo;
         }
@@ -878,7 +874,7 @@ TEST(FindReferences, WithinAST) {
         int [[foo]](int) {}
         int main() {
           auto *X = &[[^foo]];
-          [[foo]](42)
+          [[foo]](42);
         }
       )cpp",
 
@@ -1182,8 +1178,6 @@ TEST(GetNonLocalDeclRefs, All) {
   for (const Case &C : Cases) {
     Annotations File(C.AnnotatedCode);
     auto AST = TestTU::withCode(File.code()).build();
-    ASSERT_TRUE(AST.getDiagnostics().empty())
-        << AST.getDiagnostics().begin()->Message;
     SourceLocation SL = llvm::cantFail(
         sourceLocationInMainFile(AST.getSourceManager(), File.point()));