[clangd] Include-fixer: handle more "incomplete type" diags, clean up tests

[sammccall]

Summary

I started adding tests for all diags but found there were just too many cases.
Still I think cleaning up the tests and adding a few more is worthwhile.

[kadircet]

clang-tools-extra/clangd/IncludeFixer.cpp

75

have you considered updating these diagnostics to have "incomplete" as diagnostic category and do the filtering here based on that?

176

nit: if (!ET->...)

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

834–835

thanks this one looks a lot better!

I just wonder if we should send it as a separate patch though.

clang-tools-extra/clangd/IncludeFixer.cpp

std::vector<Fix> only(llvm::Optional<Fix> F) {
   return {};
 }
 
 } // namespace
 
 std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
                                    const clang::Diagnostic &Info) const {
   switch (Info.getID()) {
-  case diag::err_incomplete_nested_name_spec:
+  /*
+   There are many "incomplete type" diagnostics!
+   They are almost all Sema diagnostics with "incomplete" in the name.
+
+   sed -n '/CLASS_NOTE/! s/DIAG(\\([^,]*\\).*)/  case diag::\\1:/p' \
+     tools/clang/include/clang/Basic/DiagnosticSemaKinds.inc | grep incomplete
+  */
+  // clang-format off
+  //case diag::err_alignof_member_of_incomplete_type:
+  case diag::err_array_incomplete_or_sizeless_type:
+  case diag::err_array_size_incomplete_type:
+  case diag::err_asm_incomplete_type:
+  case diag::err_assoc_type_incomplete:
+  case diag::err_bad_cast_incomplete:
+  case diag::err_call_function_incomplete_return:
+  case diag::err_call_incomplete_argument:
+  case diag::err_call_incomplete_return:
+  case diag::err_capture_of_incomplete_or_sizeless_type:
+  case diag::err_catch_incomplete:
+  case diag::err_catch_incomplete_ptr:
+  case diag::err_catch_incomplete_ref:
+  case diag::err_cconv_incomplete_param_type:
+  case diag::err_coroutine_promise_type_incomplete:
+  case diag::err_covariant_return_incomplete:
+  //case diag::err_deduced_class_template_incomplete:
+  case diag::err_delete_incomplete_class_type:
+  case diag::err_dereference_incomplete_type:
+  case diag::err_exception_spec_incomplete_type:
+  case diag::err_field_incomplete_or_sizeless:
+  case diag::err_for_range_incomplete_type:
+  case diag::err_func_def_incomplete_result:
+  case diag::err_ice_incomplete_type:
+  case diag::err_illegal_message_expr_incomplete_type:
   case diag::err_incomplete_base_class:
+  case diag::err_incomplete_enum:
+  case diag::err_incomplete_in_exception_spec:
   case diag::err_incomplete_member_access:
+  case diag::err_incomplete_nested_name_spec:
+  case diag::err_incomplete_object_call:
+  case diag::err_incomplete_receiver_type:
+  case diag::err_incomplete_synthesized_property:
   case diag::err_incomplete_type:
-  case diag::err_typecheck_decl_incomplete_type:
-  case diag::err_typecheck_incomplete_tag:
+  case diag::err_incomplete_type_objc_at_encode:
+  case diag::err_incomplete_type_used_in_type_trait_expr:
+  case diag::err_incomplete_typeid:
+  case diag::err_init_incomplete_type:
   case diag::err_invalid_incomplete_type_use:
+  case diag::err_lambda_incomplete_result:
+  //case diag::err_matrix_incomplete_index:
+  //case diag::err_matrix_separate_incomplete_index:
+  case diag::err_memptr_incomplete:
+  case diag::err_new_incomplete_or_sizeless_type:
+  case diag::err_objc_incomplete_boxed_expression_type:
+  case diag::err_objc_index_incomplete_class_type:
+  case diag::err_offsetof_incomplete_type:
+  case diag::err_omp_firstprivate_incomplete_type:
+  case diag::err_omp_incomplete_type:
+  case diag::err_omp_lastprivate_incomplete_type:
+  case diag::err_omp_linear_incomplete_type:
+  case diag::err_omp_private_incomplete_type:
+  case diag::err_omp_reduction_incomplete_type:
+  case diag::err_omp_section_incomplete_type:
+  case diag::err_omp_threadprivate_incomplete_type:
+  case diag::err_second_parameter_to_va_arg_incomplete:
   case diag::err_sizeof_alignof_incomplete_or_sizeless_type:
-  case diag::err_for_range_incomplete_type:
-  case diag::err_func_def_incomplete_result:
-  case diag::err_field_incomplete_or_sizeless:
+  case diag::err_subscript_incomplete_or_sizeless_type:
+  case diag::err_switch_incomplete_class_type:
+  case diag::err_temp_copy_incomplete:
+  //case diag::err_template_arg_deduced_incomplete_pack:
+  case diag::err_template_nontype_parm_incomplete:
+  //case diag::err_tentative_def_incomplete_type:
+  case diag::err_throw_incomplete:
+  case diag::err_throw_incomplete_ptr:
+  case diag::err_typecheck_arithmetic_incomplete_or_sizeless_type:
+  case diag::err_typecheck_cast_to_incomplete:
+  case diag::err_typecheck_decl_incomplete_type:
+  //case diag::err_typecheck_incomplete_array_needs_initializer:
+  case diag::err_typecheck_incomplete_tag:
+  case diag::err_typecheck_incomplete_type_not_modifiable_lvalue:
+  case diag::err_typecheck_nonviable_condition_incomplete:
+  case diag::err_underlying_type_of_incomplete_enum:
+  case diag::ext_incomplete_in_exception_spec:
+  //case diag::ext_typecheck_compare_complete_incomplete_pointers:
+  case diag::ext_typecheck_decl_incomplete_type:
+  case diag::warn_delete_incomplete:
+  case diag::warn_incomplete_encoded_type:
+  //case diag::warn_printf_incomplete_specifier:
+  case diag::warn_return_value_udt_incomplete:
+  //case diag::warn_scanf_scanlist_incomplete:
+  //case diag::warn_tentative_incomplete_array:
+    //  clang-format on
     // Incomplete type diagnostics should have a QualType argument for the
     // incomplete type.
     for (unsigned Idx = 0; Idx < Info.getNumArgs(); ++Idx) {
       if (Info.getArgKind(Idx) == DiagnosticsEngine::ak_qualtype) {
         auto QT = QualType::getFromOpaquePtr((void *)Info.getRawArg(Idx));
-        if (const Type *T = QT.getTypePtrOrNull())
+        if (const Type *T = QT.getTypePtrOrNull()) {
           if (T->isIncompleteType())
             return fixIncompleteType(*T);
+          // `enum x : int;' is not formally an incomplete type.
+          // We may need a full definition anyway.
+          if (auto * ET = llvm::dyn_cast<EnumType>(T))
+            if (!ET->getDecl()->getDefinition())
+              return fixIncompleteType(*T);
+        }
       }
     }
     break;
+
   case diag::err_unknown_typename:
   case diag::err_unknown_typename_suggest:
   case diag::err_typename_nested_not_found:
   case diag::err_no_template:
   case diag::err_no_template_suggest:
   case diag::err_undeclared_use:
   case diag::err_undeclared_use_suggest:
   case diag::err_undeclared_var_use:
if (LastUnresolvedName) {
       //                      UnresolvedName
       // We only attempt to recover a diagnostic if it has the same location as
       // the last seen unresolved name.
       if (DiagLevel >= DiagnosticsEngine::Error &&
           LastUnresolvedName->Loc == Info.getLocation())
         return fixUnresolvedName();
     }
     break;
+
   // Cases where clang explicitly knows which header to include.
   // (There's no fix provided for boring formatting reasons).
   case diag::err_implied_std_initializer_list_not_found:
     return only(insertHeader("<initializer_list>"));
   case diag::err_need_header_before_typeid:
     return only(insertHeader("<typeid>"));
   case diag::err_need_header_before_ms_uuidof:
     return only(insertHeader("<guiddef.h>"));

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

for (const auto &S : Syms) {
     Sym.CanonicalDeclaration.FileURI = S.DeclaringFile.c_str();
     Sym.Definition.FileURI = S.DeclaringFile.c_str();
     Sym.IncludeHeaders.emplace_back(S.IncludeHeader, 1);
     Slab.insert(Sym);
   }
   return MemIndex::build(std::move(Slab).build(), RefSlab(), RelationSlab());
 }
 
 TEST(IncludeFixerTest, IncompleteType) {
   auto TU = TestTU::withHeaderCode("namespace ns { class X; } ns::X *x;");
   TU.ExtraArgs.push_back("-std=c++20");
   auto Index = buildIndexWithSymbol(
       {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""}});
   TU.ExternalIndex = Index.get();
 
   std::vector<std::pair<llvm::StringRef, llvm::StringRef>> Tests{
       {"incomplete_nested_name_spec", "[[ns::X::]]Nested n;"},
       {"incomplete_base_class", "class Y : [[ns::X]] {};"},
       {"incomplete_member_access", "auto i = x[[->]]f();"},
       {"incomplete_type", "auto& [[[]]m] = *x;"},
+      {"init_incomplete_type",
+       "struct C { static int f(ns::X&); }; int i = C::f([[{]]});"},
+      {"bad_cast_incomplete", "auto a = [[static_cast]]<ns::X>(0);"},
+      {"template_nontype_parm_incomplete", "template <ns::X [[foo]]> int a;"},
       {"typecheck_decl_incomplete_type", "ns::X [[var]];"},
       {"typecheck_incomplete_tag", "auto i = [[(*x)]]->f();"},
+      {"typecheck_nonviable_condition_incomplete",
+       "struct A { operator ns::X(); } a; const ns::X &[[b]] = a;"},
       {"invalid_incomplete_type_use", "auto var = [[ns::X()]];"},
       {"sizeof_alignof_incomplete_or_sizeless_type",
        "auto s = [[sizeof]](ns::X);"},
       {"for_range_incomplete_type", "void foo() { for (auto i : [[*]]x ) {} }"},
       {"func_def_incomplete_result", "ns::X [[func]] () {}"},
       {"field_incomplete_or_sizeless", "class M { ns::X [[member]]; };"},
+      {"array_incomplete_or_sizeless_type", "auto s = [[(ns::X[]){}]];"},
+      {"call_incomplete_return", "ns::X f(); auto fp = &f; auto z = [[fp()]];"},
+      {"call_function_incomplete_return", "ns::X foo(); auto a = [[foo()]];"},
+      {"call_incomplete_argument", "int m(ns::X); int i = m([[*x]]);"},
+      {"switch_incomplete_class_type", "void a() { [[switch]](*x) {} }"},
+      {"delete_incomplete_class_type", "void f() { [[delete]] *x; }"},
+      {"-Wdelete-incomplete", "void f() { [[delete]] x; }"},
+      {"dereference_incomplete_type",
+       R"cpp(void f() { asm("" : "=r"([[*]]x)::); })cpp"},
   };
   for (auto Case : Tests) {
     Annotations Main(Case.second);
     TU.Code = Main.code().str() + "\n // error-ok";
     EXPECT_THAT(
         *TU.build().getDiagnostics(),
         ElementsAre(AllOf(DiagName(Case.first), HasRange(Main.range()),
                           WithFix(Fix(Range{}, "#include \"x.h\"\n",
                                       "Include \"x.h\" for symbol ns::X")))))
         << Case.second;
   }
 }
 
