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

Source generation doesn't support non-nullable object types #3085

Closed
peppy opened this issue Nov 9, 2022 · 14 comments
Closed

Source generation doesn't support non-nullable object types #3085

peppy opened this issue Nov 9, 2022 · 14 comments
Assignees

Comments

@peppy
Copy link

peppy commented Nov 9, 2022

What happened?

We are keen to switch to the new source generator, but are blocked by it not supporting non-nullable object types. We have been using realm with non-nullable object types for a good year and it definitely works well, so I would hope that this can be supported by the source generator.

Currently it is disabled via this check:

if (!IsNullable &&
(ScalarType == ScalarType.Object))
{
return false;
}

I'm not sure if this is to be considered a bug or feature request, but it's a blocker for us to use source generation unfortunately.

Repro steps

  • Create a class with non-nullable objects.
  • Receive build errors.

Version

6.x

What SDK flavour are you using?

Local Database only

What type of application is this?

Other

Client OS and version

10.18.0

Code snippets

An example of a class that would work previously:

public partial class RealmNamedFileUsage : IEmbeddedObject
{
    public RealmFile File { get; set; } = null!; // <- problematic line

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

    public RealmNamedFileUsage(RealmFile file, string filename)
    {
        File = file ?? throw new ArgumentNullException(nameof(file));
        Filename = filename ?? throw new ArgumentNullException(nameof(filename));
    }

    private RealmNamedFileUsage()
    {
    }
}

[MapTo("File")]
public partial class RealmFile : IRealmObject
{
    [PrimaryKey]
    public string Hash { get; set; } = string.Empty;
}

Our codebase branch with all changes applied that we hope would make source generation work correctly: https://github.com/ppy/osu/compare/master...peppy:osu:realm-source-gen?expand=1

Stacktrace of the exception/crash you're getting

Program.cs(19, 5): [RLM010] RealmNamedFileUsage.File has type RealmFile, that does not support the assigned nullability annotiation.
@nirinchev
Copy link
Member

The problem is that properties linking to realm objects are inherently nullable. Since you can always do something like:

RealmNamedFileUsage usage = ...;
realm.Remove(usage.File);

Now, usage.File will be null, which makes the nullability annotation incorrect. While your app may not be doing that, there's no way to statically guarantee that, which is why we opted for the safer approach of requiring that all RealmObject properties are nullable.

@peppy
Copy link
Author

peppy commented Nov 9, 2022

I completely understand that from a safety perspective. But I do wonder if some middle-ground can be offered as we value non-nullable specs on a lot of our models (and haven't had this go wrong for us.. yet).

@nirinchev
Copy link
Member

@papafe perhaps we can offer this as a config option? I.e. allow users to explicitly enable usage of non-nullable link properties? I guess we could still return null on our end and have them deal with the consequences 😅

@J-Swift
Copy link

J-Swift commented Nov 11, 2022

I would prefer the same as any on-the-wire deserialization API does (e.g. System.Text.Json, etc), namely you can declare the nullability however you want but that doesn't guarantee anything about the actual data. Its still up to the clients to not corrupt the underlying, and allowing the programmer to specify that it shouldnt be null avoids a lot of noisy error handling throughout the entire codebase.

@papafe
Copy link
Contributor

papafe commented Nov 14, 2022

I think we can offer that as a config option. We have several things in the pipeline, including full support for nullability annotations in the model definition (#2982).
Regarding completely ignoring nullability.... this is probably something that could be made into an option as well, we'd need to discuss it internally too though. Our idea here was to remove the need for attributes such as [Required] and let the nullability be the driven directly from the properties.

@J-Swift
Copy link

J-Swift commented Nov 17, 2022

I'm definitely in favor of moving to nullability declarations (and nullable reference types) as the source of truth and avoiding the need for Required attributes if thats what you're saying.

@papafe
Copy link
Contributor

papafe commented Nov 18, 2022

@J-Swift Yes, that is the plan. I suppose that there are different preferences on this 😄

@nirinchev
Copy link
Member

This should be addressed by #3172 which should go into our next release.

@peppy
Copy link
Author

peppy commented Feb 17, 2023

Thanks for adding support for this!

I've been attempting to get it working but applying the ignore setting doesn't seem to immediately be doing anything:

2023-02-17 22 20 35@2x

2023-02-17 22 22 24@2x

It should be fine to place this in our existing .editorconfig at a solution level, correct?

@nirinchev
Copy link
Member

I think we're reading this option from the global config, so you should put it there. See https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files#global-analyzerconfig for more details.

@peppy
Copy link
Author

peppy commented Feb 18, 2023

I see, this does indeed work!

May be worth updating the link here to point to the correct anchor on that page - as it stands I misunderstood which config file was supported.

Now the remaining issues look to be inspection rules we have triggering inside the source generated files:

0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/BeatmapInfo_generated.cs(122,39): Error CS8601 : Possible null reference assignment.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/BeatmapInfo_generated.cs(128,40): Error CS8601 : Possible null reference assignment.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/RealmNamedFileUsage_generated.cs(79,36): Error CS8601 : Possible null reference assignment.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/ScoreInfo_generated.cs(110,43): Error CS8601 : Possible null reference assignment.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/ScoreInfo_generated.cs(115,39): Error CS8601 : Possible null reference assignment.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/ModPreset_generated.cs(83,39): Error CS8601 : Possible null reference assignment.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/BeatmapDifficulty_generated.cs(221,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/RealmRulesetSetting_generated.cs(209,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/BeatmapInfo_generated.cs(343,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/BeatmapCollection_generated.cs(208,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/BeatmapMetadata_generated.cs(242,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/BeatmapSetInfo_generated.cs(238,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/RealmKeyBinding_generated.cs(208,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/RealmUser_generated.cs(208,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/RealmNamedFileUsage_generated.cs(205,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/RealmFile_generated.cs(196,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/ScoreInfo_generated.cs(285,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/BeatmapUserSettings_generated.cs(195,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/ModPreset_generated.cs(221,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/RulesetInfo_generated.cs(222,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>/Users/dean/Projects/osu/osu.Game/Realm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/SkinInfo_generated.cs(234,20): Error RS0030 : The symbol 'object.Equals(object?)' is banned in this project: Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
0>------- Finished building project: osu.Game. Succeeded: False. Errors: 21. Warnings: 0
Build completed in 00:00:08.087
Rebuild failed at 11:38:46 am 

I believe this may be due to our project settings for the inspections, and if that's the case there may need to be a method of disabling this for realm sourcegen.

@nirinchev
Copy link
Member

Can you post the generated code that's triggering these warnings? I'm afraid RS0030 may be something you need to add an exemption for on your end but CS8601 is surprising and we should probably fix the generator for that one.

@peppy
Copy link
Author

peppy commented Feb 18, 2023

Here's the generated code from the above run:

Realms.SourceGenerator.RealmGenerator.zip

@peppy
Copy link
Author

peppy commented Feb 20, 2023

For anyone interested, to disable inspection rules in source generators, it looks like you basically have to disable at a .globalconfig level then re-enable locally (see dotnet/roslyn#47384). This works for me with this change.

I'm facing some remaining issues with source generation but I'll open a new discussion to keep things clean, as they are not nullability related.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants