Version 0.10.6 Released

MutabilityDetector version 0.10.6 has now been released, and is available from Maven Central. 

 

Biggest change is that Mutability Detector now supports Java bytecode up to v17. Courtesy of @l0s (https://github.com/l0s). Thank you!

Version 0.9.6 Released

MutabilityDetector version 0.9.6 has now been released, and is available from Maven Central.

The most significant changes in this release are:

• improved message when constructing collection fields with unwrapped collections. See #51, #52, thanks go to new contributors badgersow and kwahsog.
• hardcoded results can be configured to apply to MutabilityDetector.assertX methods. This is useful if you want to make assertions programatically, and don't want, for example, assertImmutable(String.class) to fail. See #64 , and thanks to new contributor alexsalesdev.
• MutabilityDetector assertion methods are now thread safe. See #63.
• The command line interface can now give a summary of the analysis, included number of classes scanned, and time taken. See #66 and thanks again to alexsalesdev.

Please also be forewarned that this will be the last release which can be run on pre-Java 8 JVMs. Future releases will still be able to analyse pre-Java 8 bytecode, but I plan to make use of Java 8 features in the codebase from now on, and won't support running on older JVMs.

This release was particularly cool because of the number of contributions made from other people interested in the project.

Happy mutability detecting folks!

New release with JDK 8 support

A new version of Mutability Detector, which supports JDK 8 bytecode,  has been released to Maven Central. Simply bump the version number from 0.9 to 0.9.1 in your build system, or hop over to the project's downloads page to grab the latest artifacts.

Supporting FindBugs v2.0.0+

Over the weekend, a whopper of a bug was reported against MutabilityDetector4FindBugs - the custom detector used to check your classes annotated with @Immutable. Turns out that it only worked on FindBugs v1.3.9, throwing exceptions on v2.0.0+. FindBugs v2.0.0 was released in December 2011, so it's not exactly recent. This is what happens when you don't dogfood (I generally use Mutability Detector in unit tests, not through FindBugs) anyhoo...

The bug has been fixed, but due to source incompatibilities, I am unable to support multiple versions of FindBugs with a single jar. As such, if you want to use MutabilityDetector4FindBugs, you'll have to grab the jar which is appropriate to your installation of FindBugs. Hop over to the project's README for details on how to get the right version.

Meet A Project: An Opportunity to Join An Open Source Project

Are you interested in getting involved in open source development? Maybe you want to improve your skills, raise your chance of landing a job, or just get a taste of what it's like to collaborate in the open source model. However, there's always that problem: "how to get involved"? Scratching your own itch is how many projects are started, but that doesn't guarantee that you'll get to work with others. Finding an existing, active project will let you work with others, but taking that first step can be daunting. You need a place where:

• You can ask questions without feeling intimidated, because you need to be able to learn and make mistakes.
• There are some people who are eager to help you get started, because thrashing around on your own in an alien codebase can be such a drag.
• You can work on real issues and features, not toy ones, because working on something that actually helps the project is a vital part of the experience.

What you need is a project that ticks those boxes. Mutability Detector is one of those projects. Based on the concept of the London Java Community's "Meet-A-Project", I want to invite you, potential contributor, to get involved.

What is Mutability Detector?

A Java library that analyses classes to find out if they are immutable. Using immutability is a popular technique for improving readability and robustness of code. Immutability is built into newer JVM languages like Scala and Clojure, but takes a little more effort to get it right in Java. Mutability Detector helps Java developers implement immutability correctly.

It uses git for version control, hosted on GitHub. It uses Maven for a build tool and CloudBees for continuous integration. The code has been written using test driven development and uses such libraries as ASM and Guava. Mutability Detector has various ways of being executed, but it's mostly standalone; there's no databases or web pages involved. It's intended to work from the command line, work within any and all unit testing frameworks like JUnit and TestNG, and work as a plugin within FindBugs.

If none of that means anything to you, Don't Panic, I can help explain all this stuff. If you're interested in coding in Java, and eager to learn, we can work on picking up that knowledge. Read on.

What is there to work on?

There are three issues in Mutability Detector's issue tracker on GitHub that I intend to get fixed, one way or the other. Two issues are new features that I think would help more users, thus improving popularity of the library. The third is an issue raised by an end user, that, although it has a workaround, would make using the library easier. Alongside the primary goal of getting these features implemented, and that bug fixed, I also want to increase contribution from others. Each of these issues are small enough that we can make progress without giving up our day jobs or taking a sabbatical, but not so small as to be trivial. If a potential employer asks in an interview what your involvement in this project was, you will be able to say "I had a major part in implementing Feature X", rather than, "I fixed a typo in the documentation"[0].

New features:

• Add support for immutable collections of Google Guava 
• Allow clicking through to source of mutability reason 
• Allow writing a single test case which will scan the current classpath for all @Immutable classes 

Again, unless you're a user, those descriptions may mean little to you. Don't Panic.

What's in it for me, as project owner?

To be blunt, I could implement these features myself. As the main developer, I understand the problems, and how to solve them. Even if other people write the code, I'm still likely to spend more time helping others write the code than I would just writing it myself. So why bother? Well, the most selfish reason is that I want experience of technical leadership. I have not had a technical lead role in my career yet (in my whopping 3 years of experience), and want to experience the challenges of mentoring, coaching and directing others. I also expect to learn something from everyone who gets involved, no matter their experience level. And I think it will be fun.

Also, what's the worst that could happen? Nobody responds and I implement the features myself? Big whoop, I'm no worse off. I spend some time helping people contribute and they lose interest? Ah, so what. I accept a contribution and a bug slips into the code? Not the end of the world, we're not developing heart monitors or air traffic control systems. I want open source development to be fun and interesting.

What's in it for you, as a contributor?

Like you, I have wanted to get involved in open source projects and either felt intimidated, didn't know where to begin, or didn't know how to be useful. Here I'm listing several features, all of which I want implemented. Hopefully I'm also making it clear that I am opening myself up to lots of questions, so you shouldn't feel shy about asking. There will be no Linus Torvalds-style flaming here. We may disagree, and I may criticise your code, or reject it until I'm happy to incorporate it, but I think that's fine -- that's collaboration.

If you are struggling to make the move into Java, you can learn by studying how someone else writes it. Not that I'm saying my code is perfect, it's most definitely not, but it can give you a frame of reference, and I'm happy to explain why I wrote code a certain way, or made a particular design choice.

Since I'm strict about it, any code which is eventually accepted will:

• have a high level of test coverage, preferably test driven
• be self documenting
• follow the SOLID principles
• be clean and crafted

So if you don't know what those things are, you'll learn about them in relation to actual code.

If you are looking for something to put on a CV, contributing to open source can help distinguish you from other candidates. It's not just about coding, it's about the collaboration aspect of it. An employer being able to see not just how you code, but how you communicate with people, helps take the risk out of the hiring decision. Fun fact: I wrote and released the first version of Mutability Detector before leaving university; and it was one of the factors in securing a job to go to after I graduated. An interviewer at TIM Group told me he looked at my code and was impressed by the quality.

So how to get started?

Post to the mailing list and announce your interest in contributing, tell me a little about yourself. Read this blog, and try to use Mutability Detector. Try to get a feel for its purpose; run it on some of your own code to see the results. That will help you understand the features. Even just understanding what this project is for is beyond beginner-level Java.

Follow the project README on GitHub, fork, clone and build the project, and submit a super-tedious pull request (like fixing a typo, or renaming a variable). That will get you familiar with building the project, importing it into your development environment, and going through the code submission process.

From there, pick one of the features you are interested in contributing to, and begin working on it. If, at any stage of the process, you hit a bump, ask for help on the project mailing list. I don't mind how 'basic' you think the question is, if it prevents you getting involved, I'm happy to give you pointers. Stuff like "how to install Java?", or "what does this error mean?" are fine. I can't promise I will be a great teacher, but I'll try.

I look forward to hearing from you.




[0] patches which fix typos are very much welcome, they are an important contribution -- they just don't say much about you as a software developer.

v0.9 released, java.lang.String now considered immutable!

In the past hour I just released a new version of Mutability Detector, v0.9. This release is quite significant. If you currently use Mutability Detector, I'd highly recommend upgrading. If you've never used it before, there's never been a better time to try*.

* I reserve the right to make this statement on all future releases ;-)


