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

Better warning for skipped properties with attributes #3354

Merged
merged 3 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* None

### Fixed
* None
* Improved the warning message when adding Realm attributes on a non-persisted property. (Issue [#3352](https://github.com/realm/realm-dotnet/issues/3352))

### Compatibility
* Realm Studio: 13.0.0 or later.
Expand Down
10 changes: 0 additions & 10 deletions Realm/Realm.SourceGenerator/Diagnostics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ private enum Id
DateTimeNotSupported = 23,
TypeNotSupported = 24,
RealmObjectWithoutAutomaticProperty = 25,
NotPersistedPropertyWithRealmAttributes = 26,
ParentOfNestedClassIsNotPartial = 27,
IndexedPrimaryKey = 28,
}
Expand Down Expand Up @@ -322,15 +321,6 @@ public static Diagnostic RealmObjectWithoutAutomaticProperty(string className, s
location);
}

public static Diagnostic NotPersistedPropertyWithRealmAttributes(string className, string propertyName, Location location)
{
return CreateDiagnosticWarning(
Id.NotPersistedPropertyWithRealmAttributes,
"Not persisted property with Realm attributes",
$"{className}.{propertyName} has one or more Realm attributes applied, but it's not persisted, so those attributes will be ignored.",
location);
}

#endregion

private static Diagnostic CreateDiagnostic(Id id, string title, string messageFormat, DiagnosticSeverity severity,
Expand Down
10 changes: 5 additions & 5 deletions Realm/Realm.Weaver/RealmWeaver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ private WeaveTypeResult WeaveType(TypeDefinition type)

if (realmAttributeNames.Any())
{
_logger.Warning($"{type.Name}.{prop.Name} has {string.Join(", ", realmAttributeNames)} applied, but it's not persisted, so those attributes will be ignored.", sequencePoint);
_logger.Warning($"{type.Name}.{prop.Name} has {string.Join(", ", realmAttributeNames)} applied, but it's not persisted, so these attributes will be ignored. Skip reason: {weaveResult.SkipReason}", sequencePoint);
}
}
}
Expand Down Expand Up @@ -517,7 +517,7 @@ private WeavePropertyResult WeaveProperty(PropertyDefinition prop, TypeDefinitio

if (prop.GetMethod == null)
{
return WeavePropertyResult.Skipped();
return WeavePropertyResult.Skipped("Property has no getter");
}

var indexedAttribute = prop.CustomAttributes.FirstOrDefault(a => a.AttributeType.Name == "IndexedAttribute");
Expand Down Expand Up @@ -570,7 +570,7 @@ private WeavePropertyResult WeaveProperty(PropertyDefinition prop, TypeDefinitio
return WeavePropertyResult.Warning($"{type.Name}.{prop.Name} is not an automatic property but its type is a AsymmetricObject. This usually indicates a relationship but AsymmetricObjects are not allowed to be the receiving end of any relationships.");
}

return WeavePropertyResult.Skipped();
return WeavePropertyResult.Skipped("Property is not autoimplemented");
}