+TEST(IncludeFixerTest, IncompleteEnum) {
+  Symbol Sym = enm("X");
+  Sym.Flags |= Symbol::IndexedForCodeCompletion;
+  Sym.CanonicalDeclaration.FileURI = Sym.Definition.FileURI = "unittest:///x.h";
+  Sym.IncludeHeaders.emplace_back("\"x.h\"", 1);
+  SymbolSlab::Builder Slab;
+  Slab.insert(Sym);
+  auto Index =
+      MemIndex::build(std::move(Slab).build(), RefSlab(), RelationSlab());
+
+  TestTU TU;
+  TU.ExternalIndex = Index.get();
+  TU.ExtraArgs.push_back("-std=c++20");
+
+  std::vector<std::pair<llvm::StringRef, llvm::StringRef>> Tests{
+      {"incomplete_enum", "enum class X : int; using enum [[X]];"},
+      {"underlying_type_of_incomplete_enum",
+       "[[__underlying_type]](enum X) i;"},
+  };
+  for (auto Case : Tests) {
+    Annotations Main(Case.second);
+    TU.Code = Main.code().str() + "\n // error-ok";
+    EXPECT_THAT(*TU.build().getDiagnostics(),
+                Contains(AllOf(DiagName(Case.first), HasRange(Main.range()),
+                               WithFix(Fix(Range{}, "#include \"x.h\"\n",
+                                           "Include \"x.h\" for symbol X")))))
+        << Case.second;
+  }
+}
+
 TEST(IncludeFixerTest, NoSuggestIncludeWhenNoDefinitionInHeader) {
   Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace ns {
   class X;
 }
 class Y : $base[[public ns::X]] {};
 int main() {
   ns::X *x;

clang-tools-extra/clangd/unittests/TestIndex.h

 //===-- IndexHelpers.h ------------------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTINDEX_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTINDEX_H
 
 #include "index/Index.h"
-#include "index/Merge.h"
 
 namespace clang {
 namespace clangd {
 
 // Creates Symbol instance and sets SymbolID to given QualifiedName.
 Symbol symbol(llvm::StringRef QName);
 
 // Helpers to produce fake index symbols with proper SymbolID.
 // USRFormat is a regex replacement string for the unqualified part of the USR.
 Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
            llvm::StringRef USRFormat);
 // Creats a function symbol assuming no function arg.
 Symbol func(llvm::StringRef Name);
 // Creates a class symbol.
 Symbol cls(llvm::StringRef Name);
+// Creates an enum symbol.
+Symbol enm(llvm::StringRef Name);
 // Creates a variable symbol.
 Symbol var(llvm::StringRef Name);
 // Creates a namespace symbol.
 Symbol ns(llvm::StringRef Name);
 
 // Create a slab of symbols with the given qualified names as IDs and names.
 SymbolSlab generateSymbols(std::vector<std::string> QualifiedNames);
 

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

 Symbol func(llvm::StringRef Name) { // Assumes the function has no args.
   return sym(Name, index::SymbolKind::Function, "@F@\\0#"); // no args
 }
 
 Symbol cls(llvm::StringRef Name) {
   return sym(Name, index::SymbolKind::Class, "@S@\\0");
 }
 
+Symbol enm(llvm::StringRef Name) {
+  return sym(Name, index::SymbolKind::Enum, "@E@\\0");
+}
+
 Symbol var(llvm::StringRef Name) {
   return sym(Name, index::SymbolKind::Variable, "@\\0");
 }
 
 Symbol ns(llvm::StringRef Name) {
   return sym(Name, index::SymbolKind::Namespace, "@N@\\0");
 }