just getting started

[martincha...@gmail.com]

Hi, just getting started using MD with a view to contributing. Eventually got over the hurdle of getting the code from git, and have written a couple of basic tests. it looks good I like the use of your own hamcrest matchers, I'd like to know how to do that.

So I wrote a test case for the guava collections issue, thought I might try & tackle that? I think I'll need a few pointers first though...

[Graham Allan]

Hey Martin,

Great stuff.

Could you elaborate on the hurdles you experienced getting the code checked out? I'd be eager to make this step easier for contributors. Some details would be great; e.g  if you're using git or GitHub or Maven for the first time, could the project be friendlier about how to check out and build?

In terms of Hamcrest matchers, I picked that up mostly through work, so I don't have a definitive resource to point to on how to do that, though I'd be happy to explain more/answer questions, etc. Had a quick glance over this blog entry (http://www.planetgeek.ch/2012/03/07/create-your-own-matcher/ ) it looks great. Covers the main parts of the Hamcrest API I use most: TypeSafeDiagnosingMatcher and FeatureMatcher; JUnit's assertThat vs Hamcres't assertThat.

What I was aiming for with the Guava collections issue is something similar to the support that Mutability Detector has for the built-in unmodifiable collections from java.util.Collections. Rather than performing a smart (but difficult) analysis, it has a list of definitions of 'safe' patterns. So if your constructor has a parameter which is a List of Strings, if you just assign that field your class may be immutable, because the List is abstract and can be a mutable implementation. But if inside your constructor you copy the list (with e.g. new ArrayList(sourceList);) and then wrap in an unmodiable list (with Collections.unmodifiableList(copiedList);) then we can conclude that the having a field of type List is safe*. The way Mutability Detector supports this is that it has a big list of methods that either copy or wrap a collection safely, and checks that they're invoked. 

That list is hardcoded within Mutability Detector. We could do the same thing for Guava, but ideally you should be able to configure it for your own codebase as well. To that end, I'd like to support Guava in a kind of half-state: the configuration required comes out-of-the-box with Mutability Detector, but it uses a mechanism that is also suitable for end users and their own, private codebase.

This boils down to a couple of high level pieces of work:

• adapting Mutability Detector code to look at a configuration, and recognises when safe immutable collections have been used
• finding all the collections, and how to construct them, that is safe in Guava (e.g it should recognise Lists.newArrayList(sourceList) as a copy, just like new ArrayList(sourceList))

That's all pretty high level, and we can go into more detail, I don't expect you to be able to start work from that**. I think the best way to start would be to write a series of test cases for Mutability Detector, which, when they're all passing, out-of-the-box, basically means that Mutability Detector supports Guava. That in itself is quite a big task, so how about, by this time next week, you issue a pull request which contain a single test case, that a class with a single, final field of type com.google.common.collect.ImmutableCollection, containing an immutable type (String will do) is considered immutable.

Sound reasonable?

Also, I would also like to hear if you've run Mutability Detector on some of your own code, and if it gave surprising results. Or if you think there could be improvements in terms of usability, e.g. better error messages.

I'm really thrilled you got the code checked out and want to continue!

~ Graham A

* checking that the element type is immutable, of course. It gets complicated quickly :-D

** do correct me if you know exactly where to go

[Martin Charlesworth]

Hi Graham. The main problem is that I'm not familiar with git, that's all. I tried checking it out on my home windows PC first, it took a while to figure it out (I had to install a piece of software from github) and when I got the code the project had maven errors so I gave up on using windows. I know maven, and I know what a time sink it can be fixing a build!

I think I see what you're saying - you want a general mechanism for telling MD about these safe patterns, and for it to be preconfigured for guava. That sounds sensible.

I think I've already done what you said, and I've also tried using the "hardcoded exception" mechanism you mentioned in the original bug discussion, but it doesn't work as I expect. Have a look at the attachment & tell me why the 2nd test doesn't pass pls!

When you say "issue a pull request" does that mean check in in svn-speak?

martin

[Graham Allan]

Hi Martin,

Answers inline.

On 17 September 2013 13:04, Martin Charlesworth <martincha...@gmail.com> wrote:

Hi Graham. The main problem is that I'm not familiar with git, that's all. I tried checking it out on my home windows PC first, it took a while to figure it out (I had to install a piece of software from github) and when I got the code the project had maven errors so I gave up on using windows. I know maven, and I know what a time sink it can be fixing a build!

Sorry to hear about git problems. Unfortunately, I can't help you with git on Windows, been a Linux-only user for a few years now. It sounds like you have git running correctly now. We won't be doing anything fancy with git really, just commits and pushes. This may be helpful: https://git.wiki.kernel.org/index.php/GitSvnCrashCourse

Unfortunately I think there's an extra step that must be completed before building locally. See here: https://github.com/MutabilityDetector/MutabilityDetector/wiki/How-to-Contribute#building-code

It involves installing a library used by MD, written by me, but not released to Maven Central. The instructions talk about running a shell script before invoking 'mvn test', but that obviously won't work on your Windows machine, instead try running:

mvn install:install-file -Dfile=vendor/lib/main/asm-nonclassloadingsimpleverifier-1.0-SNAPSHOT.jar -DgroupId=org.mutabilitydetector -DartifactId=asm-nonclassloadingsimpleverifier -Dversion=1.0-SNAPSHOT -Dpackaging=jar

Then hopefully mvn test should work. If it's a different error that's the problem, I'd like to hear about it so we can fix it. Would you be able to provide the error output?

 

I think I see what you're saying - you want a general mechanism for telling MD about these safe patterns, and for it to be preconfigured for guava. That sounds sensible.

I think I've already done what you said, and I've also tried using the "hardcoded exception" mechanism you mentioned in the original bug discussion, but it doesn't work as I expect. Have a look at the attachment & tell me why the 2nd test doesn't pass pls!

What you've done is a good start. It fails because MD doesn't realise that ImmutableList.of will return an immutable implementation of an abstract class. Unless MD can see that the class is concrete, then at runtime it could be any implementation, including, crucially, a mutable implementation.  See the class 'CollectionTypeWrappedInUnmodifiableIdiomChecker' for an idea of what will need to be configurable. Unfortunately it's not just as easy as declaring the type as immutable. MD will need to know that the return type of ImmutableList.of, and a host of others, returns a concrete immutable implementation. I've found it too hard to do this with automatic analysis correctly, so configuration is the way to go :-)

 

