6f24cc0632
In some cases, the `rule.adoc` at root of a rule is never included anywhere and thus is dead code. It's a maintenance cost by itself, but also it misses opportunities to inline code that seems used by two documents when in fact only one document is actually rendered. And this missed opportunity, in turn, stops us from applying the correct language tag on the code samples.
91 lines
2.7 KiB
Plaintext
91 lines
2.7 KiB
Plaintext
== Why is this an issue?
|
|
|
|
Failing to explicitly declare the visibility of a member variable could result it in having a visibility you don't expect, and potentially leave it open to unexpected modification by other classes.
|
|
|
|
|
|
The default access level modifier may be intentional; in that case, this rule can report false positives.
|
|
|
|
=== Noncompliant code example
|
|
|
|
[source,java,diff-id=1,diff-type=noncompliant]
|
|
----
|
|
class Ball {
|
|
String color = "red"; // Noncompliant
|
|
}
|
|
enum A {
|
|
B;
|
|
int a; // Noncompliant
|
|
}
|
|
----
|
|
|
|
=== Compliant solution
|
|
|
|
[source,java,diff-id=1,diff-type=compliant]
|
|
----
|
|
class Ball {
|
|
private String color = "red"; // Compliant
|
|
}
|
|
enum A {
|
|
B;
|
|
private int a; // Compliant
|
|
}
|
|
----
|
|
|
|
=== Exceptions
|
|
|
|
* Members with comments containing the word `modifier` are ignored, as it indicates the modifier is intentionally omitted.
|
|
* Members annotated with the `@VisibleForTesting` annotation are ignored, as it indicates that visibility has been purposely relaxed to make the code testable.
|
|
|
|
[source,java]
|
|
----
|
|
class Cone {
|
|
@VisibleForTesting
|
|
Logger logger; // Compliant
|
|
}
|
|
----
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
|
|
== Implementation Specification
|
|
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
Explicitly declare the visibility for "xxx".
|
|
|
|
|
|
'''
|
|
|
|
== Comments And Links
|
|
|
|
(visible only on this page)
|
|
|
|
=== relates to: S2950
|
|
|
|
=== on 16 Oct 2014, 11:36:50 Ann Campbell wrote:
|
|
Not appropriate for PHP
|
|
|
|
=== on 22 May 2019, 10:55:33 Tibor Blenessy wrote:
|
|
I believe this rule is not appropriate for Java also and we should deprecate it. In Java, no modifier means package visibility, which is a specific kind of visibility, one of the four available "private", "protected", "public" and "package". It is just an unfortunate feature of Java that this access modifier cannot be specified with a specific keyword, but it can't be replaced. We have better rules related to access specification like RSPEC-2386
|
|
|
|
|
|
cc [~nicolas.harraudeau]
|
|
|
|
=== on 22 May 2019, 11:04:18 Nicolas Harraudeau wrote:
|
|
I agree with your reasoning as package visibility is also quite restrictive. I would however first like to check where this rule comes from. In my experience it is rare to use package visibility. It might be a forgotten "private" field, which would then be a code-smell.
|
|
|
|
=== on 1 Jul 2019, 10:35:49 Tibor Blenessy wrote:
|
|
\[~nicolas.harraudeau], OK to deprecate this rule?
|
|
|
|
=== on 29 Jul 2019, 09:38:35 Tibor Blenessy wrote:
|
|
\[~nicolas.harraudeau] I would like to make a decision and deprecate this rule, unless you tell me otherwise, I will do it by the end of the week
|
|
|
|
=== on 29 Jul 2020, 14:47:24 Alexandre Gigleux wrote:
|
|
I change the type from Vulnerability to Code Smell after discussing with AppSec bubble and the fact the same decision was taken for RSPEC-1104.
|
|
|
|
|
|
endif::env-github,rspecator-view[]
|