var backlinkAttribute = prop.CustomAttributes.FirstOrDefault(a => a.AttributeType.Name == "BacklinkAttribute");
Expand All @@ -589,7 +589,7 @@ private WeavePropertyResult WeaveProperty(PropertyDefinition prop, TypeDefinitio
{
if (prop.SetMethod == null)
{
return WeavePropertyResult.Skipped();
return WeavePropertyResult.Skipped("Property has no setter");
}

var setter = isPrimaryKey ? _references.RealmObject_SetValueUnique : _references.RealmObject_SetValue;
Expand Down Expand Up @@ -707,7 +707,7 @@ private WeavePropertyResult WeaveProperty(PropertyDefinition prop, TypeDefinitio
}
else if (prop.SetMethod == null)
{
return WeavePropertyResult.Skipped();
return WeavePropertyResult.Skipped("Property has no setter");
}
else if (prop.PropertyType.FullName == "System.DateTime")
{
Expand Down
9 changes: 6 additions & 3 deletions Realm/Realm.Weaver/WeaveResults.cs
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,17 @@ public static WeavePropertyResult Error(string error)
return new WeavePropertyResult(error: error);
}

public static WeavePropertyResult Skipped()
public static WeavePropertyResult Skipped(string reason)
{
return new WeavePropertyResult();
return new WeavePropertyResult(skipReason: reason);
}

public string? ErrorMessage { get; }

public string? WarningMessage { get; }

public string? SkipReason { get; }

[MemberNotNullWhen(true, nameof(Property))]
public bool Woven { get; }

Expand All @@ -181,10 +183,11 @@ private WeavePropertyResult(PropertyDefinition property, FieldReference? field,
Woven = true;
}

private WeavePropertyResult(string? error = null, string? warning = null)
private WeavePropertyResult(string? error = null, string? warning = null, string? skipReason = null)
{
ErrorMessage = error;
WarningMessage = warning;
SkipReason = skipReason;
}

public override string ToString()
Expand Down
2 changes: 1 addition & 1 deletion Realm/Realm/Realm.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
</ProjectReference>
</ItemGroup>
<ItemGroup Label="Package">
<None Include="NuGet.md" Pack="true" PackagePath="\" />
<None Include="$(ProjectDir)..\..\NuGet.md" Pack="true" PackagePath="\" />
<EmbeddedResource Include="Properties\Realm.rd.xml" />
<None Include="..\Realm.SourceGenerator\bin\$(Configuration)\netstandard2.0\Realm.SourceGenerator.dll" PackagePath="analyzers\dotnet\cs" Pack="true" Visible="false" />
<None Include="wrappers-props\Realm.props" PackagePath="build\Realm.props" Pack="true" Visible="false" />
Expand Down
7 changes: 0 additions & 7 deletions Tests/Weaver/AnalyticsAssembly/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,6 @@ public static void ApiKeyAuthenticationMethod()
}
#endif

#if SERVER_API_KEY
public static void ServerApiKeyAuthenticationMethod()
{
_ = Credentials.ServerApiKey("serverApiKey");
}
#endif

#if FUNCTION
public static void FunctionAuthenticationMethod()
{
Expand Down
8 changes: 4 additions & 4 deletions Tests/Weaver/Realm.Fody.Tests/WeaverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,10 @@ public void MatchErrorsAndWarnings()
{
"LambdaPropertyObject.FirstPropertyObject is not an automatic property but its type is a RealmObject/EmbeddedObject which normally indicates a relationship.",
"Sensor.FirstMeasurement is not an automatic property but its type is a AsymmetricObject. This usually indicates a relationship but AsymmetricObjects are not allowed to be the receiving end of any relationships.",
"IncorrectAttributes.AutomaticId has [PrimaryKey] applied, but it's not persisted, so those attributes will be ignored.",
"IncorrectAttributes.AutomaticDate has [Indexed] applied, but it's not persisted, so those attributes will be ignored.",
"IncorrectAttributes.Email_ has [MapTo] applied, but it's not persisted, so those attributes will be ignored.",
"IncorrectAttributes.Date_ has [Indexed], [MapTo] applied, but it's not persisted, so those attributes will be ignored.",
"IncorrectAttributes.AutomaticId has [PrimaryKey] applied, but it's not persisted, so these attributes will be ignored. Skip reason: Property has no setter",
"IncorrectAttributes.AutomaticDate has [Indexed] applied, but it's not persisted, so these attributes will be ignored. Skip reason: Property has no setter",
"IncorrectAttributes.Email_ has [MapTo] applied, but it's not persisted, so these attributes will be ignored. Skip reason: Property has no setter",
"IncorrectAttributes.Date_ has [Indexed], [MapTo] applied, but it's not persisted, so these attributes will be ignored. Skip reason: Property has no setter",
"AccessorTestObject.SetterLessObject does not have a setter but its type is a RealmObject/EmbeddedObject which normally indicates a relationship.",
};

Expand Down