When you say "issue a pull request" does that mean check in in svn-speak?

Not quite. A git 'push' is equivalent to a svn commit/check in. A pull request is a way to say 'On my fork, here are some diffs you should merge into your fork'. When it comes time for you to contribute code, you'll push the changes to your fork, then issue a pull request to me, and I will merge into my fork (which also happens to be the project's main fork). The easiest way I've found to do this is through GitHub's web interface. See here for a terse example, I can explain more if necessary. https://help.github.com/articles/creating-a-pull-request

Good start, seems you worked out MD's Configuration API. Was that from online docs or exploring the code in your IDE?

Cheers!

Graham

[Mani Sarkar]

Hi Martin,

Are you still having problems with Git on windows, I'm a Windows/Linux/Mac user and can give you hand at it. I have used git on windows for a bit, I must say its not as smooth as using it the other OSes.

Cheers,

mani

[Mani Sarkar]

Hi Martin,

I have had a look at the GuavaTest.java file and would like to say, I liked your naming convention - pretty self-explanatory names.

Cheers,

mani

[Martin Charlesworth]

Cheers Mani, I started using the "should_do_x()" style after watching one of sandro mancuso's (he of software craftsmanship) videos, it really helps tell the reader the intent of the test i think.

[Martin Charlesworth]

Responses inline...

On 17 September 2013 14:03, Graham Allan <grundl...@gmail.com> wrote:

Hi Martin,

Answers inline.

On 17 September 2013 13:04, Martin Charlesworth <martincha...@gmail.com> wrote:

Hi Graham. The main problem is that I'm not familiar with git, that's all. I tried checking it out on my home windows PC first, it took a while to figure it out (I had to install a piece of software from github) and when I got the code the project had maven errors so I gave up on using windows. I know maven, and I know what a time sink it can be fixing a build!

Sorry to hear about git problems. Unfortunately, I can't help you with git on Windows, been a Linux-only user for a few years now. It sounds like you have git running correctly now. We won't be doing anything fancy with git really, just commits and pushes. This may be helpful: https://git.wiki.kernel.org/index.php/GitSvnCrashCourse

 

Thanks for the link, I think I've got a very basic grasp of the basics of git now!

 

Unfortunately I think there's an extra step that must be completed before building locally. See here: https://github.com/MutabilityDetector/MutabilityDetector/wiki/How-to-Contribute#building-code

It involves installing a library used by MD, written by me, but not released to Maven Central. The instructions talk about running a shell script before invoking 'mvn test', but that obviously won't work on your Windows machine, instead try running:

mvn install:install-file -Dfile=vendor/lib/main/asm-nonclassloadingsimpleverifier-1.0-SNAPSHOT.jar -DgroupId=org.mutabilitydetector -DartifactId=asm-nonclassloadingsimpleverifier -Dversion=1.0-SNAPSHOT -Dpackaging=jar

That worked fine. The build problems are all in the test code - lots of unresolvable imports - see the screenshot attached. (I had to do a screen shot, there appears to be no copy-paste with git bash!) So anyway I could do mvn install -DskipTests, which then allowed me to build & run the tests in the client project, they ran fine. So the question is where are those imports, eg org.mutabilitydetector.benchmarks.types.InterfaceType ?

 

Then hopefully mvn test should work. If it's a different error that's the problem, I'd like to hear about it so we can fix it. Would you be able to provide the error output?

 

I think I see what you're saying - you want a general mechanism for telling MD about these safe patterns, and for it to be preconfigured for guava. That sounds sensible.

