[clang-tools-extra] 29dc0b1 - Add the readability-redundant-access-specifiers check.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 30 10:31:22 PDT 2019
Author: Aaron Ballman
Date: 2019-10-30T13:30:57-04:00
New Revision: 29dc0b17de6b04afa6110a040053a19b02ca1a87
URL: https://github.com/llvm/llvm-project/commit/29dc0b17de6b04afa6110a040053a19b02ca1a87
DIFF: https://github.com/llvm/llvm-project/commit/29dc0b17de6b04afa6110a040053a19b02ca1a87.diff
LOG: Add the readability-redundant-access-specifiers check.
This finds redundant access specifier declarations inside classes, structs, and unions.
Patch by Mateusz Mackowski.
Added:
clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 2d226b10334a..7455e10831ce 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -21,6 +21,7 @@ add_clang_library(clangTidyReadabilityModule
NamespaceCommentCheck.cpp
NonConstParameterCheck.cpp
ReadabilityTidyModule.cpp
+ RedundantAccessSpecifiersCheck.cpp
RedundantControlFlowCheck.cpp
RedundantDeclarationCheck.cpp
RedundantFunctionPtrDereferenceCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 5005ba3df61f..2fd2870ebe66 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -27,6 +27,7 @@
#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "NonConstParameterCheck.h"
+#include "RedundantAccessSpecifiersCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantDeclarationCheck.h"
#include "RedundantFunctionPtrDereferenceCheck.h"
@@ -82,6 +83,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-misleading-indentation");
CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
"readability-misplaced-array-index");
+ CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>(
+ "readability-redundant-access-specifiers");
CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(
"readability-redundant-function-ptr-dereference");
CheckFactories.registerCheck<RedundantMemberInitCheck>(
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
new file mode 100644
index 000000000000..22625c912580
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
@@ -0,0 +1,85 @@
+//===--- RedundantAccessSpecifiersCheck.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 "RedundantAccessSpecifiersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void RedundantAccessSpecifiersCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ Finder->addMatcher(
+ cxxRecordDecl(has(accessSpecDecl())).bind("redundant-access-specifiers"),
+ this);
+}
+
+void RedundantAccessSpecifiersCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("redundant-access-specifiers");
+
+ const AccessSpecDecl *LastASDecl = nullptr;
+ for (DeclContext::specific_decl_iterator<AccessSpecDecl>
+ AS(MatchedDecl->decls_begin()),
+ ASEnd(MatchedDecl->decls_end());
+ AS != ASEnd; ++AS) {
+ const AccessSpecDecl *ASDecl = *AS;
+
+ // Ignore macro expansions.
+ if (ASDecl->getLocation().isMacroID()) {
+ LastASDecl = ASDecl;
+ continue;
+ }
+
+ if (LastASDecl == nullptr) {
+ // First declaration.
+ LastASDecl = ASDecl;
+
+ if (CheckFirstDeclaration) {
+ AccessSpecifier DefaultSpecifier =
+ MatchedDecl->isClass() ? AS_private : AS_public;
+ if (ASDecl->getAccess() == DefaultSpecifier) {
+ diag(ASDecl->getLocation(),
+ "redundant access specifier has the same accessibility as the "
+ "implicit access specifier")
+ << FixItHint::CreateRemoval(ASDecl->getSourceRange());
+ }
+ }
+
+ continue;
+ }
+
+ if (LastASDecl->getAccess() == ASDecl->getAccess()) {
+ // Ignore macro expansions.
+ if (LastASDecl->getLocation().isMacroID()) {
+ LastASDecl = ASDecl;
+ continue;
+ }
+
+ diag(ASDecl->getLocation(),
+ "redundant access specifier has the same accessibility as the "
+ "previous access specifier")
+ << FixItHint::CreateRemoval(ASDecl->getSourceRange());
+ diag(LastASDecl->getLocation(), "previously declared here",
+ DiagnosticIDs::Note);
+ } else {
+ LastASDecl = ASDecl;
+ }
+ }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h
new file mode 100644
index 000000000000..2aee22f1077c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h
@@ -0,0 +1,39 @@
+//===--- RedundantAccessSpecifiersCheck.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_READABILITY_REDUNDANTACCESSSPECIFIERSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTACCESSSPECIFIERSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Detects redundant access specifiers inside classes, structs, and unions.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-access-specifiers.html
+class RedundantAccessSpecifiersCheck : public ClangTidyCheck {
+public:
+ RedundantAccessSpecifiersCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ CheckFirstDeclaration(
+ Options.getLocalOrGlobal("CheckFirstDeclaration", false)) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const bool CheckFirstDeclaration;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTACCESSSPECIFIERSCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index fc5336602c83..d2819258f17e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -135,6 +135,12 @@ Improvements to include-fixer
The improvements are...
+- New :doc:`readability-redundant-access-specifiers
+ <clang-tidy/checks/readability-redundant-access-specifiers>` check.
+
+ Finds classes, structs, and unions that contain redundant member
+ access specifiers.
+
Improvements to clang-include-fixer
-----------------------------------
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5d3bedc603b2..ac9b999969e5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -365,6 +365,7 @@ Clang-Tidy Checks
readability-misplaced-array-index
readability-named-parameter
readability-non-const-parameter
+ readability-redundant-access-specifiers
readability-redundant-control-flow
readability-redundant-declaration
readability-redundant-function-ptr-dereference
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
new file mode 100644
index 000000000000..74e1a1539b50
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
@@ -0,0 +1,51 @@
+.. title:: clang-tidy - readability-redundant-access-specifiers
+
+readability-redundant-access-specifiers
+=======================================
+
+Finds classes, structs, and unions containing redundant member (field and
+method) access specifiers.
+
+Example
+-------
+
+.. code-block:: c++
+
+ class Foo {
+ public:
+ int x;
+ int y;
+ public:
+ int z;
+ protected:
+ int a;
+ public:
+ int c;
+ }
+
+In the example above, the second ``public`` declaration can be removed without
+any changes of behavior.
+
+Options
+-------
+
+.. option:: CheckFirstDeclaration
+
+ If set to non-zero, the check will also diagnose if the first access
+ specifier declaration is redundant (e.g. ``private`` inside ``class``,
+ or ``public`` inside ``struct`` or ``union``).
+ Default is `0`.
+
+Example
+^^^^^^^
+
+.. code-block:: c++
+
+ struct Bar {
+ public:
+ int x;
+ }
+
+If `CheckFirstDeclaration` option is enabled, a warning about redundant
+access specifier will be emitted, because ``public`` is the default member access
+for structs.
diff --git a/clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp b/clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
new file mode 100644
index 000000000000..ca318f3fb1c4
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t -- \
+// RUN: -config="{CheckOptions: [{key: readability-redundant-access-specifiers.CheckFirstDeclaration, value: 1}]}" --
+
+class FooPublic {
+private: // comment-0
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the implicit access specifier [readability-redundant-access-specifiers]
+ // CHECK-FIXES: {{^}}// comment-0{{$}}
+ int a;
+};
+
+struct StructPublic {
+public: // comment-1
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the implicit access specifier [readability-redundant-access-specifiers]
+ // CHECK-FIXES: {{^}}// comment-1{{$}}
+ int a;
+};
+
+union UnionPublic {
+public: // comment-2
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the implicit access specifier [readability-redundant-access-specifiers]
+ // CHECK-FIXES: {{^}}// comment-2{{$}}
+ int a;
+};
+
+class FooMacro {
+#if defined(ZZ)
+private:
+#endif
+ int a;
+};
+
+class ValidInnerStruct {
+ struct Inner {
+ private:
+ int b;
+ };
+};
+
+#define MIXIN private: int b;
+
+class ValidMacro {
+ MIXIN
+};
diff --git a/clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp b/clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp
new file mode 100644
index 000000000000..c1218e30a4ba
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t
+
+class FooPublic {
+public:
+ int a;
+public: // comment-0
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
+ // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+ // CHECK-FIXES: {{^}}// comment-0{{$}}
+ int b;
+private:
+ int c;
+};
+
+struct StructPublic {
+public:
+ int a;
+public: // comment-1
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
+ // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+ // CHECK-FIXES: {{^}}// comment-1{{$}}
+ int b;
+private:
+ int c;
+};
+
+union UnionPublic {
+public:
+ int a;
+public: // comment-2
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
+ // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+ // CHECK-FIXES: {{^}}// comment-2{{$}}
+ int b;
+private:
+ int c;
+};
+
+class FooProtected {
+protected:
+ int a;
+protected: // comment-3
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
+ // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+ // CHECK-FIXES: {{^}}// comment-3{{$}}
+ int b;
+private:
+ int c;
+};
+
+class FooPrivate {
+private:
+ int a;
+private: // comment-4
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
+ // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+ // CHECK-FIXES: {{^}}// comment-4{{$}}
+ int b;
+public:
+ int c;
+};
+
+class FooMacro {
+private:
+ int a;
+#if defined(ZZ)
+ public:
+ int b;
+#endif
+private: // comment-5
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
+ // CHECK-MESSAGES: :[[@LINE-8]]:1: note: previously declared here
+ // CHECK-FIXES: {{^}}// comment-5{{$}}
+ int c;
+protected:
+ int d;
+public:
+ int e;
+};
+
+class Valid {
+private:
+ int a;
+public:
+ int b;
+private:
+ int c;
+protected:
+ int d;
+public:
+ int e;
+};
+
+class ValidInnerClass {
+public:
+ int a;
+
+ class Inner {
+ public:
+ int b;
+ };
+};
+
+#define MIXIN private: int b;
+
+class ValidMacro {
+private:
+ int a;
+MIXIN
+private:
+ int c;
+protected:
+ int d;
+public:
+ int e;
+};
More information about the cfe-commits
mailing list