If I've already convinced you, here's the snippet you'll need for your pom.xml:

    <dependency>
      <groupid>org.mutabilitydetector</groupid>
      <artifactid>MutabilityDetector</artifactid>
      <version>0.9</version>
      <scope>test</scope>
    </dependency>

If I haven't convinced you yet, here's some of the new features that might get you there:

• you can now use java.lang.String in your immutable classes, without your test failing
• analysis recognises safe usage of Collections.unmodifiableXXX methods
• new out-of-the-box "allowed reasons" to help you suppress false positives, in a readable and safe way
• using the Effective Java-style builder pattern to create immutable objects should now not fail tests
• we migrated from Google Code to GitHub, so we're in with the cool kids now, right?
• the usual host of bug fixes and improvements

The String Problem

Finding java.lang.String to be mutable has long been a problem for Mutability Detector, recently rearing it's ugly head during a case study of ThreeTen, the new Date&Time API for Java 8. Although the general case of benign data races has still not been solved, I have taken the pragmatic choice to hard code it as immutable. Similarly for other JDK types, such as the primitive wrappers (Integer, Double, Character, etc), BigDecimal, BigInteger, and Class.

The way String et al have been hardcoded has been exposed to users, so that if you have false positives "tainting" your codebase, you too can tell Mutability Detector to regard certain types as immutable. For further information on this configuration, refer to the JavaDoc.

Unmodifiable Collections

Classes that follow this pattern:

public final class HasUnmodifiableCollection {
    private final List<String>myStrings;
 
    public HasUnmodifiableCollection(List<String> original) {
        this.myStrings = Collections.unmodifiableList(new ArrayList<String>(original));
    }
    // ...
}

... are now considered immutable. Provided you use the methods from the standard JDK to copy (the new ArrayList call) and wrap in an unmodifiable collection (the unmodifiableList call) an error won't be raised. That condition is rather strict, for example, I personally use Guava's Lists.newArrayList method to copy, and that is still flagged as an issue. Let me know how useful it is for you.


Those two additions should hopefully make a major difference in the usability of Mutability Detector. Feedback, as always, is welcome.

What's Next?

That kinda depends on the feedback I get, bug fixes that are getting in your way will likely be top priority. More ambitious plans include: allowing mutable fields, provided they are not mutated and don't escape; studying popular projects like Guava to do more case studies, and/or bundle more useful configuration.

Once the release hits Maven Central, I'll also push out a new version of the FindBugs plugin.

Let me know what you'd like to see being worked on!

Mutability Detector has relocated.

With thanks to some fine work from Mani (@neomatrix369) Mutability Detector has been successfully moved to GitHub. The main motivation for this was to make collaboration easier, so hopefully those pull requests can start flooding in :-)

The GitHub project is available at:

    https://github.com/MutabilityDetector/MutabilityDetector

mutabilitydetector.org now points to the GitHub Pages project at the same repository.

If you have considered fixing or adding something to Mutability Detector, it has never been easier, so get involved!

Mutability Detector In Action

Quick video demonstrating Mutability Detector In Action

Thanks to Jon Dickinson for the video editing.

ThreeTen Case Study: A Time (And Date) To Reflect

We started this series of blogs with 55 test cases, each using Mutability Detector to assert immutability of classes within ThreeTen. 6 of those test cases were failing, and worth investigating. We saw how Mutability Detector can prompt a developer to carefully analyse those classes, and if those classes are actually immutable, how those warnings can be easily and flexibly suppressed. 4/6 of those failures were false positives, and the tests were adapted to suppress those warnings. 2/6 were considered real problems, and have since been fixed in the master branch of ThreeTen.

From this case study, there are several things I have learned about Mutability Detector.

A high pass rate is possible
32 tests passed without suppressing warnings, 23 required suppressions. I am particularly pleased with this success rate, given the strictness of the tool.

The String Problem is both surprising and annoying
An issue that has consistently suprised users is that java.lang.String is found to be mutable. Users don't particularly care that String has a benign data race where its hash field is lazily calculated and assigned outside the constructor. They only care that String is so often held up as an example of an immutable Java class.

As well as being surprising, it causes problems even when you understand what causes it. For example, ThreeTen's ZoneOffset has a String field which requires a suppression like "provided(String.class).isAlsoImmutable()" in the test case. But more annoying than that, ZoneOffset's supposed mutability flows back to the other classes which have a ZoneOffset field. That causes an extra 5 suppressions in the tests for classes which have a ZoneOffset field.

