Description
Some frameworks such as Guava, Android and AssertJ include an annotation @VisibleForTesting that can be used to mark methods and fields whose visibility restrictions has been relaxed more than necessary for the API to allow for easier unit testing.
Access to such methods is fine for code in test packages but is discouraged in production code. It would be nice if there was a Sonar rule that could raise an issue if the latter case is encountered, so violations could be spotted quickly in SonarQube and would also be flagged in the IDE by SonarLint.
The rule should cover not only fields but also methods, for scenarios where using a getter is fine in production code but a setter is only allowed to be used in test code.
Noncompliant Code:
/** src/main/java/MyObject.java */
@VisibleForTesting
String foo;
/** src/main/java/Service.java */
new MyObject().foo; // NONCOMPLIANT, foo is accessed from production code
Compilant Code
/** src/main/java/MyObject.java */
@VisibleForTesting
String foo;
/** src/test/java/MyObjectTest.java */
new MyObject().foo; // COMPLIANT, foo is accessed from test code
External references
- Annotation in Guava: https://guava.dev/releases/19.0/api/docs/com/google/common/annotations/VisibleForTesting.html
- Annotation in AssertJ: https://joel-costigliola.github.io/assertj/core/api/org/assertj/core/util/VisibleForTesting.html
- Annotation in Android: https://developer.android.com/reference/android/support/annotation/VisibleForTesting
- Someone already made a plugin for this, but I feel like such a rule is useful enough to be part of Sonar’s default rule set.
- Sonar is already acknowledging the existence of @VisibleForTesting as exceptions to RSPEC-2039 and RSPEC-2156, as added in SONARJAVA-1747.
Type
Code Smell - It’s meant to raise awareness to potential violations of API contracts. Ideally the original developer recognizes the issue right there in the IDE and refrains from accessing that member in that way.
Tags
pitfall - Modifying the internal state of objects in a manner that is not sanctioned by the API in production code could cause unforeseeable problems further down the line and might happen easily by accident if a developer does not check the JavaDoc or original member definition to notice that annotation before using it.
(Currently we use @Deprecated instead of @VisibleForTesting because usage of deprecated code is at least flagged by Sonar. But that is of course diluting the semantics of the annotation and would also raise issues or IDE warnings in test code, where the accessing the members would be fine.)