tag:blogger.com,1999:blog-86564656665083532242024-03-04T22:13:33.497-08:00Mutability Detector BlogDiscussing the design, decisions and discovery involved in developing <a href="http://mutabilitydetector.github.com/MutabilityDetector/">Mutability Detector</a>, an open-source analysis tool for Java.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.comBlogger29125tag:blogger.com,1999:blog-8656465666508353224.post-2018654761168556422022-02-01T12:30:00.007-08:002022-02-01T12:31:41.715-08:00Version 0.10.6 Released<p>MutabilityDetector version 0.10.6 has now been released, and is available from <a href="http://search.maven.org/#artifactdetails%7Corg.mutabilitydetector%7CMutabilityDetector%7C0.10.6%7Cjar">Maven Central</a>. </p><p> </p><p>Biggest change is that Mutability Detector n<span class="css-901oao css-16my406 r-poiln3 r-bcqeeo r-qvutc0">ow supports Java bytecode up to v17.
Courtesy of</span><span class="r-18u37iz"> <a class="css-4rbku5 css-18t94o4 css-901oao css-16my406 r-1cvl2hr r-1loqt21 r-poiln3 r-bcqeeo r-qvutc0" dir="ltr" href="https://twitter.com/l0s" role="link">@l0s</a></span><span class="css-901oao css-16my406 r-poiln3 r-bcqeeo r-qvutc0"> (</span><a class="css-4rbku5 css-18t94o4 css-901oao css-16my406 r-1cvl2hr r-1loqt21 r-poiln3 r-bcqeeo r-qvutc0" dir="ltr" href="https://github.com/l0s" rel="noopener noreferrer" role="link" target="_blank"><span aria-hidden="true" class="css-901oao css-16my406 r-poiln3 r-hiw28u r-qvk6io r-bcqeeo r-qvutc0">https://</span>github.com/l0s</a><span class="css-901oao css-16my406 r-poiln3 r-bcqeeo r-qvutc0">). Thank you!</span></p>Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-23473351947393495862016-11-10T08:38:00.001-08:002016-11-10T08:43:04.058-08:00Version 0.9.6 ReleasedMutabilityDetector version 0.9.6 has now been released, and is available from <a href="http://search.maven.org/#artifactdetails%7Corg.mutabilitydetector%7CMutabilityDetector%7C0.9.6%7Cjar">Maven Central</a>. <br />
<br />
The most significant changes in this release are:<br />
<ul>
<li>improved message when constructing collection fields with unwrapped collections. See <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/51">#51</a>, <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/52">#52</a>, thanks go to new contributors <a href="https://github.com/badgersow">badgersow</a> and <a href="https://github.com/kwahsog">kwahsog</a>.</li>
<li>hardcoded results can be configured to apply to <span style="font-family: "courier new" , "courier" , monospace;">MutabilityDetector.assertX </span>methods. This is useful if you want to make assertions programatically, and don't want, for example, assertImmutable(String.class) to fail. See <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/64">#64</a> , and thanks to new contributor <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/assigned/alexsalesdev">alexsalesdev</a>.</li>
<li>MutabilityDetector assertion methods are now thread safe. See <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/63">#63</a>.</li>
<li>The command line interface can now give a summary of the analysis, included number of classes scanned, and time taken. See <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/66">#66</a> and thanks again to <a href="https://github.com/alexsalesdev">alexsalesdev</a>.</li>
</ul>
Please also be forewarned that <u><b>t</b><b>his will be the last release which can be run on pre-Java 8 JVMs.</b></u> 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. <br />
<ul>
</ul>
This release was particularly cool because of the number of contributions made from other people interested in the project.<br />
<br />
Happy mutability detecting folks! Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-76347002363619801942014-03-22T14:36:00.001-07:002014-03-22T14:36:49.042-07:00New release with JDK 8 supportA new version of Mutability Detector, which supports <a href="http://www.oracle.com/technetwork/java/javase/8train-relnotes-latest-2153846.html">JDK 8</a> 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 <a href="https://github.com/MutabilityDetector/MutabilityDetector/wiki/Downloads">downloads</a> page to grab the latest artifacts.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-36234537124948071772013-09-15T10:54:00.001-07:002013-09-15T10:54:33.007-07:00Supporting FindBugs v2.0.0+Over the weekend, a whopper of a bug was reported against <a href="https://github.com/MutabilityDetector/MutabilityDetector4FindBugs">MutabilityDetector4FindBugs</a> - the custom detector used to check your classes annotated with <span style="font-family: Courier New, Courier, monospace;">@Immutable</span>. Turns out that it <a href="https://github.com/MutabilityDetector/MutabilityDetector4FindBugs/issues/3">only worked on FindBugs v1.3.9, throwing exceptions on v2.0.0+</a>. 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...<br />
<br />
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 <a href="https://github.com/MutabilityDetector/MutabilityDetector4FindBugs">README</a> for details on how to get the right version.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-19148882952840491442013-08-28T15:19:00.002-07:002013-09-02T14:15:24.961-07:00Meet A Project: An Opportunity to Join An Open Source ProjectAre 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:<br />
<br />
<ul>
<li>You can ask questions without feeling intimidated, because you need to be able to learn and make mistakes.</li>
<li>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.</li>
<li>You can work on real issues and features, not toy ones, because working on something that actually <b>helps</b> the project is a vital part of the experience.</li>
</ul>
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 <a href="http://www.meetup.com/Londonjavacommunity/events/98543702/">"Meet-A-Project"</a>, I want to invite you, potential contributor, to get involved.<br />
<br />
<h3>
What is Mutability Detector?</h3>
A Java library that analyses classes to find out if they are <a href="http://docs.oracle.com/javase/tutorial/essential/concurrency/immutable.html">immutable</a>. Using immutability is a popular technique for <a href="http://www.javapractices.com/topic/TopicAction.do?Id=29">improving readability and robustness</a> of code. Immutability is built into newer JVM languages like <a href="http://www.scala-lang.org/">Scala</a> and <a href="http://clojure.org/">Clojure</a>, but takes a little more effort to get it right in Java. Mutability Detector helps Java developers implement immutability correctly.<br />
<br />
It uses <a href="http://git-scm.com/">git</a> for version control, hosted on <a href="https://github.com/">GitHub</a>. It uses <a href="http://maven.apache.org/">Maven</a> for a build tool and <a href="http://www.cloudbees.com/">CloudBees</a> for <a href="http://en.wikipedia.org/wiki/Continuous_integration">continuous integration</a>. The code has been written using <a href="http://en.wikipedia.org/wiki/Test-driven_development">test driven development</a> and uses such libraries as <a href="http://asm.ow2.org/">ASM</a> and <a href="http://code.google.com/p/guava-libraries/">Guava</a>. 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 <a href="http://junit.org/">JUnit</a> and <a href="http://testng.org/doc/index.html">TestNG</a>, and work as a plugin within <a href="http://findbugs.sourceforge.net/">FindBugs</a>.<br />
<br />
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.<br />
<br />
What is there to work on?<br />
<br />
There are three issues in Mutability Detector's issue tracker on <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues?state=open">GitHub</a> 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].<br />
<br />
New features:<br />
<ul>
<li><a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/39">Add support for immutable collections of Google Guava</a> </li>
<li><a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/43">Allow clicking through to source of mutability reason </a></li>
<li><a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/44">Allow writing a single test case which will scan the current classpath for all @Immutable classes</a> </li>
</ul>
Again, unless you're a user, those descriptions may mean little to you. Don't Panic.<br />
<h3>
What's in it for me, as project owner?</h3>
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.<br />
<br />
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.<br />
<br />
<h3>
What's in it for you, as a contributor?</h3>
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>I want implemented</i>. 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 <a href="http://en.wikipedia.org/wiki/Linus_Torvalds">Linus Torvalds</a>-<a href="http://linux.slashdot.org/story/13/07/15/2316219/kernel-dev-tells-linus-torvalds-to-stop-using-abusive-language">style flaming</a> 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.<br />
<br />
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.<br />
<br />
Since I'm strict about it, any code which is eventually accepted will:<br />
<br />
<ul>
<li>have a high level of <a href="http://en.wikipedia.org/wiki/Code_coverage">test coverage</a>, preferably test driven</li>
<li>be <a href="http://en.wikipedia.org/wiki/Self-documenting">self documenting</a></li>
<li>follow the <a href="http://en.wikipedia.org/wiki/SOLID_(object-oriented_design)">SOLID</a> principles</li>
<li>be <a href="http://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882">clean</a> and <a href="http://manifesto.softwarecraftsmanship.org/">crafted</a></li>
</ul>
<div>
So if you don't know what those things are, you'll learn about them in relation to actual code.</div>
<div>
<br /></div>
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 <a href="http://www.timgroup.com/">TIM Group</a> told me he looked at my code and was impressed by the quality.<br />
<br />
<h3>
So how to get started?</h3>
Post to the <a href="mailto:mutability-detector@googlegroups.com">mailing list</a> 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.<br />
<br />
Follow the project <a href="https://github.com/MutabilityDetector/MutabilityDetector/blob/master/README.md">README</a> on GitHub, <a href="https://help.github.com/articles/fork-a-repo">fork</a>, <a href="https://help.github.com/articles/fork-a-repo#step-2-clone-your-fork">clone</a> and <a href="http://www.mkyong.com/maven/how-to-run-unit-test-with-maven/">build</a> the project, and submit a super-tedious <a href="https://help.github.com/articles/creating-a-pull-request">pull request</a> (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.<br />
<br />
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 <a href="https://groups.google.com/forum/#!forum/mutability-detector">project mailing list</a>. 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.<br />
<br />
I look forward to hearing from you.<br />
<br />
<br />
<br />
<br />
[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.<br />
<br />
<br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com4tag:blogger.com,1999:blog-8656465666508353224.post-37434227039489934762013-01-27T15:05:00.001-08:002013-01-27T15:05:04.635-08:00v0.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*.<br />
<br />
* I reserve the right to make this statement on all future releases ;-)<br />
<br />
<br />
If I've already convinced you, here's the snippet you'll need for your pom.xml:<br />
<br />
<pre><code>
<dependency>
<groupid>org.mutabilitydetector</groupid>
<artifactid>MutabilityDetector</artifactid>
<version>0.9</version>
<scope>test</scope>
</dependency>
</code></pre>
<br />
<br />
If I haven't convinced you yet, here's some of the new features that might get you there:<br />
<br />
<ul>
<li>you can now use <span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;">java.lang.String</span></span> in your immutable classes, without your test failing</li>
<li>analysis recognises safe usage of <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/26">Collections.unmodifiableXXX </a>methods</li>
<li>new out-of-the-box "allowed reasons" to help you suppress false positives, in a readable and safe way </li>
<li>using the Effective Java-style builder pattern to create immutable objects should <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/33">now not fail tests</a></li>
<li>we migrated from <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/29">Google Code to GitHub</a>, so we're in with the cool kids now, right? </li>
<li>the usual host of <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues?page=1&state=closed">bug fixes and improvements</a></li>
</ul>
<br />
<h3>
The String Problem</h3>
Finding <a href="https://github.com/MutabilityDetector/MutabilityDetector/issues/4"><span style="font-family: "Courier New",Courier,monospace; font-size: small;">java.lang.String</span> to be mutable</a> has long been a problem for Mutability Detector, recently rearing it's ugly head during a <a href="http://mutability-detector.blogspot.co.uk/2012/07/threeten-case-study-time-and-date-to.html">case study of ThreeTen</a>, 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 (<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;">Integer</span></span>, <span style="font-family: "Courier New",Courier,monospace;">Double</span>, <span style="font-family: "Courier New",Courier,monospace;">Character</span>, etc), <span style="font-family: "Courier New",Courier,monospace;">BigDecimal</span>, <span style="font-family: "Courier New",Courier,monospace;">BigInteger</span>, and <span style="font-family: "Courier New",Courier,monospace;">Class</span>.<br />
<br />
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 <a href="http://mutabilitydetector.github.com/MutabilityDetector/mvn-site/apidocs/org/mutabilitydetector/unittesting/MutabilityAssert.html">JavaDoc</a>.<br />
<br />
<h3>
Unmodifiable Collections</h3>
Classes that follow this pattern:<br />
<span style="font-size: small;"><br /></span>
<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;">public final class HasUnmodifiableCollection {</span></span><br />
<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"> private final List<span style="font-size: small;"><</span></span></span><span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;">String<string>>myStrings;</string></span></span><br />
<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"> </span></span><br />
<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"> public HasUnmodifiableCollection(List</span></span><span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"><span style="font-size: small;"><</span></span></span><span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"><string><span style="font-family: "Courier New",Courier,monospace;">String></span> original) {</string></span></span><br />
<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"> this.myStrings = Collections.unmodifiableList(new ArrayList</span></span><span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"><span style="font-size: small;"><</span></span></span><span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"><string><span style="font-family: "Courier New",Courier,monospace;">String></span>(original));</string></span></span><br />
<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"> }</span></span><br />
<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;"> // ... </span></span><br />
<span style="font-size: small;"><span style="font-family: "Courier New",Courier,monospace;">}</span></span><br />
<br />
... are now considered immutable. Provided you use the methods from the standard JDK to copy (the <span style="font-family: "Courier New",Courier,monospace;">new ArrayList </span>call) and wrap in an unmodifiable collection (the <span style="font-family: "Courier New",Courier,monospace;">unmodifiableList</span> call) an error won't be raised. That condition is rather strict, for example, I personally use Guava's <span style="font-family: "Courier New",Courier,monospace;">Lists.newArrayList</span> method to copy, and that is still flagged as an issue. Let me know how useful it is for you.<br />
<br />
<br />
Those two additions should hopefully make a major difference in the usability of Mutability Detector. Feedback, as always, is welcome.<br />
<br />
<h3>
What's Next?</h3>
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.<br />
<br />
Once the release hits Maven Central, I'll also push out a new version of the FindBugs plugin. <br />
<br />
Let me know what you'd like to see being worked on!<br />
<br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com8tag:blogger.com,1999:blog-8656465666508353224.post-17645587255758077362012-12-09T14:23:00.003-08:002012-12-09T14:25:06.885-08:00Mutability Detector has relocated.With thanks to some fine work from Mani (<a href="https://github.com/neomatrix369">@neomatrix369</a>) 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 :-)<br />
<br />
The GitHub project is available at:<br />
<br />
<a href="https://github.com/MutabilityDetector/MutabilityDetector">https://github.com/MutabilityDetector/MutabilityDetector</a><br />
<br />
<a href="http://mutabilitydetector.org/">mutabilitydetector.org</a> now points to the GitHub Pages project at the same repository.<br />
<br />
If you have considered fixing or adding something to Mutability Detector, it has never been easier, so get involved! Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-41208912249475638182012-11-25T07:17:00.002-08:002012-11-25T07:17:21.842-08:00Mutability Detector In ActionQuick video demonstrating Mutability Detector In Action
<div class="separator" style="clear: both; text-align: center;">
</div>
<div class="separator" style="clear: both; text-align: center;">
</div>
<div class="separator" style="clear: both; text-align: center;">
</div>
<iframe allowfullscreen='allowfullscreen' webkitallowfullscreen='webkitallowfullscreen' mozallowfullscreen='mozallowfullscreen' width='800' height='600' src='https://www.youtube.com/embed/ST3wU79nwS8?feature=player_embedded' frameborder='0'></iframe><br />
<br />
Thanks to Jon Dickinson for the video editing.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-82747740233865583952012-07-19T13:30:00.002-07:002012-07-19T13:30:50.381-07:00ThreeTen Case Study: A Time (And Date) To ReflectWe started this <a href="http://mutability-detector.blogspot.co.uk/2012/07/times-have-changed-but-instant-stays.html">series of blogs</a> with <a href="http://code.google.com/p/mutability-detector/source/browse/trunk/ClientOfMutabilityDetector/trunk/ClientOfMutabilityDetector/src/test/java/org/mutabilitydetector/casestudies/ThreeTenTest.java?spec=svn610&r=610">55 test cases</a>, 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 <a href="https://github.com/ThreeTen/threeten/commit/60ad0f24569491ab44a26e929c6e6c7e26b62244">fixed in the master branch of ThreeTen</a>.<br />
<br />
From this case study, there are several things I have learned about Mutability Detector.<br />
<br />
<b>A high pass rate is possible</b><br />
32 tests passed without suppressing warnings, 23 required suppressions. I am particularly pleased with this success rate, given the strictness of the tool.<br />
<br />
<b>The String Problem is both surprising and annoying</b><br />
An issue that has consistently <a href="https://twitter.com/jodastephen/status/225164290984914945">suprised users</a> is that <span style="font-family: 'Courier New', Courier, monospace;">java.lang.String</span> is found to be mutable. Users don't particularly care that <span style="font-family: 'Courier New', Courier, monospace;">String</span> has a benign data race where its <a href="http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/lang/String.java#String.hashCode%28%29"><span style="font-family: 'Courier New', Courier, monospace;">hash</span> field is lazily calculated and assigned outside the constructor</a>. They only care that <span style="font-family: 'Courier New', Courier, monospace;">String</span> is so often held up as an example of an immutable Java class.<br />
<br />
As well as being surprising, it causes problems even when you understand what causes it. For example, ThreeTen's <span style="font-family: 'Courier New', Courier, monospace;">ZoneOffset</span> has a <span style="font-family: 'Courier New', Courier, monospace;">String</span> field which requires a suppression like "<span style="font-family: 'Courier New', Courier, monospace;">provided(String.class).isAlsoImmutable()</span>" in the test case. But more annoying than that, <span style="font-family: 'Courier New', Courier, monospace;">ZoneOffset</span>'s supposed mutability flows back to the other classes which have a <span style="font-family: 'Courier New', Courier, monospace;">ZoneOffset</span> field. That causes an extra 5 suppressions in the tests for classes which have a <span style="font-family: 'Courier New', Courier, monospace;">ZoneOffset</span> field.<br />
<br />
I am now fairly convinced that automatically detecting that <span style="font-family: 'Courier New', Courier, monospace;">String</span> is immutable, which involves correctly deciding if a data race is benign, is just too damn difficult. I have started work to allow <a href="http://code.google.com/p/mutability-detector/source/browse/trunk/MutabilityDetector/trunk/MutabilityDetector/src/main/java/org/mutabilitydetector/Configuration.java">hardcoding <span style="font-family: 'Courier New', Courier, monospace;">String</span> as immutable in Mutability Detector</a>. Since ThreeTen has shown that benign data races/internal caching is not limited to <span style="font-family: 'Courier New', Courier, monospace;">String</span>, there should also be an out-of-the-box suppression for this particular use case.
<br />
<br />
<b>Panicking at the sight of a mutable type is simplistic</b><br />
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 <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_16.html">will raise an error</a>. 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 <span style="font-family: 'Courier New', Courier, monospace;">java.util.Date</span> field, as long as it doesn't call, e.g. <span style="font-family: 'Courier New', Courier, monospace;">setYear</span>.
With this, I would have to be very wary of the performance implication of such an analysis.<br />
<br />
Also, we saw that there is the potential for false negatives if your mutable field escapes. As in, the class never invokes <span style="font-family: 'Courier New', Courier, monospace;">setYear</span>, but it passes the instance as a parameter to a method in another class which does.<br />
<br />
<b>Warnings about visibility in Java Memory Model are naive</b><br />
We <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_15.html">saw warnings</a> 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 <span style="font-family: 'Courier New', Courier, monospace;">volatile</span>, or is of a particular type, e.g. <span style="font-family: 'Courier New', Courier, monospace;">AtomicReference</span>; 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.<br />
<br />
The former should be an easy enough change to make. The latter might be more tricky, but still doable.<br />
<br />
<b>Analysis is implementation-focussed</b><br />
The team behind ThreeTen's implementation <a href="http://sourceforge.net/mailarchive/forum.php?thread_name=4F047398.5000808%40oracle.com&forum_name=threeten-develop">previously discussed the possibility of using Mutability Detector or other tools for enforcing immutability</a>. The idea being that the test could be part of JSR310's <a href="http://en.wikipedia.org/wiki/Technology_Compatibility_Kit">Technology Compatibility Kit (TCK)</a>. 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.<br />
<br />
<b>Can help experts</b><br />
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 <a href="http://blog.joda.org/">popular examples</a> 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.
<br />
<br />
<b>API for allowing reasons for mutability is flexible and powerful</b><br />
In several of the <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310.html">examples</a>, we <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_16.html">saw</a> how we could implement a custom <a href="http://code.google.com/p/hamcrest/">Hamcrest</a> 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.<br />
<br />
In conclusion, I'm pleased with the results from analysing ThreeTen, but there's lots of room for improvement.<br />
<br />
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 <a href="https://groups.google.com/forum/?fromgroups#!forum/mutability-detector">discussion group</a>, or in email to grundlefleck at gmail dot com.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-90336032173710832142012-07-16T13:08:00.000-07:002012-07-19T13:35:33.565-07:00Checking for Mutability in JSR 310: StandardZoneRules<br />
Here we have the <a href="http://mutability-detector.blogspot.co.uk/2012/07/times-have-changed-but-instant-stays.html">final failing test case</a>, out of the <a href="http://code.google.com/p/mutability-detector/source/browse/trunk/ClientOfMutabilityDetector/trunk/ClientOfMutabilityDetector/src/test/java/org/mutabilitydetector/casestudies/ThreeTenTest.java">55 intended immutable classes in ThreeTen</a>.<br />
<br />
The assertion:<br />
<pre class="prettyprint">assertImmutable(Class.forName("javax.time.zone.StandardZoneRules"));
</pre>
Fails with:<br />
<pre class="prettyprint">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.
</pre>
This time, a huge 21 reasons why <span style="font-family: 'Courier New', Courier, monospace;">StandardZoneRules</span> 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 (<span style="font-family: 'Courier New', Courier, monospace;">ConcurrentHashMap</span>) 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 <span style="font-family: 'Courier New', Courier, monospace;">ConcurrentHashMap</span> is modified, but it's another example of an unobservable mutation, an internal caching strategy.<br />
<br />
VERDICT: False positive.<br />
WHAT TO DO: Using the allowed reason matcher implementations <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_16.html">from previous examples</a>, we can alter the test to look like this:<br />
<pre class="prettyprint">assertInstancesOf(Class.forName("javax.time.zone.StandardZoneRules"),
areImmutable(),
AssumingTheFields.named("lastRulesCache").areModifiedAsPartAsAnUnobservableCachingStrategy(),
AssumingArrayFields.named("standardTransitions",
"standardOffsets",
"savingsLocalTransitions",
"wallOffsets",
"savingsInstantTransitions",
"lastRules")
.areNotModifiedAndDoNotEscape());
</pre>
<br />
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.<br />
<br />
Final result: ThreeTen: 4, Mutability Detector 2. Convincing win from ThreeTen.<br />
<br />
Stay tuned for a conclusion and my overall assessment and takeaways from this case study into immutability in ThreeTen.<br />
<br />
Update: <a href="http://mutability-detector.blogspot.co.uk/2012/07/threeten-case-study-time-and-date-to.html">concluding post now available</a>.<br />
<br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-17549183368381437502012-07-16T11:46:00.000-07:002012-07-19T13:34:21.064-07:00Checking for Mutability in JSR 310: ResourceZoneRulesVersion<br />
This post will cover the penultimate failing test case, in a test for immutability in <a href="https://github.com/ThreeTen/threeten">ThreeTen</a>.<br />
<br />
The assertion:<br />
<br />
<pre class="prettyprint">assertImmutable(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion"));
</pre>
<br />
<br />
Fails with the message:<br />
<pre class="prettyprint">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.
</pre>
<br />
As usual, like in the previous examples, we can suppress the String problem related to the "<span style="font-family: 'Courier New', Courier, monospace;">versionID</span>" field. There is also the "<span style="font-family: 'Courier New', Courier, monospace;">provider</span>" field, of type <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesDataProvider</span>. We covered this case <a href="http://mutability-detector.blogspot.co.uk/2012/07/times-have-changed-but-instant-stays.html">in an earlier blog</a>, and decided this class is immutable. Thus our assertion can now look like:<br />
<pre class="prettyprint">assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion"),
areImmutable(),
provided(String.class).isAlsoImmutable(),
provided("javax.time.zone.ResourceZoneRulesDataProvider").isAlsoImmutable());
</pre>
<br />
Next up is the failure reason that <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesVersion</span> can be subclassed. Mutability Detector complains about this because if you are a consumer of a type <span style="font-family: 'Courier New', Courier, monospace;">Foo</span> (i.e. you have a method which takes a <span style="font-family: 'Courier New', Courier, monospace;">Foo</span>) but <span style="font-family: 'Courier New', Courier, monospace;">Foo</span> can be subclassed, then callers can pass you a subclass, like <span style="font-family: 'Courier New', Courier, monospace;">MutableFoo</span>. This issue could manifest itself in subtle, hard to detect ways. For example, if you have been adding <span style="font-family: 'Courier New', Courier, monospace;">Foo</span>'s as keys in a <span style="font-family: 'Courier New', Courier, monospace;">HashMap</span>, but they are actually <span style="font-family: 'Courier New', Courier, monospace;">MutableFoo</span>s 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 <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesVersion</span> class is an internal class, constructed and used by <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesDataProvider</span>. Since the producer is
explicitly using the <span style="font-family: 'Courier New', Courier, monospace;">new</span> 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 <span style="font-family: 'Courier New', Courier, monospace;">final</span> 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:<br />
<br />
<pre class="prettyprint">assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider$ResourceZoneRulesVersion"),
areImmutable(),
provided(String.class).isAlsoImmutable(),
provided("javax.time.zone.ResourceZoneRulesDataProvider").isAlsoImmutable(),
allowingForSubclassing());
</pre>
This would allow the class to be extended. Which leads us to the remaining failures, that "<span style="font-family: 'Courier New', Courier, monospace;">regionArray</span>" and "<span style="font-family: 'Courier New', Courier, monospace;">rulesIndices</span>" are mutable array fields. The element types of these arrays are <span style="font-family: 'Courier New', Courier, monospace;">String</span> and <span style="font-family: 'Courier New', Courier, monospace;">short</span>, 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: <span style="font-family: 'Courier New', Courier, monospace;">regionArray</span> is defensively copied into an unmodifiable <span style="font-family: 'Courier New', Courier, monospace;">Set</span>, to provide a getter; and <span style="font-family: 'Courier New', Courier, monospace;">rulesIndices</span> 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:<br />
<pre class="prettyprint">public class AssumingArrayFields {
private ImmutableSet<string> fieldNames;
public AssumingArrayFields(ImmutableSet<string> 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<mutablereasondetail> areNotModifiedAndDoNotEscape() {
return new TypeSafeDiagnosingMatcher<mutablereasondetail>() {
@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;
}
}
};
}
}
</mutablereasondetail></mutablereasondetail></string></string></pre>
Which allows us to write a passing test:<br />
<pre class="prettyprint">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());
</pre>
<br />
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.<br />
<br />
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 <span style="font-family: 'Courier New', Courier, monospace;">java.util.Arrays#binarySearch</span>, and <span style="font-family: 'Courier New', Courier, monospace;">java.util.Arrays#asList</span>. 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 <span style="font-family: 'Courier New', Courier, monospace;">com.smelly.Evil#mutateYourArray</span> and it would not complain. This is a false negative which, unfortunately, users need to be wary of.<br />
<br />
VERDICT: False positive.<br />
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.<br />
<br />
Score so far, ThreeTen 3, Mutability Detector 2.<br />
<br />
One class left to go, <span style="font-family: 'Courier New', Courier, monospace;"><a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_3574.html">StandardZoneRules</a></span>. Can Mutability Detector pull out a draw in extra time?<br />
<br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-5955048403492851202012-07-15T13:41:00.004-07:002012-07-19T13:33:34.119-07:00Checking for Mutability in JSR 310: ZoneOffsetTransitionRule<br />
Another example of a failing test case from ThreeTen. In this case, the assertion:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;">assertImmutable(javax.time.zone.ZoneOffsetTransitionRule.class);</span><br />
<br />
Fails with the message:<br />
<br />
<br />
<pre class="prettyprint">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.
</pre>
<br />
<br />
Once again, the String Problem is biting us. In this case, mutability is seen to flow back from the type <a href="https://github.com/ThreeTen/threeten/blob/c88868d3cb050941b2daf1a2667aa88cc549b6ea/src/main/java/javax/time/calendar/ZoneOffset.java">javax.time.ZoneOffset</a>. There is a separate test case for <a href="http://code.google.com/p/mutability-detector/source/browse/trunk/ClientOfMutabilityDetector/trunk/ClientOfMutabilityDetector/src/test/java/org/mutabilitydetector/casestudies/ThreeTenTest.java#271">ZoneOffset</a>, which is only considered mutable because it has a String field. Thus, we can change the assertion to look like this:<br />
<br />
<pre class="prettyprint">assertInstancesOf(javax.time.zone.ZoneOffsetTransitionRule.class,
areImmutable(),
provided(ZoneOffset.class).isAlsoImmutable());
</pre>
<br />
Which leaves us with the single error, that the boolean field <span style="font-family: 'Courier New', Courier, monospace;">timeEndOfDay</span> 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 <span style="font-family: 'Courier New', Courier, monospace;">ZoneOffsetTransitionRule</span> could be published to other threads, without extra synchronisation around the <span style="font-family: 'Courier New', Courier, monospace;">timeEndOfDay</span> 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.<br />
<br />
VERDICT: True positive (yay)<br />
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 <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_15.html">previous post)</a>. All the other fields in the class are final anyway, so it makes sense to have this one be final as well.<br />
<br />
ThreeTen: 2, Mutability Detector 2<br />
<br />
Next: <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_16.html">ResourceZoneRulesVersion</a><br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com1tag:blogger.com,1999:blog-8656465666508353224.post-44160208453565409302012-07-15T13:05:00.000-07:002012-07-16T05:26:15.854-07:00Checking for Mutability in JSR 310: ZoneRulesGroupIn seeing how Mutability Detector fared on ThreeTen, the reference implementation for JSR 310, the previous <a href="http://mutability-detector.blogspot.co.uk/2012/07/times-have-changed-but-instant-stays.html">two</a> <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310.html">blogs</a> 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.
<br />
<br />
The assertion:<br />
<span style="font-family: 'Courier New', Courier, monospace;">assertImmutable(<a href="https://github.com/ThreeTen/threeten/blob/c56213c6567f5722be9e992d2296ed56fe85720a/src/main/java/javax/time/calendar/zone/ZoneRulesGroup.java">ZoneRulesGroup</a>.class); </span><br />
<br />
Fails with the message:
<br />
<pre class="prettyprint">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.
</pre>
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).<br />
<br />
The declaration of the field in question looks like this:
<br />
<br />
<pre class="prettyprint">private AtomicReference<TreeMap<String, ZoneRulesVersion>> versions =
new AtomicReference<TreeMap<String, ZoneRulesVersion>>(new TreeMap<String, ZoneRulesVersion>(Collections.reverseOrder()));
</pre>
<br />
<span style="font-family: 'Times New Roman'; white-space: normal;">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) </span><br />
<span style="font-family: 'Times New Roman'; white-space: normal;">
</span><br />
<blockquote class="tr_bq">
<span style="font-family: 'Times New Roman'; white-space: normal;">"A properly constructed object can be safely published by [...] Storing a reference to it into a volatile field or AtomicReference". </span></blockquote>
<span style="font-family: 'Times New Roman'; white-space: normal;">
</span><br />
<span style="font-family: 'Times New Roman'; white-space: normal;">The aforementioned synchronization would also guarantee visibility.</span><br />
<span style="background-color: white;"><br /></span><br />
VERDICT: False positive.<br />
<br />
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:
<br />
<pre class="prettyprint">assertInstancesOf(javax.time.zone.ZoneRulesGroup.class,
areImmutable(),
provided(String.class).isAlsoImmutable(),
assumingTheFields("versions").areModifiedAsPartAsAnUnobservableCachingStrategy());
</pre>
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:
<br />
<pre class="prettyprint">assertInstancesOf(javax.time.zone.ZoneRulesGroup.class,
areImmutable(),
provided(String.class).isAlsoImmutable(),
assumingTheFields("versions").areModifiedAsPartAsAnUnobservableCachingStrategy(),
allowingNonFinalFields());
</pre>
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.
<br />
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.
<br />
<br />
Since I'm keeping score, it's ThreeTen: 2, Mutability Detector 1. <a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_3495.html">Next up, ZoneOffsetTransitionRule</a>.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com1tag:blogger.com,1999:blog-8656465666508353224.post-13597362288479318192012-07-15T11:37:00.001-07:002012-07-16T05:24:50.197-07:00Checking for Mutability in JSR 310: ResourceZoneRulesDataProvider<br />
Checking for Mutability in JSR 310: ResourceZoneRulesDataProvider<br />
<br />
Carrying on in my <a href="http://mutability-detector.blogspot.co.uk/2012/07/times-have-changed-but-instant-stays.html">checking of mutability in the reference implementation of JSR 310, ThreeTen</a>, I'm going to look at the next failing test case.<br />
<br />
The assertion[0]:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;">assertImmutable(Class.forName("<a href="http://javax.time.zone.resourcezonerulesdataprovider/">javax.time.zone.ResourceZoneRulesDataProvider</a>"));</span><br />
<br />
Fails with:<br />
<pre class="prettyprint"> 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.
</pre>
The first failure reason, exemplifies <a href="http://code.google.com/p/mutability-detector/issues/detail?id=3">The String Problem</a>. Which I can suppress by changing the assertion to:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;"> assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider"), </span><br />
<span style="font-family: 'Courier New', Courier, monospace;"><span class="Apple-tab-span" style="white-space: pre;"> </span> areImmutable(),</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"><span class="Apple-tab-span" style="white-space: pre;"> </span> provided(String.class).isAlsoImmutable());</span><br />
<span class="Apple-tab-span" style="white-space: pre;"> </span> <br />
Which leaves us with the three remaining failure reasons. The first two are quite similar, that the class has mutable fields of type <span style="font-family: 'Courier New', Courier, monospace;">java.util.Set</span> (the assignment is from the concrete <span style="font-family: 'Courier New', Courier, monospace;">java.util.HashSet)</span>. Although we can't tell from the bytecode, the generic type of these <span style="font-family: 'Courier New', Courier, monospace;">Set</span>s are <span style="font-family: 'Courier New', Courier, monospace;">String</span>, and a ThreeTen type, <span style="font-family: 'Courier New', Courier, monospace;"><a href="https://github.com/ThreeTen/threeten/blob/c88868d3cb050941b2daf1a2667aa88cc549b6ea/src/main/java/javax/time/calendar/zone/ZoneRulesVersion.java">ZoneRulesVersion</a></span>, 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.<br />
<br />
At first glance, this appears to be a problem. Here's a snippet which represents a subset of the code in the <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesDataProvider</span> class:<br />
<br />
<pre class="prettyprint">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;
}
}
</pre>
<br />
Taken at face value, this code represents a problem. If a user could get a hold of an instance of <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesDataProvider</span>, they could for instance, execute:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;">resourceZoneRulesDataProvider.getRegionIDs().remove("Europe/London");</span><br />
<br />
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 <span style="font-family: 'Courier New', Courier, monospace;"><a href="https://github.com/ThreeTen/threeten/blob/c88868d3cb050941b2daf1a2667aa88cc549b6ea/src/main/java/javax/time/calendar/zone/ZoneRulesGroup.java">ZoneRulesGroup</a></span>. This class is in the same package, taking advantage of the package-scoped methods in <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesDataProvider</span>. Tracing up those calls, it appears they are only triggered from the static initialisation block from <span style="font-family: 'Courier New', Courier, monospace;">ZoneRulesGroup</span>, from which neither the <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesDataProvider</span> 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.<br />
<br />
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 <a href="http://code.google.com/p/hamcrest/">Hamcrest</a> <a href="http://code.google.com/p/hamcrest/source/browse/trunk/hamcrest-java/hamcrest-core/src/main/java/org/hamcrest/Matcher.java">matcher</a> to allow mutability reasons. In the assertion example above, the call "<span style="font-family: 'Courier New', Courier, monospace;">provided(String.class).isAlsoImmutable()</span>" is nothing more than just a Hamcrest matcher. An example of how that could look is:<br />
<br />
<pre class="prettyprint">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;
}
}
};
}
}
</pre>
<br />
This allows us to write an assertion like this:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;"> assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider"), </span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> areImmutable(),</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> provided(String.class).isAlsoImmutable(),</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> assumingTheFields("regions", "versions").areNotModifiedByCallers());</span><br />
<br />
("<span style="font-family: 'Courier New', Courier, monospace;">regions</span>" and "<span style="font-family: 'Courier New', Courier, monospace;">versions</span>" being the <span style="font-family: 'Courier New', Courier, monospace;">Set</span>s which escape from <span style="font-family: 'Courier New', Courier, monospace;">ResourceZoneRulesDataProvider</span>). The result of this assertion now looks like this:<br />
<pre class="prettyprint">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]
</pre>
Allowing these fields to be mutable leaves us with just one failure. In this case the "<span style="font-family: 'Courier New', Courier, monospace;">rules</span>" field, which is of type <span style="font-family: 'Courier New', Courier, monospace;">AtomicReferenceArray</span>. 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:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;"> assertInstancesOf(Class.forName("javax.time.zone.ResourceZoneRulesDataProvider"), </span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> areImmutable(),</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> provided(String.class).isAlsoImmutable(),</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> assumingTheFields("regions", "versions").areNotModifiedByCallers(),</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> assumingTheFields("rules").areModifiedAsPartAsAnUnobservableCachingStrategy());</span><br />
<br />
Which passes. Note that the implementation for both suppressions are the same, the difference in name is to communicate to other developers.<br />
<br />
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.<br />
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].<br />
<br />
<br />
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.<br />
<br />
<a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310_15.html">Click here for the next entry, ZoneRulesGroup.</a><br />
<br />
<br />
[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).<br />
[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.<br />
<br />
<br />
<br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-83471858457233549982012-07-15T06:28:00.000-07:002012-07-16T05:23:06.874-07:00Times Have Changed, But An Instant Stays The Same: Checking for Mutability in JSR 310<br />
I recently attended a <a href="http://www.meetup.com/Londonjavacommunity/events/71709942/">London Java Community meetup</a> to try out the new date and time API scheduled for Java 8[1]. <a href="http://jcp.org/en/jsr/detail?id=310">JSR 310</a> (or <a href="https://github.com/ThreeTen/threeten">"ThreeTen"</a> 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, <a href="http://joda-time.sourceforge.net/">JodaTime</a>[2].<br />
<br />
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.<br />
<br />
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.<br />
<br />
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 <a href="http://code.google.com/p/mutability-detector/source/browse/trunk/ClientOfMutabilityDetector/trunk/ClientOfMutabilityDetector/src/test/java/org/mutabilitydetector/casestudies/ThreeTenTest.java">added a unit test case to assert the class is immutable</a>. Out of those 55, 6 tests are failing. Among the passing tests are some of ThreeTen's core classes: <span style="font-family: 'Courier New', Courier, monospace;">LocalDate</span>, <span style="font-family: 'Courier New', Courier, monospace;">LocalDateTime</span>, <span style="font-family: 'Courier New', Courier, monospace;">Duration</span>, <span style="font-family: 'Courier New', Courier, monospace;">Period</span>, 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).<br />
<br />
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 <a href="https://github.com/ThreeTen/threeten/blob/master/src-standard/main/java/javax/time/format/DateTimeFormatter.java">DateTimeFormatter</a>.<br />
<br />
The assertion:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;">assertImmutable(javax.time.format.DateTimeFormatter.class);</span><br />
<br />
Fails with:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;"> org.mutabilitydetector.unittesting.MutabilityAssertionError: </span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> Expected: javax.time.format.DateTimeFormatter to be IMMUTABLE</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> but: javax.time.format.DateTimeFormatter is actually NOT_IMMUTABLE</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> Reasons:</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"><span class="Apple-tab-span" style="white-space: pre;"> </span> Field can have a mutable type (java.util.Locale) assigned to it. [Field: locale, Class: javax.time.format.DateTimeFormatter]</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"><span class="Apple-tab-span" style="white-space: pre;"> </span> Field can have a mutable type (javax.time.format.DateTimeFormatterBuilder$CompositePrinterParser) assigned to it. [Field: printerParser, Class: javax.time.format.DateTimeFormatter]</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> Allowed reasons:</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"><span class="Apple-tab-span" style="white-space: pre;"> </span> None.</span><br />
<br />
DateTimeFormatter is considered mutable because it has two fields of mutable types. The types in question being <span style="font-family: 'Courier New', Courier, monospace;">java.util.Locale</span>, and a ThreeTen internal type, <span style="font-family: 'Courier New', Courier, monospace;"><a href="https://github.com/ThreeTen/threeten/blob/master/src-standard/main/java/javax/time/format/DateTimeFormatterBuilder.java#L1258">CompositePrinterParser</a></span>. Since the result for Locale (for the JDK 1.6u25 incarnation at least) is a false positive relating to <i><a href="http://code.google.com/p/mutability-detector/issues/detail?id=3">The String Problem</a></i>, which I can suppress in the unit test, I'll take a more detailed look at <span style="font-family: 'Courier New', Courier, monospace;">CompositePrinterParser</span>.<br />
<br />
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 <i>panics</i> at the sight of an array field, and leaves it up to us, the developer, to investigate.<br />
<br />
Out of those potential problems described:<br />
<br />
(A) doesn't happen. The class only iterates over the array, it doesn't modify the contents.<br />
(B) isn't applicable here. The element type of the array is an interface which is contracted to be immutable.<br />
(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 <u>of the same class</u>. 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.<br />
<br />
VERDICT: False Positive<br />
<br />
WHAT TO DO?:<br />
For both Locale, and CompositePrinterParser, we can add suppressions, or "allowed reasons" for mutability. Thus the test case can look like this:<br />
<br />
<span style="font-family: 'Courier New', Courier, monospace;">assertInstancesOf(javax.time.format.DateTimeFormatter.class, </span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> areImmutable(), </span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> provided(Locale.class).isAlsoImmutable(),</span><br />
<span style="font-family: 'Courier New', Courier, monospace;"> </span><span style="background-color: white; font-family: 'Courier New', Courier, monospace;">provided("javax.time.format.DateTimeFormatterBuilder$CompositePrinterParser").isAlsoImmutable());</span><br />
<br />
This will allow the test to pass, ignoring the false positives, but failing again if someone introduces mutability another way.<br />
<br />
Score so far, ThreeTen 1, Mutability Detector 0. Next up, <a href="https://github.com/ThreeTen/threeten/blob/master/src/main/java/javax/time/zone/ResourceZoneRulesDataProvider.java">ResourceZoneRulesDataProvider</a>. (<a href="http://mutability-detector.blogspot.co.uk/2012/07/checking-for-mutability-in-jsr-310.html">Click here for the next blog entry</a>).<br />
<br />
[1] Thanks to OpenGamma, Stephen Colebourne, Richard Warburton, James Gough et al for organising.<br />
[2] This is unsurprising, since one of the technical spec leads of JSR 310, Stephen Colebourne is the creator of JodaTime.<br />
<br />
<br />
<br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-74226476780226755942012-03-01T14:04:00.000-08:002012-03-01T14:04:06.818-08:00Announcing Version 0.8 and MutabilityDetector4FindBugs<br />
Version 0.8 of MutabilityDetector has been released and is now available from <a href="http://repo1.maven.org/maven2/org/mutabilitydetector/MutabilityDetector/0.8/MutabilityDetector-0.8.jar">Maven Central</a>. This release includes bug fixes for the <a href="http://code.google.com/p/mutability-detector/issues/list?can=1&q=closed-after%3A2011%2F11%2F16&colspec=ID+Type+Status+Priority+Milestone+Owner+Summary+Closed&cells=tiles">following issues</a>:<br />
<br />
<ul>
<li><a href="http://code.google.com/p/mutability-detector/issues/detail?id=17">Correctly analyse the rule "don't allow subclassing"</a> </li>
<li><a href="http://code.google.com/p/mutability-detector/issues/detail?id=19">Improve unit test error message to separate failure reasons from allowed reasons</a> </li>
<li><a href="http://code.google.com/p/mutability-detector/issues/detail?id=20">Cannot use an implementation of org.hamcrest.Matcher in unit tests </a> </li>
<li><a href="http://code.google.com/p/mutability-detector/issues/detail?id=21">Matching effectivelyImmutable classes does not permit allowing reasons for mutability.</a> </li>
<li><a href="http://code.google.com/p/mutability-detector/issues/detail?id=22">Cyclic references will cause a stack overflow</a> </li>
</ul>
Probably the most significant change is that Mutability Detector now recognizes when you have prevented subclassing by only having <b style="font-family: "Courier New",Courier,monospace;">private</b> constructors (previously you had to mark your class <b><span style="font-family: "Courier New",Courier,monospace;">final</span></b>). <br />
<br />
I'm also very pleased to announce the release of a <a href="http://findbugs.sourceforge.net/">FindBugs</a> plugin for Mutability Detector. This is also available from <a href="http://repo1.maven.org/maven2/org/mutabilitydetector/MutabilityDetector4FindBugs/0.2/MutabilityDetector4FindBugs-0.2.jar">Maven Central</a> and the <a href="http://code.google.com/p/mutability-detector/downloads/list">project's download page</a>. Just configure your FindBugs installation to <a href="http://code.google.com/p/findbugs/wiki/Plugins4FindBugs">pickup the plugin detector during analysis </a>(usually by placing it in the<span style="font-family: "Courier New",Courier,monospace;"> 'plugin' </span>directory) and it will detect classes annotated with <b><span style="font-family: "Courier New",Courier,monospace;">@Immutable</span></b>. The detector will emit a FindBugs warning for each violation of the rules for achieving immutability.<br />
<br />
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), <a href="http://code.google.com/p/mutability-detector/issues/entry">file an issue</a>, or post to the <a href="http://groups.google.com/group/mutability-detector?pli=1">Google group</a>.<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-75668224807987609252011-12-30T04:38:00.000-08:002011-12-30T04:38:06.946-08:00Imprudence Begets Impetus - No Go For GoDaddyIt'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 <a href="http://godaddyboycott.org/">boycott GoDaddy</a>, due to their stance on <a href="http://en.wikipedia.org/wiki/Stop_Online_Piracy_Act">SOPA. </a><br />
<br />
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 <span style="font-family: "Courier New",Courier,monospace;">org.mutabilitydetector</span> package name in the source code actually match Java conventions.<br />
<br />
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 <a href="http://www.namecheap.com/">Namecheap</a>, another domain name registrar like GoDaddy, but with a <a href="http://community.namecheap.com/blog/2011/12/22/we-say-no-to-sopa/">much more amenable outlook on SOPA</a>. 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.<br />
<br />
I might do more with <a href="http://www.mutabilitydetector.org/">www.mutabilitydetector.org</a> 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.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-39830841424264116522011-11-16T13:16:00.001-08:002011-11-16T13:51:28.791-08:00Version 0.7 ReleasedMutability Detector v0.7 has been released. It is available for download <a href="http://code.google.com/p/mutability-detector/downloads/list">at the project homepage</a>, or from Maven Central with the details:<br />
<br />
<table class="wikitable"><tbody>
<tr><td style="border: 1px solid #ccc; padding: 5px;">Group ID </td><td style="border: 1px solid #ccc; padding: 5px;">Artifact ID </td><td style="border: 1px solid #ccc; padding: 5px;">Version </td></tr>
<tr><td style="border: 1px solid #ccc; padding: 5px;">org.mutabilitydetector </td><td style="border: 1px solid #ccc; padding: 5px;">MutabilityDetector </td><td style="border: 1px solid #ccc; padding: 5px;">0.7 </td></tr>
</tbody></table>
<br />
This release included improvements in two areas: improve support allowing reasons for mutability in unit tests, and <a href="http://mutability-detector.blogspot.com/2011/11/new-escapedthisreferencechecker-due-for.html">the introduction of analysis on the 'this' reference escaping</a>. It also includes more documentation to help get started with <a href="https://mutability-detector.googlecode.com/svn/trunk/MutabilityDetector/trunk/javadoc/latest/org/mutabilitydetector/unittesting/MutabilityAssert.html">unit testing for immutability</a> (view online or as JavaDoc embedded in the sources jar).<br />
<br />
Other more minor improvements include:<br />
<ul>
<li>improving the <a href="http://mutability-detector.blogspot.com/2011/09/better-way-of-categorising-immutability.html">semantics of the overall result</a> for a class</li>
<li>adding an 'allowing for subclassing' reason</li>
</ul>
Any and all feedback welcome.<br />
<br />
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 <a href="http://code.google.com/p/mutability-detector/source/browse/#svn%2Ftrunk%2FClientOfMutabilityDetector%2Ftrunk%2FClientOfMutabilityDetector">pretend client codebase</a>. Potentially I could add your project to my continuous integration setup, and test my trunk code against yours to give more realistic feedback.<br />
<br />
There's already a couple of issues I want to include in the next release (issues <a href="http://code.google.com/p/mutability-detector/issues/detail?id=17">17</a>, <a href="http://code.google.com/p/mutability-detector/issues/detail?id=18">18</a>, <a href="http://code.google.com/p/mutability-detector/issues/detail?id=19">19</a> at least) and of course, there's always "<a href="http://code.google.com/p/mutability-detector/issues/detail?id=3">The String Problem</a>", but if there's anything that's affecting you, please do get in touch.<br />
<br />
<br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-54307231526390187882011-11-09T11:58:00.000-08:002011-11-09T13:35:48.608-08:00New EscapedThisReferenceChecker due for next releaseIn a <a href="http://mutability-detector.blogspot.com/2010_07_01_archive.html">previous post</a>
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.<br />
<br />
As with any other checks the tool does, the analysis is <i>unsound</i>,
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.<br />
<br />
The checker analyses <i>every constructor</i> of your class, and checks for the following problems. <br />
<br />
<br />
<span style="font-size: large;">Assigning '<b><span style="font-family: 'Courier New', Courier, monospace;">this</span></b>' to a field</span><br />
The tool will render your class mutable if it does any of the following:<br />
<ol>
<li>assigns '<span style="font-family: 'Courier New', Courier, monospace;">this</span>' to static field </li>
<ul style="font-family: "Courier New",Courier,monospace;">
<li><b>SomeOtherClass.staticField = this; </b></li>
</ul>
<li>assigns 'this' to instance of other object</li>
<ul style="font-family: "Courier New",Courier,monospace;">
<li><b>someOtherObject.instanceField = this;</b></li>
</ul>
<li>assigns 'this' to field of this instance</li>
<ul>
<li><b><span style="font-family: 'Courier New', Courier, monospace;">this.myField = this;</span></b> </li>
</ul>
<li>assigns 'this' to static field of this class</li>
<ul style="font-family: "Courier New",Courier,monospace;">
<li><b>ThisClass.staticField = this;</b></li>
</ul>
</ol>
Some of these will be a false positive, e.g. with #2, if you construct an object in the constructor, and assign '<b><span style="font-family: 'Courier New', Courier, monospace;">this</span></b>' to one of its fields, before it's published, it'll be safe. Also assigning '<b><span style="font-family: 'Courier New', Courier, monospace;">this</span></b>'
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.<br />
<br />
<br />
<span style="font-size: large;">Passing '<b><span style="font-family: 'Courier New', Courier, monospace;">this</span></b>' as a parameter</span><br />
<br />
The tool will render your class mutable if it does any of the following:<br />
<br />
<ol>
<li>passes '<b><span style="font-family: 'Courier New', Courier, monospace;">this</span></b>' as parameter to another method</li>
<ul>
<li> <b><span style="font-family: 'Courier New', Courier, monospace;">new SomeOtherClass(this);</span></b></li>
<li style="font-family: "Courier New",Courier,monospace;"><b>SomeOtherClass.staticMethod(this);</b></li>
<li style="font-family: "Courier New",Courier,monospace;"><b>someOtherObject.instanceMethod(this);</b></li>
</ul>
<li>constructs an inner class (which has an implicit reference to '<span class="Apple-style-span" style="font-family: 'Courier New', Courier, monospace;"><b>this</b></span>')<br />
</li>
<ul style="font-family: "Courier New",Courier,monospace;">
<li><b>new InnerClass()</b></li>
</ul>
</ol>
A false positive in this case could be, with #1, passing '<b><span style="font-family: 'Courier New', Courier, monospace;">this</span></b>'
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 '<b><span style="font-family: 'Courier New', Courier, monospace;">this</span></b>' reference hasn't escaped.<br />
<br />
<span style="font-size: large;">Unexpected false positives</span><br />
<span style="font-size: large;"><span style="font-size: small;">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 '<b><span style="font-family: 'Courier New', Courier, monospace;">this</span></b>' being passed to other methods, it also accidentally raises an issue when <i>instance fields are passed as well</i>.</span> </span><br />
<br />
<br />
Here's an example from <b><span style="font-family: 'Courier New', Courier, monospace;">java.util.Vector</span></b>:<br />
<br />
<div style="font-family: "Courier New",Courier,monospace;">
<b><span style="font-size: x-small;">public Vector(Collection c) {<br />
elementData = c.toArray();<br />
elementCount = elementData.length;<br />
<br />
if (elementData.getClass() != Object[].class)<br />
elementData = Arrays.copyOf(elementData, elementCount, Object[].class);<br />
}</span></b></div>
<b><span style="font-family: 'Courier New', Courier, monospace;">}</span></b><br />
<br />
<br />
<br />
The issue being the call to <b><span style="font-family: 'Courier New', Courier, monospace;">Arrays.copyOf()</span></b> with two instance fields. This piece of code will cause <b><span style="font-family: 'Courier New', Courier, monospace;">Vector</span></b>
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 <b><span style="font-family: 'Courier New', Courier, monospace;">Arrays.copyOf()</span></b> does, I can't conclude whether <b><span style="font-family: 'Courier New', Courier, monospace;">Vector</span></b> is immutable or not. I'm choosing to err on the side of caution. There is the scope for a potential <i>whitelist</i> of safe methods, but that's not on the cards just yet.
<br />
<br />
<br />
Another interesting point is that one field is a primitive <b><span style="font-family: 'Courier New', Courier, monospace;">int</span></b>, the other is an array. Since the value of the <b><span style="font-family: 'Courier New', Courier, monospace;">int</span></b>
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.<br />
<br />
<br />
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 <a href="http://code.google.com/p/mutability-detector/issues/list">raise a bug</a> (that may require sign in).<br />
<br />
<br />
<br />
I expect to release a new version within the next week, stay tuned for more news!<br />
<br />
<br />
<br />
<br />
<br />
<br />Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-13952478514874209332011-10-24T13:56:00.000-07:002011-10-24T13:56:21.049-07:00Introducing Google GroupA Google Group is now available for questions, discussions or abuse, relating to Mutability Detector. The pertinent details of the group are:<br />
<br />
<ul>
<li><span class="Apple-style-span" style="background-color: white; font-family: arial, sans-serif; font-size: 13px;">Group name: mutability-detector</span></li>
<li><span class="Apple-style-span" style="background-color: white; font-family: arial, sans-serif; font-size: 13px;">Group home page: <a href="http://groups.google.com/group/mutability-detector" style="color: #0000cc;" target="_blank">http://groups.google.com/<wbr></wbr>group/mutability-detector</a></span></li>
<li><span class="Apple-style-span" style="background-color: white; font-family: arial, sans-serif; font-size: 13px;">Group email address <a href="mailto:mutability-detector@googlegroups.com" style="color: #0000cc;">mutability-detector@<wbr></wbr>googlegroups.com</a></span></li>
</ul>
<div>
Don't be shy!</div>Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-6719012376601492992011-09-30T15:38:00.000-07:002011-09-30T15:38:46.034-07:00A better way of categorising immutability?Currently Mutability Detector categorises a class' immutability as one of the following:<br />
<br />
<br />
<span class="Apple-tab-span" style="white-space: pre;"> </span>public static enum IsImmutable {<br />
<span class="Apple-tab-span" style="white-space: pre;"> </span>COULD_NOT_ANALYSE,<br />
<span class="Apple-tab-span" style="white-space: pre;"> </span>DEFINITELY,<br />
<span class="Apple-tab-span" style="white-space: pre;"> </span>MAYBE,<br />
<span class="Apple-tab-span" style="white-space: pre;"> </span>DEFINITELY_NOT;<br />
<span class="Apple-tab-span" style="white-space: pre;"> </span>}<br />
<br />
For example:<br />
<br />
java.util.Date is DEFINITELY_NOT immutable. Reasons:<br />
(... some reasons ommitted for brevity ...)<br />
Field [fastTime] can be reassigned within method [setTime]<br />
<br />
Putting aside the general error case of COULD_NOT_ANALYSE, the three main categories are kind of... unsatisfying.<br />
<br />
For example, if your class has a primitive array field, it will be classed as MAYBE immutable. In this case it's due to the tool being naive and not doing the kind of analysis it could, and there were so many false positives being generated. At the time I thought it was a pragmatic (read "weasel") way to have my cake and eat it too - I got to reduce false positives, without the effort of making the analysis more powerful. The problem is that it's not particularly helpful. When Joe Hypothetical runs the analysis, what is he supposed to concur from seeing that his class is "maybe immutable"?<br />
<br />
Another example is the DEFINITELY category. <a href="http://mutability-detector.blogspot.com/2010/07/immutability-and-subclassing-are-they.html?showComment=1310162283671#c870004818976988079">In the discussion following a previous blog post</a> it was pointed out that where I had used a class I considered DEFINITELY immutable, a commenter pointed out that under certain threading conditions, it could be seen to mutate. These categories were not being used to communicate certain, specific, well defined, and ultimately useful information.<br />
<br />
So I've been thinking about having different categories, that will hopefully be more useful. I've borrowed a couple, which, because the book is so darn good, are based on the ideas and semantics from <a href="http://jcip.net/">Java Concurrency in Practice</a>.<br />
<br />
<ul><li>IMMUTABLE </li>
<li>EFFECTIVELY_IMMUTABLE</li>
<li>NOT_IMMUTABLE</li>
<li>COULD_NOT_ANALYSE</li>
</ul><div>Immutable is the strictest category of immutability. All fields are final and the class is final. All fields are immutable (a 'turtles all the way down' kind of arrangement). Instances of this class can be published in any way, under any threading conditions, as the Java Memory Model guarantees that writes to the final field <i>Happens-Before</i> any reads from that field. Implicitly thread safe. Issues include: that old favourite - lazily loading fields that are expensive to compute. Detecting benign data races is Hard. I may have to hard code some common cases, e.g. java.lang.String/java.math.BigDecimal.</div><div><br />
</div><div>Instances of effectively immutable classes can be safely shared across threads, as long as they are safely published (JCIP covers what 'safely published' means in more detail). The class doesn't need to be final. Fields don't need to be final, but they can only be, and must be, assigned in the constructor or private methods called only by the constructor. Fields should all be immutable, effectively immutable, or <i>mutable-but-never-mutated</i>, meaning for example, a field can be of type java.util.List, or an array, but as long as it isn't mutated after construction, it's fine. This includes allowing references to mutable instances to escape, e.g. returning the array field from a method call without copying first. Issues include: the 'method called only from constructor' clause allows for serialisation... but should it? Also, confidently identifying fields which are mutable but never mutated is non-trivial.</div><div><br />
</div><div>Not immutable is everything else: interfaces and abstract classes, classes whose fields can be reassigned after construction, whose mutable fields are published, which assign a mutable type (e.g. List or an array) to a field. The analysis will still need to get better at detecting valid and common patterns, but the more esoteric the code gets, the more likely the analysis is just going to throw it's hands up in the air in desperation and admit mutability.</div><div><br />
</div><div>This scheme, would represent a slight change in direction for the analysis. I can remember when the code was thrown together as part of a uni assignment, that the requirements for being immutable were very strict, and since then I've tried to improve the analysis to allow for more leniency. Now I'm starting to think that strictness may be more of a strength - particularly since it would be possible to manually 'override' the result using a flexible and fluent API available in unit test assertions.</div><div><br />
</div><div>So not a particularly ground breaking or earth shattering change suggestion, but hopefully something that could potentially be more useful, communicative and more broadly understood. As usual comments and suggestions welcome, thanks for reading.</div><div><br />
</div><div><br />
</div>Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com1tag:blogger.com,1999:blog-8656465666508353224.post-75973621433424885672011-04-24T14:39:00.000-07:002011-04-24T14:39:15.696-07:00Announcing *cough* Better Maven SupportIn a <a href="http://mutability-detector.blogspot.com/2011/04/maven-support-for-latest-version-of.html">previous post</a> I gave details of how to retrieve Mutability Detector in a build system such as Maven. It included details of the snapshot repository that had to be added to your pom.xml (or equivalent file), and the specifier for the latest SNAPSHOT version - just for extra complicatedness. <br />
<br />
While trying to use my own project at <a href="http://www.youdevise.com/">work</a>, where we use <a href="http://www.gradle.org/">Gradle</a> as a build system, I found that retrieving a snapshot dependency from a Maven repository was not particularly well supported. Since it at <i>best</i> required some non-default behaviour (if it's at all supported, I never found a solution), I leapt into action, much like a ninja mongoose, determined to rectify the situation immediately. So, just a short(!) 8 days later, I'm proud to announce that Mutability Detector is available from Maven Central, as a bonafide release version.<br />
<br />
The relevant details now are:<br />
<br />
<table><tbody>
<tr><td>Group ID</td><td>Artifact ID</td><td>Version</td></tr>
<tr><td>org.mutabilitydetector</td><td>MutabilityDetector</td><td>0.6</td></tr>
</tbody></table><br />
Mutability Detector is packaged as a single JAR, which can be used for runtime analysis, as an assertion mechanism in unit testing, or as a standalone application launched from the command line.<br />
<br />
The 0.6 release which made it to Maven Central also fixed two issues I mentioned in the previous post (discussed <a href="http://code.google.com/p/mutability-detector/issues/detail?id=12&can=1">here</a>, and <a href="http://code.google.com/p/mutability-detector/issues/detail?id=11&can=1">here</a>), which are fairly significant.<br />
<br />
As always, if you're a user with any questions or issues, please let me know. Well, if you're a user with no issues, it would be nice to hear that you actually exist...Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-44536686797442410722011-04-16T13:05:00.000-07:002011-04-16T13:08:05.789-07:00Announcing Maven support for the latest version of Mutability DetectorMutability Detector is now available for Maven users*. If you want to try it out on your own code, do the following:<br />
<br />
Make the Sonatype Snapshot repository available to your build system. In Maven, this means adding the following snippet within the <project> section of your pom.xml.<br />
<pre><repositories>
<repository>
<id>Sonatype Snapshots</id>
<url>http://oss.sonatype.org/content/repositories/snapshots</url>
</repository>
</repositories>
</pre><br />
Next add the Mutability Detector dependency to your pom.xml with this snippet:<br />
<pre><dependency>
<groupId>org.mutabilitydetector</groupId>
<artifactId>MutabilityDetector</artifactId>
<version>0.6-SNAPSHOT</version>
<scope>test</scope>
</dependency>
</pre><br />
For other build systems (Gradle, Ivy, etc.) the pertinent bits of information for the dependency are:<br />
<br />
<table><tbody>
<tr><td>Group ID</td><td>Artifact ID</td><td>Version</td></tr>
<tr><td>org.mutabilitydetector</td><td>MutabilityDetector</td><td>0.6-SNAPSHOT</td></tr>
</tbody></table><br />
Some points:<br />
<ul><li>Source code is not available through Maven, working on fixing this. In the meantime, you can checkout or browse the source, details available at the <a href="http://code.google.com/p/mutability-detector/">Googlecode project page</a>.</li>
<li>You may get surprising results with the current snapshot, as described in <a href="http://code.google.com/p/mutability-detector/issues/detail?id=11">this issue</a></li>
<li>The packaged jar file includes dependencies (e.g. ASM, Classpath Explorer), so there may be some issues putting this jar on your classpath. Working on fixing this also.</li>
</ul>As always, any questions, please ask.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0tag:blogger.com,1999:blog-8656465666508353224.post-68083924312926069182011-03-13T14:37:00.000-07:002011-03-13T14:37:15.021-07:00Lightning Talk Introducing Mutability DetectorHere's a video of a <a href="http://en.wikipedia.org/wiki/Lightning_Talk">lightning talk</a> I gave at work, introducing Mutability Detector to my colleagues. I had to cut it short so as not to break the rules of a maximum 5 minute duration, but I think it gives a good overview. If you find my Scots accent tough to understand, you might prefer to check out a couple of the previous blogs, particularly the <a href="http://mutability-detector.blogspot.com/2010/07/introduction-to-mutability-detector.html">introduction to the first version released</a>, and <a href="http://mutability-detector.blogspot.com/2010/10/version-002-of-mutability-detector-now.html">an overview of a more recent release</a>.<br />
<br />
<br />
<iframe frameborder="0" height="300" src="http://player.vimeo.com/video/17218830" width="600"></iframe><br />
<a href="http://vimeo.com/17218830">The Mutability Detector</a> from <a href="http://vimeo.com/user3637590">youdevisetech</a> on <a href="http://vimeo.com/">Vimeo</a>.Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com2tag:blogger.com,1999:blog-8656465666508353224.post-2316524032470505232010-10-03T13:40:00.000-07:002010-10-03T13:40:50.883-07:00Version 0.02 of Mutability Detector now available.... because nothing says "Progress!" like the increment by one-hundredth of a major release.<br />
<br />
Sarcasm aside, in this new release, I definitely see a further step towards "Actually useful". I'll use this blog entry to demonstrate some of the improvements, discuss the planned features for the next release, and list what's still keeping that v1.0 release out of reach.<br />
<br />
<b>Unit Testing Support</b><br />
I consider one of the major barriers to the adoption of a static analysis tool is the 'bootstrapping' cost: the time and resource required to configure and run the analysis as part of the build.<b> </b>There's also the issue of adapting the results to suit your codebase - trimming the false positives, and keeping the results to a meaningful and manageable level. With this problem in mind, I decided to stand on the shoulders of giants, to reuse, rather than reinvent. <br />
<br />
<a href="http://www.junit.org/">JUnit</a>, and its ilk (<a href="http://testng.org/doc/index.html">TestNG</a> being the most prominent rival) have long been a weapon in the arsenal of software development. Without attempting to be controversial, any Java development shop worth its salt will have unit tests regularly run as part of a continuous integration build. The beauty of JUnit is that while it is suitable for large teams with large projects, it also works for a single developer with a small project (about a third of the Java source files in Mutability Detector are JUnit test classes, run regularly through an IDE).<br />
<br />
With widespread adoption and an ecosystem of knowledge and tools available, it seems a sensible option to hook into that, rather than trying to roll my own. The v0.02 release of Mutability Detector enables this.<br />
<br />
By simply adding the jar to your classpath, it is possible to write this kind of test case:<br />
<br />
<pre class="prettyprint">import static org.mutabilitydetector.junit.MutabilityAssert.assertImmutable;
@Test
public void theClassIsImmutable() {
assertImmutable(MyNewClass.class);
}
</pre><br />
Which will fail if, for example, someone later comes along and adds a setter method.<br />
<br />
That's all it takes. And there's several advantages that come with this approach:<br />
<br />
<ul><li>There's no lengthy configuration: just add a jar to your development classpath, there's certainly no need to add a runtime dependency on Mutability Detector, unless you want to (there is an API for doing runtime checks).</li>
<li>There's no need to annotate your code: meaning nothing except test code has to be cleaned up if you decide to stop using the jar; no dependency on a third-party annotation; and no nasty "magic" comments to enable or disable the check. Developers who employ TDD will be familiar with the idea that test code <i>is the documentation</i>, there's no need for redundant source annotations. </li>
<li>There's also no separate build phase: since the checks are made as part of running the unit tests, if you already do that, there's nothing more to configure. But, if you don't regularly run unit tests don't worry, the option to run the tool from the command line is still there, and always will be.</li>
<li>It's extremely selective: you won't have to prune all the results to get the ones for classes you actually care about, the analysis will only be run on classes that you specify as intending to be immutable. This means no noisy false positives.</li>
</ul><br />
However, like anything else, there are downsides and caveats, and I'd be remiss for not mentioning them.<br />
<br />
<ul><li>The assertion mechanism for checking immutability assumes a JUnit-like approach. That is, failed checks will throw a (subclass of) <code>AssertionError</code> on the thread the test is run on. If your unit testing library of choice does not use this kind of mechanism, the tool may not be suitable for you. </li>
<li>The flipside of being selective is that it may require a lot of <i>busywork</i> to go around your codebase adding in single-line test methods which only call <code>assertImmutable()</code>. I intend to address this problem (somehow) but support for lessening this burden is not here yet. </li>
<li>The unit test task may require more memory than previously. I chose the option of speed over memory footprint, using a single analysis session across all unit tests run in the current JVM process. This prevents the need to re-analyse classes several times, but it does mean that more memory is used to cache the results. Unfortunately, this is not something that I have been able to benchmark in a real-world scenario, but Mutability Detector can analyse the entire rt.jar of Sun's 1.6.12 JDK (~17,000 classes) with less than 96Mb of memory, which gives a decent indication of the memory footprint.</li>
</ul><br />
The support for unit testing immutability is the most significant addition to Mutability Detector, and, in my humble opinion, represents a useful and novel way of introducing static analysis into development workflow.<br />
<br />
<br />
<b>Other Changes With This Release</b> <br />
Along with this change, there are several other improvements:<br />
<ul><li>Better detection of setter methods, covering some of the false results described in <a href="http://mutability-detector.blogspot.com/2010/09/detecting-setter-methods-aint-so-easy.html">an earlier post</a>.</li>
<li>Support for reporting only on specified classes with a --classlist option. Run <code>java -jar MutabilityDetector.jar -help</code> for further detail. </li>
<li>The <code>--match</code> option for the command line interface is now implemented. </li>
<li>Improvements to the way classes are loaded, to reduce the amount of PermGen space required.</li>
</ul><br />
<br />
But, before I get ahead of myself, I must point out, there is a very good reason why this release is not close to a v1.0: <a href="http://code.google.com/p/mutability-detector/issues/detail?id=3&can=1">java.lang.String is still reported as mutable</a>.<br />
<br />
<b>What's Next?</b><br />
Obviously the analysis of java.lang.String is a problem: since first releasing the source code I have been researching and considering ways to correctly handle the case. Since this is the major stumbling block to a v1.0, and I want to get it right, the work on this is ongoing. Ideas and thoughts are most welcome.<br />
<br />
Besides that, there are another couple of items of work which I will be working on in the near future (as time and other commitments permit, of course).<br />
<br />
<ul><li>Implement the EscapedThisReferenceChecker described in an <a href="http://mutability-detector.blogspot.com/2010/07/escapedthisreferencechecker-sketch-of.html">earlier post</a>.</li>
<li>Expose an API within the MutabilityAssert methods to allow more fine grained control to override potential false positives. For example, if your class does have one method which lazy inits a field, and the tool isn't at the level of sophistication required to detect it, there is the mechanism to say "In regards to reassigning this particular field, in this method, don't worry, I know what I'm doing". It's inevitable that there's always going to be code which can fool the tool, this should help ease that pain a little.</li>
<li>Investigate the following features: an Eclipse plugin; a FindBugs plugin; a nicer report from the command line tool (perhaps a nice HTML page with the capability of filtering and searching the results); and an ant task to wrap the command line interface. Some of these features may be worthwhile, some may not, so it would be prudent to consider them.</li>
<li>Improve the tool to be much smarter about detecting mutations. For example, having a mutable field makes the owning class mutable, whether that field is mutated or not. It should be possible for an immutable class to have mutable fields - as long as they are not <i>actually mutated</i>.</li>
<li>The usual bug fixes and incremental improvements. </li>
</ul><br />
Hopefully I've demonstrated the main improvements over the last version, and possibly convinced you to try the tool out. As always, if there are any comments, questions or flames, feel free to post to the blog, or <a href="mailto:Grundlefleck+md@gmail.com">email me</a>.<br />
<br />
So that's v0.02: roll on v0.03!Graham Allanhttp://www.blogger.com/profile/16539618370362116733noreply@blogger.com0