I am now fairly convinced that automatically detecting that String is immutable, which involves correctly deciding if a data race is benign, is just too damn difficult. I have started work to allow hardcoding String as immutable in Mutability Detector. Since ThreeTen has shown that benign data races/internal caching is not limited to String, there should also be an out-of-the-box suppression for this particular use case. 

Panicking at the sight of a mutable type is simplistic
Mutability Detector does not yet allow the safe, encapsulated use of a mutable type. When your class has a field which is a mutable type (or an array) it will raise an error. Checking for non-mutating calls is non-trivial, but I believe, achievable. For arrays it would have to check for element mutation. For mutable objects, it would involve building up a dictionary of mutating methods, and ensuring you don't call them. So your class can have java.util.Date field, as long as it doesn't call, e.g. setYear. With this, I would have to be very wary of the performance implication of such an analysis.

Also, we saw that there is the potential for false negatives if your mutable field escapes. As in, the class never invokes setYear, but it passes the instance as a parameter to a method in another class which does.

Warnings about visibility in Java Memory Model are naive
We saw warnings about non-final fields not receiving the guarantee that they will be visible, even if assigned in the constructor. This ignores that it is possibly to obtain this guarantee in other ways: if your non-final field is volatile, or is of a particular type, e.g. AtomicReference; and if there are other memory barriers e.g. assignment to a volatile field coming last in a constructor where non-final fields are assigned.

The former should be an easy enough change to make. The latter might be more tricky, but still doable.

Analysis is implementation-focussed
The team behind ThreeTen's implementation previously discussed the possibility of using Mutability Detector or other tools for enforcing immutability. The idea being that the test could be part of JSR310's Technology Compatibility Kit (TCK). However, being on the strict side, it would be difficult to meaningfully test for immutability, while also allowing internal caching, in implementations other than the OpenJDK's. This limitation may also apply to anyone wishing to use the analysis at runtime. For now, Mutability Detector is best suited for testing specific implementations of your own classes.

Can help experts
Following the blog entries which exposed real mistakes in immutable implementations, the problems were deemed important enough to warrant fixing. ThreeTen is being developed by experts in their field, for inclusion in the reference implementation, lead by a developer who created one of the most popular examples of using immutability in Java. With Mutability Detector, I am aiming to serve both beginners learning how to write immutable classes, and experienced developers familiar with the technique who have made a "typo". This case demonstrated the latter does happen, and that the tool can help, with little cost in terms of setup and maintenance.

API for allowing reasons for mutability is flexible and powerful
In several of the examples, we saw how we could implement a custom Hamcrest matcher to suppress a false positive. I used Mutability Detector in the way that users are intended to, in that I did not make any changes to the source code. In each case, we could suppress false positives, in a custom way, without any modifications to Mutability Detector itself. Over time I would expect to incorporate some of the more common matchers into the out-of-the-box offerings.

In conclusion, I'm pleased with the results from analysing ThreeTen, but there's lots of room for improvement.

Have you tried Mutability Detector? Loved it? Hated it? I want to hear from you! Any kind of feedback is welcome, either here, on the discussion group, or in email to grundlefleck at gmail dot com.

Checking for Mutability in JSR 310: StandardZoneRules

Here we have the final failing test case, out of the 55 intended immutable classes in ThreeTen.

The assertion:

assertImmutable(Class.forName("javax.time.zone.StandardZoneRules"));

Fails with:

org.mutabilitydetector.unittesting.MutabilityAssertionError: 
Expected: javax.time.zone.StandardZoneRules to be IMMUTABLE
     but: javax.time.zone.StandardZoneRules is actually NOT_IMMUTABLE
    Reasons:
        Field can have a mutable type (java.util.concurrent.ConcurrentHashMap) assigned to it. [Field: lastRulesCache, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: standardTransitions, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: standardOffsets, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: savingsLocalTransitions, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: wallOffsets, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: savingsInstantTransitions, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: lastRules, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (java.util.concurrent.ConcurrentHashMap) assigned to it. [Field: lastRulesCache, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: standardTransitions, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: standardOffsets, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: savingsInstantTransitions, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: wallOffsets, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: lastRules, Class: javax.time.zone.StandardZoneRules]
        Field can have a mutable type (a primitive array) assigned to it. [Field: savingsLocalTransitions, Class: javax.time.zone.StandardZoneRules]
        Field is a primitive array. [Field: standardTransitions, Class: javax.time.zone.StandardZoneRules]
        Field is a primitive array. [Field: standardOffsets, Class: javax.time.zone.StandardZoneRules]
        Field is a primitive array. [Field: savingsInstantTransitions, Class: javax.time.zone.StandardZoneRules]
        Field is a primitive array. [Field: savingsLocalTransitions, Class: javax.time.zone.StandardZoneRules]
        Field is a primitive array. [Field: wallOffsets, Class: javax.time.zone.StandardZoneRules]
        Field is a primitive array. [Field: lastRules, Class: javax.time.zone.StandardZoneRules]
    Allowed reasons:
        None.

This time, a huge 21 reasons why StandardZoneRules is mutable. But it's not, it's immutable. These 21 reasons fall into roughly the same category though. The class has both a mutable type (ConcurrentHashMap) and array types as fields. This causes Mutability Detector to basically panic without further analysis to see if the mutable things are actually modified. However, all the array fields are final, encapsulated, and contain elements which we consider to be immutable. The ConcurrentHashMap is modified, but it's another example of an unobservable mutation, an internal caching strategy.

VERDICT: False positive.
WHAT TO DO: Using the allowed reason matcher implementations from previous examples, we can alter the test to look like this:

assertInstancesOf(Class.forName("javax.time.zone.StandardZoneRules"),
          areImmutable(),
          AssumingTheFields.named("lastRulesCache").areModifiedAsPartAsAnUnobservableCachingStrategy(),
          AssumingArrayFields.named("standardTransitions", 
                        "standardOffsets", 
                        "savingsLocalTransitions",
                        "wallOffsets", 
                        "savingsInstantTransitions", 
                        "lastRules")
              .areNotModifiedAndDoNotEscape());

Which passes. For the initial number of failure reasons, the passing test doesn't end up too bad. A bit refactoring-unfriendly, in that changing field names will cause the test to break. Perhaps a fuzzier matcher would avoid having to change the test whenever a field name changes. Overall I'm fairly happy with that assertion.

Final result: ThreeTen: 4, Mutability Detector 2. Convincing win from ThreeTen.

Stay tuned for a conclusion and my overall assessment and takeaways from this case study into immutability in ThreeTen.

Update: concluding post now available.

Checking for Mutability in JSR 310: ResourceZoneRulesVersion

This post will cover the penultimate failing test case, in a test for immutability in ThreeTen.

The assertion:

assertImmutable(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion"));

Fails with the message:

org.mutabilitydetector.unittesting.MutabilityAssertionError: 
Expected: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion to be IMMUTABLE
     but: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion is actually NOT_IMMUTABLE
    Reasons:
        Can be subclassed, therefore parameters declared to be this type could be mutable subclasses at runtime. [Class: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion]
        Field can have a mutable type (javax.time.zone.ResourceZoneRulesDataProvider) assigned to it. [Field: provider, Class: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion]
        Field can have a mutable type (java.lang.String) assigned to it. [Field: versionID, Class: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion]
        Field can have a mutable type (a primitive array) assigned to it. [Field: regionArray, Class: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion]
        Field can have a mutable type (a primitive array) assigned to it. [Field: ruleIndices, Class: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion]
        Field is a primitive array. [Field: regionArray, Class: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion]
        Field is a primitive array. [Field: ruleIndices, Class: javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion]
    Allowed reasons:
        None.

As usual, like in the previous examples, we can suppress the String problem related to the "versionID" field. There is also the "provider" field, of type ResourceZoneRulesDataProvider. We covered this case in an earlier blog, and decided this class is immutable. Thus our assertion can now look like:

assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion"),
                  areImmutable(),
                  provided(String.class).isAlsoImmutable(),
                  provided("javax.time.zone.ResourceZoneRulesDataProvider").isAlsoImmutable());

Next up is the failure reason that ResourceZoneRulesVersion can be subclassed. Mutability Detector complains about this because if you are a consumer of a type Foo (i.e. you have a method which takes a Foo) but Foo can be subclassed, then callers can pass you a subclass, like MutableFoo. This issue could manifest itself in subtle, hard to detect ways. For example, if you have been adding Foo's as keys in a HashMap, but they are actually MutableFoos which have changed, you could see that the same instance cannot be used to correctly retrieve the relevant value. To allow consumers of your immutable class to have faith in it, so to speak, you should prevent subclassing. The situation is slightly different when you are the producer of the class. As with this example, the ResourceZoneRulesVersion class is an internal class, constructed and used by ResourceZoneRulesDataProvider. Since the producer is explicitly using the new keyword, there is no doubt that instances can be swapped out for mutable subclasses. Here we can do one of two things: we can add the final keyword, which should be a non-breaking change, since scope to extend the class is limited; or we can add suppress the warning so our assertion would look like:

assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion"),
                  areImmutable(),
                  provided(String.class).isAlsoImmutable(),
                  provided("javax.time.zone.ResourceZoneRulesDataProvider").isAlsoImmutable(),
                  allowingForSubclassing());

