[dotnet-svcutil] fix --internal inconsistent accessibility (XmlSerializer)

[imcarolwang]

For issue #5894 :

• Fixed XmlSerializer DataSet-like WSDL imports so they generate System.Data.DataSet / System.Data.DataTable instead of placeholder ArrayOfXElement (via a new DataSetSchemaImporterExtension wired into the XmlSerializer import pipeline, with msdata + wrapper/diffgram pattern detection).

• Addressed the --internal “inconsistent accessibility” problem where a public generated type exposed an internal helper (ArrayOfXElement): we now either avoid generating that helper (by mapping to DataSet) and/or make schema-generated types internal when --internal is used.

• Added a CodeDOM pass (InternalTypeVisibilityFixer) so --internal consistently makes XmlSerializer-generated data types internal too; fixed the initial build error (override visibility mismatch).

Update (follow-up fix):

• The InternalTypeVisibilityFixer approach was reverted because making XmlSerializer schema/message types internal can break WCF at runtime: XmlSerializerOperationBehavior generates serializer assemblies at runtime which cannot reference internal types, leading to XmlReflectionImporter failures when using the client.

• Keeping the ArrayOfXElement helper type public under --internal is a separate compatibility tweak to prevent compile-time “inconsistent accessibility” (public members exposing an internal helper) in the remaining xsd:any/IXmlSerializable remap cases, while DataSet-like shapes are still mapped to System.Data types.

• Added a regression test case for the DataSet-wrapper + --internal scenario.

[mconnew]

@imcarolwang, it looks like the problem is that we accept the /internal command line parameter when the service requires use of XmlSerializer and silently ignore it, and the ArrayOfXElement problem was because we were generating that as internal when it should also be public. Does that sound right?

[mconnew]

@imcarolwang, do you think we should generate a warning when you specify internal and dotnet-svcutil has to ignore it due to using XmlSerializer?

[imcarolwang]

@imcarolwang, it looks like the problem is that we accept the /internal command line parameter when the service requires use of XmlSerializer and silently ignore it, and the ArrayOfXElement problem was because we were generating that as internal when it should also be public. Does that sound right?

I think the second part is right: the ArrayOfXElement inconsistent-accessibility issue happened because we were generating ArrayOfXElement as internal while the schema wrapper types that referenced it were public. Making ArrayOfXElement public fixes that mismatch.

On the first part (“we accept /internal for XmlSerializer and silently ignore it”): it’s not being ignored everywhere. /internal still gets applied in the generators that actually support “generate internal types”:

• ServiceContractGenerator: /internal maps to ServiceContractGenerationOptions.InternalTypes.
• DataContract importer:/internalmaps to ImportOptions.GenerateInternal.

The XmlSerializer schema-import path is different: the XSD importer/exporter doesn’t really have a “generate internal schema types” option, so those wrapper types come out public by default. We can try to flip them to internal with a CodeDOM post-pass, but when we tried that it looked like it can break WCF XmlSerializer at runtime (serializer generation/reflection expects those message/schema types to be publicly accessible).

So I’d summarize it as:

• /internal is honored where it’s supported (contract + DataContract),
• XmlSerializer schema wrapper types stay public because of how that pipeline + runtime behavior works,
• and in that setup it seems safest to keep ArrayOfXElement public too (plus the DataSet/DataTable schema importer mapping to avoid ArrayOfXElement for that common pattern).

[imcarolwang]

@imcarolwang, do you think we should generate a warning when you specify internal and dotnet-svcutil has to ignore it due to using XmlSerializer?

@mconnew that makes sense. I’m thinking we can add a one-time warning when --internal is set and the generated client actually uses XmlSerializer (ideally detect that via XmlSerializerOperationBehavior, so it also covers the “Auto ended up using XmlSerializer” cases, not just --serializer XmlSerializer).

Proposed warning text:
“--internal may be only partially applied when using XmlSerializer; some schema wrapper types are generated as public for runtime compatibility.”

Does that message sound reasonable to you?