diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ 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 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -26,6 +26,7 @@ #include "MisplacedArrayIndexCheck.h" #include "NamedParameterCheck.h" #include "NonConstParameterCheck.h" +#include "RedundantAccessSpecifiersCheck.h" #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" @@ -79,6 +80,8 @@ "readability-misleading-indentation"); CheckFactories.registerCheck( "readability-misplaced-array-index"); + CheckFactories.registerCheck( + "readability-redundant-access-specifiers"); CheckFactories.registerCheck( "readability-redundant-function-ptr-dereference"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h new file mode 100644 --- /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/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp @@ -0,0 +1,81 @@ +//===--- 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("redundant-access-specifiers"); + + const AccessSpecDecl *LastASDecl = nullptr; + for (DeclContext::specific_decl_iterator + 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") + << FixItHint::CreateRemoval(ASDecl->getSourceRange()); + } + } + + continue; + } + + if (LastASDecl->getAccess() == ASDecl->getAccess()) { + // Ignore macro expansions. + if (LastASDecl->getLocation().isMacroID()) { + LastASDecl = ASDecl; + continue; + } + + diag(ASDecl->getLocation(), "duplicated 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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -149,6 +149,12 @@ but either don't specify it or the clause is specified but with the kind other than ``none``, and suggests to use the ``default(none)`` clause. +- New :doc:`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 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -259,6 +259,7 @@ 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 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst @@ -0,0 +1,49 @@ +.. title:: clang-tidy - readability-redundant-access-specifiers + +readability-redundant-access-specifiers +======================================= + +Finds classes, structs and unions containing redundant member 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 give warning if the first access + specifier declaration is redundant (e.g. `private` inside `class`). 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, since ``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 --- /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 [readability-redundant-access-specifiers] + // CHECK-FIXES: {{^}}// comment-0{{$}} + int a; +}; + +struct StructPublic { +public: // comment-1 + // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 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 [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 --- /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: duplicated 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: duplicated 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: duplicated 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: duplicated 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: duplicated 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: duplicated 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; +};