This would allow the class to be extended. Which leads us to the remaining failures, that "regionArray" and "rulesIndices" are mutable array fields. The element types of these arrays are String and short, respectively. Thus we need not worry that the elements themselves can be mutated. We need only worry that the array is protected from mutation. In this case they are: regionArray is defensively copied into an unmodifiable Set, to provide a getter; and rulesIndices is never exposed. Thus, as in previous examples, we can get the test to pass by assuming these fields are handled safely, and creating an appropriate allowed reason:

public class AssumingArrayFields {
    private ImmutableSet fieldNames;

    public AssumingArrayFields(ImmutableSet fieldNames) {
        this.fieldNames = fieldNames;
    }

    public static AssumingArrayFields named(String first, String... rest) {
        return new AssumingArrayFields(ImmutableSet.copyOf(Iterables.concat(asList(first), asList(rest))));
    }
    
    public Matcher areNotModifiedAndDoNotEscape() {
        return new TypeSafeDiagnosingMatcher() {

            @Override public void describeTo(Description description) { }

            @Override
            protected boolean matchesSafely(MutableReasonDetail reasonDetail, Description mismatchDescription) {
                if (reasonDetail.codeLocation() instanceof FieldLocation) {
                    return reasonDetail.reason().isOneOf(MUTABLE_TYPE_TO_FIELD, ARRAY_TYPE_INHERENTLY_MUTABLE)
                            && fieldNames.contains(((FieldLocation)reasonDetail.codeLocation()).fieldName());
                } else {
                    return false;
                }
            }
            
        };
    }
}

Which allows us to write a passing test:

assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion"),
                  areImmutable(),
                  provided(String.class).isAlsoImmutable(),
                  provided("javax.time.zone.ResourceZoneRulesDataProvider").isAlsoImmutable(),
                  allowingForSubclassing(),
                  AssumingArrayFields.named("regionArray", "ruleIndices").areNotModifiedAndDoNotEscape());

We finally have a passing test. But it's taken a lot of suppressions to get there. I'll leave it as an exercise to the reader to decide if the test, with all its allowed reasons, is pulling its weight.

An interesting issue is brought up by this class. Although the array fields do not escape through, for example, getter methods, they do escape to method invoked by the class itself. These include java.util.Arrays#binarySearch, and java.util.Arrays#asList. In this case, we can trust these methods are safe havens, just from knowing the implementation. However, Mutability Detector does not know anything about these methods. For all Mutability Detector knows, the class could be invoking com.smelly.Evil#mutateYourArray and it would not complain. This is a false negative which, unfortunately, users need to be wary of.

VERDICT: False positive.
WHAT TO DO: In this case, I would make the class final, and allow the other reasons. Personally I feel the test is still worth the cost. It's still preventing setter methods and non-final fields, for me that's good enough, but I understand the number of allowed reasons is getting rather annoying.

Score so far, ThreeTen 3, Mutability Detector 2.

One class left to go, StandardZoneRules. Can Mutability Detector pull out a draw in extra time?
               

Checking for Mutability in JSR 310: ZoneOffsetTransitionRule

Another example of a failing test case from ThreeTen. In this case, the assertion:

assertImmutable(javax.time.zone.ZoneOffsetTransitionRule.class);

Fails with the message:

org.mutabilitydetector.unittesting.MutabilityAssertionError: 
Expected: javax.time.zone.ZoneOffsetTransitionRule to be IMMUTABLE
     but: javax.time.zone.ZoneOffsetTransitionRule is actually NOT_IMMUTABLE
    Reasons:
        Field is not final, if shared across threads the Java Memory Model will not guarantee it is initialised before it is read. [Field: timeEndOfDay, Class: javax.time.zone.ZoneOffsetTransitionRule]
        Field can have a mutable type (javax.time.ZoneOffset) assigned to it. [Field: standardOffset, Class: javax.time.zone.ZoneOffsetTransitionRule]
        Field can have a mutable type (javax.time.ZoneOffset) assigned to it. [Field: offsetBefore, Class: javax.time.zone.ZoneOffsetTransitionRule]
        Field can have a mutable type (javax.time.ZoneOffset) assigned to it. [Field: offsetAfter, Class: javax.time.zone.ZoneOffsetTransitionRule]
    Allowed reasons:
        None.

Once again, the String Problem is biting us. In this case, mutability is seen to flow back from the type javax.time.ZoneOffset. There is a separate test case for ZoneOffset, which is only considered mutable because it has a String field. Thus, we can change the assertion to look like this:

assertInstancesOf(javax.time.zone.ZoneOffsetTransitionRule.class, 
                  areImmutable(),
                  provided(ZoneOffset.class).isAlsoImmutable());

Which leaves us with the single error, that the boolean field timeEndOfDay is not final. In this case, I think Mutability Detector has found a real problem. The field is never reassigned, so callers cannot change the object. However, instances of ZoneOffsetTransitionRule could be published to other threads, without extra synchronisation around the timeEndOfDay field, Without guarantees of visibility from the Java Memory Model, users could see it change from its default value (false) to its real value (true or false) once it's been made visible correctly.

VERDICT: True positive (yay)
WHAT TO DO: Change the field to be final. Making it volatile would also prevent the aforementioned problem, but Mutability Detector wouldn't take account of it (see the previous post). All the other fields in the class are final anyway, so it makes sense to have this one be final as well.

ThreeTen: 2, Mutability Detector 2

Next: ResourceZoneRulesVersion

Checking for Mutability in JSR 310: ZoneRulesGroup

In seeing how Mutability Detector fared on ThreeTen, the reference implementation for JSR 310, the previous two blogs described an in depth analysis of classes which cause failing immutability tests. Hopefully the following examples will be a bit more simple, and clear cut. 

The assertion:
      assertImmutable(ZoneRulesGroup.class); 

Fails with the message:

org.mutabilitydetector.unittesting.MutabilityAssertionError: 
Expected: javax.time.zone.ZoneRulesGroup to be IMMUTABLE
     but: javax.time.zone.ZoneRulesGroup is actually NOT_IMMUTABLE
    Reasons:
        Field is not final, if shared across threads the Java Memory Model will not guarantee it is initialised before it is read. [Field: versions, Class: javax.time.zone.ZoneRulesGroup]
        Field can have a mutable type (java.util.concurrent.atomic.AtomicReference) assigned to it. [Field: versions, Class: javax.time.zone.ZoneRulesGroup]
        Field can have a mutable type (java.lang.String) assigned to it. [Field: groupID, Class: javax.time.zone.ZoneRulesGroup]
    Allowed reasons:
        None.

It's becomming a common and irritating theme, but we can suppress the String Problem as per the previous examples. That leaves us with two issues: that ZoneRulesGroup has a non-final field, and that same field is a mutable type (AtomicReference).

The declaration of the field in question looks like this:

private AtomicReference<TreeMap<String, ZoneRulesVersion>> versions =
            new AtomicReference<TreeMap<String, ZoneRulesVersion>>(new TreeMap<String, ZoneRulesVersion>(Collections.reverseOrder()));

The field itself contains a mutable type, but it's another example of an unobservable caching stretegy. There is also extra synchronization around modification, but thread safety != immutability, and Mutability Detector isn't intended to analyse this. The other complaint from Mutability Detector is that the field may not be visible if published to other threads. This however, is a special case that Mutability Detector does not account for. As described in Java Concurrency In Practice (p52, ch3.5.3 Safe Publication Idioms) 

"A properly constructed object can be safely published by [...] Storing a reference to it into a volatile field or AtomicReference". 

The aforementioned synchronization would also guarantee visibility.


 VERDICT: False positive.

 WHAT TO DO?: I don't see any reason to not make the field final, since it is never reassigned anyway. Thus we could add the final modifier to the declaration. You could say that it's overkill, and just noise, but I think the semantics of how the field is used matches the final keyword, so I'd be happy to add it. To solve the mutable field problem, we could borrow the same allowed reason from an earlier example. The assertion would look like this:

assertInstancesOf(javax.time.zone.ZoneRulesGroup.class,
                  areImmutable(),
                  provided(String.class).isAlsoImmutable(),
                  assumingTheFields("versions").areModifiedAsPartAsAnUnobservableCachingStrategy());

If there's some reason that the field cannot be final that I don't understand, Mutability Detector ships with an allowed reason implementation which can help here. Keeping the field non-final, an assertion could be:

assertInstancesOf(javax.time.zone.ZoneRulesGroup.class,
                  areImmutable(),
                  provided(String.class).isAlsoImmutable(),
                 assumingTheFields("versions").areModifiedAsPartAsAnUnobservableCachingStrategy(),
                  allowingNonFinalFields());

But, "allowingNonFinalFields()" can be overly permissive, allowing new non-final fields to be added in such a way that they don't guarantee safe publication. Perhaps it would be better to specify the field by name, as with the other allowed reason.
For me, this false positive is a good find. In terms of visibility in the Java Memory Model, Mutability Detector could fairly easily permit a field if it is either volatile or an instance of AtomicReference.

Since I'm keeping score, it's ThreeTen: 2, Mutability Detector 1. Next up, ZoneOffsetTransitionRule.

Checking for Mutability in JSR 310: ResourceZoneRulesDataProvider

Checking for Mutability in JSR 310: ResourceZoneRulesDataProvider

Carrying on in my checking of mutability in the reference implementation of JSR 310, ThreeTen, I'm going to look at the next failing test case.

