Make generated source code for equals static code analysis friendly

[GoogleCodeExporter]

Run Delombok on any @Data class (like the DataExample.java).

You will see an equals implementation with several if statements, none having 
braces.  Running static code analysis checks against the generated source 
results in numerous exceptions.  For example, it violates the PMD rule 
IfStmtsMustUseBraces:
http://pmd.sourceforge.net/rules/braces.html#IfStmtsMustUseBraces

Here is an example:

        @java.lang.Override
        @java.lang.SuppressWarnings("all")
        public boolean equals(final java.lang.Object o) {
                if (o == this) return true;
                if (!(o instanceof DataExample)) return false;
                final DataExample other = (DataExample)o;
                if (!other.canEqual((java.lang.Object)this)) return false;
                if (this.getName() == null ? other.getName() != null : !this.getName().equals((java.lang.Object)other.getName())) return false;
                if (this.getAge() != other.getAge()) return false;
                if (java.lang.Double.compare(this.getScore(), other.getScore()) != 0) return false;
                if (!java.util.Arrays.deepEquals(this.getTags(), other.getTags())) return false;
                return true;
        }

I would like to see the generated source friendlier to static code analysis 
checks; I am worried that the generated source creates so much noise that we 
lose focus on the static code analysis results on the non-generated source.  At 
the very least, adding braces would reduce many exceptions.  Another idea is to 
consolidate statements.  For example:

        @java.lang.Override
        @java.lang.SuppressWarnings("all")
        public boolean equals(final java.lang.Object o) {
                if (o == this) {
                    return true;
                }
                if (!(o instanceof DataExample)) {
                    return false;
                }
                final DataExample other = (DataExample)o;
                if (
                    (!other.canEqual((java.lang.Object)this)) ||
                    (this.getName() == null ? other.getName() != null : !this.getName().equals((java.lang.Object)other.getName())) ||
                    (this.getAge() != other.getAge()) ||
                    (java.lang.Double.compare(this.getScore(), other.getScore()) != 0) ||
                    (!java.util.Arrays.deepEquals(this.getTags(), other.getTags())) {
                    return false;
                }
                return true;
        }

PMD may still complain that this isn't single-entry, single-exit:  
http://pmd.sourceforge.net/rules/controversial.html#OnlyOneReturn
This could be addressed like:

        @java.lang.Override
        @java.lang.SuppressWarnings("all")
        public boolean equals(final java.lang.Object o) {
            boolean isEquals = true;
            if (o != this) {
                if (!(o instanceof DataExample)) {
                    isEquals = false;
                } else {
                    final DataExample other = (DataExample)o;
                    if (
                        (!other.canEqual((java.lang.Object)this)) ||
                        (this.getName() == null ? other.getName() != null : !this.getName().equals((java.lang.Object)other.getName())) ||
                        (this.getAge() != other.getAge()) ||
                        (java.lang.Double.compare(this.getScore(), other.getScore()) != 0) ||
                        (!java.util.Arrays.deepEquals(this.getTags(), other.getTags())) {
                        isEquals = false;
                    }
                }
            }
            return isEquals;
        }

Cyclomatic complexity can be further reduced by changing the if condition into 
a boolean assignment and applying De Morgan's law:

        @java.lang.Override
        @java.lang.SuppressWarnings("all")
        public boolean equals(final java.lang.Object o) {
            boolean isEquals = true;
            if (o != this) {
                if (!(o instanceof DataExample)) {
                    isEquals = false;
                } else {
                    final DataExample other = (DataExample)o;
                    isEquals =
                        (other.canEqual((java.lang.Object)this)) &&
                        (this.getName() == null ? other.getName() == null : this.getName().equals((java.lang.Object)other.getName())) &&
                        (this.getAge() == other.getAge()) &&
                        (java.lang.Double.compare(this.getScore(), other.getScore()) == 0) &&
                        (java.util.Arrays.deepEquals(this.getTags(), other.getTags()));
                }
            }
            return isEquals;
        }

Note that another idea is to disable the static code analysis checks for the 
generated source code.  Alas, PMD does not really have a nice way to do this (I 
think largely because of how it is designed around doing pattern matching 
against an Abstract Syntax Tree).  Of course, I tried to kickstart a discussion 
around this idea:
    http://sourceforge.net/projects/pmd/forums/forum/188192/topic/4549418
If they provide a mechanism, then perhaps that could be leveraged for delombok.

Original issue reported on code.google.com by anth...@whitford.com on 8 Aug 2011 at 5:30

[GoogleCodeExporter]

About the converting the if one-liners to blocks; will do, but not in 0.10.0. 
About everything else: we add SuppressWarnings("all"). PMD should respect that.

In fact, _any_ inclusion of anything lombok does in any code metric is silly, 
however @SuppressWarnings("all") and the idea 'this is generated code' don't 
overlap perfectly. There's unfortunately no @java.lang.Generated (yet). 
Nevertheless, in PMD's case it is a tool that creates warnings for your code 
and therefore it is very obvious to us that we've done all we should do here.

To be specific, we will not clean up the code generated by lombok just to 
satisfy style checker rules, for a very simple reason: Everybody has their own 
rules, and conflicts are likely, where we simply can't make everybody happy.

Accepted only for if-blocks.

Original comment by r.spilker on 8 Aug 2011 at 7:41

• Changed state: Accepted
• Added labels: Priority-Low, Type-Enhancement
• Removed labels: Priority-Medium, Type-Defect

[GoogleCodeExporter]

Just curious: Why would you run PMD on the generated source instead of the real 
source?

Original comment by Lord.Qua...@gmail.com on 9 Aug 2011 at 3:28

[GoogleCodeExporter]

Because then you end up with lots of other issues.  For example, if you are 
using @Data to generate getters and setters, PMD will complain that fields are 
unreferenced (because it does not see the public getField and setField).

Original comment by anth...@whitford.com on 19 Aug 2011 at 4:30

[GoogleCodeExporter]

I discovered a nice workaround for PMD 5, and documented it in my blog:  
http://anthonywhitford.blogspot.com/2013/09/suppressing-pmd-alerts-for-lombok.ht
ml

Original comment by anth...@whitford.com on 22 Sep 2013 at 4:50