== Why is this an issue?

Redundant access specifiers should be removed because they needlessly clutter the code.


=== Noncompliant code example

[source,cpp,diff-id=1,diff-type=noncompliant]
----
struct S {
public: // Noncompliant; does not affect any declaration
private:
  void f1();
private: // Noncompliant; does not change accessibility level
  void f2();
private: // Noncompliant; does not affect any declaration
};
class C {
  void f1();
private: // Noncompliant;  does not change accessibility level
  void f2();
};
----


=== Compliant solution

[source,cpp,diff-id=1,diff-type=compliant]
----
struct S {
private:
  void f1();
  void f2();
};
class C {
  void f1();
  void f2();
};
----


=== Exceptions

An access specifier at the very beginning of a `class` or `struct` that matches the default access level is ignored even when it doesn't change any accessibility levels.

[source,cpp]
----
class C {
  private: // redundant but accepted
    // ...
};
struct S {
  public: // redundant but accepted
    // ...
};
----
Such a specifier is redundant but ignored to allow ``class``es and ``struct``s to be described uniformly.

[source,cpp]
----
class C {
  public:
    void call();

  protected:
    int delete();

  private:
    int code;
};
struct S {
  public: // redundant but accepted
    int sum();

  protected:
    int min();

  private:
    int count;
};
----

Additionally, many people use an access specifier not to change the access level but as a visual separator between member functions and member variables of a class. This rule does not raise an issue on this pattern:

[source,cpp]
----
class C {
public:
  void f1();

private:
  void f2();
  void f3();

private: // redundant but accepted: separates functions from variables
  int m1;

}
----

Finally, Qt meta-object system makes use of some custom (macro-based) access specifiers. Even when they have no impact on the access level of the following members according to the {cpp} definition, they are accepted as long as they differ in spelling:

[source,cpp]
----
class Counter : public QObject {
  Q_OBJECT
public:
  Counter() { m_value = 0; }
  int value() const { return m_value; }

public slots: // equivalent to "public" but accepted
  void setValue(int value);

signals: // equivalent to "public" but accepted
  void keyChanged(int newValue);

signals: // Noncompliant; does not change accessibility level
  void valueChanged(int newValue);

private:
  int m_key;
  int m_value;
};
----

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

* Remove this redundant access specifier; it does not change the accessibility level.
* Remove this access specifier; it does not affect any declaration.


=== Highlighting

access specifier


'''
== Comments And Links
(visible only on this page)

=== on 29 Feb 2016, 11:45:02 Alban Auzeill wrote:
I doubt that there's a consensus on this rules. This is an example of discussion:

http://stackoverflow.com/questions/4962942/is-it-confusing-to-omit-the-private-keyword-from-a-class-definition


And bellow the use cases that could lead into exception on this rule:


*Use case 1:*

Some developers like to always write the first access specifier, even if it is redundant with the default access. So the code is easier to read for multi-language developers that don't master {cpp}.

https://github.com/SonarSource/it-sources/blob/master/cpp/ruling/cpp/clang-3.4/include/clang/ASTMatchers/Dynamic/Diagnostics.h#L102

----
class A {
  private:
    void reset();
  // ...
};
struct B {
  public:
    void run();
  // ...
};
----

*Use case 2:*

Developers that separate methods and attributes with always the same pattern: a redundant access specifier + a comment.

----
class A {
  public: // methods
    void run();
  private: // methods
    void reset();
    // ... 50 more lines of declarations ....
  private: // attributes
    int speed;
};
----

*My current view*

* "Use case 1", I don't have a strong opinion, I prefer to not add an exception and see the findings on real code to decide if there is too much noise.
* "Use case 2", I disagree and would not accept an exception about this use case.

=== on 1 Mar 2016, 14:01:14 Alban Auzeill wrote:
The bellow "Use Case 1" generate too many issues, so Massimo and I decided to accept a redundant access specifier before the first member of a class declaration. I'm adding an Exception to the RSPEC.


Now I'm confident with the findings of this rule, so I changed "Default Quality Profiles" from "None" to "Sonar Way".


But concerning "Default Severity" I have no idea.

=== on 1 Mar 2016, 14:59:25 Ann Campbell wrote:
\[~alban.auzeill] I almost updated the Noncompliant example; ``++struct S++`` starts with a "Noncompliant" declaration that will be ignored according to this new exception...? On the other hand, it's ``++public++``, and ``++private++`` is the default...

=== on 2 Mar 2016, 06:35:19 Alban Auzeill wrote:
\[~ann.campbell.2] I have updated the exception paragraph to separate what is accepted ( ``++class++`` starting by ``++private:++`` and ``++struct++`` starting by ``++public:++`` ) from an example with the ``++public/protected/private++`` template.

=== on 2 Mar 2016, 08:56:19 Alban Auzeill wrote:
I've replaced "to be described with uniform templates." by "to be described uniformly " to avoid confusion with {cpp} templates

=== on 2 Mar 2016, 17:37:57 Ann Campbell wrote:
\[~alban.auzeill] I've updated the exception description slightly

endif::env-github,rspecator-view[]
