[Java] Class members annotated with @VisibleForTesting should not be accessed from production code

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 */
String foo;

/** src/main/java/Service.java */
new MyObject().foo; // NONCOMPLIANT, foo is accessed from production code

Compilant Code

/** src/main/java/MyObject.java */
String foo;

/** src/test/java/MyObjectTest.java */
new MyObject().foo; // COMPLIANT, foo is accessed from test code

External references

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.

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.)


Thank you very much for this nice description, I like the idea and think it would make a great rule!
We are currently working on rules related to tests, it could enter this scope one way or another.

I don’t expect it to be trivial to implement, but at first glance, we have everything needed in order to implement it. I created a rule specification (RSPEC-5803), may I ask you to have a look (description, severity, message, …), and let me know if it reflects what you had in mind?


Looks good to me. I was about to add that it should of course not raise flags if the member is accessed from production code that could already do so even with more restrictive visibility (i.e. within the defining class itself). But I think the wording “only possible thanks to this relaxed visibility” in the rule description already covers that.