eed3si9n / scalaxb

scalaxb is an XML data binding tool for Scala.
http://scalaxb.org/
MIT License
337 stars 156 forks source link

base trait is not generated in case when one of input files does not contain subtypes, but others do #118

Open OlegYch opened 13 years ago

OlegYch commented 13 years ago

steps

compile two input files with the command

>scalaxb-0.6.7-SNAPSHOT Service3.wsdl Service2.wsdl

Service2.wsdl

<?xml version="1.0" encoding="UTF-8"?>
<wsdl:definitions xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/"
                  xmlns:ns1="http://schemas.datacontract.org/Entities"
                  xmlns:xsd="http://www.w3.org/2001/XMLSchema"
                  targetNamespace="http://tempuri.org/">
  <wsdl:types>
    <xsd:schema
        attributeFormDefault="unqualified" elementFormDefault="qualified"
        targetNamespace="http://schemas.datacontract.org/Entities">
      <xsd:complexType name="BaseProperty">
        <xsd:sequence>
          <xsd:element minOccurs="0" name="Definition" nillable="true" type="xsd:string"/>
        </xsd:sequence>
      </xsd:complexType>
    </xsd:schema>
  </wsdl:types>
</wsdl:definitions>

Service3.wsdl

<?xml version="1.0" encoding="UTF-8"?>
<wsdl:definitions xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/"
                  xmlns:ns1="http://schemas.datacontract.org/Entities"
                  xmlns:xsd="http://www.w3.org/2001/XMLSchema"
                  targetNamespace="http://tempuri.org/">
  <wsdl:types>
    <xsd:schema
        attributeFormDefault="unqualified" elementFormDefault="qualified"
        targetNamespace="http://schemas.datacontract.org/Entities">
      <xsd:complexType name="BaseProperty">
        <xsd:sequence>
          <xsd:element minOccurs="0" name="Definition" nillable="true" type="xsd:string"/>
        </xsd:sequence>
      </xsd:complexType>
      <xsd:complexType name="PropertyOfint">
        <xsd:complexContent>
          <xsd:extension base="ns1:BaseProperty">
            <xsd:sequence>
              <xsd:element minOccurs="0" name="Value" type="xsd:int"/>
            </xsd:sequence>
          </xsd:extension>
        </xsd:complexContent>
      </xsd:complexType>
    </xsd:schema>
  </wsdl:types>
</wsdl:definitions>

problem

Service3_type1.scala

case class PropertyOfint(Definition: Option[Option[String]] = None,
  Value: Option[Int] = None) extends BaseProperty

Service2_type1.scala

case class BaseProperty(Definition: Option[Option[String]] = None)
eed3si9n commented 13 years ago

What if you flip the processing order?

 > scalaxb-0.6.7-SNAPSHOT Service2.wsdl Service3.wsdl

BaseProperty is masked because it's duplicated. If the semantics are not identical, I don't know if I want to keep support it..

OlegYch commented 13 years ago

In this case it will work correctly: generate base trait and place all three types in one file.

OlegYch commented 13 years ago

I think it's a good idea to always generate the trait regardless of whether the type has subtypes. Looks like this way the issue would be resolved without much hassle.

eed3si9n commented 13 years ago

Eventually there will be a better support via #53, but we can use last-win principle as a workaround. In case there is circular situation, you can always add the third wsdl at the end that repeats all inheritances.

eed3si9n commented 13 years ago

I think it's a good idea to always generate the trait regardless of whether the type has subtypes.

Wouldn't that make the generated code more noisy?

OlegYch commented 13 years ago

It will, but it will also allow someone to define subtypes manually. And hopefully resolve this issue without resorting to modifying schemas (which are quite often regenerated). Maybe an option would be sufficient.

eed3si9n commented 13 years ago

So, if there's a complex type called ipo:Address, and scalaxb always generated Addressable, all other complex types then need to refer to ipo:Address as Addressable in Scala code like:

trait Fooable {
  val address: Addressable
}

case class Foo(address: Addressable) extends Fooable

and so on and so forth. This will make pattern matching annoying because you then need to make sure you're handling the sub type of Addressable that may be written in the future.

foo match {
  case Foo(Address(name, street, city)) => 
}

The above is no longer exhaustive. First, foo may or may not be Foo, and foo.address may or may not be Address.

OlegYch commented 13 years ago

Heh. Ensuring exhaustiveness in case of just one subtype is not a big deal. On the other hand currently traits are generated as not sealed so it becomes a problem when there are several subtypes anyway. That's actually another point for enhancement - you can add an option to generate sealed tratis so that compiler could check for exhaustiveness. Going back to the original issue of incorrect merging - i still think it should be fixed one way or another, because the goal of being able to generate valid code for a valid input imo is crucial.

eed3si9n commented 13 years ago

the goal of being able to generate valid code for a valid input imo is crucial.

Right. In my opinion, the moment you have a duplicated types across the schemas, it's already invalid. I guess I could try to detect that the merged type needs trait or not, but this would be an enhancement, not a bug fix.