Skip to content

Commit

Permalink
Emit errors for collection assignments
Browse files Browse the repository at this point in the history
  • Loading branch information
@nirinchev
nirinchev committed Oct 3, 2023
1 parent 15b1e1a commit e554ef5
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 48 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
realm.custom_ignore_attribute = [System.Text.Json.Serialization.JsonIgnore]
```
(Issue [#2579](https://github.com/realm/realm-dotnet/issues/2579))
* The Realm source generator will now error out in case a collection in the model classes is assigned to a non-null value either in a property initializer or in a constructor. Realm collections are initialized internally and assigning non-null values to the property is not supported, where the `null!` assignment is only useful to silence nullable reference type warnings, in reality the collection will never be null. (Issue [#3455](https://github.com/realm/realm-dotnet/issues/3455))

### Fixed
* None
Expand Down
2 changes: 1 addition & 1 deletion Realm/Realm.SourceGenerator/ClassCodeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ private string GenerateManagedAccessor()
}
else
{
var forceNotNullable = type == "string" || type == "byte[]" ? "!" : string.Empty;
var forceNotNullable = type is "string" or "byte[]" ? "!" : string.Empty;

var getterString = $@"get => ({type})GetValue(""{stringName}""){forceNotNullable};";

Expand Down
22 changes: 22 additions & 0 deletions Realm/Realm.SourceGenerator/Diagnostics.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ private enum Id
RealmObjectWithoutAutomaticProperty = 25,
ParentOfNestedClassIsNotPartial = 27,
IndexedPrimaryKey = 28,
InvalidCollectionInitializer = 29,
InvalidCollectionInitializerInCtor = 30,
InvalidGeneratorConfiguration = 1000,
}

Expand Down Expand Up @@ -318,6 +320,26 @@ public static Diagnostic ParentOfNestedClassIsNotPartial(string className, strin
location);
}

public static Diagnostic InvalidCollectionInitializer(string className, string propertyName, Location location)
{
return CreateDiagnosticError(
Id.InvalidCollectionInitializer,
"Invalid collection initializer",
$"{className}.{propertyName} is a collection with an initializer that is not supported. Realm collections are always initialized internally and initializing them to a non-null value is not supported.",
location,
description: $"Either remove the initializer or replace it with '= null!' to silence the 'Non-nullable field '{propertyName}' is uninitialized' error.");
}

public static Diagnostic InvalidCollectionInitializerInCtor(string className, string propertyName, Location location)
{
return CreateDiagnosticError(
Id.InvalidCollectionInitializerInCtor,
"Invalid collection initializer in constructor",
$"{className}.{propertyName} is a collection that is initialized in a constructor. Realm collections are always initialized internally and initializing them to a non-null value is not supported.",
location,
description: $"Either remove the initializer or replace it with '= null!' to silence the 'Non-nullable field '{propertyName}' is uninitialized' error.");
}

#endregion

#region Warnings
Expand Down
50 changes: 47 additions & 3 deletions Realm/Realm.SourceGenerator/Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand All @@ -27,6 +28,9 @@ namespace Realms.SourceGenerator
{
internal class Parser
{
private static readonly Regex _equalsNullRegex = new(@"\s*=\s*null!?");
private static readonly Regex _nullRegex = new(@"^null!?$");

private readonly GeneratorExecutionContext _context;
private readonly GeneratorConfig _generatorConfig;

Expand Down Expand Up @@ -112,6 +116,8 @@ public ParsingResults Parse(IEnumerable<RealmClassDefinition> realmClasses)
classInfo.Diagnostics.Add(Diagnostics.MultiplePrimaryKeys(classInfo.Name, firstClassDeclarationSyntax.GetIdentifierLocation()));
}

ValidateConstructorBody(classInfo, classDeclarations);

result.ClassInfo.Add(classInfo);
}
catch (Exception ex)
Expand Down Expand Up @@ -269,6 +275,11 @@ private IEnumerable<PropertyInfo> GetProperties(ClassInfo classInfo, IEnumerable
}
}

if (info.TypeInfo.IsCollection && !string.IsNullOrEmpty(info.Initializer) && !_equalsNullRegex.IsMatch(info.Initializer!))
{
classInfo.Diagnostics.Add(Diagnostics.InvalidCollectionInitializer(classInfo.Name, info.Name, propSyntax.GetLocation()));
}

yield return info;
}
}
Expand Down Expand Up @@ -461,12 +472,45 @@ INamedTypeSymbol when typeSymbol.IsAnyRealmObjectType() => PropertyTypeInfo.Obje
return propInfo;
}

private static bool HasParameterlessConstructor(List<ClassDeclarationSyntax> classDeclarations)
private static bool HasParameterlessConstructor(IEnumerable<ClassDeclarationSyntax> classDeclarations)
{
var constructors = classDeclarations.SelectMany(cd => cd.ChildNodes().OfType<ConstructorDeclarationSyntax>()).ToArray();
return !constructors.Any() || constructors.Any(c => !c.ParameterList.Parameters.Any());
}

private static void ValidateConstructorBody(ClassInfo classInfo, IEnumerable<ClassDeclarationSyntax> classDeclarations)
{
// This finds all assignment expressions in the ctor body
var assignments = classDeclarations.SelectMany(cd => cd.ChildNodes().OfType<ConstructorDeclarationSyntax>())
.Where(c => c.Body != null)
.SelectMany(c => c.Body!.Statements)
.OfType<ExpressionStatementSyntax>()
.Select(e => e.Expression)
.OfType<AssignmentExpressionSyntax>();

foreach (var assignment in assignments)
{
if (assignment.Left is not IdentifierNameSyntax leftIdentifier)
{
continue;
}

// If the assignment is to `null`, we ignore it
if (_nullRegex.IsMatch(assignment.Right.ToString()))
{
continue;
}

// If the left operand of the assignment is a collection property, we should report an error as it's not valid to
// initialize a collection to something other than null.
var initializedProp = classInfo.Properties.FirstOrDefault(p => p.TypeInfo.IsCollection && p.Name == leftIdentifier.Identifier.ValueText);
if (initializedProp != null)
{
classInfo.Diagnostics.Add(Diagnostics.InvalidCollectionInitializerInCtor(classInfo.Name, initializedProp.Name, assignment.GetLocation()));
}
}
}

private static IList<EnclosingClassInfo> GetEnclosingClassList(ClassInfo classInfo, ITypeSymbol classSymbol, ClassDeclarationSyntax classDeclarationSyntax)
{
var enclosingClassList = new List<EnclosingClassInfo>();
Expand All @@ -480,8 +524,8 @@ private static IList<EnclosingClassInfo> GetEnclosingClassList(ClassInfo classIn
classInfo.Diagnostics.Add(Diagnostics.ParentOfNestedClassIsNotPartial(classSymbol.Name, ts.Name, cs.GetIdentifierLocation()));
}

var enclosingClassinfo = new EnclosingClassInfo(ts.Name, ts.DeclaredAccessibility);
enclosingClassList.Add(enclosingClassinfo);
var enclosingClassInfo = new EnclosingClassInfo(ts.Name, ts.DeclaredAccessibility);
enclosingClassList.Add(enclosingClassInfo);

currentSymbol = ts;
currentClassDeclaration = cs;
Expand Down
35 changes: 22 additions & 13 deletions Tests/SourceGenerators/Realm.SourceGenerator.Tests/ComparisonTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,28 @@ public async Task ComparisonTest(string filename, params string[] classNames)
await RunComparisonTest(filename, classNames);
}

[TestCase("ClassWithBaseType")]
[TestCase("MultiplePrimaryKeys")]
[TestCase("NoPartialClass")]
[TestCase("RealmintegerErrors")]
[TestCase("RealmObjectAndEmbeddedObjectClass")]
[TestCase("UnsupportedIndexableTypes")]
[TestCase("UnsupportedPrimaryKeyTypes")]
[TestCase("UnsupportedRequiredTypes")]
[TestCase("NestedClassWithoutPartialParent")]
[TestCase("NullableErrorClass")]
[TestCase("IgnoreObjectNullabilityClass")]
[TestCase("UnsupportedBacklink", "UnsupportedBacklink", "BacklinkObj")]
public async Task ErrorComparisonTest(string filename, params string[] classNames)
public static object[][] ErrorTestCases = Directory.GetFiles(_errorClassesPath)
.Select(Path.GetFileNameWithoutExtension)
.Select(f =>
{
var classNames = Array.Empty<string>();
if (f == "UnsupportedBacklink")
{
classNames = new[]
{
"UnsupportedBacklink", "BacklinkObj"
};
}

return new object[]
{
f!, classNames
};
})
.ToArray();

[TestCaseSource(nameof(ErrorTestCases))]
public async Task ErrorComparisonTest(string filename, string[] classNames)
{
await RunErrorTest(filename, classNames);
}
Expand Down
23 changes: 10 additions & 13 deletions Tests/SourceGenerators/Realm.SourceGenerator.Tests/SourceGenerationTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,24 @@ namespace SourceGeneratorTests
{
internal abstract class SourceGenerationTest
{
private string _testClassesPath;
private string _errorClassesPath;
private string _supportClassesPath;
private string _generatedFilesPath;
private static readonly string _buildFolder = Path.GetDirectoryName(typeof(SourceGenerationTest).Assembly.Location)!;
private static readonly string _testFolder = _buildFolder.Substring(0, _buildFolder.IndexOf("Realm.SourceGenerator.Test", StringComparison.InvariantCulture));
private static readonly string _assemblyToProcessFolder = Path.Combine(_testFolder, "SourceGeneratorAssemblyToProcess");

private string[] _supportClasses = new[] {
private static readonly string _testClassesPath = Path.Combine(_assemblyToProcessFolder, "TestClasses");
protected static readonly string _errorClassesPath = Path.Combine(_assemblyToProcessFolder, "ErrorClasses");
private static readonly string _supportClassesPath = Path.Combine(_assemblyToProcessFolder, "SupportClasses");
private static readonly string _generatedFilesPath = Path.Combine(_assemblyToProcessFolder, "Generated",
"Realm.SourceGenerator", "Realms.SourceGenerator.RealmGenerator");

private readonly string[] _supportClasses = {
"RealmObj",
"EmbeddedObj",
};

[OneTimeSetUp]
public void Setup()
{
var buildFolder = Path.GetDirectoryName(typeof(SourceGenerationTest).Assembly.Location)!;
var testFolder = buildFolder.Substring(0, buildFolder.IndexOf("Realm.SourceGenerator.Test", StringComparison.InvariantCulture));
var assemblyToProcessFolder = Path.Combine(testFolder, "SourceGeneratorAssemblyToProcess");
_testClassesPath = Path.Combine(assemblyToProcessFolder, "TestClasses");
_errorClassesPath = Path.Combine(assemblyToProcessFolder, "ErrorClasses");
_supportClassesPath = Path.Combine(assemblyToProcessFolder, "SupportClasses");
_generatedFilesPath = Path.Combine(assemblyToProcessFolder, "Generated",
"Realm.SourceGenerator", "Realms.SourceGenerator.RealmGenerator");
Environment.SetEnvironmentVariable("NO_GENERATOR_DIAGNOSTICS", "true");
}

Expand Down
31 changes: 25 additions & 6 deletions Tests/SourceGenerators/SourceGeneratorAssemblyToProcess/ErrorClasses/CollectionErrors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,44 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Realms;

namespace SourceGeneratorPlayground
{
public partial class CollectionErrors : IRealmObject
{
public IDictionary<int, string> UnsupportetDictionaryKeyProp { get; }

public ISet<EmbeddedObj> SetOfEmbeddedObj { get; }

public IList<int> CollectionWithSetter { get; set; }

public IList<RealmInteger<int>> CollectionOfRealmInteger { get; }

public IList<DateTime> CollectionOfUnsupportedType { get; }

public List<int> ListInsteadOfIList { get; }

public IList<int> CollectionWithInitializer { get; } = new List<int>
{
1,
2,
3
};

public IList<string> CollectionWithCtorInitializer { get; }

// This should not generate error as it's initialized to null
public IList<string> ValidCollectionInitializer { get; } = null!;

public IList<string> ValidCollectionInitializerInCtor { get; }

public CollectionErrors()
{
CollectionWithCtorInitializer = new List<string>();

// This should not generate error as it's initialized to null
ValidCollectionInitializerInCtor = null!;
}
}
}
46 changes: 34 additions & 12 deletions ...alm.SourceGenerator/Realms.SourceGenerator.RealmGenerator/CollectionErrors.diagnostics.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
"Severity": 3,
"Message": "CollectionErrors.UnsupportetDictionaryKeyProp is a Dictionary<int, string> but only string keys are currently supported by Realm.",
"Location": {
"StartLine": 30,
"StartLine": 27,
"StartColumn": 9,
"EndLine": 30,
"EndLine": 27,
"EndColumn": 78
}
},
Expand All @@ -15,9 +15,9 @@
"Severity": 3,
"Message": "CollectionErrors.SetOfEmbeddedObj is a Set<EmbeddedObject> which is not supported. Embedded objects are always unique which is why List<EmbeddedObject> already has Set semantics.",
"Location": {
"StartLine": 32,
"StartLine": 29,
"StartColumn": 9,
"EndLine": 32,
"EndLine": 29,
"EndColumn": 59
}
},
Expand All @@ -26,9 +26,9 @@
"Severity": 3,
"Message": "CollectionErrors.CollectionWithSetter has a setter but its type is a List which only supports getters.",
"Location": {
"StartLine": 34,
"StartLine": 31,
"StartColumn": 9,
"EndLine": 34,
"EndLine": 31,
"EndColumn": 61
}
},
Expand All @@ -37,9 +37,9 @@
"Severity": 3,
"Message": "CollectionErrors.CollectionOfRealmInteger is an List<RealmInteger> which is not supported.",
"Location": {
"StartLine": 36,
"StartLine": 33,
"StartColumn": 9,
"EndLine": 36,
"EndLine": 33,
"EndColumn": 74
}
},
Expand All @@ -48,9 +48,9 @@
"Severity": 3,
"Message": "CollectionErrors.CollectionOfUnsupportedType is an List but its generic type is System.DateTime which is not supported by Realm.",
"Location": {
"StartLine": 38,
"StartLine": 35,
"StartColumn": 9,
"EndLine": 38,
"EndLine": 35,
"EndColumn": 68
}
},
Expand All @@ -59,10 +59,32 @@
"Severity": 3,
"Message": "CollectionErrors.ListInsteadOfIList is declared as List which is not the correct way to declare to-many relationships in Realm. If you want to persist the collection, use the interface IList, otherwise annotate the property with the [Ignored] attribute.",
"Location": {
"StartLine": 40,
"StartLine": 37,
"StartColumn": 9,
"EndLine": 40,
"EndLine": 37,
"EndColumn": 53
}
},
{
"Id": "RLM029",
"Severity": 3,
"Message": "CollectionErrors.CollectionWithInitializer is a collection with an initializer that is not supported. Realm collections are always initialized internally and initializing them to a non-null value is not supported.",
"Location": {
"StartLine": 39,
"StartColumn": 9,
"EndLine": 44,
"EndColumn": 11
}
},
{
"Id": "RLM030",
"Severity": 3,
"Message": "CollectionErrors.CollectionWithCtorInitializer is a collection that is initialized in a constructor. Realm collections are always initialized internally and initializing them to a non-null value is not supported.",
"Location": {
"StartLine": 55,
"StartColumn": 13,
"EndLine": 55,
"EndColumn": 63
}
}
]

0 comments on commit e554ef5

Please sign in to comment.