The assertion[0]:

 assertImmutable(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider"));

Fails with:

  org.mutabilitydetector.unittesting.MutabilityAssertionError: 
  Expected: javax.time.zone.ResourceZoneRulesDataProvider to be IMMUTABLE
      but: javax.time.zone.ResourceZoneRulesDataProvider is actually NOT_IMMUTABLE
      Reasons:
   Field can have a mutable type (java.lang.String) assigned to it. [Field: groupID, Class: javax.time.zone.ResourceZoneRulesDataProvider]
   Field can have a mutable type (java.util.HashSet) assigned to it. [Field: regions, Class: javax.time.zone.ResourceZoneRulesDataProvider]
   Field can have a mutable type (java.util.HashSet) assigned to it. [Field: versions, Class: javax.time.zone.ResourceZoneRulesDataProvider]
   Field can have a mutable type (java.util.concurrent.atomic.AtomicReferenceArray) assigned to it. [Field: rules, Class: javax.time.zone.ResourceZoneRulesDataProvider]
      Allowed reasons:
   None.

The first failure reason, exemplifies The String Problem. Which I can suppress by changing the assertion to:

  assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider"), 
   areImmutable(),
   provided(String.class).isAlsoImmutable());
 
Which leaves us with the three remaining failure reasons. The first two are quite similar, that the class has mutable fields of type java.util.Set (the assignment is from the concrete java.util.HashSet). Although we can't tell from the bytecode, the generic type of these Sets are String, and a ThreeTen type, ZoneRulesVersion, which is expected to be immutable.Thus, element types of both fields can be considered immutable. This means that in terms of these fields, we only have to worry about the collection type, not the element type.

At first glance, this appears to be a problem. Here's a snippet which represents a subset of the code in the ResourceZoneRulesDataProvider class:

final class ResourceZoneRulesDataProvider {
  private final Set<ZoneRulesVersion> versions;
  private final Set<String> regions;
  
  private ResourceZoneRulesDataProvider(URL url) {
    // constructor code reading from an input stream
    this.regions = new HashSet<String>(Arrays.asList(aLocallyPopulatedArrayVariable));
    this.versions = aLocallyPopulatedHashSet;
  }
  
  public Set<ZoneRulesVersion> getVersions() {
    return versions;
  }

  public Set<String> getRegionIDs() {
    return regions;
  }
}

Taken at face value, this code represents a problem. If a user could get a hold of an instance of ResourceZoneRulesDataProvider, they could for instance, execute:

 resourceZoneRulesDataProvider.getRegionIDs().remove("Europe/London");

This represents a trade off in Mutability Detector's design, and the choice to not perform whole program analysis. Those methods to retrieve the mutable sets are only invoked from one place, the class ZoneRulesGroup. This class is in the same package, taking advantage of the package-scoped methods in ResourceZoneRulesDataProvider. Tracing up those calls, it appears they are only triggered from the static initialisation block from ZoneRulesGroup, from which neither the ResourceZoneRulesDataProvider instances, or their mutable fields escape. Such a situation would take much more analysis than Mutability Detector currently performs, increasing the time taken to analyse, and greatly complicate the implementation. That's not likely to be an avenue I'd be willing to take, so the next best thing is to offer users the ability to overrule Mutability Detector, and tell the tool that this particular example is safe.

Fortunately, an API for doing so already exists in Mutability Detector, it just takes a bit more effort from the user. It involves implementing a Hamcrest matcher to allow mutability reasons. In the assertion example above, the call "provided(String.class).isAlsoImmutable()" is nothing more than just a Hamcrest matcher. An example of how that could look is:

static final class AssumingTheFields  {
    
    private Set<String> fieldNames;

    private AssumingTheFields(Set<String> fieldNames) {
 this.fieldNames = fieldNames;
    }
    
    public static AssumingTheFields assumingTheFields(String first, String... rest) {
 return new AssumingTheFields(Sets.newHashSet(concat(asList(first), asList(rest))));
    }
    
    Matcher<MutableReasonDetail> areNotModifiedByCallers() {
 return new TypeSafeDiagnosingMatcher<MutableReasonDetail>() {
     @Override public void describeTo(Description description) { }

     @Override
     protected boolean matchesSafely(MutableReasonDetail item, Description mismatchDescription) {
  CodeLocation<?> locationOfMutability = item.codeLocation();
  if (locationOfMutability instanceof FieldLocation) {
      return item.reason().isOneOf(MUTABLE_TYPE_TO_FIELD)
       && fieldNames.contains(((FieldLocation)locationOfMutability).fieldName());
  } else {
      return false;
  }
     }
 };
    }
}

This allows us to write an assertion like this:

    assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider"), 
                      areImmutable(),
                      provided(String.class).isAlsoImmutable(),
                      assumingTheFields("regions", "versions").areNotModifiedByCallers());
                   
("regions" and "versions" being the Sets which escape from ResourceZoneRulesDataProvider). The result of this assertion now looks like this:

org.mutabilitydetector.unittesting.MutabilityAssertionError: 
Expected: javax.time.zone.ResourceZoneRulesDataProvider to be IMMUTABLE
     but: javax.time.zone.ResourceZoneRulesDataProvider is actually NOT_IMMUTABLE
    Reasons:
        Field can have a mutable type (java.util.concurrent.atomic.AtomicReferenceArray) assigned to it. [Field: rules, Class: javax.time.zone.ResourceZoneRulesDataProvider]
    Allowed reasons:
        Field can have a mutable type (java.lang.String) assigned to it. [Field: groupID, Class: javax.time.zone.ResourceZoneRulesDataProvider]
        Field can have a mutable type (java.util.HashSet) assigned to it. [Field: regions, Class: javax.time.zone.ResourceZoneRulesDataProvider]
        Field can have a mutable type (java.util.HashSet) assigned to it. [Field: versions, Class: javax.time.zone.ResourceZoneRulesDataProvider]

Allowing these fields to be mutable leaves us with just one failure. In this case the "rules" field, which is of type AtomicReferenceArray. This is a very similar problem to the previous one. In this case, the collection does not escape, but its contents are modified in one of the instance methods. This appears to be implementation detail of a caching strategy, not an observable change. Again, the scope to invoke this method is limited, and with the current state of the code, it looks to be used safely. As such, we can just treat this field like the escaping Sets, so our assertion can look like this:

  assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider"), 
                    areImmutable(),
                    provided(String.class).isAlsoImmutable(),
                    assumingTheFields("regions", "versions").areNotModifiedByCallers(),
                    assumingTheFields("rules").areModifiedAsPartAsAnUnobservableCachingStrategy());
                 
Which passes. Note that the implementation for both suppressions are the same, the difference in name is to communicate to other developers.

