Consistent visibilities of methods and their return types

A quite easy way of getting more control on dependencies between classes and packages is working actively with visibility modifiers. Most people create their classes (with active support by their IDE) using a public modifier:

public class Item { ... }

This means that the class can now be used from any other type located in any other package. Creating it without a modifier instead will make it only be visible to classes which reside in the same package:

class Item { ... } 

Any attempt of using it from the outside now builds up a little hurdle to the developer and should make him think about the situation. Oliver Gierke created a nice blog post about this approach.

In response to a post from last week about replacing a FindBugs check with a jQAssistant rule he commented that he also misses checks for inconsistencies of visibility declarations, e.g.

class Item { // default visibility
   ...
}

public class ItemRepository {
    public Item findByName(String name) { // public visibility
        return ...;
    }
}

What’s the problem? The method „findByName“ is declared public but the type it returns is only visible to the package. Thus calling the method makes only sense to members of the same package – so why is it public?

Let’s create a constraint to find such inconsistencies:

<constraint id="visibility:PublicMethodMustReturnPublicType">
    <description>The return type of a public method must also be public.</description>
    <cypher><![CDATA[
        MATCH
          (method:Method)-[:RETURNS]->(returnType)
        WHERE
          has(method.visibility)
          and method.visibility = "public"
          and has(returnType.visibility)
          and returnType.visibility <> "public"
        RETURN
          method as PublicMethod, returnType as NonPublicType
        ]]></cypher>
    </constraint>

The rule is quite straight forward: Find all methods and their return types which declare a visibility modifier and check their values.