diff --git a/clang-tools-extra/clang-apply-replacements/CMakeLists.txt b/clang-tools-extra/clang-apply-replacements/CMakeLists.txt index 02da0851a72be..5bfdcb487e17a 100644 --- a/clang-tools-extra/clang-apply-replacements/CMakeLists.txt +++ b/clang-tools-extra/clang-apply-replacements/CMakeLists.txt @@ -10,7 +10,7 @@ add_clang_library(clangApplyReplacements clangBasic clangRewrite clangToolingCore - clangToolingRefactor + clangToolingRefactoring ) include_directories( diff --git a/clang-tools-extra/clang-apply-replacements/tool/CMakeLists.txt b/clang-tools-extra/clang-apply-replacements/tool/CMakeLists.txt index 945b486103b2e..26aa760c731d8 100644 --- a/clang-tools-extra/clang-apply-replacements/tool/CMakeLists.txt +++ b/clang-tools-extra/clang-apply-replacements/tool/CMakeLists.txt @@ -12,7 +12,7 @@ target_link_libraries(clang-apply-replacements clangFormat clangRewrite clangToolingCore - clangToolingRefactor + clangToolingRefactoring ) install(TARGETS clang-apply-replacements diff --git a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp index b529f72ddae32..14f5e1532474b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp @@ -16,6 +16,18 @@ namespace clang { namespace tidy { namespace bugprone { +UnhandledSelfAssignmentCheck::UnhandledSelfAssignmentCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + WarnOnlyIfThisHasSuspiciousField( + Options.get("WarnOnlyIfThisHasSuspiciousField", true)) {} + +void UnhandledSelfAssignmentCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "WarnOnlyIfThisHasSuspiciousField", + WarnOnlyIfThisHasSuspiciousField); +} + void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus) return; @@ -61,29 +73,32 @@ void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) { cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl( hasName("operator="), ofClass(equalsBoundNode("class")))))))); - // Matcher for standard smart pointers. - const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType( - recordType(hasDeclaration(classTemplateSpecializationDecl( - hasAnyName("::std::shared_ptr", "::std::unique_ptr", - "::std::weak_ptr", "::std::auto_ptr"), - templateArgumentCountIs(1)))))); - - // We will warn only if the class has a pointer or a C array field which - // probably causes a problem during self-assignment (e.g. first resetting the - // pointer member, then trying to access the object pointed by the pointer, or - // memcpy overlapping arrays). - const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl( - has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType), - hasType(arrayType()))))))); - - Finder->addMatcher( - cxxMethodDecl(ofClass(cxxRecordDecl().bind("class")), - isCopyAssignmentOperator(), IsUserDefined, - HasReferenceParam, HasNoSelfCheck, - unless(HasNonTemplateSelfCopy), unless(HasTemplateSelfCopy), - HasNoNestedSelfAssign, ThisHasSuspiciousField) - .bind("copyAssignmentOperator"), - this); + DeclarationMatcher AdditionalMatcher = cxxMethodDecl(); + if (WarnOnlyIfThisHasSuspiciousField) { + // Matcher for standard smart pointers. + const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(classTemplateSpecializationDecl( + hasAnyName("::std::shared_ptr", "::std::unique_ptr", + "::std::weak_ptr", "::std::auto_ptr"), + templateArgumentCountIs(1)))))); + + // We will warn only if the class has a pointer or a C array field which + // probably causes a problem during self-assignment (e.g. first resetting + // the pointer member, then trying to access the object pointed by the + // pointer, or memcpy overlapping arrays). + AdditionalMatcher = cxxMethodDecl(ofClass(cxxRecordDecl( + has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType), + hasType(arrayType()))))))); + } + + Finder->addMatcher(cxxMethodDecl(ofClass(cxxRecordDecl().bind("class")), + isCopyAssignmentOperator(), IsUserDefined, + HasReferenceParam, HasNoSelfCheck, + unless(HasNonTemplateSelfCopy), + unless(HasTemplateSelfCopy), + HasNoNestedSelfAssign, AdditionalMatcher) + .bind("copyAssignmentOperator"), + this); } void UnhandledSelfAssignmentCheck::check( diff --git a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h index 1747246143552..d7a2b7c619ff8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h @@ -23,10 +23,14 @@ namespace bugprone { /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-unhandled-self-assignment.html class UnhandledSelfAssignmentCheck : public ClangTidyCheck { public: - UnhandledSelfAssignmentCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + UnhandledSelfAssignmentCheck(StringRef Name, ClangTidyContext *Context); + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool WarnOnlyIfThisHasSuspiciousField; }; } // namespace bugprone diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp index cd8da0c663643..341968b6fa6b1 100644 --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "../bugprone/UnhandledSelfAssignmentCheck.h" #include "../google/UnnamedNamespaceInHeaderCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" #include "../misc/NonCopyableObjects.h" @@ -49,6 +50,8 @@ class CERTModule : public ClangTidyModule { // OOP CheckFactories.registerCheck( "cert-oop11-cpp"); + CheckFactories.registerCheck( + "cert-oop54-cpp"); // ERR CheckFactories.registerCheck( "cert-err09-cpp"); @@ -85,6 +88,7 @@ class CERTModule : public ClangTidyModule { ClangTidyOptions Options; ClangTidyOptions::OptionMap &Opts = Options.CheckOptions; Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU"; + Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "0"; return Options; } }; diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt index b50ddf014c634..474d9356adfbf 100644 --- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyCERTModule clangBasic clangLex clangTidy + clangTidyBugproneModule clangTidyGoogleModule clangTidyMiscModule clangTidyPerformanceModule diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp index c6cfe5ec0002f..fc334fb5b3baf 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -250,7 +250,8 @@ void fixInitializerList(const ASTContext &Context, DiagnosticBuilder &Diag, ProTypeMemberInitCheck::ProTypeMemberInitCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - IgnoreArrays(Options.get("IgnoreArrays", false)) {} + IgnoreArrays(Options.get("IgnoreArrays", false)), + UseAssignment(Options.getLocalOrGlobal("UseAssignment", false)) {} void ProTypeMemberInitCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus) @@ -314,6 +315,7 @@ void ProTypeMemberInitCheck::check(const MatchFinder::MatchResult &Result) { void ProTypeMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreArrays", IgnoreArrays); + Options.store(Opts, "UseAssignment", UseAssignment); } // FIXME: Copied from clang/lib/Sema/SemaDeclCXX.cpp. @@ -338,6 +340,56 @@ static bool isEmpty(ASTContext &Context, const QualType &Type) { return isIncompleteOrZeroLengthArrayType(Context, Type); } +static const char *getInitializer(QualType QT, bool UseAssignment) { + const char *DefaultInitializer = "{}"; + if (!UseAssignment) + return DefaultInitializer; + + if (QT->isPointerType()) + return " = nullptr"; + + const BuiltinType *BT = + dyn_cast(QT.getCanonicalType().getTypePtr()); + if (!BT) + return DefaultInitializer; + + switch (BT->getKind()) { + case BuiltinType::Bool: + return " = false"; + case BuiltinType::Float: + return " = 0.0F"; + case BuiltinType::Double: + return " = 0.0"; + case BuiltinType::LongDouble: + return " = 0.0L"; + case BuiltinType::SChar: + case BuiltinType::Char_S: + case BuiltinType::WChar_S: + case BuiltinType::Char16: + case BuiltinType::Char32: + case BuiltinType::Short: + case BuiltinType::Int: + return " = 0"; + case BuiltinType::UChar: + case BuiltinType::Char_U: + case BuiltinType::WChar_U: + case BuiltinType::UShort: + case BuiltinType::UInt: + return " = 0U"; + case BuiltinType::Long: + return " = 0L"; + case BuiltinType::ULong: + return " = 0UL"; + case BuiltinType::LongLong: + return " = 0LL"; + case BuiltinType::ULongLong: + return " = 0ULL"; + + default: + return DefaultInitializer; + } +} + void ProTypeMemberInitCheck::checkMissingMemberInitializer( ASTContext &Context, const CXXRecordDecl &ClassDecl, const CXXConstructorDecl *Ctor) { @@ -420,7 +472,7 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer( for (const FieldDecl *Field : FieldsToFix) { Diag << FixItHint::CreateInsertion( getLocationForEndOfToken(Context, Field->getSourceRange().getEnd()), - "{}"); + getInitializer(Field->getType(), UseAssignment)); } } else if (Ctor) { // Otherwise, rewrite the constructor's initializer list. diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h index 807acfe3bb220..2ec8fb16342b5 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h @@ -64,6 +64,11 @@ class ProTypeMemberInitCheck : public ClangTidyCheck { // Whether arrays need to be initialized or not. Default is false. bool IgnoreArrays; + + // Whether fix-its for initialization of fundamental type use assignment + // instead of brace initalization. Only effective in C++11 mode. Default is + // false. + bool UseAssignment; }; } // namespace cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp new file mode 100644 index 0000000000000..1b147ace89868 --- /dev/null +++ b/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp @@ -0,0 +1,130 @@ +//===--- AvoidNSObjectNewCheck.cpp - clang-tidy ---------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#include "AvoidNSObjectNewCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "llvm/Support/FormatVariadic.h" +#include +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace google { +namespace objc { + +static bool isMessageExpressionInsideMacro(const ObjCMessageExpr *Expr) { + SourceLocation ReceiverLocation = Expr->getReceiverRange().getBegin(); + if (ReceiverLocation.isMacroID()) + return true; + + SourceLocation SelectorLocation = Expr->getSelectorStartLoc(); + if (SelectorLocation.isMacroID()) + return true; + + return false; +} + +// Walk up the class hierarchy looking for an -init method, returning true +// if one is found and has not been marked unavailable. +static bool isInitMethodAvailable(const ObjCInterfaceDecl *ClassDecl) { + while (ClassDecl != nullptr) { + for (const auto *MethodDecl : ClassDecl->instance_methods()) { + if (MethodDecl->getSelector().getAsString() == "init") + return !MethodDecl->isUnavailable(); + } + ClassDecl = ClassDecl->getSuperClass(); + } + + // No -init method found in the class hierarchy. This should occur only rarely + // in Objective-C code, and only really applies to classes not derived from + // NSObject. + return false; +} + +// Returns the string for the Objective-C message receiver. Keeps any generics +// included in the receiver class type, which are stripped if the class type is +// used. While the generics arguments will not make any difference to the +// returned code at this time, the style guide allows them and they should be +// left in any fix-it hint. +static StringRef getReceiverString(SourceRange ReceiverRange, + const SourceManager &SM, + const LangOptions &LangOpts) { + CharSourceRange CharRange = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(ReceiverRange), SM, LangOpts); + return Lexer::getSourceText(CharRange, SM, LangOpts); +} + +static FixItHint getCallFixItHint(const ObjCMessageExpr *Expr, + const SourceManager &SM, + const LangOptions &LangOpts) { + // Check whether the messaged class has a known factory method to use instead + // of -init. + StringRef Receiver = + getReceiverString(Expr->getReceiverRange(), SM, LangOpts); + // Some classes should use standard factory methods instead of alloc/init. + std::map ClassToFactoryMethodMap = {{"NSDate", "date"}, + {"NSNull", "null"}}; + auto FoundClassFactory = ClassToFactoryMethodMap.find(Receiver); + if (FoundClassFactory != ClassToFactoryMethodMap.end()) { + StringRef ClassName = FoundClassFactory->first; + StringRef FactorySelector = FoundClassFactory->second; + std::string NewCall = + llvm::formatv("[{0} {1}]", ClassName, FactorySelector); + return FixItHint::CreateReplacement(Expr->getSourceRange(), NewCall); + } + + if (isInitMethodAvailable(Expr->getReceiverInterface())) { + std::string NewCall = llvm::formatv("[[{0} alloc] init]", Receiver); + return FixItHint::CreateReplacement(Expr->getSourceRange(), NewCall); + } + + return {}; // No known replacement available. +} + +void AvoidNSObjectNewCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().ObjC) + return; + + // Add two matchers, to catch calls to +new and implementations of +new. + Finder->addMatcher( + objcMessageExpr(isClassMessage(), hasSelector("new")).bind("new_call"), + this); + Finder->addMatcher( + objcMethodDecl(isClassMethod(), isDefinition(), hasName("new")) + .bind("new_override"), + this); +} + +void AvoidNSObjectNewCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *CallExpr = + Result.Nodes.getNodeAs("new_call")) { + // Don't warn if the call expression originates from a macro expansion. + if (isMessageExpressionInsideMacro(CallExpr)) + return; + + diag(CallExpr->getExprLoc(), "do not create objects with +new") + << getCallFixItHint(CallExpr, *Result.SourceManager, + Result.Context->getLangOpts()); + } + + if (const auto *DeclExpr = + Result.Nodes.getNodeAs("new_override")) { + diag(DeclExpr->getBeginLoc(), "classes should not override +new"); + } +} + +} // namespace objc +} // namespace google +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.h b/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.h new file mode 100644 index 0000000000000..97988c903e8e5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.h @@ -0,0 +1,38 @@ +//===--- AvoidNSObjectNewCheck.h - clang-tidy -------------------*- 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_CLANG_TIDY_GOOGLE_AVOIDNSOBJECTNEWCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_AVOIDNSOBJECTNEWCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace google { +namespace objc { + +/// This check finds Objective-C code that uses +new to create object instances, +/// or overrides +new in classes. Both are forbidden by Google's Objective-C +/// style guide. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/google-avoid-nsobject-new.html +class AvoidNSObjectNewCheck : public ClangTidyCheck { +public: + AvoidNSObjectNewCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace objc +} // namespace google +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_AVOIDNSOBJECTNEWCHECK_H diff --git a/clang-tools-extra/clang-tidy/google/CMakeLists.txt b/clang-tools-extra/clang-tidy/google/CMakeLists.txt index 4d0a326f73b16..b78088cf06bab 100644 --- a/clang-tools-extra/clang-tidy/google/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/google/CMakeLists.txt @@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyGoogleModule AvoidCStyleCastsCheck.cpp + AvoidNSObjectNewCheck.cpp AvoidThrowingObjCExceptionCheck.cpp AvoidUnderscoreInGoogletestNameCheck.cpp DefaultArgumentsCheck.cpp diff --git a/clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp b/clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp index ce833906dd5c5..30ab04c08c008 100644 --- a/clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp @@ -23,29 +23,35 @@ namespace objc { namespace { -AST_MATCHER(VarDecl, isLocalVariable) { - return Node.isLocalVarDecl(); -} +AST_MATCHER(VarDecl, isLocalVariable) { return Node.isLocalVarDecl(); } FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) { + if (IsConst && (Decl->getStorageClass() != SC_Static)) { + // No fix available if it is not a static constant, since it is difficult + // to determine the proper fix in this case. + return FixItHint(); + } + char FC = Decl->getName()[0]; if (!llvm::isAlpha(FC) || Decl->getName().size() == 1) { // No fix available if first character is not alphabetical character, or it - // is a single-character variable, since it is difficult to determine the + // is a single-character variable, since it is difficult to determine the // proper fix in this case. Users should create a proper variable name by // their own. return FixItHint(); } char SC = Decl->getName()[1]; if ((FC == 'k' || FC == 'g') && !llvm::isAlpha(SC)) { - // No fix available if the prefix is correct but the second character is not - // alphabetical, since it is difficult to determine the proper fix in this - // case. + // No fix available if the prefix is correct but the second character is + // not alphabetical, since it is difficult to determine the proper fix in + // this case. return FixItHint(); } + auto NewName = (IsConst ? "k" : "g") + llvm::StringRef(std::string(1, FC)).upper() + Decl->getName().substr(1).str(); + return FixItHint::CreateReplacement( CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())), llvm::StringRef(NewName)); @@ -71,7 +77,7 @@ void GlobalVariableDeclarationCheck::registerMatchers(MatchFinder *Finder) { this); Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()), unless(isLocalVariable()), - unless(matchesName("::(k[A-Z]|[A-Z]{2,})"))) + unless(matchesName("::(k[A-Z])|([A-Z][A-Z0-9])"))) .bind("global_const"), this); } diff --git a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp index c2a9ec5edbc48..1e3410fb8a5ef 100644 --- a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp @@ -13,6 +13,7 @@ #include "../readability/FunctionSizeCheck.h" #include "../readability/NamespaceCommentCheck.h" #include "AvoidCStyleCastsCheck.h" +#include "AvoidNSObjectNewCheck.h" #include "AvoidThrowingObjCExceptionCheck.h" #include "AvoidUnderscoreInGoogletestNameCheck.h" #include "DefaultArgumentsCheck.h" @@ -49,6 +50,8 @@ class GoogleModule : public ClangTidyModule { "google-explicit-constructor"); CheckFactories.registerCheck( "google-global-names-in-headers"); + CheckFactories.registerCheck( + "google-objc-avoid-nsobject-new"); CheckFactories.registerCheck( "google-objc-avoid-throwing-exception"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp index a36f307b1bf93..a496e3b292fde 100644 --- a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp @@ -124,13 +124,16 @@ void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) { } } - bool is_full_spec = FD->getTemplateSpecializationKind() != TSK_Undeclared; + bool IsFullSpec = FD->getTemplateSpecializationKind() != TSK_Undeclared; diag(FD->getLocation(), "%select{function|full function template specialization}0 %1 defined " "in a header file; function definitions in header files can lead to " "ODR violations") - << is_full_spec << FD << FixItHint::CreateInsertion( - FD->getReturnTypeSourceRange().getBegin(), "inline "); + << IsFullSpec << FD; + diag(FD->getLocation(), /*FixDescription=*/"make as 'inline'", + DiagnosticIDs::Note) + << FixItHint::CreateInsertion(FD->getReturnTypeSourceRange().getBegin(), + "inline "); } else if (const auto *VD = dyn_cast(ND)) { // Static data members of a class template are allowed. if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember()) diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp index 179a09745e87d..5fbc7be3749b6 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -298,11 +298,20 @@ bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag, return true; // Check whether we implicitly construct a class from a // std::initializer_list. - if (const auto *ImplicitCE = dyn_cast(Arg)) { - if (ImplicitCE->isStdInitListInitialization()) + if (const auto *CEArg = dyn_cast(Arg)) { + // Strip the elidable move constructor, it is present in the AST for + // C++11/14, e.g. Foo(Bar{1, 2}), the move constructor is around the + // init-list constructor. + if (CEArg->isElidable()) { + if (const auto *TempExp = CEArg->getArg(0)) { + if (const auto *UnwrappedCE = + dyn_cast(TempExp->IgnoreImplicit())) + CEArg = UnwrappedCE; + } + } + if (CEArg->isStdInitListInitialization()) return true; } - return false; } return false; }; diff --git a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp index 0c5eacef2e5c6..303833d73ec1d 100644 --- a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "DeleteNullPointerCheck.h" +#include "../utils/LexerUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" @@ -62,9 +63,11 @@ void DeleteNullPointerCheck::check(const MatchFinder::MatchResult &Result) { Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( IfWithDelete->getBeginLoc(), - Lexer::getLocForEndOfToken(IfWithDelete->getCond()->getEndLoc(), 0, - *Result.SourceManager, - Result.Context->getLangOpts()))); + utils::lexer::getPreviousToken(IfWithDelete->getThen()->getBeginLoc(), + *Result.SourceManager, + Result.Context->getLangOpts()) + .getLocation())); + if (Compound) { Diag << FixItHint::CreateRemoval( CharSourceRange::getTokenRange(Compound->getLBracLoc())); diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp index 7e56fe16d9b38..1bdfe2124e90c 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -800,10 +800,11 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { // Fix type aliases in value declarations if (const auto *Value = Result.Nodes.getNodeAs("decl")) { - if (const auto *Typedef = - Value->getType().getTypePtr()->getAs()) { - addUsage(NamingCheckFailures, Typedef->getDecl(), - Value->getSourceRange()); + if (const auto *TypePtr = Value->getType().getTypePtrOrNull()) { + if (const auto *Typedef = TypePtr->getAs()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } } } diff --git a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt index e093215b8e8e8..5b2cc93296420 100644 --- a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -23,5 +23,5 @@ add_clang_library(clangTidyUtils clangBasic clangLex clangTidy - clangToolingRefactor + clangToolingRefactoring ) diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp index f142de4644184..80a829808681e 100644 --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp @@ -7,12 +7,24 @@ //===----------------------------------------------------------------------===// #include "TransformerClangTidyCheck.h" +#include "llvm/ADT/STLExtras.h" namespace clang { namespace tidy { namespace utils { using tooling::RewriteRule; +TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R, + StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), Rule(std::move(R)) { + assert(llvm::all_of(Rule.Cases, [](const RewriteRule::Case &C) { + return C.Explanation != nullptr; + }) && + "clang-tidy checks must have an explanation by default;" + " explicitly provide an empty explanation if none is desired"); +} + void TransformerClangTidyCheck::registerMatchers( ast_matchers::MatchFinder *Finder) { Finder->addDynamicMatcher(tooling::detail::buildMatcher(Rule), this); @@ -44,15 +56,13 @@ void TransformerClangTidyCheck::check( if (Transformations->empty()) return; - StringRef Message = "no explanation"; - if (Case.Explanation) { - if (Expected E = Case.Explanation(Result)) - Message = *E; - else - llvm::errs() << "Error in explanation: " << llvm::toString(E.takeError()) - << "\n"; + Expected Explanation = Case.Explanation(Result); + if (!Explanation) { + llvm::errs() << "Error in explanation: " + << llvm::toString(Explanation.takeError()) << "\n"; + return; } - DiagnosticBuilder Diag = diag(RootLoc, Message); + DiagnosticBuilder Diag = diag(RootLoc, *Explanation); for (const auto &T : *Transformations) { Diag << FixItHint::CreateReplacement(T.Range, T.Replacement); } diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h index 6d0f86795bfdc..faf946ceb0feb 100644 --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h @@ -31,10 +31,14 @@ namespace utils { // }; class TransformerClangTidyCheck : public ClangTidyCheck { public: + // All cases in \p R must have a non-null \c Explanation, even though \c + // Explanation is optional for RewriteRule in general. Because the primary + // purpose of clang-tidy checks is to provide users with diagnostics, we + // assume that a missing explanation is a bug. If no explanation is desired, + // indicate that explicitly (for example, by passing `text("no explanation")` + // to `makeRule` as the `Explanation` argument). TransformerClangTidyCheck(tooling::RewriteRule R, StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), Rule(std::move(R)) {} - + ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) final; void check(const ast_matchers::MatchFinder::MatchResult &Result) final; diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt index 0a10b6d0d2039..68bf0aa50b479 100644 --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -77,6 +77,7 @@ add_clang_library(clangDaemon index/MemIndex.cpp index/Merge.cpp index/Ref.cpp + index/Relation.cpp index/Serialization.cpp index/Symbol.cpp index/SymbolCollector.cpp @@ -125,7 +126,7 @@ add_clang_library(clangDaemon clangTooling clangToolingCore clangToolingInclusions - clangToolingRefactor + clangToolingRefactoring ${LLVM_PTHREAD_LIB} ${CLANGD_ATOMIC_LIB} ) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 6bc8499730f55..0c09eaaeabf9f 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -8,6 +8,7 @@ #include "ClangdLSPServer.h" #include "Diagnostics.h" +#include "FormattedString.h" #include "Protocol.h" #include "SourceCode.h" #include "Trace.h" @@ -358,6 +359,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, SupportsHierarchicalDocumentSymbol = Params.capabilities.HierarchicalDocumentSymbol; SupportFileStatus = Params.initializationOptions.FileStatus; + HoverContentFormat = Params.capabilities.HoverContentFormat; + SupportsOffsetsInSignatureHelp = Params.capabilities.OffsetsInSignatureHelp; llvm::json::Object Result{ {{"capabilities", llvm::json::Object{ @@ -759,7 +762,22 @@ void ClangdLSPServer::onCompletion(const CompletionParams &Params, void ClangdLSPServer::onSignatureHelp(const TextDocumentPositionParams &Params, Callback Reply) { Server->signatureHelp(Params.textDocument.uri.file(), Params.position, - std::move(Reply)); + Bind( + [this](decltype(Reply) Reply, + llvm::Expected Signature) { + if (!Signature) + return Reply(Signature.takeError()); + if (SupportsOffsetsInSignatureHelp) + return Reply(std::move(*Signature)); + // Strip out the offsets from signature help for + // clients that only support string labels. + for (auto &SigInfo : Signature->signatures) { + for (auto &Param : SigInfo.parameters) + Param.labelOffsets.reset(); + } + return Reply(std::move(*Signature)); + }, + std::move(Reply))); } // Go to definition has a toggle function: if def and decl are distinct, then @@ -842,7 +860,30 @@ void ClangdLSPServer::onDocumentHighlight( void ClangdLSPServer::onHover(const TextDocumentPositionParams &Params, Callback> Reply) { Server->findHover(Params.textDocument.uri.file(), Params.position, - std::move(Reply)); + Bind( + [this](decltype(Reply) Reply, + llvm::Expected> H) { + if (!H) + return Reply(H.takeError()); + if (!*H) + return Reply(llvm::None); + + Hover R; + R.contents.kind = HoverContentFormat; + R.range = (*H)->SymRange; + switch (HoverContentFormat) { + case MarkupKind::PlainText: + R.contents.value = + (*H)->present().renderAsPlainText(); + return Reply(std::move(R)); + case MarkupKind::Markdown: + R.contents.value = + (*H)->present().renderAsMarkdown(); + return Reply(std::move(R)); + }; + llvm_unreachable("unhandled MarkupKind"); + }, + std::move(Reply))); } void ClangdLSPServer::onTypeHierarchy( diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index f0b10a2f89667..2b2d9c0b0080d 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -154,6 +154,10 @@ class ClangdLSPServer : private DiagnosticsConsumer { bool SupportsHierarchicalDocumentSymbol = false; /// Whether the client supports showing file status. bool SupportFileStatus = false; + /// Which kind of markup should we use in textDocument/hover responses. + MarkupKind HoverContentFormat = MarkupKind::PlainText; + /// Whether the client supports offsets for parameter info labels. + bool SupportsOffsetsInSignatureHelp = false; // Store of the current versions of the open documents. DraftStore DraftMgr; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 449c0c980991d..ca1500be0e36e 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -10,11 +10,13 @@ #include "ClangdUnit.h" #include "CodeComplete.h" #include "FindSymbols.h" +#include "FormattedString.h" #include "Headers.h" #include "Protocol.h" #include "SourceCode.h" #include "TUScheduler.h" #include "Trace.h" +#include "XRefs.h" #include "index/CanonicalIncludes.h" #include "index/FileIndex.h" #include "index/Merge.h" @@ -461,15 +463,18 @@ void ClangdServer::findDocumentHighlights( } void ClangdServer::findHover(PathRef File, Position Pos, - Callback> CB) { - auto Action = [Pos](Callback> CB, + Callback> CB) { + auto Action = [Pos](decltype(CB) CB, Path File, llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); - CB(clangd::getHover(InpAST->AST, Pos)); + format::FormatStyle Style = getFormatStyleForFile( + File, InpAST->Inputs.Contents, InpAST->Inputs.FS.get()); + CB(clangd::getHover(InpAST->AST, Pos, std::move(Style))); }; - WorkScheduler.runWithAST("Hover", File, Bind(Action, std::move(CB))); + WorkScheduler.runWithAST("Hover", File, + Bind(Action, std::move(CB), File.str())); } void ClangdServer::typeHierarchy(PathRef File, Position Pos, int Resolve, diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 1038982dc7a4a..c0389f098193d 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -14,6 +14,7 @@ #include "ClangdUnit.h" #include "CodeComplete.h" #include "FSProvider.h" +#include "FormattedString.h" #include "Function.h" #include "GlobalCompilationDatabase.h" #include "Protocol.h" @@ -186,7 +187,7 @@ class ClangdServer { /// Get code hover for a given position. void findHover(PathRef File, Position Pos, - Callback> CB); + Callback> CB); /// Get information about type hierarchy for a given position. void typeHierarchy(PathRef File, Position Pos, int Resolve, diff --git a/clang-tools-extra/clangd/ClangdUnit.h b/clang-tools-extra/clangd/ClangdUnit.h index 042bb0d6a036f..16246eb27843d 100644 --- a/clang-tools-extra/clangd/ClangdUnit.h +++ b/clang-tools-extra/clangd/ClangdUnit.h @@ -95,6 +95,13 @@ class ParsedAST { std::shared_ptr getPreprocessorPtr(); const Preprocessor &getPreprocessor() const; + SourceManager &getSourceManager() { + return getASTContext().getSourceManager(); + } + const SourceManager &getSourceManager() const { + return getASTContext().getSourceManager(); + } + /// This function returns top-level decls present in the main file of the AST. /// The result does not include the decls that come from the preamble. /// (These should be const, but RecursiveASTVisitor requires Decl*). diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 7f811c31de5bc..32fdccc410092 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -28,6 +28,7 @@ #include "FuzzyMatch.h" #include "Headers.h" #include "Logger.h" +#include "Protocol.h" #include "Quality.h" #include "SourceCode.h" #include "TUScheduler.h" @@ -56,6 +57,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Compiler.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Error.h" #include "llvm/Support/Format.h" @@ -92,8 +94,10 @@ CompletionItemKind toCompletionItemKind(index::SymbolKind Kind) { case SK::Extension: case SK::Union: return CompletionItemKind::Class; - // FIXME(ioeric): figure out whether reference is the right type for aliases. case SK::TypeAlias: + // We use the same kind as the VSCode C++ extension. + // FIXME: pick a better option when we have one. + return CompletionItemKind::Interface; case SK::Using: return CompletionItemKind::Reference; case SK::Function: @@ -146,46 +150,6 @@ toCompletionItemKind(CodeCompletionResult::ResultKind ResKind, llvm_unreachable("Unhandled CodeCompletionResult::ResultKind."); } -/// Get the optional chunk as a string. This function is possibly recursive. -/// -/// The parameter info for each parameter is appended to the Parameters. -std::string getOptionalParameters(const CodeCompletionString &CCS, - std::vector &Parameters, - SignatureQualitySignals &Signal) { - std::string Result; - for (const auto &Chunk : CCS) { - switch (Chunk.Kind) { - case CodeCompletionString::CK_Optional: - assert(Chunk.Optional && - "Expected the optional code completion string to be non-null."); - Result += getOptionalParameters(*Chunk.Optional, Parameters, Signal); - break; - case CodeCompletionString::CK_VerticalSpace: - break; - case CodeCompletionString::CK_Placeholder: - // A string that acts as a placeholder for, e.g., a function call - // argument. - // Intentional fallthrough here. - case CodeCompletionString::CK_CurrentParameter: { - // A piece of text that describes the parameter that corresponds to - // the code-completion location within a function call, message send, - // macro invocation, etc. - Result += Chunk.Text; - ParameterInformation Info; - Info.label = Chunk.Text; - Parameters.push_back(std::move(Info)); - Signal.ContainsActiveParameter = true; - Signal.NumberOfOptionalParameters++; - break; - } - default: - Result += Chunk.Text; - break; - } - } - return Result; -} - // Identifier code completion result. struct RawIdentifier { llvm::StringRef Name; @@ -394,8 +358,9 @@ struct CodeCompletionBuilder { Bundled.emplace_back(); BundledEntry &S = Bundled.back(); if (C.SemaResult) { + bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern; getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, - &Completion.RequiredQualifier); + &Completion.RequiredQualifier, IsPattern); S.ReturnType = getReturnType(*SemaCCS); } else if (C.IndexResult) { S.Signature = C.IndexResult->Signature; @@ -480,7 +445,8 @@ struct CodeCompletionBuilder { return EmptyArgs ? "()" : "($0)"; return *Snippet; // Not an arg snippet? } - if (Completion.Kind == CompletionItemKind::Reference || + // 'CompletionItemKind::Interface' matches template type aliases. + if (Completion.Kind == CompletionItemKind::Interface || Completion.Kind == CompletionItemKind::Class) { if (Snippet->front() != '<') return *Snippet; // Not an arg snippet? @@ -826,8 +792,7 @@ class SignatureHelpCollector final : public CodeCompleteConsumer { public: SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts, const SymbolIndex *Index, SignatureHelp &SigHelp) - : CodeCompleteConsumer(CodeCompleteOpts), - SigHelp(SigHelp), + : CodeCompleteConsumer(CodeCompleteOpts), SigHelp(SigHelp), Allocator(std::make_shared()), CCTUInfo(Allocator), Index(Index) {} @@ -940,6 +905,50 @@ class SignatureHelpCollector final : public CodeCompleteConsumer { CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; } private: + void processParameterChunk(llvm::StringRef ChunkText, + SignatureInformation &Signature, + SignatureQualitySignals Signal) const { + // (!) this is O(n), should still be fast compared to building ASTs. + unsigned ParamStartOffset = lspLength(Signature.label); + unsigned ParamEndOffset = ParamStartOffset + lspLength(ChunkText); + // A piece of text that describes the parameter that corresponds to + // the code-completion location within a function call, message send, + // macro invocation, etc. + Signature.label += ChunkText; + ParameterInformation Info; + Info.labelOffsets.emplace(ParamStartOffset, ParamEndOffset); + // FIXME: only set 'labelOffsets' when all clients migrate out of it. + Info.labelString = ChunkText; + + Signature.parameters.push_back(std::move(Info)); + // FIXME: this should only be set on CK_CurrentParameter. + Signal.ContainsActiveParameter = true; + } + + void processOptionalChunk(const CodeCompletionString &CCS, + SignatureInformation &Signature, + SignatureQualitySignals &Signal) const { + for (const auto &Chunk : CCS) { + switch (Chunk.Kind) { + case CodeCompletionString::CK_Optional: + assert(Chunk.Optional && + "Expected the optional code completion string to be non-null."); + processOptionalChunk(*Chunk.Optional, Signature, Signal); + break; + case CodeCompletionString::CK_VerticalSpace: + break; + case CodeCompletionString::CK_CurrentParameter: + case CodeCompletionString::CK_Placeholder: + processParameterChunk(Chunk.Text, Signature, Signal); + Signal.NumberOfOptionalParameters++; + break; + default: + Signature.label += Chunk.Text; + break; + } + } + } + // FIXME(ioeric): consider moving CodeCompletionString logic here to // CompletionString.h. ScoredSignature processOverloadCandidate(const OverloadCandidate &Candidate, @@ -960,28 +969,16 @@ class SignatureHelpCollector final : public CodeCompleteConsumer { assert(!ReturnType && "Unexpected CK_ResultType"); ReturnType = Chunk.Text; break; + case CodeCompletionString::CK_CurrentParameter: case CodeCompletionString::CK_Placeholder: - // A string that acts as a placeholder for, e.g., a function call - // argument. - // Intentional fallthrough here. - case CodeCompletionString::CK_CurrentParameter: { - // A piece of text that describes the parameter that corresponds to - // the code-completion location within a function call, message send, - // macro invocation, etc. - Signature.label += Chunk.Text; - ParameterInformation Info; - Info.label = Chunk.Text; - Signature.parameters.push_back(std::move(Info)); + processParameterChunk(Chunk.Text, Signature, Signal); Signal.NumberOfParameters++; - Signal.ContainsActiveParameter = true; break; - } case CodeCompletionString::CK_Optional: { // The rest of the parameters are defaulted/optional. assert(Chunk.Optional && "Expected the optional code completion string to be non-null."); - Signature.label += getOptionalParameters(*Chunk.Optional, - Signature.parameters, Signal); + processOptionalChunk(*Chunk.Optional, Signature, Signal); break; } case CodeCompletionString::CK_VerticalSpace: @@ -1033,7 +1030,7 @@ void loadMainFilePreambleMacros(const Preprocessor &PP, PP.getIdentifierTable().getExternalIdentifierLookup(); if (!PreambleIdentifiers || !PreambleMacros) return; - for (const auto& MacroName : Preamble.MainFileMacros) + for (const auto &MacroName : Preamble.MainFileMacros) if (auto *II = PreambleIdentifiers->get(MacroName)) if (II->isOutOfDate()) PreambleMacros->updateOutOfDateIdentifier(*II); @@ -1209,7 +1206,7 @@ class CodeCompleteFlow { int NSema = 0, NIndex = 0, NSemaAndIndex = 0, NIdent = 0; bool Incomplete = false; // Would more be available with a higher limit? CompletionPrefix HeuristicPrefix; - llvm::Optional Filter; // Initialized once Sema runs. + llvm::Optional Filter; // Initialized once Sema runs. Range ReplacedRange; std::vector QueryScopes; // Initialized once Sema runs. // Initialized once QueryScopes is initialized, if there are scopes. @@ -1703,8 +1700,8 @@ clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const { return Result; } -CompletionPrefix -guessCompletionPrefix(llvm::StringRef Content, unsigned Offset) { +CompletionPrefix guessCompletionPrefix(llvm::StringRef Content, + unsigned Offset) { assert(Offset <= Content.size()); StringRef Rest = Content.take_front(Offset); CompletionPrefix Result; diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp index 586be67e92c30..bf3cabc269820 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -11,6 +11,8 @@ #include "clang/AST/DeclObjC.h" #include "clang/AST/RawCommentList.h" #include "clang/Basic/SourceManager.h" +#include "clang/Sema/CodeCompleteConsumer.h" +#include #include namespace clang { @@ -73,8 +75,23 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) { } void getSignature(const CodeCompletionString &CCS, std::string *Signature, - std::string *Snippet, std::string *RequiredQualifiers) { - unsigned ArgCount = 0; + std::string *Snippet, std::string *RequiredQualifiers, + bool CompletingPattern) { + // Placeholder with this index will be ${0:…} to mark final cursor position. + // Usually we do not add $0, so the cursor is placed at end of completed text. + unsigned CursorSnippetArg = std::numeric_limits::max(); + if (CompletingPattern) { + // In patterns, it's best to place the cursor at the last placeholder, to + // handle cases like + // namespace ${1:name} { + // ${0:decls} + // } + CursorSnippetArg = + llvm::count_if(CCS, [](const CodeCompletionString::Chunk &C) { + return C.Kind == CodeCompletionString::CK_Placeholder; + }); + } + unsigned SnippetArg = 0; bool HadObjCArguments = false; for (const auto &Chunk : CCS) { // Informative qualifier chunks only clutter completion results, skip @@ -124,8 +141,10 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature, break; case CodeCompletionString::CK_Placeholder: *Signature += Chunk.Text; - ++ArgCount; - *Snippet += "${" + std::to_string(ArgCount) + ':'; + ++SnippetArg; + *Snippet += + "${" + + std::to_string(SnippetArg == CursorSnippetArg ? 0 : SnippetArg) + ':'; appendEscapeSnippet(Chunk.Text, Snippet); *Snippet += '}'; break; diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h index 153e0af1189e6..6733d0231df49 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.h +++ b/clang-tools-extra/clangd/CodeCompletionStrings.h @@ -38,12 +38,16 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D); /// Formats the signature for an item, as a display string and snippet. /// e.g. for const_reference std::vector::at(size_type) const, this returns: /// *Signature = "(size_type) const" -/// *Snippet = "(${0:size_type})" +/// *Snippet = "(${1:size_type})" /// If set, RequiredQualifiers is the text that must be typed before the name. /// e.g "Base::" when calling a base class member function that's hidden. +/// +/// When \p CompletingPattern is true, the last placeholder will be of the form +/// ${0:…}, indicating the cursor should stay there. void getSignature(const CodeCompletionString &CCS, std::string *Signature, std::string *Snippet, - std::string *RequiredQualifiers = nullptr); + std::string *RequiredQualifiers = nullptr, + bool CompletingPattern = false); /// Assembles formatted documentation for a completion result. This includes /// documentation comments and other relevant information like annotations. diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index 5f42841db7717..a7bc1f1dcdb86 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -25,7 +25,9 @@ #include "llvm/Support/Path.h" #include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/Signals.h" +#include "llvm/Support/raw_ostream.h" #include +#include namespace clang { namespace clangd { @@ -437,6 +439,21 @@ void StoreDiags::EndSourceFile() { LangOpts = None; } +/// Sanitizes a piece for presenting it in a synthesized fix message. Ensures +/// the result is not too large and does not contain newlines. +static void writeCodeToFixMessage(llvm::raw_ostream &OS, llvm::StringRef Code) { + constexpr unsigned MaxLen = 50; + + // Only show the first line if there are many. + llvm::StringRef R = Code.split('\n').first; + // Shorten the message if it's too long. + R = R.take_front(MaxLen); + + OS << R; + if (R.size() != Code.size()) + OS << "…"; +} + void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) { DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info); @@ -494,12 +511,21 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, llvm::StringRef Insert = FixIt.CodeToInsert; if (!Invalid) { llvm::raw_svector_ostream M(Message); - if (!Remove.empty() && !Insert.empty()) - M << "change '" << Remove << "' to '" << Insert << "'"; - else if (!Remove.empty()) - M << "remove '" << Remove << "'"; - else if (!Insert.empty()) - M << "insert '" << Insert << "'"; + if (!Remove.empty() && !Insert.empty()) { + M << "change '"; + writeCodeToFixMessage(M, Remove); + M << "' to '"; + writeCodeToFixMessage(M, Insert); + M << "'"; + } else if (!Remove.empty()) { + M << "remove '"; + writeCodeToFixMessage(M, Remove); + M << "'"; + } else if (!Insert.empty()) { + M << "insert '"; + writeCodeToFixMessage(M, Insert); + M << "'"; + } // Don't allow source code to inject newlines into diagnostics. std::replace(Message.begin(), Message.end(), '\n', ' '); } diff --git a/clang-tools-extra/clangd/ExpectedTypes.cpp b/clang-tools-extra/clangd/ExpectedTypes.cpp index 886b5db0cd0c6..3b0779ea66bc6 100644 --- a/clang-tools-extra/clangd/ExpectedTypes.cpp +++ b/clang-tools-extra/clangd/ExpectedTypes.cpp @@ -8,9 +8,11 @@ #include "ExpectedTypes.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Type.h" #include "clang/Index/USRGeneration.h" #include "clang/Sema/CodeCompleteConsumer.h" +#include "llvm/ADT/None.h" #include "llvm/ADT/STLExtras.h" namespace clang { @@ -41,7 +43,13 @@ static const Type *toEquivClass(ASTContext &Ctx, QualType T) { static llvm::Optional typeOfCompletion(const CodeCompletionResult &R) { - auto *VD = dyn_cast_or_null(R.Declaration); + const NamedDecl *D = R.Declaration; + if (!D) + return llvm::None; + // Templates do not have a type on their own, look at the templated decl. + if (auto *Template = dyn_cast(D)) + D = Template->getTemplatedDecl(); + auto *VD = dyn_cast(D); if (!VD) return llvm::None; // We handle only variables and functions below. auto T = VD->getType(); diff --git a/clang-tools-extra/clangd/FormattedString.cpp b/clang-tools-extra/clangd/FormattedString.cpp index 3ae1a3c6fa8a7..3be179bbdc2ce 100644 --- a/clang-tools-extra/clangd/FormattedString.cpp +++ b/clang-tools-extra/clangd/FormattedString.cpp @@ -9,6 +9,7 @@ #include "clang/Basic/CharInfo.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/FormatVariadic.h" #include #include @@ -169,5 +170,27 @@ std::string FormattedString::renderAsPlainText() const { R.pop_back(); return R; } + +std::string FormattedString::renderForTests() const { + std::string R; + for (const auto &C : Chunks) { + switch (C.Kind) { + case ChunkKind::PlainText: + R += "text[" + C.Contents + "]"; + break; + case ChunkKind::InlineCodeBlock: + R += "code[" + C.Contents + "]"; + break; + case ChunkKind::CodeBlock: + if (!R.empty()) + R += "\n"; + R += llvm::formatv("codeblock({0}) [\n{1}\n]\n", C.Language, C.Contents); + break; + } + } + while (!R.empty() && isWhitespace(R.back())) + R.pop_back(); + return R; +} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/FormattedString.h b/clang-tools-extra/clangd/FormattedString.h index f20c19af85680..10313a4408cf6 100644 --- a/clang-tools-extra/clangd/FormattedString.h +++ b/clang-tools-extra/clangd/FormattedString.h @@ -35,6 +35,7 @@ class FormattedString { std::string renderAsMarkdown() const; std::string renderAsPlainText() const; + std::string renderForTests() const; private: enum class ChunkKind { diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index 50f5408cff362..b40ae26cd3e72 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -173,6 +173,7 @@ tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const { std::lock_guard Lock(Mutex); Cmd.CommandLine.insert(Cmd.CommandLine.end(), FallbackFlags.begin(), FallbackFlags.end()); + adjustArguments(Cmd, ResourceDir); return Cmd; } diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index a8b1c43732d33..4714c6c11da57 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -17,6 +17,7 @@ #include "llvm/ADT/Hashing.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Format.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/JSON.h" @@ -302,6 +303,25 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R) { DocumentSymbol->getBoolean("hierarchicalDocumentSymbolSupport")) R.HierarchicalDocumentSymbol = *HierarchicalSupport; } + if (auto *Hover = TextDocument->getObject("hover")) { + if (auto *ContentFormat = Hover->getArray("contentFormat")) { + for (const auto &Format : *ContentFormat) { + MarkupKind K = MarkupKind::PlainText; + if (fromJSON(Format, K)) { + R.HoverContentFormat = K; + break; + } + } + } + } + if (auto *Help = TextDocument->getObject("signatureHelp")) { + if (auto *Info = Help->getObject("signatureInformation")) { + if (auto *Parameter = Info->getObject("parameterInformation")) { + if (auto OffsetSupport = Parameter->getBoolean("labelOffsetSupport")) + R.OffsetsInSignatureHelp = *OffsetSupport; + } + } + } } if (auto *Workspace = O->getObject("workspace")) { if (auto *Symbol = Workspace->getObject("symbol")) { @@ -683,6 +703,27 @@ static llvm::StringRef toTextKind(MarkupKind Kind) { llvm_unreachable("Invalid MarkupKind"); } +bool fromJSON(const llvm::json::Value &V, MarkupKind &K) { + auto Str = V.getAsString(); + if (!Str) { + elog("Failed to parse markup kind: expected a string"); + return false; + } + if (*Str == "plaintext") + K = MarkupKind::PlainText; + else if (*Str == "markdown") + K = MarkupKind::Markdown; + else { + elog("Unknown markup kind: {0}", *Str); + return false; + } + return true; +} + +llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, MarkupKind K) { + return OS << toTextKind(K); +} + llvm::json::Value toJSON(const MarkupContent &MC) { if (MC.value.empty()) return nullptr; @@ -791,8 +832,14 @@ llvm::json::Value toJSON(const CompletionList &L) { } llvm::json::Value toJSON(const ParameterInformation &PI) { - assert(!PI.label.empty() && "parameter information label is required"); - llvm::json::Object Result{{"label", PI.label}}; + assert((PI.labelOffsets.hasValue() || !PI.labelString.empty()) && + "parameter information label is required"); + llvm::json::Object Result; + if (PI.labelOffsets) + Result["label"] = + llvm::json::Array({PI.labelOffsets->first, PI.labelOffsets->second}); + else + Result["label"] = PI.labelString; if (!PI.documentation.empty()) Result["documentation"] = PI.documentation; return std::move(Result); diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 957e2f3da444c..1ebe8071cdad8 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -353,6 +353,15 @@ llvm::json::Value toJSON(const OffsetEncoding &); bool fromJSON(const llvm::json::Value &, OffsetEncoding &); llvm::raw_ostream &operator<<(llvm::raw_ostream &, OffsetEncoding); +// Describes the content type that a client supports in various result literals +// like `Hover`, `ParameterInfo` or `CompletionItem`. +enum class MarkupKind { + PlainText, + Markdown, +}; +bool fromJSON(const llvm::json::Value &, MarkupKind &); +llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, MarkupKind); + // This struct doesn't mirror LSP! // The protocol defines deeply nested structures for client capabilities. // Instead of mapping them all, this just parses out the bits we care about. @@ -381,6 +390,9 @@ struct ClientCapabilities { /// Client supports hierarchical document symbols. bool HierarchicalDocumentSymbol = false; + /// Client supports processing label offsets instead of a simple label string. + bool OffsetsInSignatureHelp = false; + /// The supported set of CompletionItemKinds for textDocument/completion. /// textDocument.completion.completionItemKind.valueSet llvm::Optional CompletionItemKinds; @@ -391,6 +403,9 @@ struct ClientCapabilities { /// Supported encodings for LSP character offsets. (clangd extension). llvm::Optional> offsetEncoding; + + /// The content format that should be used for Hover requests. + MarkupKind HoverContentFormat = MarkupKind::PlainText; }; bool fromJSON(const llvm::json::Value &, ClientCapabilities &); @@ -861,11 +876,6 @@ struct CompletionParams : TextDocumentPositionParams { }; bool fromJSON(const llvm::json::Value &, CompletionParams &); -enum class MarkupKind { - PlainText, - Markdown, -}; - struct MarkupContent { MarkupKind kind = MarkupKind::PlainText; std::string value; @@ -972,8 +982,14 @@ llvm::json::Value toJSON(const CompletionList &); /// A single parameter of a particular signature. struct ParameterInformation { - /// The label of this parameter. Mandatory. - std::string label; + /// The label of this parameter. Ignored when labelOffsets is set. + std::string labelString; + + /// Inclusive start and exclusive end offsets withing the containing signature + /// label. + /// Offsets are computed by lspLength(), which counts UTF-16 code units by + /// default but that can be overriden, see its documentation for details. + llvm::Optional> labelOffsets; /// The documentation of this parameter. Optional. std::string documentation; diff --git a/clang-tools-extra/clangd/StdSymbolMap.inc b/clang-tools-extra/clangd/StdSymbolMap.inc index 85fd474eed4b5..3ec94860ae792 100644 --- a/clang-tools-extra/clangd/StdSymbolMap.inc +++ b/clang-tools-extra/clangd/StdSymbolMap.inc @@ -454,11 +454,24 @@ SYMBOL(includes, std::, ) SYMBOL(inclusive_scan, std::, ) SYMBOL(independent_bits_engine, std::, ) SYMBOL(indirect_array, std::, ) +SYMBOL(initializer_list, std::, ) SYMBOL(inner_product, std::, ) SYMBOL(inplace_merge, std::, ) SYMBOL(input_iterator_tag, std::, ) SYMBOL(insert_iterator, std::, ) SYMBOL(inserter, std::, ) +SYMBOL(int16_t, std::, ) +SYMBOL(int32_t, std::, ) +SYMBOL(int64_t, std::, ) +SYMBOL(int8_t, std::, ) +SYMBOL(int_fast16_t, std::, ) +SYMBOL(int_fast32_t, std::, ) +SYMBOL(int_fast64_t, std::, ) +SYMBOL(int_fast8_t, std::, ) +SYMBOL(int_least16_t, std::, ) +SYMBOL(int_least32_t, std::, ) +SYMBOL(int_least64_t, std::, ) +SYMBOL(int_least8_t, std::, ) SYMBOL(integer_sequence, std::, ) SYMBOL(integral_constant, std::, ) SYMBOL(internal, std::, ) @@ -1150,6 +1163,18 @@ SYMBOL(u16string_view, std::, ) SYMBOL(u32streampos, std::, ) SYMBOL(u32string, std::, ) SYMBOL(u32string_view, std::, ) +SYMBOL(uint16_t, std::, ) +SYMBOL(uint32_t, std::, ) +SYMBOL(uint64_t, std::, ) +SYMBOL(uint8_t, std::, ) +SYMBOL(uint_fast16_t, std::, ) +SYMBOL(uint_fast32_t, std::, ) +SYMBOL(uint_fast64_t, std::, ) +SYMBOL(uint_fast8_t, std::, ) +SYMBOL(uint_least16_t, std::, ) +SYMBOL(uint_least32_t, std::, ) +SYMBOL(uint_least64_t, std::, ) +SYMBOL(uint_least8_t, std::, ) SYMBOL(uintmax_t, std::, ) SYMBOL(uintptr_t, std::, ) SYMBOL(uncaught_exceptions, std::, ) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index c51631ad1934b..818dbc53a4d5f 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -7,21 +7,38 @@ //===----------------------------------------------------------------------===// #include "XRefs.h" #include "AST.h" +#include "CodeCompletionStrings.h" #include "FindSymbols.h" +#include "FormattedString.h" #include "Logger.h" +#include "Protocol.h" #include "SourceCode.h" #include "URI.h" #include "index/Merge.h" #include "index/SymbolCollector.h" #include "index/SymbolLocation.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" #include "clang/AST/DeclTemplate.h" +#include "clang/AST/PrettyPrinter.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Type.h" +#include "clang/Basic/LLVM.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "clang/Index/IndexDataConsumer.h" #include "clang/Index/IndexSymbol.h" #include "clang/Index/IndexingAction.h" #include "clang/Index/USRGeneration.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/None.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" +#include "llvm/Support/raw_ostream.h" namespace clang { namespace clangd { @@ -241,17 +258,17 @@ IdentifiedSymbol getSymbolAtPosition(ParsedAST &AST, SourceLocation Pos) { return {DeclMacrosFinder.getFoundDecls(), DeclMacrosFinder.takeMacroInfos()}; } -Range getTokenRange(ParsedAST &AST, SourceLocation TokLoc) { - const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); - SourceLocation LocEnd = Lexer::getLocForEndOfToken( - TokLoc, 0, SourceMgr, AST.getASTContext().getLangOpts()); +Range getTokenRange(ASTContext &AST, SourceLocation TokLoc) { + const SourceManager &SourceMgr = AST.getSourceManager(); + SourceLocation LocEnd = + Lexer::getLocForEndOfToken(TokLoc, 0, SourceMgr, AST.getLangOpts()); return {sourceLocToPosition(SourceMgr, TokLoc), sourceLocToPosition(SourceMgr, LocEnd)}; } -llvm::Optional makeLocation(ParsedAST &AST, SourceLocation TokLoc, +llvm::Optional makeLocation(ASTContext &AST, SourceLocation TokLoc, llvm::StringRef TUPath) { - const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); + const SourceManager &SourceMgr = AST.getSourceManager(); const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc)); if (!F) return None; @@ -270,7 +287,7 @@ llvm::Optional makeLocation(ParsedAST &AST, SourceLocation TokLoc, std::vector locateSymbolAt(ParsedAST &AST, Position Pos, const SymbolIndex *Index) { - const auto &SM = AST.getASTContext().getSourceManager(); + const auto &SM = AST.getSourceManager(); auto MainFilePath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM); if (!MainFilePath) { @@ -299,8 +316,8 @@ std::vector locateSymbolAt(ParsedAST &AST, Position Pos, // As a consequence, there's no need to look them up in the index either. std::vector Result; for (auto M : Symbols.Macros) { - if (auto Loc = - makeLocation(AST, M.Info->getDefinitionLoc(), *MainFilePath)) { + if (auto Loc = makeLocation(AST.getASTContext(), M.Info->getDefinitionLoc(), + *MainFilePath)) { LocatedSymbol Macro; Macro.Name = M.Name; Macro.PreferredDeclaration = *Loc; @@ -320,7 +337,7 @@ std::vector locateSymbolAt(ParsedAST &AST, Position Pos, // Emit all symbol locations (declaration or definition) from AST. for (const Decl *D : Symbols.Decls) { - auto Loc = makeLocation(AST, findNameLoc(D), *MainFilePath); + auto Loc = makeLocation(AST.getASTContext(), findNameLoc(D), *MainFilePath); if (!Loc) continue; @@ -445,7 +462,7 @@ findRefs(const std::vector &Decls, ParsedAST &AST) { std::vector findDocumentHighlights(ParsedAST &AST, Position Pos) { - const SourceManager &SM = AST.getASTContext().getSourceManager(); + const SourceManager &SM = AST.getSourceManager(); auto Symbols = getSymbolAtPosition( AST, getBeginningOfIdentifier(AST, Pos, SM.getMainFileID())); auto References = findRefs(Symbols.Decls, AST); @@ -453,7 +470,7 @@ std::vector findDocumentHighlights(ParsedAST &AST, std::vector Result; for (const auto &Ref : References) { DocumentHighlight DH; - DH.range = getTokenRange(AST, Ref.Loc); + DH.range = getTokenRange(AST.getASTContext(), Ref.Loc); if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write)) DH.kind = DocumentHighlightKind::Write; else if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Read)) @@ -477,102 +494,238 @@ static PrintingPolicy printingPolicyForDecls(PrintingPolicy Base) { return Policy; } -/// Return a string representation (e.g. "class MyNamespace::MyClass") of -/// the type declaration \p TD. -static std::string typeDeclToString(const TypeDecl *TD) { - QualType Type = TD->getASTContext().getTypeDeclType(TD); +/// Given a declaration \p D, return a human-readable string representing the +/// local scope in which it is declared, i.e. class(es) and method name. Returns +/// an empty string if it is not local. +static std::string getLocalScope(const Decl *D) { + std::vector Scopes; + const DeclContext *DC = D->getDeclContext(); + auto GetName = [](const Decl *D) { + const NamedDecl *ND = dyn_cast(D); + std::string Name = ND->getNameAsString(); + if (!Name.empty()) + return Name; + if (auto RD = dyn_cast(D)) + return ("(anonymous " + RD->getKindName() + ")").str(); + return std::string(""); + }; + while (DC) { + if (const TypeDecl *TD = dyn_cast(DC)) + Scopes.push_back(GetName(TD)); + else if (const FunctionDecl *FD = dyn_cast(DC)) + Scopes.push_back(FD->getNameAsString()); + DC = DC->getParent(); + } - PrintingPolicy Policy = - printingPolicyForDecls(TD->getASTContext().getPrintingPolicy()); + return llvm::join(llvm::reverse(Scopes), "::"); +} + +/// Returns the human-readable representation for namespace containing the +/// declaration \p D. Returns empty if it is contained global namespace. +static std::string getNamespaceScope(const Decl *D) { + const DeclContext *DC = D->getDeclContext(); - std::string Name; - llvm::raw_string_ostream Stream(Name); - Type.print(Stream, Policy); + if (const TypeDecl *TD = dyn_cast(DC)) + return getNamespaceScope(TD); + if (const FunctionDecl *FD = dyn_cast(DC)) + return getNamespaceScope(FD); + if (const NamedDecl *ND = dyn_cast(DC)) + return ND->getQualifiedNameAsString(); - return Stream.str(); + return ""; } -/// Return a string representation (e.g. "namespace ns1::ns2") of -/// the named declaration \p ND. -static std::string namedDeclQualifiedName(const NamedDecl *ND, - llvm::StringRef Prefix) { +static std::string printDefinition(const Decl *D) { + std::string Definition; + llvm::raw_string_ostream OS(Definition); PrintingPolicy Policy = - printingPolicyForDecls(ND->getASTContext().getPrintingPolicy()); - - std::string Name; - llvm::raw_string_ostream Stream(Name); - Stream << Prefix << ' '; - ND->printQualifiedName(Stream, Policy); + printingPolicyForDecls(D->getASTContext().getPrintingPolicy()); + Policy.IncludeTagDefinition = false; + D->print(OS, Policy); + return Definition; +} - return Stream.str(); +static void printParams(llvm::raw_ostream &OS, + const std::vector &Params) { + for (size_t I = 0, E = Params.size(); I != E; ++I) { + if (I) + OS << ", "; + OS << Params.at(I); + } } -/// Given a declaration \p D, return a human-readable string representing the -/// scope in which it is declared. If the declaration is in the global scope, -/// return the string "global namespace". -static llvm::Optional getScopeName(const Decl *D) { - const DeclContext *DC = D->getDeclContext(); +static std::vector +fetchTemplateParameters(const TemplateParameterList *Params, + const PrintingPolicy &PP) { + assert(Params); + std::vector TempParameters; + + for (const Decl *Param : *Params) { + HoverInfo::Param P; + P.Type.emplace(); + if (const auto TTP = dyn_cast(Param)) { + P.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class"; + if (TTP->isParameterPack()) + *P.Type += "..."; + + if (!TTP->getName().empty()) + P.Name = TTP->getNameAsString(); + if (TTP->hasDefaultArgument()) + P.Default = TTP->getDefaultArgument().getAsString(PP); + } else if (const auto NTTP = dyn_cast(Param)) { + if (IdentifierInfo *II = NTTP->getIdentifier()) + P.Name = II->getName().str(); + + llvm::raw_string_ostream Out(*P.Type); + NTTP->getType().print(Out, PP); + if (NTTP->isParameterPack()) + Out << "..."; + + if (NTTP->hasDefaultArgument()) { + P.Default.emplace(); + llvm::raw_string_ostream Out(*P.Default); + NTTP->getDefaultArgument()->printPretty(Out, nullptr, PP); + } + } else if (const auto TTPD = dyn_cast(Param)) { + llvm::raw_string_ostream OS(*P.Type); + OS << "template <"; + printParams(OS, + fetchTemplateParameters(TTPD->getTemplateParameters(), PP)); + OS << "> class"; // FIXME: TemplateTemplateParameter doesn't store the + // info on whether this param was a "typename" or + // "class". + if (!TTPD->getName().empty()) + P.Name = TTPD->getNameAsString(); + if (TTPD->hasDefaultArgument()) { + P.Default.emplace(); + llvm::raw_string_ostream Out(*P.Default); + TTPD->getDefaultArgument().getArgument().print(PP, Out); + } + } + TempParameters.push_back(std::move(P)); + } - if (isa(DC)) - return std::string("global namespace"); - if (const TypeDecl *TD = dyn_cast(DC)) - return typeDeclToString(TD); - else if (const NamespaceDecl *ND = dyn_cast(DC)) - return namedDeclQualifiedName(ND, "namespace"); - else if (const FunctionDecl *FD = dyn_cast(DC)) - return namedDeclQualifiedName(FD, "function"); + return TempParameters; +} - return None; +static llvm::Optional getTokenRange(SourceLocation Loc, + const ASTContext &Ctx) { + if (!Loc.isValid()) + return llvm::None; + SourceLocation End = Lexer::getLocForEndOfToken( + Loc, 0, Ctx.getSourceManager(), Ctx.getLangOpts()); + if (!End.isValid()) + return llvm::None; + return halfOpenToRange(Ctx.getSourceManager(), + CharSourceRange::getCharRange(Loc, End)); } /// Generate a \p Hover object given the declaration \p D. -static Hover getHoverContents(const Decl *D) { - Hover H; - llvm::Optional NamedScope = getScopeName(D); - - // Generate the "Declared in" section. - if (NamedScope) { - assert(!NamedScope->empty()); - - H.contents.value += "Declared in "; - H.contents.value += *NamedScope; - H.contents.value += "\n\n"; +static HoverInfo getHoverContents(const Decl *D) { + HoverInfo HI; + const ASTContext &Ctx = D->getASTContext(); + + HI.NamespaceScope = getNamespaceScope(D); + if (!HI.NamespaceScope->empty()) + HI.NamespaceScope->append("::"); + HI.LocalScope = getLocalScope(D); + if (!HI.LocalScope.empty()) + HI.LocalScope.append("::"); + + PrintingPolicy Policy = printingPolicyForDecls(Ctx.getPrintingPolicy()); + if (const NamedDecl *ND = llvm::dyn_cast(D)) { + HI.Documentation = getDeclComment(Ctx, *ND); + HI.Name = printName(Ctx, *ND); } - // We want to include the template in the Hover. - if (TemplateDecl *TD = D->getDescribedTemplate()) - D = TD; - - std::string DeclText; - llvm::raw_string_ostream OS(DeclText); - - PrintingPolicy Policy = - printingPolicyForDecls(D->getASTContext().getPrintingPolicy()); + HI.Kind = indexSymbolKindToSymbolKind(index::getSymbolInfo(D).Kind); - D->print(OS, Policy); + // Fill in template params. + if (const TemplateDecl *TD = D->getDescribedTemplate()) { + HI.TemplateParameters = + fetchTemplateParameters(TD->getTemplateParameters(), Policy); + D = TD; + } else if (const FunctionDecl *FD = D->getAsFunction()) { + if (const auto FTD = FD->getDescribedTemplate()) { + HI.TemplateParameters = + fetchTemplateParameters(FTD->getTemplateParameters(), Policy); + D = FTD; + } + } - OS.flush(); + // Fill in types and params. + if (const FunctionDecl *FD = D->getAsFunction()) { + HI.ReturnType.emplace(); + llvm::raw_string_ostream OS(*HI.ReturnType); + FD->getReturnType().print(OS, Policy); + + HI.Type.emplace(); + llvm::raw_string_ostream TypeOS(*HI.Type); + FD->getReturnType().print(TypeOS, Policy); + TypeOS << '('; + + HI.Parameters.emplace(); + for (const ParmVarDecl *PVD : FD->parameters()) { + if (HI.Parameters->size()) + TypeOS << ", "; + HI.Parameters->emplace_back(); + auto &P = HI.Parameters->back(); + if (!PVD->getType().isNull()) { + P.Type.emplace(); + llvm::raw_string_ostream OS(*P.Type); + PVD->getType().print(OS, Policy); + PVD->getType().print(TypeOS, Policy); + } else { + std::string Param; + llvm::raw_string_ostream OS(Param); + PVD->dump(OS); + OS.flush(); + elog("Got param with null type: {0}", Param); + } + if (!PVD->getName().empty()) + P.Name = PVD->getNameAsString(); + if (PVD->hasDefaultArg()) { + P.Default.emplace(); + llvm::raw_string_ostream Out(*P.Default); + PVD->getDefaultArg()->printPretty(Out, nullptr, Policy); + } + } + TypeOS << ')'; + // FIXME: handle variadics. + } else if (const auto *VD = dyn_cast(D)) { + // FIXME: Currently lambdas are also handled as ValueDecls, they should be + // more similar to functions. + HI.Type.emplace(); + llvm::raw_string_ostream OS(*HI.Type); + VD->getType().print(OS, Policy); + } - H.contents.value += DeclText; - return H; + HI.Definition = printDefinition(D); + return HI; } /// Generate a \p Hover object given the type \p T. -static Hover getHoverContents(QualType T, ASTContext &ASTCtx) { - Hover H; - std::string TypeText; - llvm::raw_string_ostream OS(TypeText); +static HoverInfo getHoverContents(QualType T, const Decl *D, + ASTContext &ASTCtx) { + HoverInfo HI; + llvm::raw_string_ostream OS(HI.Name); PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy()); T.print(OS, Policy); - OS.flush(); - H.contents.value += TypeText; - return H; + + if (D) + HI.Kind = indexSymbolKindToSymbolKind(index::getSymbolInfo(D).Kind); + return HI; } /// Generate a \p Hover object given the macro \p MacroDecl. -static Hover getHoverContents(MacroDecl Decl, ParsedAST &AST) { - SourceManager &SM = AST.getASTContext().getSourceManager(); - std::string Definition = Decl.Name; +static HoverInfo getHoverContents(MacroDecl Decl, ParsedAST &AST) { + HoverInfo HI; + SourceManager &SM = AST.getSourceManager(); + HI.Name = Decl.Name; + HI.Kind = indexSymbolKindToSymbolKind( + index::getSymbolInfoForMacro(*Decl.Info).Kind); + // FIXME: Populate documentation + // FIXME: Pupulate parameters // Try to get the full definition, not just the name SourceLocation StartLoc = Decl.Info->getDefinitionLoc(); @@ -586,14 +739,12 @@ static Hover getHoverContents(MacroDecl Decl, ParsedAST &AST) { unsigned StartOffset = SM.getFileOffset(StartLoc); unsigned EndOffset = SM.getFileOffset(EndLoc); if (EndOffset <= Buffer.size() && StartOffset < EndOffset) - Definition = Buffer.substr(StartOffset, EndOffset - StartOffset).str(); + HI.Definition = + ("#define " + Buffer.substr(StartOffset, EndOffset - StartOffset)) + .str(); } } - - Hover H; - H.contents.kind = MarkupKind::PlainText; - H.contents.value = "#define " + Definition; - return H; + return HI; } namespace { @@ -607,14 +758,11 @@ namespace { /// a deduced type set. The AST should be improved to simplify this scenario. class DeducedTypeVisitor : public RecursiveASTVisitor { SourceLocation SearchedLocation; - llvm::Optional DeducedType; public: DeducedTypeVisitor(SourceLocation SearchedLocation) : SearchedLocation(SearchedLocation) {} - llvm::Optional getDeducedType() { return DeducedType; } - // Handle auto initializers: //- auto i = 1; //- decltype(auto) i = 1; @@ -626,8 +774,10 @@ class DeducedTypeVisitor : public RecursiveASTVisitor { return true; if (auto *AT = D->getType()->getContainedAutoType()) { - if (!AT->getDeducedType().isNull()) + if (!AT->getDeducedType().isNull()) { DeducedType = AT->getDeducedType(); + this->D = D; + } } return true; } @@ -655,13 +805,17 @@ class DeducedTypeVisitor : public RecursiveASTVisitor { const AutoType *AT = D->getReturnType()->getContainedAutoType(); if (AT && !AT->getDeducedType().isNull()) { DeducedType = AT->getDeducedType(); + this->D = D; } else if (auto DT = dyn_cast(D->getReturnType())) { // auto in a trailing return type just points to a DecltypeType and // getContainedAutoType does not unwrap it. - if (!DT->getUnderlyingType().isNull()) + if (!DT->getUnderlyingType().isNull()) { DeducedType = DT->getUnderlyingType(); + this->D = D; + } } else if (!D->getReturnType().isNull()) { DeducedType = D->getReturnType(); + this->D = D; } return true; } @@ -680,16 +834,19 @@ class DeducedTypeVisitor : public RecursiveASTVisitor { const DecltypeType *DT = dyn_cast(TL.getTypePtr()); while (DT && !DT->getUnderlyingType().isNull()) { DeducedType = DT->getUnderlyingType(); - DT = dyn_cast(DeducedType->getTypePtr()); + D = DT->getAsTagDecl(); + DT = dyn_cast(DeducedType.getTypePtr()); } return true; } + + QualType DeducedType; + const Decl *D = nullptr; }; } // namespace /// Retrieves the deduced type at a given location (auto, decltype). -llvm::Optional getDeducedType(ParsedAST &AST, - SourceLocation SourceLocationBeg) { +bool hasDeducedType(ParsedAST &AST, SourceLocation SourceLocationBeg) { Token Tok; auto &ASTCtx = AST.getASTContext(); // Only try to find a deduced type if the token is auto or decltype. @@ -697,35 +854,45 @@ llvm::Optional getDeducedType(ParsedAST &AST, Lexer::getRawToken(SourceLocationBeg, Tok, ASTCtx.getSourceManager(), ASTCtx.getLangOpts(), false) || !Tok.is(tok::raw_identifier)) { - return {}; + return false; } AST.getPreprocessor().LookUpIdentifierInfo(Tok); if (!(Tok.is(tok::kw_auto) || Tok.is(tok::kw_decltype))) - return {}; - - DeducedTypeVisitor V(SourceLocationBeg); - V.TraverseAST(AST.getASTContext()); - return V.getDeducedType(); + return false; + return true; } -llvm::Optional getHover(ParsedAST &AST, Position Pos) { - const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); - SourceLocation SourceLocationBeg = - getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID()); +llvm::Optional getHover(ParsedAST &AST, Position Pos, + format::FormatStyle Style) { + llvm::Optional HI; + SourceLocation SourceLocationBeg = getBeginningOfIdentifier( + AST, Pos, AST.getSourceManager().getMainFileID()); // Identified symbols at a specific position. auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg); if (!Symbols.Macros.empty()) - return getHoverContents(Symbols.Macros[0], AST); - - if (!Symbols.Decls.empty()) - return getHoverContents(Symbols.Decls[0]); + HI = getHoverContents(Symbols.Macros[0], AST); + else if (!Symbols.Decls.empty()) + HI = getHoverContents(Symbols.Decls[0]); + else { + if (!hasDeducedType(AST, SourceLocationBeg)) + return None; + + DeducedTypeVisitor V(SourceLocationBeg); + V.TraverseAST(AST.getASTContext()); + if (V.DeducedType.isNull()) + return None; + HI = getHoverContents(V.DeducedType, V.D, AST.getASTContext()); + } - auto DeducedType = getDeducedType(AST, SourceLocationBeg); - if (DeducedType && !DeducedType->isNull()) - return getHoverContents(*DeducedType, AST.getASTContext()); + auto Replacements = format::reformat( + Style, HI->Definition, tooling::Range(0, HI->Definition.size())); + if (auto Formatted = + tooling::applyAllReplacements(HI->Definition, Replacements)) + HI->Definition = *Formatted; - return None; + HI->SymRange = getTokenRange(SourceLocationBeg, AST.getASTContext()); + return HI; } std::vector findReferences(ParsedAST &AST, Position Pos, @@ -733,7 +900,7 @@ std::vector findReferences(ParsedAST &AST, Position Pos, if (!Limit) Limit = std::numeric_limits::max(); std::vector Results; - const SourceManager &SM = AST.getASTContext().getSourceManager(); + const SourceManager &SM = AST.getSourceManager(); auto MainFilePath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM); if (!MainFilePath) { @@ -748,7 +915,7 @@ std::vector findReferences(ParsedAST &AST, Position Pos, auto MainFileRefs = findRefs(Symbols.Decls, AST); for (const auto &Ref : MainFileRefs) { Location Result; - Result.range = getTokenRange(AST, Ref.Loc); + Result.range = getTokenRange(AST.getASTContext(), Ref.Loc); Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath); Results.push_back(std::move(Result)); } @@ -782,7 +949,7 @@ std::vector findReferences(ParsedAST &AST, Position Pos, } std::vector getSymbolInfo(ParsedAST &AST, Position Pos) { - const SourceManager &SM = AST.getASTContext().getSourceManager(); + const SourceManager &SM = AST.getSourceManager(); auto Loc = getBeginningOfIdentifier(AST, Pos, SM.getMainFileID()); auto Symbols = getSymbolAtPosition(AST, Loc); @@ -917,10 +1084,8 @@ getTypeAncestors(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx, } const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos) { - ASTContext &ASTCtx = AST.getASTContext(); - const SourceManager &SourceMgr = ASTCtx.getSourceManager(); - SourceLocation SourceLocationBeg = - getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID()); + SourceLocation SourceLocationBeg = getBeginningOfIdentifier( + AST, Pos, AST.getSourceManager().getMainFileID()); IdentifiedSymbol Symbols = getSymbolAtPosition(AST, SourceLocationBeg); if (Symbols.Decls.empty()) return nullptr; @@ -991,5 +1156,40 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, return Result; } +FormattedString HoverInfo::present() const { + FormattedString Output; + if (NamespaceScope) { + Output.appendText("Declared in"); + // Drop trailing "::". + if (!LocalScope.empty()) + Output.appendInlineCode(llvm::StringRef(LocalScope).drop_back(2)); + else if (NamespaceScope->empty()) + Output.appendInlineCode("global namespace"); + else + Output.appendInlineCode(llvm::StringRef(*NamespaceScope).drop_back(2)); + } + + if (!Definition.empty()) { + Output.appendCodeBlock(Definition); + } else { + // Builtin types + Output.appendCodeBlock(Name); + } + return Output; +} + +llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, + const HoverInfo::Param &P) { + std::vector Output; + if (P.Type) + Output.push_back(*P.Type); + if (P.Name) + Output.push_back(*P.Name); + OS << llvm::join(Output, " "); + if (P.Default) + OS << " = " << *P.Default; + return OS; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h index 008bba50a0920..180e3f755c3d9 100644 --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -14,9 +14,13 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H #include "ClangdUnit.h" +#include "FormattedString.h" #include "Protocol.h" #include "index/Index.h" +#include "index/SymbolLocation.h" +#include "clang/Index/IndexSymbol.h" #include "llvm/ADT/Optional.h" +#include "llvm/Support/raw_ostream.h" #include namespace clang { @@ -46,8 +50,73 @@ std::vector locateSymbolAt(ParsedAST &AST, Position Pos, std::vector findDocumentHighlights(ParsedAST &AST, Position Pos); +/// Contains detailed information about a Symbol. Especially useful when +/// generating hover responses. It can be rendered as a hover panel, or +/// embedding clients can use the structured information to provide their own +/// UI. +struct HoverInfo { + /// Represents parameters of a function, a template or a macro. + /// For example: + /// - void foo(ParamType Name = DefaultValue) + /// - #define FOO(Name) + /// - template class Foo {}; + struct Param { + /// The pretty-printed parameter type, e.g. "int", or "typename" (in + /// TemplateParameters) + llvm::Optional Type; + /// None for unnamed parameters. + llvm::Optional Name; + /// None if no default is provided. + llvm::Optional Default; + }; + + /// For a variable named Bar, declared in clang::clangd::Foo::getFoo the + /// following fields will hold: + /// - NamespaceScope: clang::clangd:: + /// - LocalScope: Foo::getFoo:: + /// - Name: Bar + + /// Scopes might be None in cases where they don't make sense, e.g. macros and + /// auto/decltype. + /// Contains all of the enclosing namespaces, empty string means global + /// namespace. + llvm::Optional NamespaceScope; + /// Remaining named contexts in symbol's qualified name, empty string means + /// symbol is not local. + std::string LocalScope; + /// Name of the symbol, does not contain any "::". + std::string Name; + llvm::Optional SymRange; + /// Scope containing the symbol. e.g, "global namespace", "function x::Y" + /// - None for deduced types, e.g "auto", "decltype" keywords. + SymbolKind Kind; + std::string Documentation; + /// Source code containing the definition of the symbol. + std::string Definition; + + /// Pretty-printed variable type. + /// Set only for variables. + llvm::Optional Type; + /// Set for functions and lambadas. + llvm::Optional ReturnType; + /// Set for functions, lambdas and macros with parameters. + llvm::Optional> Parameters; + /// Set for all templates(function, class, variable). + llvm::Optional> TemplateParameters; + + /// Produce a user-readable information. + FormattedString present() const; +}; +llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &); +inline bool operator==(const HoverInfo::Param &LHS, + const HoverInfo::Param &RHS) { + return std::tie(LHS.Type, LHS.Name, LHS.Default) == + std::tie(RHS.Type, RHS.Name, RHS.Default); +} + /// Get the hover information when hovering at \p Pos. -llvm::Optional getHover(ParsedAST &AST, Position Pos); +llvm::Optional getHover(ParsedAST &AST, Position Pos, + format::FormatStyle Style); /// Returns reference locations of the symbol at a specified \p Pos. /// \p Limit limits the number of results returned (0 means no limit). diff --git a/clang-tools-extra/clangd/clients/clangd-vscode/package-lock.json b/clang-tools-extra/clangd/clients/clangd-vscode/package-lock.json index 47373624481ae..8108e789b2e36 100644 --- a/clang-tools-extra/clangd/clients/clangd-vscode/package-lock.json +++ b/clang-tools-extra/clangd/clients/clangd-vscode/package-lock.json @@ -1,6 +1,6 @@ { "name": "vscode-clangd", - "version": "0.0.12", + "version": "0.0.13", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/clang-tools-extra/clangd/clients/clangd-vscode/package.json b/clang-tools-extra/clangd/clients/clangd-vscode/package.json index b8125808d53af..1a423292d8afd 100644 --- a/clang-tools-extra/clangd/clients/clangd-vscode/package.json +++ b/clang-tools-extra/clangd/clients/clangd-vscode/package.json @@ -2,7 +2,7 @@ "name": "vscode-clangd", "displayName": "vscode-clangd", "description": "Clang Language Server", - "version": "0.0.12", + "version": "0.0.13", "publisher": "llvm-vs-code-extensions", "homepage": "https://clang.llvm.org/extra/clangd.html", "engines": { diff --git a/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts b/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts index 2cb97d97ab70a..7b80967151883 100644 --- a/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts +++ b/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts @@ -1,6 +1,5 @@ import * as vscode from 'vscode'; import * as vscodelc from 'vscode-languageclient'; -import { realpathSync } from 'fs'; /** * Method to get workspace configuration option @@ -87,17 +86,6 @@ export function activate(context: vscode.ExtensionContext) { fileEvents: vscode.workspace.createFileSystemWatcher(filePattern) }, initializationOptions: { clangdFileStatus: true }, - // Resolve symlinks for all files provided by clangd. - // This is a workaround for a bazel + clangd issue - bazel produces a symlink tree to build in, - // and when navigating to the included file, clangd passes its path inside the symlink tree - // rather than its filesystem path. - // FIXME: remove this once clangd knows enough about bazel to resolve the - // symlinks where needed (or if this causes problems for other workflows). - uriConverters: { - code2Protocol: (value: vscode.Uri) => value.toString(), - protocol2Code: (value: string) => - vscode.Uri.file(realpathSync(vscode.Uri.parse(value).fsPath)) - }, // Do not switch to output window when clangd returns output revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never }; diff --git a/clang-tools-extra/clangd/include-mapping/gen_std.py b/clang-tools-extra/clangd/include-mapping/gen_std.py index bfa6d0d49224b..c5824a0c42efa 100755 --- a/clang-tools-extra/clangd/include-mapping/gen_std.py +++ b/clang-tools-extra/clangd/include-mapping/gen_std.py @@ -84,10 +84,9 @@ def ParseSymbolPage(symbol_page_html, symbol_name): for row in table.select('tr'): if HasClass(row, 't-dcl', 't-dsc'): was_decl = True - # Declaration is in the first cell. - text = row.find('td').text - # Decl may not be for the symbol name we're looking for. - if not re.search("\\b%s\\b" % symbol_name, text): + # Symbols are in the first cell. + found_symbols = row.find('td').stripped_strings + if not symbol_name in found_symbols: continue headers.update(current_headers) elif HasClass(row, 't-dsc-header'): diff --git a/clang-tools-extra/clangd/include-mapping/test.py b/clang-tools-extra/clangd/include-mapping/test.py index 107257698d35b..3f17b53189c11 100755 --- a/clang-tools-extra/clangd/include-mapping/test.py +++ b/clang-tools-extra/clangd/include-mapping/test.py @@ -85,7 +85,11 @@ def testParseSymbolPage_MulHeaders(self): - void foo() + + void + foo + () + this is matched @@ -108,7 +112,11 @@ def testParseSymbolPage_MulHeadersInSameDiv(self): - void foo() + + void + foo + () + this is matched @@ -116,6 +124,32 @@ def testParseSymbolPage_MulHeadersInSameDiv(self): self.assertEqual(ParseSymbolPage(html, "foo"), set(['', ''])) + def testParseSymbolPage_MulSymbolsInSameTd(self): + # defined in header + # int8_t + # int16_t + html = """ + + + + + + + + + +
+ Defined in header <cstdint>
+
+ int8_t + int16_t + this is matched
+""" + self.assertEqual(ParseSymbolPage(html, "int8_t"), + set([''])) + self.assertEqual(ParseSymbolPage(html, "int16_t"), + set([''])) + if __name__ == '__main__': unittest.main() diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp index 25bbffb13dcd9..017547ce05755 100644 --- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp +++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp @@ -88,30 +88,17 @@ collectIWYUHeaderMaps(CanonicalIncludes *Includes) { void addSystemHeadersMapping(CanonicalIncludes *Includes) { static const std::vector> SymbolMap = { - // Map symbols in to their preferred includes. - {"std::basic_filebuf", ""}, - {"std::filebuf", ""}, - {"std::wfilebuf", ""}, - {"std::basic_istream", ""}, - {"std::istream", ""}, - {"std::wistream", ""}, - {"std::basic_ostream", ""}, - {"std::ostream", ""}, - {"std::wostream", ""}, - {"std::uint_least16_t", ""}, // redeclares these - {"std::uint_least32_t", ""}, #define SYMBOL(Name, NameSpace, Header) { #NameSpace#Name, #Header }, #include "StdSymbolMap.inc" #undef SYMBOL }; + for (const auto &Pair : SymbolMap) Includes->addSymbolMapping(Pair.first, Pair.second); // FIXME: remove the std header mapping once we support ambiguous symbols, now // it serves as a fallback to disambiguate: // - symbols with mulitiple headers (e.g. std::move) - // - symbols with a primary template in one header and a specialization in - // another (std::abs) static const std::vector> SystemHeaderMap = { {"include/__stddef_max_align_t.h", ""}, diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index 0a271a7cf7845..87a777e72df88 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H #include "Ref.h" +#include "Relation.h" #include "Symbol.h" #include "SymbolID.h" #include "llvm/ADT/DenseSet.h" diff --git a/clang-tools-extra/clangd/index/Relation.cpp b/clang-tools-extra/clangd/index/Relation.cpp new file mode 100644 index 0000000000000..e46aa23664151 --- /dev/null +++ b/clang-tools-extra/clangd/index/Relation.cpp @@ -0,0 +1,40 @@ +//===--- Relation.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 +// +//===----------------------------------------------------------------------===// + +#include "Relation.h" + +#include + +namespace clang { +namespace clangd { + +llvm::iterator_range +RelationSlab::lookup(const SymbolID &Subject, + index::SymbolRole Predicate) const { + auto IterPair = std::equal_range(Relations.begin(), Relations.end(), + Relation{Subject, Predicate, SymbolID{}}, + [](const Relation &A, const Relation &B) { + return std::tie(A.Subject, A.Predicate) < + std::tie(B.Subject, B.Predicate); + }); + return {IterPair.first, IterPair.second}; +} + +RelationSlab RelationSlab::Builder::build() && { + // Sort in SPO order. + std::sort(Relations.begin(), Relations.end()); + + // Remove duplicates. + Relations.erase(std::unique(Relations.begin(), Relations.end()), + Relations.end()); + + return RelationSlab{std::move(Relations)}; +} + +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/index/Relation.h b/clang-tools-extra/clangd/index/Relation.h new file mode 100644 index 0000000000000..deca3d7a2e7e0 --- /dev/null +++ b/clang-tools-extra/clangd/index/Relation.h @@ -0,0 +1,88 @@ +//===--- Relation.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_INDEX_RELATION_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H + +#include "SymbolID.h" +#include "SymbolLocation.h" +#include "clang/Index/IndexSymbol.h" +#include "llvm/ADT/iterator_range.h" +#include +#include + +namespace clang { +namespace clangd { + +/// Represents a relation between two symbols. +/// For an example "A is a base class of B" may be represented +/// as { Subject = A, Predicate = RelationBaseOf, Object = B }. +struct Relation { + SymbolID Subject; + index::SymbolRole Predicate; + SymbolID Object; + + bool operator==(const Relation &Other) const { + return std::tie(Subject, Predicate, Object) == + std::tie(Other.Subject, Other.Predicate, Other.Object); + } + // SPO order + bool operator<(const Relation &Other) const { + return std::tie(Subject, Predicate, Object) < + std::tie(Other.Subject, Other.Predicate, Other.Object); + } +}; + +class RelationSlab { +public: + using value_type = Relation; + using const_iterator = std::vector::const_iterator; + using iterator = const_iterator; + + RelationSlab() = default; + RelationSlab(RelationSlab &&Slab) = default; + RelationSlab &operator=(RelationSlab &&RHS) = default; + + const_iterator begin() const { return Relations.begin(); } + const_iterator end() const { return Relations.end(); } + size_t size() const { return Relations.size(); } + bool empty() const { return Relations.empty(); } + + size_t bytes() const { + return sizeof(*this) + sizeof(value_type) * Relations.capacity(); + } + + /// Lookup all relations matching the given subject and predicate. + llvm::iterator_range lookup(const SymbolID &Subject, + index::SymbolRole Predicate) const; + + /// RelationSlab::Builder is a mutable container that can 'freeze' to + /// RelationSlab. + class Builder { + public: + /// Adds a relation to the slab. + void insert(const Relation &R) { Relations.push_back(R); } + + /// Consumes the builder to finalize the slab. + RelationSlab build() &&; + + private: + std::vector Relations; + }; + +private: + RelationSlab(std::vector Relations) + : Relations(std::move(Relations)) {} + + std::vector Relations; +}; + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H diff --git a/clang-tools-extra/clangd/index/Serialization.cpp b/clang-tools-extra/clangd/index/Serialization.cpp index 12993eb631ca6..521e5bd04ad71 100644 --- a/clang-tools-extra/clangd/index/Serialization.cpp +++ b/clang-tools-extra/clangd/index/Serialization.cpp @@ -24,6 +24,29 @@ llvm::Error makeError(const llvm::Twine &Msg) { return llvm::make_error(Msg, llvm::inconvertibleErrorCode()); } +} // namespace + +RelationKind symbolRoleToRelationKind(index::SymbolRole Role) { + // SymbolRole is used to record relations in the index. + // Only handle the relations we actually store currently. + // If we start storing more relations, this list can be expanded. + switch (Role) { + case index::SymbolRole::RelationBaseOf: + return RelationKind::BaseOf; + default: + llvm_unreachable("Unsupported symbol role"); + } +} + +index::SymbolRole relationKindToSymbolRole(RelationKind Kind) { + switch (Kind) { + case RelationKind::BaseOf: + return index::SymbolRole::RelationBaseOf; + } + llvm_unreachable("Invalid relation kind"); +} + +namespace { // IO PRIMITIVES // We use little-endian 32 bit ints, sometimes with variable-length encoding. @@ -358,6 +381,28 @@ readRefs(Reader &Data, llvm::ArrayRef Strings) { return Result; } +// RELATIONS ENCODING +// A relations section is a flat list of relations. Each relation has: +// - SymbolID (subject): 8 bytes +// - relation kind (predicate): 1 byte +// - SymbolID (object): 8 bytes +// In the future, we might prefer a packed representation if the need arises. + +void writeRelation(const Relation &R, llvm::raw_ostream &OS) { + OS << R.Subject.raw(); + RelationKind Kind = symbolRoleToRelationKind(R.Predicate); + OS.write(static_cast(Kind)); + OS << R.Object.raw(); +} + +Relation readRelation(Reader &Data) { + SymbolID Subject = Data.consumeID(); + index::SymbolRole Predicate = + relationKindToSymbolRole(static_cast(Data.consume8())); + SymbolID Object = Data.consumeID(); + return {Subject, Predicate, Object}; +} + // FILE ENCODING // A file is a RIFF chunk with type 'CdIx'. // It contains the sections: @@ -434,6 +479,17 @@ llvm::Expected readRIFF(llvm::StringRef Data) { return makeError("malformed or truncated refs"); Result.Refs = std::move(Refs).build(); } + if (Chunks.count("rela")) { + Reader RelationsReader(Chunks.lookup("rela")); + RelationSlab::Builder Relations; + while (!RelationsReader.eof()) { + auto Relation = readRelation(RelationsReader); + Relations.insert(Relation); + } + if (RelationsReader.err()) + return makeError("malformed or truncated relations"); + Result.Relations = std::move(Relations).build(); + } return std::move(Result); } @@ -483,6 +539,14 @@ void writeRIFF(const IndexFileOut &Data, llvm::raw_ostream &OS) { } } + std::vector Relations; + if (Data.Relations) { + for (const auto &Relation : *Data.Relations) { + Relations.emplace_back(Relation); + // No strings to be interned in relations. + } + } + std::string StringSection; { llvm::raw_string_ostream StringOS(StringSection); @@ -508,6 +572,16 @@ void writeRIFF(const IndexFileOut &Data, llvm::raw_ostream &OS) { RIFF.Chunks.push_back({riff::fourCC("refs"), RefsSection}); } + std::string RelationSection; + if (Data.Relations) { + { + llvm::raw_string_ostream RelationOS{RelationSection}; + for (const auto &Relation : Relations) + writeRelation(Relation, RelationOS); + } + RIFF.Chunks.push_back({riff::fourCC("rela"), RelationSection}); + } + std::string SrcsSection; { { @@ -561,6 +635,7 @@ std::unique_ptr loadIndex(llvm::StringRef SymbolFilename, SymbolSlab Symbols; RefSlab Refs; + RelationSlab Relations; { trace::Span Tracer("ParseIndex"); if (auto I = readIndexFile(Buffer->get()->getBuffer())) { @@ -568,6 +643,8 @@ std::unique_ptr loadIndex(llvm::StringRef SymbolFilename, Symbols = std::move(*I->Symbols); if (I->Refs) Refs = std::move(*I->Refs); + if (I->Relations) + Relations = std::move(*I->Relations); } else { llvm::errs() << "Bad Index: " << llvm::toString(I.takeError()) << "\n"; return nullptr; @@ -576,15 +653,17 @@ std::unique_ptr loadIndex(llvm::StringRef SymbolFilename, size_t NumSym = Symbols.size(); size_t NumRefs = Refs.numRefs(); + size_t NumRelations = Relations.size(); trace::Span Tracer("BuildIndex"); auto Index = UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs)) : MemIndex::build(std::move(Symbols), std::move(Refs)); vlog("Loaded {0} from {1} with estimated memory usage {2} bytes\n" " - number of symbols: {3}\n" - " - number of refs: {4}\n", + " - number of refs: {4}\n" + " - numnber of relations: {5}", UseDex ? "Dex" : "MemIndex", SymbolFilename, - Index->estimateMemoryUsage(), NumSym, NumRefs); + Index->estimateMemoryUsage(), NumSym, NumRefs, NumRelations); return Index; } diff --git a/clang-tools-extra/clangd/index/Serialization.h b/clang-tools-extra/clangd/index/Serialization.h index 3788693e04246..03e5e387c7bcd 100644 --- a/clang-tools-extra/clangd/index/Serialization.h +++ b/clang-tools-extra/clangd/index/Serialization.h @@ -41,6 +41,7 @@ enum class IndexFileFormat { struct IndexFileIn { llvm::Optional Symbols; llvm::Optional Refs; + llvm::Optional Relations; // Keys are URIs of the source files. llvm::Optional Sources; }; @@ -51,6 +52,7 @@ llvm::Expected readIndexFile(llvm::StringRef); struct IndexFileOut { const SymbolSlab *Symbols = nullptr; const RefSlab *Refs = nullptr; + const RelationSlab *Relations = nullptr; // Keys are URIs of the source files. const IncludeGraph *Sources = nullptr; // TODO: Support serializing Dex posting lists. @@ -59,7 +61,8 @@ struct IndexFileOut { IndexFileOut() = default; IndexFileOut(const IndexFileIn &I) : Symbols(I.Symbols ? I.Symbols.getPointer() : nullptr), - Refs(I.Refs ? I.Refs.getPointer() : nullptr) {} + Refs(I.Refs ? I.Refs.getPointer() : nullptr), + Relations(I.Relations ? I.Relations.getPointer() : nullptr) {} }; // Serializes an index file. llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const IndexFileOut &O); @@ -67,12 +70,18 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const IndexFileOut &O); // Convert a single symbol to YAML, a nice debug representation. std::string toYAML(const Symbol &); std::string toYAML(const std::pair> &); +std::string toYAML(const Relation &); // Build an in-memory static index from an index file. // The size should be relatively small, so data can be managed in memory. std::unique_ptr loadIndex(llvm::StringRef Filename, bool UseDex = true); +// Used for serializing SymbolRole as used in Relation. +enum class RelationKind : uint8_t { BaseOf }; +RelationKind symbolRoleToRelationKind(index::SymbolRole); +index::SymbolRole relationKindToSymbolRole(RelationKind); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index af1938dafa8ab..f7b027c3240b5 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -193,6 +193,11 @@ RefKind toRefKind(index::SymbolRoleSet Roles) { return static_cast(static_cast(RefKind::All) & Roles); } +bool shouldIndexRelation(const index::SymbolRelation &R) { + // We currently only index BaseOf relations, for type hierarchy subtypes. + return R.Roles & static_cast(index::SymbolRole::RelationBaseOf); +} + } // namespace SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {} @@ -291,6 +296,16 @@ bool SymbolCollector::handleDeclOccurence( SM.getFileID(SpellingLoc) == SM.getMainFileID()) ReferencedDecls.insert(ND); + auto ID = getSymbolID(ND); + if (!ID) + return true; + + // Note: we need to process relations for all decl occurrences, including + // refs, because the indexing code only populates relations for specific + // occurrences. For example, RelationBaseOf is only populated for the + // occurrence inside the base-specifier. + processRelations(*ND, *ID, Relations); + bool CollectRef = static_cast(Opts.RefFilter) & Roles; bool IsOnlyRef = !(Roles & (static_cast(index::SymbolRole::Declaration) | @@ -315,10 +330,6 @@ bool SymbolCollector::handleDeclOccurence( if (IsOnlyRef) return true; - auto ID = getSymbolID(ND); - if (!ID) - return true; - // FIXME: ObjCPropertyDecl are not properly indexed here: // - ObjCPropertyDecl may have an OrigD of ObjCPropertyImplDecl, which is // not a NamedDecl. @@ -338,6 +349,7 @@ bool SymbolCollector::handleDeclOccurence( if (Roles & static_cast(index::SymbolRole::Definition)) addDefinition(*OriginalDecl, *BasicSymbol); + return true; } @@ -416,8 +428,38 @@ bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name, return true; } -void SymbolCollector::setIncludeLocation(const Symbol &S, - SourceLocation Loc) { +void SymbolCollector::processRelations( + const NamedDecl &ND, const SymbolID &ID, + ArrayRef Relations) { + // Store subtype relations. + if (!dyn_cast(&ND)) + return; + + for (const auto &R : Relations) { + if (!shouldIndexRelation(R)) + continue; + + const Decl *Object = R.RelatedSymbol; + + auto ObjectID = getSymbolID(Object); + if (!ObjectID) + continue; + + // Record the relation. + // TODO: There may be cases where the object decl is not indexed for some + // reason. Those cases should probably be removed in due course, but for + // now there are two possible ways to handle it: + // (A) Avoid storing the relation in such cases. + // (B) Store it anyways. Clients will likely lookup() the SymbolID + // in the index and find nothing, but that's a situation they + // probably need to handle for other reasons anyways. + // We currently do (B) because it's simpler. + this->Relations.insert( + Relation{ID, index::SymbolRole::RelationBaseOf, *ObjectID}); + } +} + +void SymbolCollector::setIncludeLocation(const Symbol &S, SourceLocation Loc) { if (Opts.CollectIncludePath) if (shouldCollectIncludePath(S.SymInfo.Kind)) // Use the expansion location to get the #include header since this is @@ -681,7 +723,7 @@ static bool isErrorAboutInclude(llvm::StringRef Line) { if (!Line.consume_front("#")) return false; Line = Line.ltrim(); - if (! Line.startswith("error")) + if (!Line.startswith("error")) return false; return Line.contains_lower("includ"); // Matches "include" or "including". } @@ -689,7 +731,7 @@ static bool isErrorAboutInclude(llvm::StringRef Line) { bool SymbolCollector::isDontIncludeMeHeader(llvm::StringRef Content) { llvm::StringRef Line; // Only sniff up to 100 lines or 10KB. - Content = Content.take_front(100*100); + Content = Content.take_front(100 * 100); for (unsigned I = 0; I < 100 && !Content.empty(); ++I) { std::tie(Line, Content) = Content.split('\n'); if (isIf(Line) && isErrorAboutInclude(Content.split('\n').first)) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h index f746002bbd3e3..3c28a451406dd 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -110,6 +110,7 @@ class SymbolCollector : public index::IndexDataConsumer { SymbolSlab takeSymbols() { return std::move(Symbols).build(); } RefSlab takeRefs() { return std::move(Refs).build(); } + RelationSlab takeRelations() { return std::move(Relations).build(); } void finish() override; @@ -117,6 +118,8 @@ class SymbolCollector : public index::IndexDataConsumer { const Symbol *addDeclaration(const NamedDecl &, SymbolID, bool IsMainFileSymbol); void addDefinition(const NamedDecl &, const Symbol &DeclSymbol); + void processRelations(const NamedDecl &ND, const SymbolID &ID, + ArrayRef Relations); llvm::Optional getIncludeHeader(llvm::StringRef QName, FileID); bool isSelfContainedHeader(FileID); @@ -135,6 +138,8 @@ class SymbolCollector : public index::IndexDataConsumer { // Only symbols declared in preamble (from #include) and referenced from the // main file will be included. RefSlab::Builder Refs; + // All relations collected from the AST. + RelationSlab::Builder Relations; ASTContext *ASTCtx; std::shared_ptr PP; std::shared_ptr CompletionAllocator; diff --git a/clang-tools-extra/clangd/index/YAMLSerialization.cpp b/clang-tools-extra/clangd/index/YAMLSerialization.cpp index 6bf0cb789ca42..4e30abaa60e0e 100644 --- a/clang-tools-extra/clangd/index/YAMLSerialization.cpp +++ b/clang-tools-extra/clangd/index/YAMLSerialization.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "Index.h" +#include "Relation.h" #include "Serialization.h" #include "SymbolLocation.h" #include "SymbolOrigin.h" @@ -35,10 +36,11 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Ref) namespace { using RefBundle = std::pair>; -// This is a pale imitation of std::variant +// This is a pale imitation of std::variant struct VariantEntry { llvm::Optional Symbol; llvm::Optional Refs; + llvm::Optional Relation; }; // A class helps YAML to serialize the 32-bit encoded position (Line&Column), // as YAMLIO can't directly map bitfields. @@ -53,6 +55,8 @@ namespace yaml { using clang::clangd::Ref; using clang::clangd::RefKind; +using clang::clangd::Relation; +using clang::clangd::RelationKind; using clang::clangd::Symbol; using clang::clangd::SymbolID; using clang::clangd::SymbolLocation; @@ -60,6 +64,7 @@ using clang::clangd::SymbolOrigin; using clang::index::SymbolInfo; using clang::index::SymbolKind; using clang::index::SymbolLanguage; +using clang::index::SymbolRole; // Helper to (de)serialize the SymbolID. We serialize it as a hex string. struct NormalizedSymbolID { @@ -275,6 +280,37 @@ template <> struct MappingTraits { } }; +struct NormalizedSymbolRole { + NormalizedSymbolRole(IO &) {} + NormalizedSymbolRole(IO &IO, SymbolRole R) { + Kind = static_cast(clang::clangd::symbolRoleToRelationKind(R)); + } + + SymbolRole denormalize(IO &IO) { + return clang::clangd::relationKindToSymbolRole( + static_cast(Kind)); + } + + uint8_t Kind = 0; +}; + +template <> struct MappingTraits { + static void mapping(IO &IO, SymbolID &ID) { + MappingNormalization NSymbolID(IO, ID); + IO.mapRequired("ID", NSymbolID->HexString); + } +}; + +template <> struct MappingTraits { + static void mapping(IO &IO, Relation &Relation) { + MappingNormalization NRole( + IO, Relation.Predicate); + IO.mapRequired("Subject", Relation.Subject); + IO.mapRequired("Predicate", NRole->Kind); + IO.mapRequired("Object", Relation.Object); + } +}; + template <> struct MappingTraits { static void mapping(IO &IO, VariantEntry &Variant) { if (IO.mapTag("!Symbol", Variant.Symbol.hasValue())) { @@ -285,6 +321,10 @@ template <> struct MappingTraits { if (!IO.outputting()) Variant.Refs.emplace(); MappingTraits::mapping(IO, *Variant.Refs); + } else if (IO.mapTag("!Relations", Variant.Relation.hasValue())) { + if (!IO.outputting()) + Variant.Relation.emplace(); + MappingTraits::mapping(IO, *Variant.Relation); } } }; @@ -308,11 +348,18 @@ void writeYAML(const IndexFileOut &O, llvm::raw_ostream &OS) { Entry.Refs = Sym; Yout << Entry; } + if (O.Relations) + for (auto &R : *O.Relations) { + VariantEntry Entry; + Entry.Relation = R; + Yout << Entry; + } } llvm::Expected readYAML(llvm::StringRef Data) { SymbolSlab::Builder Symbols; RefSlab::Builder Refs; + RelationSlab::Builder Relations; llvm::BumpPtrAllocator Arena; // store the underlying data of Position::FileURI. llvm::UniqueStringSaver Strings(Arena); @@ -329,12 +376,15 @@ llvm::Expected readYAML(llvm::StringRef Data) { if (Variant.Refs) for (const auto &Ref : Variant.Refs->second) Refs.insert(Variant.Refs->first, Ref); + if (Variant.Relation) + Relations.insert(*Variant.Relation); Yin.nextDocument(); } IndexFileIn Result; Result.Symbols.emplace(std::move(Symbols).build()); Result.Refs.emplace(std::move(Refs).build()); + Result.Relations.emplace(std::move(Relations).build()); return std::move(Result); } @@ -360,5 +410,16 @@ std::string toYAML(const std::pair> &Data) { return Buf; } +std::string toYAML(const Relation &R) { + std::string Buf; + { + llvm::raw_string_ostream OS(Buf); + llvm::yaml::Output Yout(OS); + Relation Rel = R; // copy: Yout<< requires mutability. + Yout << Rel; + } + return Buf; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 98d6fd488bb39..8b21e635c9557 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -1,3 +1,11 @@ +//===--- Rename.cpp - Symbol-rename refactorings -----------------*- 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 +// +//===----------------------------------------------------------------------===// + #include "refactor/Rename.h" #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h" #include "clang/Tooling/Refactoring/Rename/RenamingAction.h" @@ -45,10 +53,9 @@ renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos, llvm::StringRef NewName) { RefactoringResultCollector ResultCollector; ASTContext &ASTCtx = AST.getASTContext(); - const SourceManager &SourceMgr = ASTCtx.getSourceManager(); - SourceLocation SourceLocationBeg = - clangd::getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID()); - tooling::RefactoringRuleContext Context(ASTCtx.getSourceManager()); + SourceLocation SourceLocationBeg = clangd::getBeginningOfIdentifier( + AST, Pos, AST.getSourceManager().getMainFileID()); + tooling::RefactoringRuleContext Context(AST.getSourceManager()); Context.setASTContext(ASTCtx); auto Rename = clang::tooling::RenameOccurrences::initiate( Context, SourceRange(SourceLocationBeg), NewName); @@ -66,7 +73,7 @@ renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos, // Right now we only support renaming the main file, so we // drop replacements not for the main file. In the future, we might // also support rename with wider scope. - // Rename sometimes returns duplicate edits (which is a bug). A side-effect of + // Rename sometimes returns duplicate edits (which is a bug). A side-effect of // adding them to a single Replacements object is these are deduplicated. for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) { for (const auto &Rep : Change.getReplacements()) { diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp index 34634e64b6f97..6a19751e375de 100644 --- a/clang-tools-extra/clangd/refactor/Tweak.cpp +++ b/clang-tools-extra/clangd/refactor/Tweak.cpp @@ -41,7 +41,7 @@ void validateRegistry() { Tweak::Selection::Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd) : AST(AST), ASTSelection(AST.getASTContext(), RangeBegin, RangeEnd) { - auto &SM = AST.getASTContext().getSourceManager(); + auto &SM = AST.getSourceManager(); Code = SM.getBufferData(SM.getMainFileID()); Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin); } diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt index 837853139689a..d8bb7bc4bf4cc 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangDaemonTweaks OBJECT LINK_LIBS clangAST + clangBasic clangDaemon clangToolingCore ) diff --git a/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp b/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp index e3eaba501922e..7feadd1eb7854 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp @@ -82,15 +82,14 @@ bool RawStringLiteral::prepare(const Selection &Inputs) { return false; Str = dyn_cast_or_null(N->ASTNode.get()); return Str && - isNormalString(*Str, Inputs.Cursor, - Inputs.AST.getASTContext().getSourceManager()) && + isNormalString(*Str, Inputs.Cursor, Inputs.AST.getSourceManager()) && needsRaw(Str->getBytes()) && canBeRaw(Str->getBytes()); } Expected RawStringLiteral::apply(const Selection &Inputs) { return tooling::Replacements( - tooling::Replacement(Inputs.AST.getASTContext().getSourceManager(), Str, + tooling::Replacement(Inputs.AST.getSourceManager(), Str, ("R\"(" + Str->getBytes() + ")\"").str(), Inputs.AST.getASTContext().getLangOpts())); } diff --git a/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp b/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp index 9b0b72d94ca52..12838d2a06a4b 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp @@ -62,7 +62,7 @@ bool SwapIfBranches::prepare(const Selection &Inputs) { Expected SwapIfBranches::apply(const Selection &Inputs) { auto &Ctx = Inputs.AST.getASTContext(); - auto &SrcMgr = Ctx.getSourceManager(); + auto &SrcMgr = Inputs.AST.getSourceManager(); auto ThenRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(), If->getThen()->getSourceRange()); diff --git a/clang-tools-extra/clangd/test/delimited-input-comment-at-the-end.test b/clang-tools-extra/clangd/test/delimited-input-comment-at-the-end.test index 34a248dfde6d0..bbbd72f8c59f6 100644 --- a/clang-tools-extra/clangd/test/delimited-input-comment-at-the-end.test +++ b/clang-tools-extra/clangd/test/delimited-input-comment-at-the-end.test @@ -1,4 +1,4 @@ -# RUN: clangd -input-style=delimited -run-synchronously -input-mirror-file %t < %s +# RUN: clangd -input-style=delimited -sync -input-mirror-file %t < %s # RUN: grep '{"jsonrpc":"2.0","id":3,"method":"exit"}' %t # # RUN: clangd -lit-test -input-mirror-file %t < %s diff --git a/clang-tools-extra/clangd/test/diagnostics-no-tidy.test b/clang-tools-extra/clangd/test/diagnostics-no-tidy.test new file mode 100644 index 0000000000000..f17ab1794990b --- /dev/null +++ b/clang-tools-extra/clangd/test/diagnostics-no-tidy.test @@ -0,0 +1,39 @@ +# RUN: clangd -lit-test -clang-tidy=false < %s | FileCheck -strict-whitespace %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {\n(void)sizeof(42);\n}"}}} +# CHECK: "method": "textDocument/publishDiagnostics", +# CHECK-NEXT: "params": { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "code": "-Wmain-return-type", +# CHECK-NEXT: "message": "Return type of 'main' is not 'int' (fix available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 4, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 0, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "uri": "file://{{.*}}/foo.c" +# CHECK-NEXT: } +--- +{"jsonrpc":"2.0","id":2,"method":"sync","params":null} +--- +{"jsonrpc":"2.0","method":"textDocument/didClose","params":{"textDocument":{"uri":"test:///foo.c"}}} +# CHECK: "method": "textDocument/publishDiagnostics", +# CHECK-NEXT: "params": { +# CHECK-NEXT: "diagnostics": [], +# CHECK-NEXT: "uri": "file://{{.*}}/foo.c" +# CHECK-NEXT: } +--- +{"jsonrpc":"2.0","id":5,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} diff --git a/clang-tools-extra/clangd/test/hover.test b/clang-tools-extra/clangd/test/hover.test index 8f1ead055954f..e45164b346ea5 100644 --- a/clang-tools-extra/clangd/test/hover.test +++ b/clang-tools-extra/clangd/test/hover.test @@ -10,6 +10,16 @@ # CHECK-NEXT: "contents": { # CHECK-NEXT: "kind": "plaintext", # CHECK-NEXT: "value": "Declared in global namespace\n\nvoid foo()" +# CHECK-NEXT: }, +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 28, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 25, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } # CHECK-NEXT: } # CHECK-NEXT: } # CHECK-NEXT:} @@ -19,6 +29,29 @@ # CHECK-NEXT: "jsonrpc": "2.0", # CHECK-NEXT: "result": null --- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main2.cpp","languageId":"cpp","version":1,"text":"enum foo{}; int main() { foo f; }\n"}}} +--- +{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main2.cpp"},"position":{"line":0,"character":27}}} +# CHECK: "id": 1, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": { +# CHECK-NEXT: "contents": { +# CHECK-NEXT: "kind": "plaintext", +# CHECK-NEXT: "value": "Declared in global namespace\n\nenum foo {}" +# CHECK-NEXT: }, +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 28, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 25, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT:} +--- {"jsonrpc":"2.0","id":3,"method":"shutdown"} --- {"jsonrpc":"2.0","method":"exit"} diff --git a/clang-tools-extra/clangd/test/input-mirror.test b/clang-tools-extra/clangd/test/input-mirror.test index 52845621e9e57..a34a4a08cf60c 100644 --- a/clang-tools-extra/clangd/test/input-mirror.test +++ b/clang-tools-extra/clangd/test/input-mirror.test @@ -1,4 +1,4 @@ -# RUN: clangd -pretty -run-synchronously -input-mirror-file %t < %s +# RUN: clangd -pretty -sync -input-mirror-file %t < %s # Note that we have to use '-b' as -input-mirror-file does not have a newline at the end of file. # RUN: diff -b %t %s # It is absolutely vital that this file has CRLF line endings. diff --git a/clang-tools-extra/clangd/test/protocol.test b/clang-tools-extra/clangd/test/protocol.test index c218763de206e..3e16c9ec9b334 100644 --- a/clang-tools-extra/clangd/test/protocol.test +++ b/clang-tools-extra/clangd/test/protocol.test @@ -1,5 +1,5 @@ -# RUN: not clangd -pretty -run-synchronously -enable-test-uri-scheme < %s | FileCheck -strict-whitespace %s -# RUN: not clangd -pretty -run-synchronously -enable-test-uri-scheme < %s 2>&1 | FileCheck -check-prefix=STDERR %s +# RUN: not clangd -pretty -sync -enable-test-uri-scheme < %s | FileCheck -strict-whitespace %s +# RUN: not clangd -pretty -sync -enable-test-uri-scheme < %s 2>&1 | FileCheck -check-prefix=STDERR %s # vim: fileformat=dos # It is absolutely vital that this file has CRLF line endings. # diff --git a/clang-tools-extra/clangd/test/signature-help-with-offsets.test b/clang-tools-extra/clangd/test/signature-help-with-offsets.test new file mode 100644 index 0000000000000..825dbc6c79bdb --- /dev/null +++ b/clang-tools-extra/clangd/test/signature-help-with-offsets.test @@ -0,0 +1,50 @@ +# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s +# Start a session. +{ + "jsonrpc": "2.0", + "id": 0, + "method": "initialize", + "params": { + "processId": 123, + "rootPath": "clangd", + "capabilities": { + "textDocument": { + "signatureHelp": { + "signatureInformation": { + "parameterInformation": { + "labelOffsetSupport": true + } + } + } + } + }, + "trace": "off" + } +} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void x(int);\nint main(){\nx("}}} +--- +{"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":2}}} +# CHECK: "id": 1, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": { +# CHECK-NEXT: "activeParameter": 0, +# CHECK-NEXT: "activeSignature": 0, +# CHECK-NEXT: "signatures": [ +# CHECK-NEXT: { +# CHECK-NEXT: "label": "x(int) -> void", +# CHECK-NEXT: "parameters": [ +# CHECK-NEXT: { +# CHECK-NEXT: "label": [ +# CHECK-NEXT: 2, +# CHECK-NEXT: 5 +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: } +--- +{"jsonrpc":"2.0","id":100000,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} diff --git a/clang-tools-extra/clangd/test/spaces-in-delimited-input.test b/clang-tools-extra/clangd/test/spaces-in-delimited-input.test index 9636425ea373e..dc2e2f5ea0f64 100644 --- a/clang-tools-extra/clangd/test/spaces-in-delimited-input.test +++ b/clang-tools-extra/clangd/test/spaces-in-delimited-input.test @@ -1,5 +1,5 @@ -# RUN: clangd -input-style=delimited -run-synchronously < %s 2>&1 | FileCheck %s -# RUN: clangd -lit-test -run-synchronously < %s 2>&1 | FileCheck %s +# RUN: clangd -input-style=delimited -sync < %s 2>&1 | FileCheck %s +# RUN: clangd -lit-test -sync < %s 2>&1 | FileCheck %s # {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} diff --git a/clang-tools-extra/clangd/test/too_large.test b/clang-tools-extra/clangd/test/too_large.test index 7b846c37f0804..7df981e794207 100644 --- a/clang-tools-extra/clangd/test/too_large.test +++ b/clang-tools-extra/clangd/test/too_large.test @@ -1,4 +1,4 @@ -# RUN: not clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s +# RUN: not clangd -sync < %s 2>&1 | FileCheck -check-prefix=STDERR %s # vim: fileformat=dos # It is absolutely vital that this file has CRLF line endings. # diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 44ca8a3363a29..91d82f9c4a1b0 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -37,14 +37,14 @@ namespace clangd { // FIXME: remove this option when Dex is cheap enough. static llvm::cl::opt UseDex("use-dex-index", - llvm::cl::desc("Use experimental Dex dynamic index."), + llvm::cl::desc("Use experimental Dex dynamic index"), llvm::cl::init(true), llvm::cl::Hidden); static llvm::cl::opt CompileCommandsDir( "compile-commands-dir", llvm::cl::desc("Specify a path to look for compile_commands.json. If path " "is invalid, clangd will look in the current directory and " - "parent paths of each source file.")); + "parent paths of each source file")); static llvm::cl::opt WorkerThreadsCount("j", @@ -59,10 +59,10 @@ static llvm::cl::opt CompletionStyle( llvm::cl::values( clEnumValN(Detailed, "detailed", "One completion item for each semantically distinct " - "completion, with full type information."), + "completion, with full type information"), clEnumValN(Bundled, "bundled", "Similar completion items (e.g. function overloads) are " - "combined. Type information shown where possible.")), + "combined. Type information shown where possible")), llvm::cl::init(Detailed)); // FIXME: Flags are the wrong mechanism for user preferences. @@ -96,14 +96,14 @@ static llvm::cl::opt LogLevel( static llvm::cl::opt Test("lit-test", - llvm::cl::desc("Abbreviation for -input-style=delimited -pretty " - "-run-synchronously -enable-test-scheme -log=verbose. " - "Intended to simplify lit tests."), + llvm::cl::desc("Abbreviation for -input-style=delimited -pretty -sync " + "-enable-test-scheme -log=verbose." + "Intended to simplify lit tests"), llvm::cl::init(false), llvm::cl::Hidden); static llvm::cl::opt EnableTestScheme( "enable-test-uri-scheme", - llvm::cl::desc("Enable 'test:' URI scheme. Only use in lit tests."), + llvm::cl::desc("Enable 'test:' URI scheme. Only use in lit tests"), llvm::cl::init(false), llvm::cl::Hidden); enum PCHStorageFlag { Disk, Memory }; @@ -119,13 +119,12 @@ static llvm::cl::opt PCHStorage( static llvm::cl::opt LimitResults( "limit-results", llvm::cl::desc("Limit the number of results returned by clangd. " - "0 means no limit. (default=100)"), + "0 means no limit (default=100)"), llvm::cl::init(100)); -static llvm::cl::opt RunSynchronously( - "run-synchronously", - llvm::cl::desc("Parse on main thread. If set, -j is ignored"), - llvm::cl::init(false), llvm::cl::Hidden); +static llvm::cl::opt + Sync("sync", llvm::cl::desc("Parse on main thread. If set, -j is ignored"), + llvm::cl::init(false), llvm::cl::Hidden); static llvm::cl::opt ResourceDir("resource-dir", @@ -135,7 +134,7 @@ static llvm::cl::opt static llvm::cl::opt InputMirrorFile( "input-mirror-file", llvm::cl::desc( - "Mirror all LSP input to the specified file. Useful for debugging."), + "Mirror all LSP input to the specified file. Useful for debugging"), llvm::cl::init(""), llvm::cl::Hidden); static llvm::cl::opt EnableIndex( @@ -143,7 +142,7 @@ static llvm::cl::opt EnableIndex( llvm::cl::desc( "Enable index-based features. By default, clangd maintains an index " "built from symbols in opened files. Global index support needs to " - "enabled separatedly."), + "enabled separatedly"), llvm::cl::init(true), llvm::cl::Hidden); static llvm::cl::opt AllScopesCompletion( @@ -152,7 +151,7 @@ static llvm::cl::opt AllScopesCompletion( "If set to true, code completion will include index symbols that are " "not defined in the scopes (e.g. " "namespaces) visible from the code completion point. Such completions " - "can insert scope qualifiers."), + "can insert scope qualifiers"), llvm::cl::init(true)); static llvm::cl::opt ShowOrigins( @@ -168,7 +167,7 @@ static llvm::cl::opt HeaderInsertion( "Include what you use. " "Insert the owning header for top-level symbols, unless the " "header is already directly included or the symbol is " - "forward-declared."), + "forward-declared"), clEnumValN( CodeCompleteOptions::NeverInsert, "never", "Never insert #include directives as part of code completion"))); @@ -177,16 +176,16 @@ static llvm::cl::opt HeaderInsertionDecorators( "header-insertion-decorators", llvm::cl::desc("Prepend a circular dot or space before the completion " "label, depending on whether " - "an include line will be inserted or not."), + "an include line will be inserted or not"), llvm::cl::init(true)); static llvm::cl::opt IndexFile( "index-file", llvm::cl::desc( "Index file to build the static index. The file must have been created " - "by a compatible clangd-indexer.\n" + "by a compatible clangd-indexer\n" "WARNING: This option is experimental only, and will be removed " - "eventually. Don't rely on it."), + "eventually. Don't rely on it"), llvm::cl::init(""), llvm::cl::Hidden); static llvm::cl::opt EnableBackgroundIndex( @@ -201,7 +200,7 @@ static llvm::cl::opt BackgroundIndexRebuildPeriod( llvm::cl::desc( "If set to non-zero, the background index rebuilds the symbol index " "periodically every X milliseconds; otherwise, the " - "symbol index will be updated for each indexed file."), + "symbol index will be updated for each indexed file"), llvm::cl::init(5000), llvm::cl::Hidden); enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs }; @@ -219,20 +218,20 @@ static llvm::cl::opt EnableFunctionArgSnippets( "function-arg-placeholders", llvm::cl::desc("When disabled, completions contain only parentheses for " "function calls. When enabled, completions also contain " - "placeholders for method parameters."), + "placeholders for method parameters"), llvm::cl::init(CodeCompleteOptions().EnableFunctionArgSnippets)); static llvm::cl::opt ClangTidyChecks( "clang-tidy-checks", llvm::cl::desc( "List of clang-tidy checks to run (this will override " - ".clang-tidy files). Only meaningful when -clang-tidy flag is on."), + ".clang-tidy files). Only meaningful when -clang-tidy flag is on"), llvm::cl::init("")); -static llvm::cl::opt EnableClangTidy( - "clang-tidy", - llvm::cl::desc("Enable clang-tidy diagnostics."), - llvm::cl::init(true)); +static llvm::cl::opt + EnableClangTidy("clang-tidy", + llvm::cl::desc("Enable clang-tidy diagnostics"), + llvm::cl::init(true)); static llvm::cl::opt FallbackStyle("fallback-style", @@ -243,13 +242,13 @@ static llvm::cl::opt static llvm::cl::opt SuggestMissingIncludes( "suggest-missing-includes", llvm::cl::desc("Attempts to fix diagnostic errors caused by missing " - "includes using index."), + "includes using index"), llvm::cl::init(true)); static llvm::cl::opt ForceOffsetEncoding( "offset-encoding", llvm::cl::desc("Force the offsetEncoding used for character positions. " - "This bypasses negotiation via client capabilities."), + "This bypasses negotiation via client capabilities"), llvm::cl::values(clEnumValN(OffsetEncoding::UTF8, "utf-8", "Offsets are in UTF-8 bytes"), clEnumValN(OffsetEncoding::UTF16, "utf-16", @@ -343,7 +342,7 @@ int main(int argc, char *argv[]) { "\n\thttps://clang.llvm.org/extra/clangd.html" "\n\thttps://microsoft.github.io/language-server-protocol/"); if (Test) { - RunSynchronously = true; + Sync = true; InputStyle = JSONStreamStyle::Delimited; LogLevel = Logger::Verbose; PrettyPrint = true; @@ -355,15 +354,15 @@ int main(int argc, char *argv[]) { "test", "Test scheme for clangd lit tests."); } - if (!RunSynchronously && WorkerThreadsCount == 0) { + if (!Sync && WorkerThreadsCount == 0) { llvm::errs() << "A number of worker threads cannot be 0. Did you mean to " - "specify -run-synchronously?"; + "specify -sync?"; return 1; } - if (RunSynchronously) { + if (Sync) { if (WorkerThreadsCount.getNumOccurrences()) - llvm::errs() << "Ignoring -j because -run-synchronously is set.\n"; + llvm::errs() << "Ignoring -j because -sync is set.\n"; WorkerThreadsCount = 0; } if (FallbackStyle.getNumOccurrences()) @@ -461,7 +460,7 @@ int main(int argc, char *argv[]) { if (auto Idx = loadIndex(IndexFile, /*UseDex=*/true)) Placeholder->reset(std::move(Idx)); }); - if (RunSynchronously) + if (Sync) AsyncIndexLoad.wait(); } Opts.StaticIndex = StaticIdx.get(); @@ -513,14 +512,14 @@ int main(int argc, char *argv[]) { tidy::ClangTidyGlobalOptions(), /* Default */ tidy::ClangTidyOptions::getDefaults(), /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem()); + Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &, + llvm::StringRef File) { + // This function must be thread-safe and tidy option providers are not. + std::lock_guard Lock(ClangTidyOptMu); + // FIXME: use the FS provided to the function. + return ClangTidyOptProvider->getOptions(File); + }; } - Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &, - llvm::StringRef File) { - // This function must be thread-safe and tidy option providers are not. - std::lock_guard Lock(ClangTidyOptMu); - // FIXME: use the FS provided to the function. - return ClangTidyOptProvider->getOptions(File); - }; Opts.SuggestMissingIncludes = SuggestMissingIncludes; llvm::Optional OffsetEncodingFromFlag; if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding) diff --git a/clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp b/clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp index 2c239ce76acd6..100e92c3c65da 100644 --- a/clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp @@ -49,7 +49,7 @@ Bar* bar; std::string WithPreamble = Preamble + Text; Annotations TestCase(WithPreamble); auto AST = TestTU::withCode(TestCase.code()).build(); - const auto &SourceMgr = AST.getASTContext().getSourceManager(); + const auto &SourceMgr = AST.getSourceManager(); SourceLocation Actual = getBeginningOfIdentifier( AST, TestCase.points().back(), SourceMgr.getMainFileID()); Position ActualPos = offsetToPosition( diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index e584597e7a90c..2f846e9c83781 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -49,6 +49,9 @@ class IgnoreDiagnostics : public DiagnosticsConsumer { // GMock helpers for matching completion items. MATCHER_P(Named, Name, "") { return arg.Name == Name; } +MATCHER_P(NameStartsWith, Prefix, "") { + return llvm::StringRef(arg.Name).startswith(Prefix); +} MATCHER_P(Scope, S, "") { return arg.Scope == S; } MATCHER_P(Qualifier, Q, "") { return arg.RequiredQualifier == Q; } MATCHER_P(Labeled, Label, "") { @@ -446,6 +449,45 @@ TEST(CompletionTest, Kinds) { Results = completions("nam^"); EXPECT_THAT(Results.Completions, Has("namespace", CompletionItemKind::Snippet)); + + // Members of anonymous unions are of kind 'field'. + Results = completions( + R"cpp( + struct X{ + union { + void *a; + }; + }; + auto u = X().^ + )cpp"); + EXPECT_THAT( + Results.Completions, + UnorderedElementsAre(AllOf(Named("a"), Kind(CompletionItemKind::Field)))); + + // Completion kinds for templates should not be unknown. + Results = completions( + R"cpp( + template struct complete_class {}; + template void complete_function(); + template using complete_type_alias = int; + template int complete_variable = 10; + + struct X { + template static int complete_static_member = 10; + + static auto x = complete_^ + } + )cpp"); + EXPECT_THAT( + Results.Completions, + UnorderedElementsAre( + AllOf(Named("complete_class"), Kind(CompletionItemKind::Class)), + AllOf(Named("complete_function"), Kind(CompletionItemKind::Function)), + AllOf(Named("complete_type_alias"), + Kind(CompletionItemKind::Interface)), + AllOf(Named("complete_variable"), Kind(CompletionItemKind::Variable)), + AllOf(Named("complete_static_member"), + Kind(CompletionItemKind::Property)))); } TEST(CompletionTest, NoDuplicates) { @@ -925,19 +967,37 @@ SignatureHelp signatures(llvm::StringRef Text, return signatures(Test.code(), Test.point(), std::move(IndexSymbols)); } +struct ExpectedParameter { + std::string Text; + std::pair Offsets; +}; MATCHER_P(ParamsAre, P, "") { if (P.size() != arg.parameters.size()) return false; - for (unsigned I = 0; I < P.size(); ++I) - if (P[I] != arg.parameters[I].label) + for (unsigned I = 0; I < P.size(); ++I) { + if (P[I].Text != arg.parameters[I].labelString || + P[I].Offsets != arg.parameters[I].labelOffsets) return false; + } return true; } MATCHER_P(SigDoc, Doc, "") { return arg.documentation == Doc; } -Matcher Sig(std::string Label, - std::vector Params) { - return AllOf(SigHelpLabeled(Label), ParamsAre(Params)); +/// \p AnnotatedLabel is a signature label with ranges marking parameters, e.g. +/// foo([[int p1]], [[double p2]]) -> void +Matcher Sig(llvm::StringRef AnnotatedLabel) { + llvm::Annotations A(AnnotatedLabel); + std::string Label = A.code(); + std::vector Parameters; + for (auto Range : A.ranges()) { + Parameters.emplace_back(); + + ExpectedParameter &P = Parameters.back(); + P.Text = Label.substr(Range.Begin, Range.End - Range.Begin); + P.Offsets.first = lspLength(llvm::StringRef(Label).substr(0, Range.Begin)); + P.Offsets.second = lspLength(llvm::StringRef(Label).substr(1, Range.End)); + } + return AllOf(SigHelpLabeled(Label), ParamsAre(Parameters)); } TEST(SignatureHelpTest, Overloads) { @@ -950,11 +1010,10 @@ TEST(SignatureHelpTest, Overloads) { int main() { foo(^); } )cpp"); EXPECT_THAT(Results.signatures, - UnorderedElementsAre( - Sig("foo(float x, float y) -> void", {"float x", "float y"}), - Sig("foo(float x, int y) -> void", {"float x", "int y"}), - Sig("foo(int x, float y) -> void", {"int x", "float y"}), - Sig("foo(int x, int y) -> void", {"int x", "int y"}))); + UnorderedElementsAre(Sig("foo([[float x]], [[float y]]) -> void"), + Sig("foo([[float x]], [[int y]]) -> void"), + Sig("foo([[int x]], [[float y]]) -> void"), + Sig("foo([[int x]], [[int y]]) -> void"))); // We always prefer the first signature. EXPECT_EQ(0, Results.activeSignature); EXPECT_EQ(0, Results.activeParameter); @@ -968,9 +1027,8 @@ TEST(SignatureHelpTest, DefaultArgs) { )cpp"); EXPECT_THAT(Results.signatures, UnorderedElementsAre( - Sig("bar(int x, int y = 0) -> void", {"int x", "int y = 0"}), - Sig("bar(float x = 0, int y = 42) -> void", - {"float x = 0", "int y = 42"}))); + Sig("bar([[int x]], [[int y = 0]]) -> void"), + Sig("bar([[float x = 0]], [[int y = 42]]) -> void"))); EXPECT_EQ(0, Results.activeSignature); EXPECT_EQ(0, Results.activeParameter); } @@ -981,8 +1039,7 @@ TEST(SignatureHelpTest, ActiveArg) { int main() { baz(baz(1,2,3), ^); } )cpp"); EXPECT_THAT(Results.signatures, - ElementsAre(Sig("baz(int a, int b, int c) -> int", - {"int a", "int b", "int c"}))); + ElementsAre(Sig("baz([[int a]], [[int b]], [[int c]]) -> int"))); EXPECT_EQ(0, Results.activeSignature); EXPECT_EQ(1, Results.activeParameter); } @@ -1719,14 +1776,12 @@ TEST(SignatureHelpTest, OverloadsOrdering) { void foo(int x, int y = 0); int main() { foo(^); } )cpp"); - EXPECT_THAT( - Results.signatures, - ElementsAre( - Sig("foo(int x) -> void", {"int x"}), - Sig("foo(int x, int y = 0) -> void", {"int x", "int y = 0"}), - Sig("foo(float x, int y) -> void", {"float x", "int y"}), - Sig("foo(int x, float y) -> void", {"int x", "float y"}), - Sig("foo(float x, float y) -> void", {"float x", "float y"}))); + EXPECT_THAT(Results.signatures, + ElementsAre(Sig("foo([[int x]]) -> void"), + Sig("foo([[int x]], [[int y = 0]]) -> void"), + Sig("foo([[float x]], [[int y]]) -> void"), + Sig("foo([[int x]], [[float y]]) -> void"), + Sig("foo([[float x]], [[float y]]) -> void"))); // We always prefer the first signature. EXPECT_EQ(0, Results.activeSignature); EXPECT_EQ(0, Results.activeParameter); @@ -1743,7 +1798,7 @@ TEST(SignatureHelpTest, InstantiatedSignatures) { )cpp"; EXPECT_THAT(signatures(Sig0).signatures, - ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"}))); + ElementsAre(Sig("foo([[T]], [[T]], [[T]]) -> void"))); StringRef Sig1 = R"cpp( template @@ -1754,7 +1809,7 @@ TEST(SignatureHelpTest, InstantiatedSignatures) { })cpp"; EXPECT_THAT(signatures(Sig1).signatures, - ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"}))); + ElementsAre(Sig("foo([[T]], [[T]], [[T]]) -> void"))); StringRef Sig2 = R"cpp( template @@ -1766,7 +1821,7 @@ TEST(SignatureHelpTest, InstantiatedSignatures) { )cpp"; EXPECT_THAT(signatures(Sig2).signatures, - ElementsAre(Sig("foo(T...) -> void", {"T..."}))); + ElementsAre(Sig("foo([[T...]]) -> void"))); // It is debatable whether we should substitute the outer template parameter // ('T') in that case. Currently we don't substitute it in signature help, but @@ -1786,7 +1841,7 @@ TEST(SignatureHelpTest, InstantiatedSignatures) { )cpp"; EXPECT_THAT(signatures(Sig3).signatures, - ElementsAre(Sig("foo(T, U) -> void", {"T", "U"}))); + ElementsAre(Sig("foo([[T]], [[U]]) -> void"))); } TEST(SignatureHelpTest, IndexDocumentation) { @@ -1807,8 +1862,8 @@ TEST(SignatureHelpTest, IndexDocumentation) { EXPECT_THAT( signatures(Sig0, {Foo0}).signatures, - ElementsAre(AllOf(Sig("foo() -> int", {}), SigDoc("Doc from the index")), - AllOf(Sig("foo(double) -> int", {"double"}), SigDoc("")))); + ElementsAre(AllOf(Sig("foo() -> int"), SigDoc("Doc from the index")), + AllOf(Sig("foo([[double]]) -> int"), SigDoc("")))); StringRef Sig1 = R"cpp( int foo(); @@ -1824,11 +1879,10 @@ TEST(SignatureHelpTest, IndexDocumentation) { EXPECT_THAT( signatures(Sig1, {Foo0, Foo1, Foo2}).signatures, - ElementsAre(AllOf(Sig("foo() -> int", {}), SigDoc("Doc from the index")), - AllOf(Sig("foo(int) -> int", {"int"}), - SigDoc("Overriden doc from sema")), - AllOf(Sig("foo(int, int) -> int", {"int", "int"}), - SigDoc("Doc from sema")))); + ElementsAre( + AllOf(Sig("foo() -> int"), SigDoc("Doc from the index")), + AllOf(Sig("foo([[int]]) -> int"), SigDoc("Overriden doc from sema")), + AllOf(Sig("foo([[int]], [[int]]) -> int"), SigDoc("Doc from sema")))); } TEST(SignatureHelpTest, DynamicIndexDocumentation) { @@ -1859,7 +1913,7 @@ TEST(SignatureHelpTest, DynamicIndexDocumentation) { EXPECT_THAT( llvm::cantFail(runSignatureHelp(Server, File, FileContent.point())) .signatures, - ElementsAre(AllOf(Sig("foo() -> int", {}), SigDoc("Member doc")))); + ElementsAre(AllOf(Sig("foo() -> int"), SigDoc("Member doc")))); } TEST(CompletionTest, CompletionFunctionArgsDisabled) { @@ -1946,10 +2000,13 @@ TEST(CompletionTest, SuggestOverrides) { }; )cpp"); const auto Results = completions(Text); - EXPECT_THAT(Results.Completions, - AllOf(Contains(Labeled("void vfunc(bool param, int p) override")), - Contains(Labeled("void ttt(bool param) const override")), - Not(Contains(Labeled("void vfunc(bool param) override"))))); + EXPECT_THAT( + Results.Completions, + AllOf(Contains(AllOf(Labeled("void vfunc(bool param, int p) override"), + NameStartsWith("vfunc"))), + Contains(AllOf(Labeled("void ttt(bool param) const override"), + NameStartsWith("ttt"))), + Not(Contains(Labeled("void vfunc(bool param) override"))))); } TEST(CompletionTest, OverridesNonIdentName) { @@ -2134,10 +2191,9 @@ TEST(SignatureHelpTest, InsideArgument) { void foo(int x, int y); int main() { foo(1+^); } )cpp"); - EXPECT_THAT( - Results.signatures, - ElementsAre(Sig("foo(int x) -> void", {"int x"}), - Sig("foo(int x, int y) -> void", {"int x", "int y"}))); + EXPECT_THAT(Results.signatures, + ElementsAre(Sig("foo([[int x]]) -> void"), + Sig("foo([[int x]], [[int y]]) -> void"))); EXPECT_EQ(0, Results.activeParameter); } { @@ -2146,10 +2202,9 @@ TEST(SignatureHelpTest, InsideArgument) { void foo(int x, int y); int main() { foo(1^); } )cpp"); - EXPECT_THAT( - Results.signatures, - ElementsAre(Sig("foo(int x) -> void", {"int x"}), - Sig("foo(int x, int y) -> void", {"int x", "int y"}))); + EXPECT_THAT(Results.signatures, + ElementsAre(Sig("foo([[int x]]) -> void"), + Sig("foo([[int x]], [[int y]]) -> void"))); EXPECT_EQ(0, Results.activeParameter); } { @@ -2158,10 +2213,9 @@ TEST(SignatureHelpTest, InsideArgument) { void foo(int x, int y); int main() { foo(1^0); } )cpp"); - EXPECT_THAT( - Results.signatures, - ElementsAre(Sig("foo(int x) -> void", {"int x"}), - Sig("foo(int x, int y) -> void", {"int x", "int y"}))); + EXPECT_THAT(Results.signatures, + ElementsAre(Sig("foo([[int x]]) -> void"), + Sig("foo([[int x]], [[int y]]) -> void"))); EXPECT_EQ(0, Results.activeParameter); } { @@ -2171,8 +2225,8 @@ TEST(SignatureHelpTest, InsideArgument) { int bar(int x, int y); int main() { bar(foo(2, 3^)); } )cpp"); - EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo(int x, int y) -> void", - {"int x", "int y"}))); + EXPECT_THAT(Results.signatures, + ElementsAre(Sig("foo([[int x]], [[int y]]) -> void"))); EXPECT_EQ(1, Results.activeParameter); } } @@ -2189,9 +2243,8 @@ TEST(SignatureHelpTest, ConstructorInitializeFields) { }; )cpp"); EXPECT_THAT(Results.signatures, - UnorderedElementsAre(Sig("A(int)", {"int"}), - Sig("A(A &&)", {"A &&"}), - Sig("A(const A &)", {"const A &"}))); + UnorderedElementsAre(Sig("A([[int]])"), Sig("A([[A &&]])"), + Sig("A([[const A &]])"))); } { const auto Results = signatures(R"cpp( @@ -2208,9 +2261,8 @@ TEST(SignatureHelpTest, ConstructorInitializeFields) { }; )cpp"); EXPECT_THAT(Results.signatures, - UnorderedElementsAre(Sig("A(int)", {"int"}), - Sig("A(A &&)", {"A &&"}), - Sig("A(const A &)", {"const A &"}))); + UnorderedElementsAre(Sig("A([[int]])"), Sig("A([[A &&]])"), + Sig("A([[const A &]])"))); } } @@ -2376,6 +2428,28 @@ TEST(CompletionTest, ObjectiveCMethodTwoArgumentsFromMiddle) { EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}"))); } +TEST(CompletionTest, CursorInSnippets) { + clangd::CodeCompleteOptions Options; + Options.EnableSnippets = true; + auto Results = completions( + R"cpp( + void while_foo(int a, int b); + void test() { + whil^ + })cpp", + /*IndexSymbols=*/{}, Options); + + // Last placeholder in code patterns should be $0 to put the cursor there. + EXPECT_THAT( + Results.Completions, + Contains(AllOf(Named("while"), + SnippetSuffix(" (${1:condition}) {\n${0:statements}\n}")))); + // However, snippets for functions must *not* end with $0. + EXPECT_THAT(Results.Completions, + Contains(AllOf(Named("while_foo"), + SnippetSuffix("(${1:int a}, ${2:int b})")))); +} + TEST(CompletionTest, WorksWithNullType) { auto R = completions(R"cpp( int main() { diff --git a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp index 43429c864655c..83b3826f6fc2b 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp @@ -22,10 +22,12 @@ class CompletionStringTest : public ::testing::Test { CCTUInfo(Allocator), Builder(*Allocator, CCTUInfo) {} protected: - void computeSignature(const CodeCompletionString &CCS) { + void computeSignature(const CodeCompletionString &CCS, + bool CompletingPattern = false) { Signature.clear(); Snippet.clear(); - getSignature(CCS, &Signature, &Snippet); + getSignature(CCS, &Signature, &Snippet, /*RequiredQualifier=*/nullptr, + CompletingPattern); } std::shared_ptr Allocator; @@ -99,6 +101,25 @@ TEST_F(CompletionStringTest, EscapeSnippet) { EXPECT_EQ(Snippet, "(${1:\\$p\\}1\\\\})"); } +TEST_F(CompletionStringTest, SnippetsInPatterns) { + auto MakeCCS = [this]() -> const CodeCompletionString & { + CodeCompletionBuilder Builder(*Allocator, CCTUInfo); + Builder.AddTypedTextChunk("namespace"); + Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace); + Builder.AddPlaceholderChunk("name"); + Builder.AddChunk(CodeCompletionString::CK_Equal); + Builder.AddPlaceholderChunk("target"); + Builder.AddChunk(CodeCompletionString::CK_SemiColon); + return *Builder.TakeString(); + }; + computeSignature(MakeCCS(), /*CompletingPattern=*/false); + EXPECT_EQ(Snippet, " ${1:name} = ${2:target};"); + + // When completing a pattern, the last placeholder holds the cursor position. + computeSignature(MakeCCS(), /*CompletingPattern=*/true); + EXPECT_EQ(Snippet, " ${1:name} = ${0:target};"); +} + TEST_F(CompletionStringTest, IgnoreInformativeQualifier) { Builder.AddTypedTextChunk("X"); Builder.AddInformativeChunk("info ok"); diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 9d0492aa84d20..02126833df65c 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -120,7 +120,7 @@ o]](); "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\\ o' to 'foo'")), + 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 @@ -247,6 +247,36 @@ TEST(DiagnosticTest, ClangTidyWarningAsError) { DiagSeverity(DiagnosticsEngine::Error)))); } +TEST(DiagnosticTest, LongFixMessages) { + // We limit the size of printed code. + Annotations Source(R"cpp( + int main() { + int somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier; + [[omereallyreallyreallyreallyreallyreallyreallyreallylongidentifier]]= 10; + } + )cpp"); + TestTU TU = TestTU::withCode(Source.code()); + EXPECT_THAT( + TU.build().getDiagnostics(), + ElementsAre(WithFix(Fix( + Source.range(), + "somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier", + "change 'omereallyreallyreallyreallyreallyreallyreallyreall…' to " + "'somereallyreallyreallyreallyreallyreallyreallyreal…'")))); + // Only show changes up to a first newline. + Source = Annotations(R"cpp( + int main() { + int ident; + [[ide\ +n]] = 10; + } + )cpp"); + TU = TestTU::withCode(Source.code()); + EXPECT_THAT(TU.build().getDiagnostics(), + ElementsAre(WithFix( + Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'")))); +} + TEST(DiagnosticTest, ClangTidyWarningAsErrorTrumpsSuppressionComment) { Annotations Main(R"cpp( int main() { diff --git a/clang-tools-extra/clangd/unittests/ExpectedTypeTest.cpp b/clang-tools-extra/clangd/unittests/ExpectedTypeTest.cpp index 8d2d60ebe5547..0315f4de7482b 100644 --- a/clang-tools-extra/clangd/unittests/ExpectedTypeTest.cpp +++ b/clang-tools-extra/clangd/unittests/ExpectedTypeTest.cpp @@ -11,6 +11,7 @@ #include "TestTU.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" +#include "llvm/ADT/None.h" #include "llvm/ADT/StringRef.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -31,16 +32,14 @@ class ExpectedTypeConversionTest : public ::testing::Test { AST = TestTU::withCode(Code).build(); } - const ValueDecl *decl(llvm::StringRef Name) { - return &cast(findDecl(*AST, Name)); - } + const NamedDecl *decl(llvm::StringRef Name) { return &findDecl(*AST, Name); } QualType typeOf(llvm::StringRef Name) { - return decl(Name)->getType().getCanonicalType(); + return cast(decl(Name))->getType().getCanonicalType(); } /// An overload for convenience. - llvm::Optional fromCompletionResult(const ValueDecl *D) { + llvm::Optional fromCompletionResult(const NamedDecl *D) { return OpaqueType::fromCompletionResult( ASTCtx(), CodeCompletionResult(D, CCP_Declaration)); } @@ -148,6 +147,29 @@ TEST_F(ExpectedTypeConversionTest, FunctionReturns) { EXPECT_EQ(fromCompletionResult(decl("returns_ptr")), IntPtrTy); } +TEST_F(ExpectedTypeConversionTest, Templates) { + build(R"cpp( +template +int* returns_not_dependent(); +template +T* returns_dependent(); + +template +int* var_not_dependent = nullptr; +template +T* var_dependent = nullptr; + +int* int_ptr_; + )cpp"); + + auto IntPtrTy = *OpaqueType::fromType(ASTCtx(), typeOf("int_ptr_")); + EXPECT_EQ(fromCompletionResult(decl("returns_not_dependent")), IntPtrTy); + EXPECT_EQ(fromCompletionResult(decl("returns_dependent")), llvm::None); + + EXPECT_EQ(fromCompletionResult(decl("var_not_dependent")), IntPtrTy); + EXPECT_EQ(fromCompletionResult(decl("var_dependent")), llvm::None); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp index 7c7993cc0f9f7..6761deb70acbd 100644 --- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp +++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp @@ -21,6 +21,7 @@ using ::testing::Contains; using ::testing::ElementsAre; using ::testing::EndsWith; using ::testing::Not; +using ::testing::StartsWith; TEST(GlobalCompilationDatabaseTest, FallbackCommand) { DirectoryBasedGlobalCompilationDatabase DB(None); @@ -85,7 +86,8 @@ TEST_F(OverlayCDBTest, GetCompileCommand) { TEST_F(OverlayCDBTest, GetFallbackCommand) { OverlayCDB CDB(Base.get(), {"-DA=4"}); EXPECT_THAT(CDB.getFallbackCommand(testPath("bar.cc")).CommandLine, - ElementsAre("clang", "-DA=2", testPath("bar.cc"), "-DA=4")); + ElementsAre("clang", "-DA=2", testPath("bar.cc"), "-DA=4", + "-fsyntax-only", StartsWith("-resource-dir"))); } TEST_F(OverlayCDBTest, NoBase) { @@ -97,7 +99,8 @@ TEST_F(OverlayCDBTest, NoBase) { Contains("-DA=5")); EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine, - ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6")); + ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6", + "-fsyntax-only")); } TEST_F(OverlayCDBTest, Watch) { diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp b/clang-tools-extra/clangd/unittests/IndexTests.cpp index 8d8c8da771562..27b23d2f75748 100644 --- a/clang-tools-extra/clangd/unittests/IndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp @@ -76,6 +76,45 @@ TEST(SymbolSlab, FindAndIterate) { EXPECT_THAT(*S.find(SymbolID(Sym)), Named(Sym)); } +TEST(RelationSlab, Lookup) { + SymbolID A{"A"}; + SymbolID B{"B"}; + SymbolID C{"C"}; + SymbolID D{"D"}; + + RelationSlab::Builder Builder; + Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, B}); + Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, C}); + Builder.insert(Relation{B, index::SymbolRole::RelationBaseOf, D}); + Builder.insert(Relation{C, index::SymbolRole::RelationBaseOf, D}); + Builder.insert(Relation{B, index::SymbolRole::RelationChildOf, A}); + Builder.insert(Relation{C, index::SymbolRole::RelationChildOf, A}); + Builder.insert(Relation{D, index::SymbolRole::RelationChildOf, B}); + Builder.insert(Relation{D, index::SymbolRole::RelationChildOf, C}); + + RelationSlab Slab = std::move(Builder).build(); + EXPECT_THAT( + Slab.lookup(A, index::SymbolRole::RelationBaseOf), + UnorderedElementsAre(Relation{A, index::SymbolRole::RelationBaseOf, B}, + Relation{A, index::SymbolRole::RelationBaseOf, C})); +} + +TEST(RelationSlab, Duplicates) { + SymbolID A{"A"}; + SymbolID B{"B"}; + SymbolID C{"C"}; + + RelationSlab::Builder Builder; + Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, B}); + Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, C}); + Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, B}); + + RelationSlab Slab = std::move(Builder).build(); + EXPECT_THAT(Slab, UnorderedElementsAre( + Relation{A, index::SymbolRole::RelationBaseOf, B}, + Relation{A, index::SymbolRole::RelationBaseOf, C})); +} + TEST(SwapIndexTest, OldIndexRecycled) { auto Token = std::make_shared(); std::weak_ptr WeakToken = Token; diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp index ac9facca83901..deae9f40b33e5 100644 --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -37,7 +37,7 @@ SelectionTree makeSelectionTree(const StringRef MarkedCode, ParsedAST &AST) { Range nodeRange(const SelectionTree::Node *N, ParsedAST &AST) { if (!N) return Range{}; - SourceManager &SM = AST.getASTContext().getSourceManager(); + SourceManager &SM = AST.getSourceManager(); StringRef Buffer = SM.getBufferData(SM.getMainFileID()); SourceRange SR = N->ASTNode.getSourceRange(); SR.setBegin(SM.getFileLoc(SR.getBegin())); diff --git a/clang-tools-extra/clangd/unittests/SerializationTests.cpp b/clang-tools-extra/clangd/unittests/SerializationTests.cpp index 792da77031039..1723bb867bc35 100644 --- a/clang-tools-extra/clangd/unittests/SerializationTests.cpp +++ b/clang-tools-extra/clangd/unittests/SerializationTests.cpp @@ -82,6 +82,14 @@ ID: 057557CEBF6E6B2D End: Line: 5 Column: 8 +... +--- !Relations +Subject: + ID: 6481EE7AF2841756 +Predicate: 0 +Object: + ID: 6512AEC512EA3A2D +... )"; MATCHER_P(ID, I, "") { return arg.ID == cantFail(SymbolID::fromStr(I)); } @@ -139,6 +147,13 @@ TEST(SerializationTest, YAMLConversions) { auto Ref1 = ParsedYAML->Refs->begin()->second.front(); EXPECT_EQ(Ref1.Kind, RefKind::Reference); EXPECT_EQ(StringRef(Ref1.Location.FileURI), "file:///path/foo.cc"); + + SymbolID Base = cantFail(SymbolID::fromStr("6481EE7AF2841756")); + SymbolID Derived = cantFail(SymbolID::fromStr("6512AEC512EA3A2D")); + ASSERT_TRUE(bool(ParsedYAML->Relations)); + EXPECT_THAT(*ParsedYAML->Relations, + UnorderedElementsAre( + Relation{Base, index::SymbolRole::RelationBaseOf, Derived})); } std::vector YAMLFromSymbols(const SymbolSlab &Slab) { @@ -149,8 +164,15 @@ std::vector YAMLFromSymbols(const SymbolSlab &Slab) { } std::vector YAMLFromRefs(const RefSlab &Slab) { std::vector Result; - for (const auto &Sym : Slab) - Result.push_back(toYAML(Sym)); + for (const auto &Refs : Slab) + Result.push_back(toYAML(Refs)); + return Result; +} + +std::vector YAMLFromRelations(const RelationSlab &Slab) { + std::vector Result; + for (const auto &Rel : Slab) + Result.push_back(toYAML(Rel)); return Result; } @@ -167,12 +189,15 @@ TEST(SerializationTest, BinaryConversions) { ASSERT_TRUE(bool(In2)) << In.takeError(); ASSERT_TRUE(In2->Symbols); ASSERT_TRUE(In2->Refs); + ASSERT_TRUE(In2->Relations); // Assert the YAML serializations match, for nice comparisons and diffs. EXPECT_THAT(YAMLFromSymbols(*In2->Symbols), UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols))); EXPECT_THAT(YAMLFromRefs(*In2->Refs), UnorderedElementsAreArray(YAMLFromRefs(*In->Refs))); + EXPECT_THAT(YAMLFromRelations(*In2->Relations), + UnorderedElementsAreArray(YAMLFromRelations(*In->Relations))); } TEST(SerializationTest, SrcsTest) { diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp index d372b1d672280..4a714388016b1 100644 --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -123,11 +123,11 @@ class ShouldCollectSymbolTest : public ::testing::Test { assert(AST.hasValue()); const NamedDecl &ND = Qualified ? findDecl(*AST, Name) : findUnqualifiedDecl(*AST, Name); - ASTContext& Ctx = AST->getASTContext(); - const SourceManager& SM = Ctx.getSourceManager(); - bool MainFile = SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getBeginLoc())); + const SourceManager &SM = AST->getSourceManager(); + bool MainFile = + SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getBeginLoc())); return SymbolCollector::shouldCollectSymbol( - ND, Ctx, SymbolCollector::Options(), MainFile); + ND, AST->getASTContext(), SymbolCollector::Options(), MainFile); } protected: @@ -273,13 +273,14 @@ class SymbolCollectorTest : public ::testing::Test { Args, Factory->create(), Files.get(), std::make_shared()); - InMemoryFileSystem->addFile( - TestHeaderName, 0, llvm::MemoryBuffer::getMemBuffer(HeaderCode)); + InMemoryFileSystem->addFile(TestHeaderName, 0, + llvm::MemoryBuffer::getMemBuffer(HeaderCode)); InMemoryFileSystem->addFile(TestFileName, 0, llvm::MemoryBuffer::getMemBuffer(MainCode)); Invocation.run(); Symbols = Factory->Collector->takeSymbols(); Refs = Factory->Collector->takeRefs(); + Relations = Factory->Collector->takeRelations(); return true; } @@ -291,6 +292,7 @@ class SymbolCollectorTest : public ::testing::Test { std::string TestFileURI; SymbolSlab Symbols; RefSlab Refs; + RelationSlab Relations; SymbolCollector::Options CollectorOpts; std::unique_ptr PragmaHandler; }; @@ -635,6 +637,19 @@ TEST_F(SymbolCollectorTest, RefsInHeaders) { HaveRanges(Header.ranges())))); } +TEST_F(SymbolCollectorTest, Relations) { + std::string Header = R"( + class Base {}; + class Derived : public Base {}; + )"; + runSymbolCollector(Header, /*Main=*/""); + const Symbol &Base = findSymbol(Symbols, "Base"); + const Symbol &Derived = findSymbol(Symbols, "Derived"); + EXPECT_THAT(Relations, + Contains(Relation{Base.ID, index::SymbolRole::RelationBaseOf, + Derived.ID})); +} + TEST_F(SymbolCollectorTest, References) { const std::string Header = R"( class W; @@ -784,10 +799,9 @@ TEST_F(SymbolCollectorTest, SymbolsInMainFile) { void f1() {} )"; runSymbolCollector(/*Header=*/"", Main); - EXPECT_THAT(Symbols, - UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2"), - QName("ff"), QName("foo"), QName("foo::Bar"), - QName("main_f"))); + EXPECT_THAT(Symbols, UnorderedElementsAre( + QName("Foo"), QName("f1"), QName("f2"), QName("ff"), + QName("foo"), QName("foo::Bar"), QName("main_f"))); } TEST_F(SymbolCollectorTest, Documentation) { diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp index cccb71c39a502..2c7b595af4821 100644 --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -59,8 +59,7 @@ ParsedAST TestTU::build() const { /*OldPreamble=*/nullptr, /*OldCompileCommand=*/Inputs.CompileCommand, Inputs, /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr); - auto AST = buildAST(FullFilename, createInvocationFromCommandLine(Cmd), - Inputs, Preamble); + auto AST = buildAST(FullFilename, std::move(CI), Inputs, Preamble); if (!AST.hasValue()) { ADD_FAILURE() << "Failed to build code:\n" << Code; llvm_unreachable("Failed to build TestTU!"); diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp index 66806b3d52e99..f39f4886de0b0 100644 --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -21,7 +21,6 @@ #include using llvm::Failed; -using llvm::HasValue; using llvm::Succeeded; namespace clang { diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 77fa042c5277b..cf2d726637157 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -9,6 +9,7 @@ #include "ClangdUnit.h" #include "Compiler.h" #include "Matchers.h" +#include "Protocol.h" #include "SyncAPI.h" #include "TestFS.h" #include "TestTU.h" @@ -16,6 +17,7 @@ #include "index/FileIndex.h" #include "index/SymbolCollector.h" #include "clang/Index/IndexingAction.h" +#include "llvm/ADT/None.h" #include "llvm/Support/Path.h" #include "llvm/Support/ScopedPrinter.h" #include "gmock/gmock.h" @@ -497,6 +499,17 @@ TEST(LocateSymbol, Ambiguous) { ElementsAre(Sym("Foo"), Sym("Foo"))); } +TEST(LocateSymbol, TemplateTypedefs) { + auto T = Annotations(R"cpp( + template struct function {}; + template using callback = function; + + c^allback foo; + )cpp"); + auto AST = TestTU::withCode(T.code()).build(); + EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback"))); +} + TEST(LocateSymbol, RelPathsInCompileCommand) { // The source is in "/clangd-test/src". // We build in "/clangd-test/build". @@ -558,6 +571,306 @@ int [[bar_not_preamble]]; HeaderNotInPreambleAnnotations.range()))); } +TEST(Hover, Structured) { + struct { + const char *const Code; + const std::function ExpectedBuilder; + } Cases[] = { + // Global scope. + {R"cpp( + // Best foo ever. + void [[fo^o]]() {} + )cpp", + [](HoverInfo &HI) { + HI.NamespaceScope = ""; + HI.Name = "foo"; + HI.Kind = SymbolKind::Function; + HI.Documentation = "Best foo ever."; + HI.Definition = "void foo()"; + HI.ReturnType = "void"; + HI.Type = "void()"; + HI.Parameters.emplace(); + }}, + // Inside namespace + {R"cpp( + namespace ns1 { namespace ns2 { + /// Best foo ever. + void [[fo^o]]() {} + }} + )cpp", + [](HoverInfo &HI) { + HI.NamespaceScope = "ns1::ns2::"; + HI.Name = "foo"; + HI.Kind = SymbolKind::Function; + HI.Documentation = "Best foo ever."; + HI.Definition = "void foo()"; + HI.ReturnType = "void"; + HI.Type = "void()"; + HI.Parameters.emplace(); + }}, + // Field + {R"cpp( + namespace ns1 { namespace ns2 { + struct Foo { + int [[b^ar]]; + }; + }} + )cpp", + [](HoverInfo &HI) { + HI.NamespaceScope = "ns1::ns2::"; + HI.LocalScope = "Foo::"; + HI.Name = "bar"; + HI.Kind = SymbolKind::Field; + HI.Definition = "int bar"; + HI.Type = "int"; + }}, + // Local to class method. + {R"cpp( + namespace ns1 { namespace ns2 { + struct Foo { + void foo() { + int [[b^ar]]; + } + }; + }} + )cpp", + [](HoverInfo &HI) { + HI.NamespaceScope = "ns1::ns2::"; + HI.LocalScope = "Foo::foo::"; + HI.Name = "bar"; + HI.Kind = SymbolKind::Variable; + HI.Definition = "int bar"; + HI.Type = "int"; + }}, + // Anon namespace and local scope. + {R"cpp( + namespace ns1 { namespace { + struct { + int [[b^ar]]; + } T; + }} + )cpp", + [](HoverInfo &HI) { + HI.NamespaceScope = "ns1::(anonymous)::"; + HI.LocalScope = "(anonymous struct)::"; + HI.Name = "bar"; + HI.Kind = SymbolKind::Field; + HI.Definition = "int bar"; + HI.Type = "int"; + }}, + // Variable with template type + {R"cpp( + template class Foo {}; + Foo [[fo^o]]; + )cpp", + [](HoverInfo &HI) { + HI.NamespaceScope = ""; + HI.Name = "foo"; + HI.Kind = SymbolKind::Variable; + HI.Definition = "Foo foo"; + HI.Type = "Foo"; + }}, + // Implicit template instantiation + {R"cpp( + template class vector{}; + [[vec^tor]] foo; + )cpp", + [](HoverInfo &HI) { + HI.NamespaceScope = ""; + HI.Name = "vector"; + HI.Kind = SymbolKind::Class; + HI.Definition = "template class vector {}"; + HI.TemplateParameters = { + {std::string("typename"), std::string("T"), llvm::None}, + }; + }}, + // Class template + {R"cpp( + template class C, + typename = char, + int = 0, + bool Q = false, + class... Ts> class Foo {}; + template class T> + [[F^oo]] foo; + )cpp", + [](HoverInfo &HI) { + HI.NamespaceScope = ""; + HI.Name = "Foo"; + HI.Kind = SymbolKind::Class; + HI.Definition = + R"cpp(template