VERDICT: This case is tricky. It's a false positive in the context of whole program analysis. However, the class in isolation is definitely mutable. Classes within the same package have the necessary access to create and mutate instances of this class.
WHAT TO DO?: It is possible to suppress the reasons to get a passing test. Personally, I would either: stop claiming the class is immutable in JavaDoc, and document that instances "depend on the kindness of strangers" as it were, to be effectively immutable; or be more strict, and wrap mutable collection types in unmodifiable versions[1].


In conclusion, in a blatant show of bias, I'm going to consider this a very narrow win for Mutability Detector.That put's us at ThreeTen: 1, Mutability Detector: 1.

Click here for the next entry, ZoneRulesGroup.


[0] This assertion demonstrates using the String version of the method, which is useful when your test is for a class which is out of scope (e.g. private, protected, etc).
[1] Though there are two issues with that: the overhead in creating an unmodifiable wrapper; and that Mutability Detector doesn't yet correctly analyse defensive copies into unmodifiable wrappers, so users would still have to add a suppression anyway.

Times Have Changed, But An Instant Stays The Same: Checking for Mutability in JSR 310

I recently attended a London Java Community meetup to try out the new date and time API scheduled for Java 8[1]. JSR 310 (or "ThreeTen" as named on GitHub) provides an upgrade to the standard date and time libraries available in the JDK. At first use, ThreeTen looks to have addressed the mistakes of old JDK Date API, and draws influence from the popular, defacto standard date/time library, JodaTime[2].

Overall I was very impressed with the design and discoverability of the API. And of course, I'm a big fan of it's design choice to use immutability wherever possible.

Since JodaTime is somewhat of a poster-child for immutability in Java, and it's unsurprising the same design principle has transferred to ThreeTen. I was curious to see how effective Mutability Detector could be in helping to enforce this principle.

Using the highly rigourous and scientific method of grep'ing the codebase for the term " immutable ", as included in the JavaDoc for such types, I found 55 immutable classes in ThreeTen. For each of those 55, I added a unit test case to assert the class is immutable. Out of those 55, 6 tests are failing. Among the passing tests are some of ThreeTen's core classes: LocalDate, LocalDateTime, Duration, Period, and more. I'm really impressed by how 'cleanly' these immutable implementations are. I'm also happy with the apparent success rate of Mutability Detector on this real-world codebase (though I know that the results assume zero false negatives).

There's two ways to look at 6 failing tests from 55. Firstly, assuming that when a ThreeTen author documents a class as immutable, it is, Mutability Detector produces 6 false positives. Secondly, assuming that Mutability Detector correctly analyzes, ThreeTen has unexpected mutability in 6 classes. Since these conditions are mutually exclusive, over the next few blog entries, I'm going to take a look at the classes in question. First up is DateTimeFormatter.

The assertion:

  assertImmutable(javax.time.format.DateTimeFormatter.class);

Fails with:

  org.mutabilitydetector.unittesting.MutabilityAssertionError: 
  Expected: javax.time.format.DateTimeFormatter to be IMMUTABLE
      but: javax.time.format.DateTimeFormatter is actually NOT_IMMUTABLE
      Reasons:
 Field can have a mutable type (java.util.Locale) assigned to it. [Field: locale, Class: javax.time.format.DateTimeFormatter]
 Field can have a mutable type (javax.time.format.DateTimeFormatterBuilder$CompositePrinterParser) assigned to it. [Field: printerParser, Class: javax.time.format.DateTimeFormatter]
      Allowed reasons:
 None.

DateTimeFormatter is considered mutable because it has two fields of mutable types. The types in question being java.util.Locale, and a ThreeTen internal type, CompositePrinterParser.  Since the result for Locale (for the JDK 1.6u25 incarnation at least) is a false positive relating to The String Problem, which I can suppress in the unit test, I'll take a more detailed look at CompositePrinterParser.

The problem that Mutability Detector has with this class is that it contains a field which is an array type. There are several ways of (ab)using an array field that breaks immutability. The class can: (A) modify the contents of the array field after construction; (B) let a mutable element of the array escape to where it can be modified; (C) let the array escape so that callers can modify the contents; (D) take the array passed to the constructor without performing a defensive copy. Note that Mutability Detector isn't yet capable of this kind of analysis, so it just panics at the sight of an array field, and leaves it up to us, the developer, to investigate.

Out of those potential problems described:

(A) doesn't happen. The class only iterates over the array, it doesn't modify the contents.
(B) isn't applicable here. The element type of the array is an interface which is contracted to be immutable.
(C) and (D) are the interesting parts. There is no getter for the array field, and it's private. The only way the array escapes, is that it is passed as the constructor argument to another instance of the same class. Since we know that same class doesn't modify the array, the destination for the escaping array is a "safe haven". That particular constructor is also only ever called by the same class, so it doesn't need to copy the passed in array. There is a second constructor, but that takes a List, and performs a defensive copy of it, so the class really is in control of that array field.

VERDICT: False Positive

WHAT TO DO?:
For both Locale, and CompositePrinterParser, we can add suppressions, or "allowed reasons" for mutability. Thus the test case can look like this:

assertInstancesOf(javax.time.format.DateTimeFormatter.class, 
                  areImmutable(), 
                  provided(Locale.class).isAlsoImmutable(),
                  provided("javax.time.format.DateTimeFormatterBuilder$CompositePrinterParser").isAlsoImmutable());
               
This will allow the test to pass, ignoring the false positives, but failing again if someone introduces mutability another way.
               
Score so far, ThreeTen 1, Mutability Detector 0. Next up, ResourceZoneRulesDataProvider. (Click here for the next blog entry).

[1] Thanks to OpenGamma, Stephen Colebourne, Richard Warburton, James Gough et al for organising.
[2] This is unsurprising, since one of the technical spec leads of JSR 310, Stephen Colebourne is the creator of JodaTime.

Announcing Version 0.8 and MutabilityDetector4FindBugs

Version 0.8 of MutabilityDetector has been released and is now available from Maven Central. This release includes bug fixes for the following issues:

• Correctly analyse the rule "don't allow subclassing" 
• Improve unit test error message to separate failure reasons from allowed reasons    
• Cannot use an implementation of org.hamcrest.Matcher in unit tests  
• Matching effectivelyImmutable classes does not permit allowing reasons for mutability. 
• Cyclic references will cause a stack overflow

Probably the most significant change is that Mutability Detector now recognizes when you have prevented subclassing by only having private constructors (previously you had to mark your class final).

