mwyman updated this revision to Diff 248611.
mwyman added a comment.
Don't provide fix-it hints when the unavailable attribute is inside a macro,
unless within a config-whitelisted macro.
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,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
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.
+
+ 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
- <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,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<std::string> FixMacroNames;
+};
+
+} // 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,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 <algorithm>
+
+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<std::string> &Macros) {
+ if (!Loc.isMacroID() || Macros.empty())
+ return false;
+
+ Token Tok;
+ if (Lexer::getRawToken(Loc, Tok, ASTContext.getSourceManager(),
+ ASTContext.getLangOpts()))
+ return false;
+
+ auto Iter =
+ std::find(Macros.begin(), Macros.end(), Tok.getRawIdentifier().str());
+ return Iter != Macros.end();
+}
+
+static FixItHint
+fixItRemovingUnavailableMethod(const ObjCMethodDecl *MD,
+ const std::vector<std::string> &FixedMacros) {
+ const auto *Attr = MD->getAttr<UnavailableAttr>();
+
+ // 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 FixItHint();
+ }
+ 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) {
+ if (!getLangOpts().ObjC)
+ return;
+
+ Finder->addMatcher(
+ objcMethodDecl(allOf(hasAttr(clang::attr::Unavailable), isNotOverriding(),
+ hasDeclContext(objcInterfaceDecl())))
+ .bind("decl"),
+ this);
+}
+
+void MethodUnavailableNotOverrideCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MD = Result.Nodes.getNodeAs<ObjCMethodDecl>("decl");
+ SourceLocation AttrLoc = MD->getAttr<UnavailableAttr>()->getLocation();
+ diag(AttrLoc, "method %0 is marked unavailable but does not override a "
+ "superclass method")
+ << MD << fixItRemovingUnavailableMethod(MD, FixMacroNames);
+}
+
+} // 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
