Skip to content

Commit 29dc0b1

Browse files
AaronBallmanAaronBallman
committed
Add the readability-redundant-access-specifiers check.
This finds redundant access specifier declarations inside classes, structs, and unions. Patch by Mateusz Mackowski.
1 parent e5972f2 commit 29dc0b1

9 files changed

+345
-0
lines changed

‎clang-tools-extra/clang-tidy/readability/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ add_clang_library(clangTidyReadabilityModule
2121
NamespaceCommentCheck.cpp
2222
NonConstParameterCheck.cpp
2323
ReadabilityTidyModule.cpp
24+
RedundantAccessSpecifiersCheck.cpp
2425
RedundantControlFlowCheck.cpp
2526
RedundantDeclarationCheck.cpp
2627
RedundantFunctionPtrDereferenceCheck.cpp

‎clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "MisplacedArrayIndexCheck.h"
2828
#include "NamedParameterCheck.h"
2929
#include "NonConstParameterCheck.h"
30+
#include "RedundantAccessSpecifiersCheck.h"
3031
#include "RedundantControlFlowCheck.h"
3132
#include "RedundantDeclarationCheck.h"
3233
#include "RedundantFunctionPtrDereferenceCheck.h"
@@ -82,6 +83,8 @@ class ReadabilityModule : public ClangTidyModule {
8283
"readability-misleading-indentation");
8384
CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
8485
"readability-misplaced-array-index");
86+
CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>(
87+
"readability-redundant-access-specifiers");
8588
CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(
8689
"readability-redundant-function-ptr-dereference");
8790
CheckFactories.registerCheck<RedundantMemberInitCheck>(

‎clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
//===--- RedundantAccessSpecifiersCheck.cpp - clang-tidy ------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "RedundantAccessSpecifiersCheck.h"
10+
#include "clang/AST/ASTContext.h"
11+
#include "clang/ASTMatchers/ASTMatchFinder.h"
12+
13+
using namespace clang::ast_matchers;
14+
15+
namespace clang {
16+
namespace tidy {
17+
namespace readability {
18+
19+
void RedundantAccessSpecifiersCheck::registerMatchers(MatchFinder *Finder) {
20+
if (!getLangOpts().CPlusPlus)
21+
return;
22+
23+
Finder->addMatcher(
24+
cxxRecordDecl(has(accessSpecDecl())).bind("redundant-access-specifiers"),
25+
this);
26+
}
27+
28+
void RedundantAccessSpecifiersCheck::check(
29+
const MatchFinder::MatchResult &Result) {
30+
const auto *MatchedDecl =
31+
Result.Nodes.getNodeAs<CXXRecordDecl>("redundant-access-specifiers");
32+
33+
const AccessSpecDecl *LastASDecl = nullptr;
34+
for (DeclContext::specific_decl_iterator<AccessSpecDecl>
35+
AS(MatchedDecl->decls_begin()),
36+
ASEnd(MatchedDecl->decls_end());
37+
AS != ASEnd; ++AS) {
38+
const AccessSpecDecl *ASDecl = *AS;
39+
40+
// Ignore macro expansions.
41+
if (ASDecl->getLocation().isMacroID()) {
42+
LastASDecl = ASDecl;
43+
continue;
44+
}
45+
46+
if (LastASDecl == nullptr) {
47+
// First declaration.
48+
LastASDecl = ASDecl;
49+
50+
if (CheckFirstDeclaration) {
51+
AccessSpecifier DefaultSpecifier =
52+
MatchedDecl->isClass() ? AS_private : AS_public;
53+
if (ASDecl->getAccess() == DefaultSpecifier) {
54+
diag(ASDecl->getLocation(),
55+
"redundant access specifier has the same accessibility as the "
56+
"implicit access specifier")
57+
<< FixItHint::CreateRemoval(ASDecl->getSourceRange());
58+
}
59+
}
60+
61+
continue;
62+
}
63+
64+
if (LastASDecl->getAccess() == ASDecl->getAccess()) {
65+
// Ignore macro expansions.
66+
if (LastASDecl->getLocation().isMacroID()) {
67+
LastASDecl = ASDecl;
68+
continue;
69+
}
70+
71+
diag(ASDecl->getLocation(),
72+
"redundant access specifier has the same accessibility as the "
73+
"previous access specifier")
74+
<< FixItHint::CreateRemoval(ASDecl->getSourceRange());
75+
diag(LastASDecl->getLocation(), "previously declared here",
76+
DiagnosticIDs::Note);
77+
} else {
78+
LastASDecl = ASDecl;
79+
}
80+
}
81+
}
82+
83+
} // namespace readability
84+
} // namespace tidy
85+
} // namespace clang

‎clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
//===--- RedundantAccessSpecifiersCheck.h - clang-tidy ----------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTACCESSSPECIFIERSCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTACCESSSPECIFIERSCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang {
15+
namespace tidy {
16+
namespace readability {
17+
18+
/// Detects redundant access specifiers inside classes, structs, and unions.
19+
///
20+
/// For the user-facing documentation see:
21+
/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-access-specifiers.html
22+
class RedundantAccessSpecifiersCheck : public ClangTidyCheck {
23+
public:
24+
RedundantAccessSpecifiersCheck(StringRef Name, ClangTidyContext *Context)
25+
: ClangTidyCheck(Name, Context),
26+
CheckFirstDeclaration(
27+
Options.getLocalOrGlobal("CheckFirstDeclaration", false)) {}
28+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
29+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
30+
31+
private:
32+
const bool CheckFirstDeclaration;
33+
};
34+
35+
} // namespace readability
36+
} // namespace tidy
37+
} // namespace clang
38+
39+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTACCESSSPECIFIERSCHECK_H

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ Improvements to include-fixer
135135