I think I've already done what you said, and I've also tried using the "hardcoded exception" mechanism you mentioned in the original bug discussion, but it doesn't work as I expect. Have a look at the attachment & tell me why the 2nd test doesn't pass pls!

What you've done is a good start. It fails because MD doesn't realise that ImmutableList.of will return an immutable implementation of an abstract class. Unless MD can see that the class is concrete, then at runtime it could be any implementation, including, crucially, a mutable implementation.  

Yeah I suspected that might be it.

 

See the class 'CollectionTypeWrappedInUnmodifiableIdiomChecker' for an idea of what will need to be configurable. Unfortunately it's not just as easy as declaring the type as immutable. MD will need to know that the return type of ImmutableList.of, and a host of others, returns a concrete immutable implementation. I've found it too hard to do this with automatic analysis correctly, so configuration is the way to go :-)

 

When you say "issue a pull request" does that mean check in in svn-speak?

Not quite. A git 'push' is equivalent to a svn commit/check in. A pull request is a way to say 'On my fork, here are some diffs you should merge into your fork'. When it comes time for you to contribute code, you'll push the changes to your fork, then issue a pull request to me, and I will merge into my fork (which also happens to be the project's main fork). The easiest way I've found to do this is through GitHub's web interface. See here for a terse example, I can explain more if necessary. https://help.github.com/articles/creating-a-pull-request

Good start, seems you worked out MD's Configuration API. Was that from online docs or exploring the code in your IDE?

From online docs. I haven't really looked at any code yet till I can get it building.

[Mani Sarkar]

Hi Martin,

I ran the mvn commands both the long and short versions you mention and I didn't get any test errors as mentioned in your screen-shot.

Have you followed all the instructions Graham mentioned in his previous response? Let me know and I'll investigate with you along side to help resolve your test failures.

Cheers,

mani

--
Twitter: @theNeomatrix369          Blog: http://neomatrix369.wordpress.com

JUG activity: LJC Advocate (@adoptopenjdk & @adoptajsr programs)

Meet-a-Project: https://github.com/MutabilityDetector

Bitbucket: https://bitbucket.org/neomatrix369   Github: https://github.com/neomatrix369

LinkedIn: http://uk.linkedin.com/pub/mani-sarkar/71/a77/39b

Devoxx UK 2013 was a grand success: http://www.devoxx.com/display/UK13/Home

Don't chase success, rather aim for "Excellence", and success will come chasing after you!

[Graham Allan]

Hey Martin,

From the screenshot you showed, it looks like Maven has had a problem adding a source directory. The classes which are missing are 'benchmark' classes which have MD's analysis run against them in tests. They're not executed as tests, and definitely don't form part of the main code, so I put them in a different source directory, 'src/test/benchmarks'.

It seems like a Maven issue, are there any earlier errors, perhaps from the build-helper-plugin? If you could get the entire log of mvn test as a text file, that would be helpful*.

~ Graham

* I would use "mvn test > results.txt" on Linux, don't know how to do the equivalent on Windows. Mani, do you know?

[Mani Sarkar]

Yes that should work. I have a Windows instance and I will try to set it up this evening and test it out to see if I get the same errors.

Cheers,

mani 

--
You received this message because you are subscribed to a topic in the Google Groups "mutability-detector" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/mutability-detector/ZY0efcQ2G58/unsubscribe.
To unsubscribe from this group and all its topics, send an email to mutability-dete...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Martin Charlesworth]

today it builds! can't explain that. Though in eclipse I had to manually add the benchmarks folder to the build path as m2e doesn't seem to play nice with the build-helper-plugin, but now all my java errors are gone.

BTW Graham - when you install git on windows you get "git bash" which is basically a bash shell with git in it, so you can use normal linux style redirection as you mentioned.

right, now to look at some code!

To unsubscribe from this group and all its topics, send an email to mutability-detector+unsub...@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

[Mani Sarkar]

Hi Martin,

Its great news, I tried building the MD in git under Windows and it build successfully for both the basic mvn clean install command and the extended mvn install command.

But just when I was about to write to you, I saw the below message.

btw git bash is a good implementation of git on windows.

Keep us posted with your progress.

Cheers,

mani

On Thursday, 19 September 2013 21:45:30 UTC+1, Martin Charlesworth wrote:

today it builds! can't explain that. Though in eclipse I had to manually add the benchmarks folder to the build path as m2e doesn't seem to play nice with the build-helper-plugin, but now all my java errors are gone.

BTW Graham - when you install git on windows you get "git bash" which is basically a bash shell with git in it, so you can use normal linux style redirection as you mentioned.

right, now to look at some code!

[Graham Allan]

Hi Martin,

I don't use m2eclipse, but if you know of a way to configure the POM such that it works for m2eclipse and also generates the correct .classpath file from 'mvn eclipse:eclipse', I'd be happy to support it.

Regards,

Graham A

--
You received this message because you are subscribed to the Google Groups "mutability-detector" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mutability-dete...@googlegroups.com.