I'm also very pleased to announce the release of a FindBugs plugin for Mutability Detector. This is also available from Maven Central and the project's download page. Just configure your FindBugs installation to pickup the plugin detector during analysis (usually by placing it in the 'plugin' directory) and it will detect classes annotated with @Immutable. The detector will emit a FindBugs warning for each violation of the rules for achieving immutability.

Disclaimer, the Mutability Detector FindBugs plugin is marked as version 0.2 for a reason. It's a first cut, and since I personally use Mutability Detector as a unit testing tool, I don't plan to 'dog food' the plugin so much. So any and all feedback is welcome. Please feel free to email me (Grundlefleck at gmail dot com), file an issue, or post to the Google group.

Imprudence Begets Impetus - No Go For GoDaddy

It's funny how unrelated things can finally give you the motivation to check off an item that's been on the TODO list for a while. In this case it was the online movement to boycott GoDaddy, due to their stance on SOPA.

One of the items I've been meaning to get to Real Soon Now, was to register a domain for Mutability Detector. If for no other reason than making the org.mutabilitydetector package name in the source code actually match Java conventions.

Since I had no existing GoDaddy accounts, I couldn't lend my support to the boycott by moving my domains away. But I could vote with my wallet and go with a competitor. Enter Namecheap, another domain name registrar like GoDaddy, but with a much more amenable outlook on SOPA. Since this was the first domain name purchase I've made, and I'm currently only using it as a redirect, I can't comment on the service in too much detail. But the process was cheap and convenient and their interface was quite friendly and easy to use.

I might do more with www.mutabilitydetector.org in the future, but for now it just redirects to the project homepage, and gives me an ever-so-slight warm and fuzzy for doing a teensy-tiny bit in the efforts against SOPA.

Version 0.7 Released

Mutability Detector v0.7 has been released. It is available for download at the project homepage, or from Maven Central with the details:

Group ID	Artifact ID	Version
org.mutabilitydetector	MutabilityDetector	0.7

This release included improvements in two areas: improve support allowing reasons for mutability in unit tests,  and the introduction of analysis on the 'this' reference escaping. It also includes more documentation to help get started with unit testing for immutability (view online or as JavaDoc embedded in the sources jar).

Other more minor improvements include:

• improving the semantics of the overall result for a class
• adding an 'allowing for subclassing' reason

Any and all feedback welcome.

Also, if you do use Mutability Detector, and your source is accessible, please get in touch. When releasing new versions I want to have as little inconvenience for users as possible, and currently I only judge this using a pretend client codebase. Potentially I could add your project to my continuous integration setup, and test my trunk code against yours to give more realistic feedback.

There's already a couple of issues I want to include in the next release (issues 17, 18, 19 at least) and of course, there's always "The String Problem", but if there's anything that's affecting you, please do get in touch.

New EscapedThisReferenceChecker due for next release

In a previous post I discussed a new check that Mutability Detector should make to provide a more accurate result. The implementation of that checker is now complete, and will be included in an upcoming 0.7 release.

As with any other checks the tool does, the analysis is unsound, that is, it doesn't guarantee correctness. It makes as best a guess as it can, and I hope that empirical study of its use would show that it's good enough to rely on, but it is possible that the tool can be fooled into emitting false positives or false negatives. I thought it would be useful to talk about the checker does and doesn't do.

The checker analyses every constructor of your class, and checks for the following problems.


Assigning 'this' to a field
The tool will render your class mutable if it does any of the following:

1. assigns 'this' to static field 
• SomeOtherClass.staticField = this;
2. assigns 'this' to instance of other object
• someOtherObject.instanceField = this;
3. assigns 'this' to field of this instance
• this.myField = this;
4. assigns 'this' to static field of this class
• ThisClass.staticField = this;

Some of these will be a false positive, e.g. with #2, if you construct an object in the constructor, and assign 'this' to one of its fields, before it's published, it'll be safe.  Also assigning 'this' to an instance field is safe as well (or at least, I can't think why it is unsafe), but it's weird enough that I don't want to lose too much sleep trying to be correct about it.


Passing 'this' as a parameter

The tool will render your class mutable if it does any of the following:

1. passes 'this' as parameter to another method
• new SomeOtherClass(this);
• SomeOtherClass.staticMethod(this);
• someOtherObject.instanceMethod(this);
2. constructs an inner class (which has an implicit reference to 'this')
   
• new InnerClass()

A false positive in this case could be, with #1, passing 'this' to somewhere that only saves the reference, without trying to read any values from it, or with #2, the instance of the inner class might not escape, meaning the 'this' reference hasn't escaped.

Unexpected false positives
The last examples of false positives are ones that can occur with a correct implementation. However, there's one false positive that I didn't expect to create. While the tool is trying to check for 'this' being passed to other methods, it also accidentally raises an issue when instance fields are passed as well.


Here's an example from java.util.Vector:

public Vector(Collection c) {
    elementData = c.toArray();
    elementCount = elementData.length;

    if (elementData.getClass() != Object[].class)
        elementData = Arrays.copyOf(elementData, elementCount, Object[].class);
    }

}



The issue being the call to Arrays.copyOf() with two instance fields. This piece of code will cause Vector to be reported as mutable (among many other reasons), and quite nicely demonstrates the issues involved. Here, the method is single threaded, but it hypothetically could change at any point to publish the references it receives to multiple threads. Without further (expensive) analysis into what Arrays.copyOf() does, I can't conclude whether Vector is immutable or not. I'm choosing to err on the side of caution. There is the scope for a potential whitelist of safe methods, but that's not on the cards just yet.


Another interesting point is that one field is a primitive int, the other is an array. Since the value of the int will be copied to the method, it's probably safe anyway, no matter whether threads are involved or not. However, for the array, the reference to the field will be passed, not the contents. If the array is not completely populated and made visible when it's passed, the receiving method could observe it to change, breaking immutability. This leads on to the wider question of any mutable fields escaping at any point in the object's life cycle, not just in the constructor. For now, I'm embracing the happy accident that the checker will warn you of some potentially dodgy code, and will deliberately leave that particular false positive active.


I don't expect any of these false positives to really cause an issue If you disagree, get in touch, email me, leave a comment, or raise a bug (that may require sign in).



I expect to release a new version within the next week, stay tuned for more news!

Introducing Google Group

A Google Group is now available for questions, discussions or abuse, relating to Mutability Detector. The pertinent details of the group are:

• Group name: mutability-detector
• Group home page: http://groups.google.com/group/mutability-detector
• Group email address mutability-detector@googlegroups.com

Don't be shy!