136136
The improvements are...
137137

138+
- New :doc:`readability-redundant-access-specifiers
139+
<clang-tidy/checks/readability-redundant-access-specifiers>` check.
140+
141+
Finds classes, structs, and unions that contain redundant member
142+
access specifiers.
143+
138144
Improvements to clang-include-fixer
139145
-----------------------------------
140146

‎clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ Clang-Tidy Checks
365365
readability-misplaced-array-index
366366
readability-named-parameter
367367
readability-non-const-parameter
368+
readability-redundant-access-specifiers
368369
readability-redundant-control-flow
369370
readability-redundant-declaration
370371
readability-redundant-function-ptr-dereference

‎clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
.. title:: clang-tidy - readability-redundant-access-specifiers
2+
3+
readability-redundant-access-specifiers
4+
=======================================
5+
6+
Finds classes, structs, and unions containing redundant member (field and
7+
method) access specifiers.
8+
9+
Example
10+
-------
11+
12+
.. code-block:: c++
13+
14+
class Foo {
15+
public:
16+
int x;
17+
int y;
18+
public:
19+
int z;
20+
protected:
21+
int a;
22+
public:
23+
int c;
24+
}
25+
26+
In the example above, the second ``public`` declaration can be removed without
27+
any changes of behavior.
28+
29+
Options
30+
-------
31+
32+
.. option:: CheckFirstDeclaration
33+
34+
If set to non-zero, the check will also diagnose if the first access
35+
specifier declaration is redundant (e.g. ``private`` inside ``class``,
36+
or ``public`` inside ``struct`` or ``union``).
37+
Default is `0`.
38+
39+
Example
40+
^^^^^^^
41+
42+
.. code-block:: c++
43+
44+
struct Bar {
45+
public:
46+
int x;
47+
}
48+
49+
If `CheckFirstDeclaration` option is enabled, a warning about redundant
50+
access specifier will be emitted, because ``public`` is the default member access
51+
for structs.

‎clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t -- \
2+
// RUN: -config="{CheckOptions: [{key: readability-redundant-access-specifiers.CheckFirstDeclaration, value: 1}]}" --
3+
4+
class FooPublic {
5+
private: // comment-0
6+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the implicit access specifier [readability-redundant-access-specifiers]
7+
// CHECK-FIXES: {{^}}// comment-0{{$}}
8+
int a;
9+
};
10+
11+
struct StructPublic {
12+
public: // comment-1
13+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the implicit access specifier [readability-redundant-access-specifiers]
14+
// CHECK-FIXES: {{^}}// comment-1{{$}}
15+
int a;
16+
};
17+
18+
union UnionPublic {
19+
public: // comment-2
20+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the implicit access specifier [readability-redundant-access-specifiers]
21+
// CHECK-FIXES: {{^}}// comment-2{{$}}
22+
int a;
23+
};
24+
25+
class FooMacro {
26+
#if defined(ZZ)
27+
private:
28+
#endif
29+
int a;
30+
};
31+
32+
class ValidInnerStruct {
33+
struct Inner {
34+
private:
35+
int b;
36+
};
37+
};
38+
39+
#define MIXIN private: int b;
40+
41+
class ValidMacro {
42+
MIXIN
43+
};

‎clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t
2+
3+
class FooPublic {
4+
public:
5+
int a;
6+
public: // comment-0
7+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
8+
// CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
9+
// CHECK-FIXES: {{^}}// comment-0{{$}}
10+
int b;
11+
private:
12+
int c;
13+
};
14+
15+
struct StructPublic {
16+
public:
17+
int a;
18+
public: // comment-1
19+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
20+
// CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
21+
// CHECK-FIXES: {{^}}// comment-1{{$}}
22+
int b;
23+
private:
24+
int c;
25+
};
26+
27+
union UnionPublic {
28+
public:
29+
int a;
30+
public: // comment-2
31+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
32+
// CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
33+
// CHECK-FIXES: {{^}}// comment-2{{$}}
34+
int b;
35+
private:
36+
int c;
37+
};
38+
39+
class FooProtected {
40+
protected:
41+
int a;
42+
protected: // comment-3
43+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
44+
// CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
45+
// CHECK-FIXES: {{^}}// comment-3{{$}}
46+
int b;
47+
private:
48+
int c;
49+
};
50+
51+
class FooPrivate {
52+
private:
53+
int a;
54+
private: // comment-4
55+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
56+
// CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
57+
// CHECK-FIXES: {{^}}// comment-4{{$}}
58+
int b;
59+
public:
60+
int c;
61+
};
62+
63+
class FooMacro {
64+
private:
65+
int a;
66+
#if defined(ZZ)
67+
public:
68+
int b;
69+
#endif
70+
private: // comment-5
71+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
72+
// CHECK-MESSAGES: :[[@LINE-8]]:1: note: previously declared here
73+
// CHECK-FIXES: {{^}}// comment-5{{$}}
74+
int c;
75+
protected:
76+
int d;
77+
public:
78+
int e;
79+
};
80+
81+
class Valid {
82+
private:
83+
int a;
84+
public:
85+
int b;
86+
private:
87+
int c;
88+
protected:
89+
int d;
90+
public:
91+
int e;
92+
};
93+
94+
class ValidInnerClass {
95+
public:
96+
int a;
97+
98+
class Inner {
99+
public:
100+
int b;
101+
};
102+
};
103+
104+
#define MIXIN private: int b;
105+
106+
class ValidMacro {
107+
private:
108+
int a;
109+
MIXIN
110+
private:
111+
int c;
112+
protected:
113+
int d;
114+
public:
115+
int e;
116+
};

0 commit comments

Comments
 (0)