mwyman updated this revision to Diff 248396. mwyman added a comment. Updated to explicitly check for __attribute__((unavailable)), to avoid flagging methods marked based on platform availability. Updated test file to validate this.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75569/new/ https://reviews.llvm.org/D75569 Files: clang-tools-extra/clang-tidy/objc/CMakeLists.txt clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.h clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m
Index: clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m @@ -0,0 +1,51 @@ +// RUN: %check_clang_tidy %s objc-method-unavailable-not-override -target x86_64-apple-macosx %t + +__attribute__((objc_root_class)) +@interface Object +- (instancetype)init; + +// Define conditionally-available methods; when unavailable due to platform +// availability, these methods should not warn. +- (void)macMethod __attribute__((availability(macosx,introduced=10.10))) __attribute__((availability(ios,unavailable))); +- (void)iosMethod __attribute__((availability(ios,introduced=2.0))) __attribute__((availability(macosx,unavailable))); +@end + +@interface MyObject : Object +- (instancetype)init __attribute__((unavailable)); + +// A new method that is not overriding, and is available, should not trigger. +- (void)notOverridingMethod; + +// Test that marking methods unavailable, that were defined in a superclass but +// conditionally unavailable based on target, don't warn. +- (void)macMethod __attribute__((unavailable)); +- (void)iosMethod __attribute__((unavailable)); + +- (void)methodA __attribute__((unavailable)); +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: method 'methodA' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override] + +// Verify check when unavailable attribute has a message. +- (void)methodB __attribute__((unavailable("use methodD"))); +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: method 'methodB' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override] + +#define NS_UNAVAILABLE __attribute__((unavailable)) + +// Verify check when using a macro that expands to the unavailable attribute. +- (void)methodC NS_UNAVAILABLE; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: method 'methodC' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override] +@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 + +// Verify that fixes exist to delete entire method declarations: +// CHECK-FIXES: {{^\s*$}} Index: clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst =================================================================== --- /dev/null +++ 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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -236,6 +236,7 @@ `objc-avoid-nserror-init <objc-avoid-nserror-init.html>`_, `objc-dealloc-in-category <objc-dealloc-in-category.html>`_, `objc-forbidden-subclassing <objc-forbidden-subclassing.html>`_, + `objc-method-unavailable-not-override <objc-method-unavailable-not-override.html>`_, "Yes" `objc-missing-hash <objc-missing-hash.html>`_, `objc-property-declaration <objc-property-declaration.html>`_, "Yes" `objc-super-self <objc-super-self.html>`_, "Yes" @@ -283,7 +284,7 @@ `readability-redundant-member-init <readability-redundant-member-init.html>`_, "Yes" `readability-redundant-preprocessor <readability-redundant-preprocessor.html>`_, `readability-redundant-smartptr-get <readability-redundant-smartptr-get.html>`_, "Yes" - `readability-redundant-string-cstr <readability-redundant-string-cstr.html>`_, + `readability-redundant-string-cstr <readability-redundant-string-cstr.html>`_, "Yes" `readability-redundant-string-init <readability-redundant-string-init.html>`_, "Yes" `readability-simplify-boolean-expr <readability-simplify-boolean-expr.html>`_, "Yes" `readability-simplify-subscript-expr <readability-simplify-subscript-expr.html>`_, "Yes" Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -98,6 +98,12 @@ Finds recursive functions and diagnoses them. +- New :doc:`objc-method-unavailable-not-override + <clang-tidy/checks/objc-method-unavailable-not-override>` check. + + Finds methods marked unavailable that do not override any method from a + superclass. + New check aliases ^^^^^^^^^^^^^^^^^ @@ -115,7 +121,7 @@ ^^^^^^^^^^^^^^^^^^^^^^^^^^ - Improved :doc:`readability-qualified-auto - <clang-tidy/checks/readability-qualified-auto>` check now supports a + <clang-tidy/checks/readability-qualified-auto>` check now supports a `AddConstToQualified` to enable adding ``const`` qualifiers to variables typed with ``auto *`` and ``auto &``. Index: clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp +++ 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<ForbiddenSubclassingCheck>( "objc-forbidden-subclassing"); + CheckFactories.registerCheck<MethodUnavailableNotOverrideCheck>( + "objc-method-unavailable-not-override"); CheckFactories.registerCheck<MissingHashCheck>( "objc-missing-hash"); CheckFactories.registerCheck<PropertyDeclarationCheck>( Index: clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.h @@ -0,0 +1,32 @@ +//===--- 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) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace objc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_METHODUNAVAILABLENOTOVERRIDECHECK_H Index: clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp @@ -0,0 +1,53 @@ +//===--- 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 "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/Diagnostic.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace objc { + +// Check for Objective-C methods that are marked unavailable but are not +// overriding another method declaration. Being unavailable for not satisfying +// a platform requirement via an availability() attribute is explicitly not +// intended to be matched here. +AST_MATCHER(ObjCMethodDecl, isUnavailableMethodNotOverriding) { + return !Node.isOverriding() && Node.hasAttr<UnavailableAttr>(); +} + +void MethodUnavailableNotOverrideCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().ObjC) + return; + + Finder->addMatcher(objcMethodDecl(allOf(isUnavailableMethodNotOverriding(), + hasDeclContext(objcInterfaceDecl()))) + .bind("decl"), + this); +} + +void MethodUnavailableNotOverrideCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MD = Result.Nodes.getNodeAs<ObjCMethodDecl>("decl"); + diag(MD->getLocation(), "method %0 is marked unavailable but " + "does not override a superclass method") + << MD << FixItHint::CreateRemoval(MD->getSourceRange()); +} + +} // namespace objc +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/objc/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/objc/CMakeLists.txt +++ 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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits