diff --git a/clang-tools-extra/clang-tidy/objc/CMakeLists.txt b/clang-tools-extra/clang-tidy/objc/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/objc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/objc/CMakeLists.txt @@ -4,6 +4,7 @@ AvoidNSErrorInitCheck.cpp DeallocInCategoryCheck.cpp ForbiddenSubclassingCheck.cpp + MethodUnavailableNotOverrideCheck.cpp MissingHashCheck.cpp ObjCTidyModule.cpp PropertyDeclarationCheck.cpp diff --git a/clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.h b/clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.h @@ -0,0 +1,38 @@ +//===--- MethodUnavailableNotOverrideCheck.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_OBJC_METHODUNAVAILABLENOTOVERRIDECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_METHODUNAVAILABLENOTOVERRIDECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace objc { + +/// Finds methods marked with __attribute__((unavailable)) that do not override +/// any declaration from a superclass. +class MethodUnavailableNotOverrideCheck : public ClangTidyCheck { +public: + MethodUnavailableNotOverrideCheck(StringRef Name, ClangTidyContext *Context); + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.ObjC; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + std::vector FixMacroNames; +}; + +} // namespace objc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_METHODUNAVAILABLENOTOVERRIDECHECK_H diff --git a/clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp b/clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp @@ -0,0 +1,101 @@ +//===--- MethodUnavailableNotOverrideCheck.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 "MethodUnavailableNotOverrideCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Attr.h" +#include "clang/AST/Attrs.inc" +#include "clang/AST/DeclObjC.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/AttrKinds.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/STLExtras.h" +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace objc { + +namespace { + +// Matches Objective-C methods that are not overriding a superclass method. +AST_MATCHER(ObjCMethodDecl, isNotOverriding) { return !Node.isOverriding(); } + +// Returns true if the source location `Loc` is expanded from inside a macro +// named in `Macros`. +static bool isExpandedFromMacros(SourceLocation Loc, + const ASTContext &ASTContext, + const std::vector &Macros) { + if (!Loc.isMacroID() || Macros.empty()) + return false; + + Token Tok; + if (Lexer::getRawToken(Loc, Tok, ASTContext.getSourceManager(), + ASTContext.getLangOpts())) + return false; + + return llvm::is_contained(Macros, Tok.getRawIdentifier().str()); +} + +static llvm::Optional +fixItRemovingUnavailableMethod(const ObjCMethodDecl *MD, + const std::vector &FixedMacros) { + const auto *Attr = MD->getAttr(); + + // If the attribute was expanded from a macro, don't offer a fix-it unless + // the top-level macro is one of the whitelisted macros. + if (Attr->getLoc().isMacroID() && + !isExpandedFromMacros(Attr->getLoc(), MD->getASTContext(), FixedMacros)) { + return llvm::None; + } + return FixItHint::CreateRemoval(MD->getSourceRange()); +} +} // anonymous namespace + +MethodUnavailableNotOverrideCheck::MethodUnavailableNotOverrideCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + FixMacroNames( + utils::options::parseStringList(Options.get("FixMacroNames", ""))) {} + +void MethodUnavailableNotOverrideCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "FixMacroNames", + utils::options::serializeStringList(FixMacroNames)); +} + +void MethodUnavailableNotOverrideCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(objcMethodDecl(hasAttr(clang::attr::Unavailable), + isNotOverriding(), + hasDeclContext(objcInterfaceDecl())) + .bind("decl"), + this); +} + +void MethodUnavailableNotOverrideCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MD = Result.Nodes.getNodeAs("decl"); + SourceLocation AttrLoc = MD->getAttr()->getLocation(); + auto Diagnostic = diag(AttrLoc, "method %0 is marked unavailable but does " + "not override a superclass method") + << MD; + if (auto FixIt = fixItRemovingUnavailableMethod(MD, FixMacroNames)) { + Diagnostic << *FixIt; + } +} + +} // namespace objc +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp b/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp --- a/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp @@ -12,6 +12,7 @@ #include "AvoidNSErrorInitCheck.h" #include "DeallocInCategoryCheck.h" #include "ForbiddenSubclassingCheck.h" +#include "MethodUnavailableNotOverrideCheck.h" #include "MissingHashCheck.h" #include "PropertyDeclarationCheck.h" #include "SuperSelfCheck.h" @@ -31,6 +32,8 @@ "objc-dealloc-in-category"); CheckFactories.registerCheck( "objc-forbidden-subclassing"); + CheckFactories.registerCheck( + "objc-method-unavailable-not-override"); CheckFactories.registerCheck( "objc-missing-hash"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -98,6 +98,12 @@ Finds recursive functions and diagnoses them. +- New :doc:`objc-method-unavailable-not-override + ` check. + + Checks that a method marked with ``__attribute__((unavailable))`` is + overriding a method declaration from a superclass. + New check aliases ^^^^^^^^^^^^^^^^^ @@ -115,7 +121,7 @@ ^^^^^^^^^^^^^^^^^^^^^^^^^^ - Improved :doc:`readability-qualified-auto - ` check now supports a + ` check now supports a `AddConstToQualified` to enable adding ``const`` qualifiers to variables typed with ``auto *`` and ``auto &``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -236,6 +236,7 @@ `objc-avoid-nserror-init `_, `objc-dealloc-in-category `_, `objc-forbidden-subclassing `_, + `objc-method-unavailable-not-override `_, "Yes" `objc-missing-hash `_, `objc-property-declaration `_, "Yes" `objc-super-self `_, "Yes" @@ -283,7 +284,7 @@ `readability-redundant-member-init `_, "Yes" `readability-redundant-preprocessor `_, `readability-redundant-smartptr-get `_, "Yes" - `readability-redundant-string-cstr `_, + `readability-redundant-string-cstr `_, "Yes" `readability-redundant-string-init `_, "Yes" `readability-simplify-boolean-expr `_, "Yes" `readability-simplify-subscript-expr `_, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst b/clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst @@ -0,0 +1,20 @@ +.. title:: clang-tidy - objc-method-unavailable-not-override + +objc-method-unavailable-not-override +==================================== + +Checks that a method marked with ``__attribute__((unavailable))`` is overriding +a method declaration from a superclass. That declaration can usually be +deleted entirely. + +.. code-block:: objc + + @interface ClassA : NSObject + // Neither ClassA nor any superclasses define method -foo. + @end + + @interface ClassB : ClassA + - (void)foo __attribute__((unavailable)); + @end + +Suggests a fix to remove the method declaration entirely. diff --git a/clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m b/clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m @@ -0,0 +1,52 @@ +// RUN: %check_clang_tidy %s objc-method-unavailable-not-override %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: objc-method-unavailable-not-override.FixMacroNames, \ +// RUN: value: "UNAVAILABLE_FIXIT"} \ +// RUN: ]}' -- + +__attribute__((objc_root_class)) +@interface Object +- (instancetype)init; +@end + +@interface MyObject : Object +- (instancetype)init __attribute__((unavailable)); + +// A new method that is not overriding, and is available, should not trigger. +- (void)notOverridingMethod; + +- (void)methodA __attribute__((unavailable)); // methodA +// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: method 'methodA' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override] +// CHECK-FIXES: {{^\s*}}// methodA{{$}} + +// Verify check when unavailable attribute has a message. +- (void)methodB __attribute__((unavailable("use methodE"))); // methodB +// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: method 'methodB' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override] +// CHECK-FIXES: {{^\s*}}// methodB{{$}} + +#define UNAVAILABLE_ATTRIBUTE __attribute__((unavailable)) +#define UNAVAILABLE_FIXIT UNAVAILABLE_ATTRIBUTE +#define UNAVAILABLE_NO_FIXIT UNAVAILABLE_ATTRIBUTE + +// Verify check with fix-it when using an configured macro that expands to the unavailable attribute. +- (void)methodC UNAVAILABLE_FIXIT; // methodC +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: method 'methodC' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override] +// CHECK-FIXES: {{^\s*}}// methodC{{$}} + +// Verify check without fix-it when using a non-configured macro that expands to the unavailable attribute. +- (void)methodD UNAVAILABLE_NO_FIXIT; // methodD +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: method 'methodD' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override] +// CHECK-FIXES-NOT: {{^\s*}}// methodD{{$}} +@end + + +@implementation MyObject + +- (void)notOverridingMethod {} + +// Should not flag implementations for methods declared unavailable; sometimes +// implemementations are provided that assert, to catch such codepaths that +// lead to those calls during development. +- (void)methodA {} + +@end