Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flag to emit #nullable enable #344

Open
Pretasoc opened this issue Jul 28, 2022 · 4 comments
Open

Add flag to emit #nullable enable #344

Pretasoc opened this issue Jul 28, 2022 · 4 comments

Comments

@Pretasoc
Copy link
Contributor

When the --nullableReferenceAttributes is set some members are annotated with nullable attributes. At least in my usecases most of the members won't receive any attribute, but should be treates as non nullable. Because the compiler does not emit nullable attributes for tool generated source code. There is currently no easy way to get non oblivious nullability annotations. The only way to enable nullable annotions from the compiler is to explicitly enable #nullable in the generated source file.

The global nullable context does not apply for generated code files. Under either strategy, the nullable context is disabled for any source file marked as generated. This means any APIs in generated files are not annotated. There are four ways a file is marked as generated: [...]

Therefore i suggest to add a new flag, that will emit a #nullable at the start of the generated file.

From a pure "does it compile" standing point, it should be save the emit #nullable enable whenever --nullableReferenceAttributes and --netCore is set. But there might be xsd schemas, that would lead to property falsely annotated as non nullable.

@Pretasoc Pretasoc changed the title Add flat to emit #nullable enable Add flag to emit #nullable enable Jul 28, 2022
@mganss
Copy link
Owner

mganss commented Aug 9, 2022

Sorry this took so long 😳

From a pure "does it compile" standing point, it should be save the emit #nullable enable whenever --nullableReferenceAttributes and --netCore is set. But there might be xsd schemas, that would lead to property falsely annotated as non nullable.

Can you give an example?

Where should we put the #nullable enable? Before the leading comment or after?

@Pretasoc
Copy link
Contributor Author

Pretasoc commented Aug 9, 2022

No Problem.

Actually i don't have an example, where adding #nullable enable would result in wrong annotations. Its simply that i dont feel confident enough to say, there will never be a problem. I think a very common source of wrongly annotated classes could be bad designed schemas, but thats a scope we can't handle here. However i will do some investigations, whether its save or not to emit the #nullable enable in the cases mentioned above.

Where should we put the #nullable enable? Before the leading comment or after?

The microsoft documentation is not really specific about it. I tested sucessfully with putting it between the leading comment and the first namespace declaration

@Eddie-Hartman
Copy link

Eddie-Hartman commented Jan 16, 2025

@mganss sorry to revive a "dead" thread, but I feel like I'm very close to what I want the generated code to look like with a couple changes.

This is my schema: https://docs.oasis-open.org/emergency/cap/v1.2/CAP-v1.2-os.html (point 3.4 has the actual definition)

and my current arguments (remove the slash after list if you don't need it):

xscgen -p EmergencyResponseData.Data.CAP -0 -f --ct System.Collections.Generic.List\`1 --csm PublicWithoutConstructorInitialization --sf --cn --nc --nr CAP-v1.2.xsd
  1. I'd like to change default collections to nullable lists IF applicable. public List<AlertInfo>? Info instead of public List<AlertInfo> Info. (Example from Alert.cs in output file.)
    • Also I'd like to remove the backing field since it's not used, which looks like it comes from Auto-Implemented Properties not used #122 Would that still require an enhancement or is there some way to accomplish this I'm unaware of? I don't fully understand if it wasn't implemented just due to lack of interest/time or if there was some other reason.
  2. The above issue would help to make the change from Nullable<decimal> to the preferred decimal? syntax (I think? I'm less confident in this since the IDE doesn't actually complain about that change). Either way, I'd prefer that kind of syntax when possible on both collections and basic properties. (Example from the AlertInfoArea.cs outputted file)

I apologize if any of this is covered elsewhere, I've spent a lot of time trying different things to try and get it exactly how I'd prefer the output to be.

@mganss
Copy link
Owner

mganss commented Jan 27, 2025

Like the Nullable<int> syntax, we're using [MaybeNull] and [AllowNull] attributes because CodeDom doesn't support the shorthand syntax. I'm open to PRs that would change this.

Also, currently the output is not fully compatible with nullable reference, e.g. this results in a warning:

[RequiredAttribute(AllowEmptyStrings=true)]
[XmlElementAttribute("event")]
public string Event { get; set; }

Not sure what the best course of action would be for these cases. This?

public string Event { get; set; } = null!;

Or perhaps the required keyword can be used for these cases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants