highsource / jaxb2-basics

Useful plugins and tools for JAXB2.
BSD 2-Clause "Simplified" License
109 stars 54 forks source link

Ability to turn off super.equals, super.hashCode, super.appendFields in generated methods #100

Closed er1c closed 5 years ago

er1c commented 5 years ago

In the jaxb file I have some interfaces I was forced to make into abstract classes (e.g. <inheritance:extends>fm.aaia.aces.xml.DigitalFileInformationBase&lt;DigitalFileInformationType&gt;</inheritance:extends>), in order to access some of the protected variables after-XImmutable`

The abstract classes are basically just interfaces with some default-implementations, and the ability to access the the protected variables, one of the issues I am encountering in the generated code adds of:

-Xequals

        if (!super.equals(thisLocator, thatLocator, object, strategy)) {
            return false;
        }

-XhashCode

    int currentHashCode = super.hashCode(locator, strategy);

-XtoString

    super.appendFields(locator, buffer, strategy);

Instead of trying to create implementations for these in the abstract class, I would prefer to disable the code generation of these super. calls.

highsource commented 5 years ago

I have to say I'm not quite enthusiastic to implement this feature. I think the best solution would be to add an implementation of equals, hashCode etc. in the abstract classes you extend.

Otherwise we'll probably need some kind of global configuration or per-class customization to turn calling appropriate super methods on or off. While absolutely doable this does not feel elegant.

er1c commented 5 years ago

I was just sitting down to dig into jaxb2-basics source code, but banging my head against some other environment-related issue with the tests failing. Hopefully I'll get a PR for that in a bit :)

I guess I was trying to figure out if there was anything "smarter" that could be added to this block:

https://github.com/highsource/jaxb2-basics/blob/660cac190bff9abe8a985a8904d60244d1bcd091/basic/src/main/java/org/jvnet/jaxb2_commons/plugin/equals/EqualsPlugin.java#L162-L178

That would check for for the abstract modifier for the superClass. The part I don't have a firm grasp on, is what the implications are. In scala I would simply mark it sealed so it couldn't be accidentally extended.

This also might be a dumb question, but how can I enable the debug logger when running mvn test for the actual org.jvnet.jaxb2_commons.test.AbstractSamplesTest? I added the slf4-simple dependency to the testing\pom.xml to get around the NO-OP error, and tried adding

$ cat testing/src/test/resources/log4j.properties
log4j.rootCategory=DEBUG, stdout
log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.target=system.out
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=%d %p [%c] - <%m>%n%

But its still not logging to console at the debug level...sorry for squishing issues!

highsource commented 5 years ago

That would check for for the abstract modifier for the superClass.

That won't be quite correct. It is perfectly OK to call super equals method on abstract superclasses. So you can't just opt out on abstract class.

As I see it, if superclass does implement Equals2, it is a pretty good sign that you have to call super.equals(...). If superclass does not implement Equals2 then there are two options:

The behaviour so far was to call super.equals(...). You want the "do not call" option.

For backwards compatibility reasons, calling super.equals(...) must remain the default behaviour. This means "do not call" should be configurable in some way. Options here are: global configuration or per-class customization.

Global configuration is quite easy to implement. If you add a boolean property whatever, you can set it via -Xequals-whatever true from the command line. The property will be set when the plugin is invoked.

Alternatively you can implement per-class customization. See how Ignoring works.

I would prefer per-class customization instead of inflating command-line options.

Sorry, I can't help much with logging. It simply doesn't work quite right.

er1c commented 5 years ago

I'm going to close this as a "bad idea", I'll simply add the super methods to the abstract classes (hopefully via an interface so I'm not duplicating code)