Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ NamespaceCommentCheck.cpp NonConstParameterCheck.cpp ReadabilityTidyModule.cpp + RedundantAccessSpecifiersCheck.cpp RedundantControlFlowCheck.cpp RedundantDeclarationCheck.cpp RedundantFunctionPtrDereferenceCheck.cpp Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ 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" @@ -95,6 +96,8 @@ "readability-named-parameter"); CheckFactories.registerCheck( "readability-non-const-parameter"); + CheckFactories.registerCheck( + "readability-redundant-access-specifiers"); CheckFactories.registerCheck( "readability-redundant-control-flow"); CheckFactories.registerCheck( Index: clang-tidy/readability/RedundantAccessSpecifiersCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantAccessSpecifiersCheck.h @@ -0,0 +1,37 @@ +//===--- RedundantAccessSpecifiersCheck.h - clang-tidy ----------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTACCESSSPECIFIERSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTACCESSSPECIFIERSCHECK_H + +#include "../ClangTidy.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); + 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 Index: clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp @@ -0,0 +1,88 @@ +//===--- RedundantAccessSpecifiersCheck.cpp - clang-tidy ------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "RedundantAccessSpecifiersCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +RedundantAccessSpecifiersCheck::RedundantAccessSpecifiersCheck( + llvm::StringRef Name, clang::tidy::ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + CheckFirstDeclaration( + Options.getLocalOrGlobal("CheckFirstDeclaration", false)) {} + +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 Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -198,6 +198,12 @@ Detects usage of magic numbers, numbers that are used as literals instead of introduced via constants or symbols. +- New :doc:`readability-redundant-access-specifiers + ` check. + + Finds classes, structs, and unions that contain redundant member + access specifiers. + - New :doc:`readability-uppercase-literal-suffix ` check. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -249,6 +249,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 Index: docs/clang-tidy/checks/readability-redundant-access-specifiers.rst =================================================================== --- /dev/null +++ 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. Index: test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp =================================================================== --- /dev/null +++ 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 +}; Index: test/clang-tidy/readability-redundant-access-specifiers.cpp =================================================================== --- /dev/null +++ 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; +};