10 Best IDEA Inspections You're Not Using

Let's clarify. By "Best" I mean the ones I like. By "You're Not Using" I mean they aren't enabled by default. By "Inspections" I mean those little code warnings that IDEA gives you which can be configured under Settings (Ctrl+Alt+S) Errors (6). You have gone an edited your inspection settings, haven't you?

Anyway, here's my list of my most favorite sweet awesome inspections.

1. Illegal package dependencies
Got a "common" package where StringUtils sits? So you've told everyone not to create dependencies out of common back into your product specific packages, yet they still do it, don't they? This inspection allows you to define package dependencies that are allowed or not allowed, and you'll receive warnings when you violate them. So common.StringUtils can be referenced from product.MyClass, but referencing product.MyClass from common.StringUtils creates an error. This is actually enabled by default, but no hierarchy rules are defined. Learning about IDEA scopes is a good thing... go play with this.

2. 'this' reference escaped in object construction
Letting a reference to 'this' escape inside your constructor opens the door to allowing other objects to start using you before your constructor has completed. If you have an immutable object, whose member fields are all set within its constructor, and you let a 'this' reference escape... then it is entirely possible that some other object will see some of those private final fields with null values. This is not good for concurrency (yet sometimes not easily avoided).

3. Field accessed in both synchronized and unsynchronized contexts
Partial synchronization leads to inconsistent data structures. Sometimes your field is updated from multiple threads correctly and sometimes it isn't. As far as I know, partial synchronization has no real world uses and is almost always a mistake. This inspection is a life saver for a condition that is easily overlooked.

4. non private field accessed in synchronized context
This is another case of partial synchronization. Maybe you've synchronized perfectly within your class... yet if you let field references escape out of your synchronized object then its likely that the callers will not update that field in a way consistent with your synchronization policy. Simply put: there's no guarantee that external writes to fields will be seen within your object. Partial synchronization leads to inconsistent data structures, as said eariler.

5. Synchronization on 'this' and 'synchronized' method
Hey, it's a two-fer! These two separate inspections are closely related. Some say it's a style issue, but I feel strongly that 'this' should not be part of your synchronization policy. It effectively makes your synchronization policy public, rather than private and encapsulated, and it opens the door for denial of service attacks on your object. In my opinion, synchronization on 'this' and synchronized methods (which are the same thing, really) should be avoided, and this inspection can help you clean that up.

6. return of collection or array field
Everyone goes through phases where you are introduced to a new idea, fall in love with it, overuse it, and then finally back off from using it so extensively. With immutable objects, I'm still waiting for the phase where I learn not to use them so much. So far it hasn't happened. Well, returning a collection or array out of a public method is a great way to take your nicely designed immutable object and let anyone at all monkey with its state. Collections.unmodifiable* is your friend. Too bad that in legacy codebases returning arrays is so ubiquitous that turning on this inspection produces more noise than signal.

7. call to 'Thread.run()'
I do this all the time. Thread.run() either runs the enclosed Runnable once on the current thread, or it does nothing (if the Thread was not constructed with a Runnable). You want Thread.start(). This issue has never made it into production code for me, its more of a code-time "oh yeah, oops" check.

8. expression.equals("literal") rather than "literal".equals(expression)
The first example might result in a NullPointerException, the second never will. Simple and useful.

9. equals method does not check class of parameter
I open Effective Java every time I have to write an equals() method. And when I'm done I flip pages so I can write a proper hashCode(). Shouldn't it be easier than this? Anyway, this inspection has kicked in a couple times when I wrote bad equals() methods. For me, it's worth turning on.

10. method may be static
I saved the best for last. This is listed under the category "Performance Issues" in IDEA, but I consider it a design issue. If a private method can be made static, then go ahead and make it static. But realize it is a code smell. Once your object contains a handful of static private methods you should start wondering what they're all doing there. If your object contains a bunch of private static methods, then how cohesive is it really? Chances are you have a new class waiting to be extracted. This is the gist of emergent design: adhere to principles and refactor as necessary. So what about public methods? What in the world is a public method doing on your object that has no dependency on any fields within the object? This is certainly a code smell. The problem is that the "auto-fix" for the inspection is to apply the static keyword. Nooooo. That's not what you want to do. A public method without any dependency on object state can't possibly be part of an object that has one clearly stated charter. It's just not cohesive and should be placed somewhere else. So: if the method is private, accept the auto-fix, but if the method is public then don't. The irony is that I've never actually received this warning on a public method I've written. If you're doing test driven development then I don't quite see how this scenario would arise, barring a lapse in judgment. But not everyone is doing that and there are exceptions to every rule.

Well, that's 10 tips. I hear posts with top 10 lists generate more traffic, so let the pageview flood begin!