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.

Sign up for GitHub

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

Ignore self referencing property values #348

Merged
merged 1 commit into from
Aug 11, 2024
Merged

Ignore self referencing property values #348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 33 additions & 0 deletions ModelBuilder.UnitTests/DefaultPropertyResolverTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
namespace ModelBuilder.UnitTests
{
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -870,6 +871,22 @@ public void IsIgnoredReturnsTrueWhenParametersMatchPropertyValueType()
actual.Should().BeTrue();
}

[Fact]
public void IsIgnoredReturnsTrueWhenPropertyValueMatchesOwningInstance()
{
var configuration = Model.UsingDefaultConfiguration();
var instance = new Hashtable();
var args = Array.Empty<object?>();

var propertyInfo = instance.GetType().GetProperty(nameof(Hashtable.SyncRoot))!;

var sut = new DefaultPropertyResolver(CacheLevel.PerInstance);

var actual = sut.IsIgnored(configuration, instance, propertyInfo, args);

actual.Should().BeTrue();
}

[Fact]
public void IsIgnoredThrowsExceptionWhenConstructorNotFound()
{
Expand Down Expand Up @@ -931,6 +948,22 @@ public void IsIgnoredThrowsExceptionWithNullPropertyInfo()
action.Should().Throw<ArgumentNullException>();
}

[Fact]
public void IsIgnoredTreatsFailedConstructorForDefaultValueDetectionAsNull()
{
var configuration = Model.UsingDefaultConfiguration();
var instance = new PropertyModel<StructConstructorException>();
var args = Array.Empty<object?>();

var propertyInfo = instance.GetType().GetProperty(nameof(PropertyModel<StructConstructorException>.Value))!;

var sut = new DefaultPropertyResolver(CacheLevel.PerInstance);

var actual = sut.IsIgnored(configuration, instance, propertyInfo, args);

actual.Should().BeFalse();
}

private class ClassMatchingNameWrapper<T> where T : class
{
public ClassMatchingNameWrapper(T value)
Expand Down
9 changes: 9 additions & 0 deletions ModelBuilder.UnitTests/Models/SelfReferenceInstance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace ModelBuilder.UnitTests.Models
{
internal class SelfReferenceInstance
{
private SelfReferenceInstance? _value;

public SelfReferenceInstance Instance { get => _value ?? this; set => _value = value; }
}
}
12 changes: 12 additions & 0 deletions ModelBuilder.UnitTests/Models/StructConstructorException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace ModelBuilder.UnitTests.Models
{
using System;

internal struct StructConstructorException
{
public StructConstructorException()
{
throw new InvalidOperationException("This is an exception from the constructor.");
}
}
}
28 changes: 28 additions & 0 deletions ModelBuilder.UnitTests/Scenarios/RegressionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
namespace ModelBuilder.UnitTests.Scenarios
{
using FluentAssertions;
using ModelBuilder.UnitTests.Models;
using Xunit;
using Xunit.Abstractions;

public class RegressionTests
{
private readonly ITestOutputHelper _output;

public RegressionTests(ITestOutputHelper output)
{
_output = output;
}

[Fact]
public void CreateShouldSkipSelfReferencingPropertiesOnPopulateException()
{
// Fixes https://github.com/roryprimrose/ModelBuilder/issues/347
var action = () => Model.WriteLog<SelfReferenceInstance>(_output.WriteLine).Create();

var actual = action.Should().NotThrow().Subject;

actual.Instance.Should().BeSameAs(actual);
}
}
}
5 changes: 5 additions & 0 deletions ModelBuilder.sln.DotSettings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,10 @@
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateInstanceFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/UserRules/=15b5b1f1_002D457c_002D4ca6_002Db278_002D5615aedc07d3/@EntryIndexedValue">&lt;Policy&gt;&lt;Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static readonly fields (private)"&gt;&lt;ElementKinds&gt;&lt;Kind Name="READONLY_FIELD" /&gt;&lt;/ElementKinds&gt;&lt;/Descriptor&gt;&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;&lt;/Policy&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/UserRules/=4a98fdf6_002D7d98_002D4f5a_002Dafeb_002Dea44ad98c70c/@EntryIndexedValue">&lt;Policy&gt;&lt;Descriptor Staticness="Instance" AccessRightKinds="Private" Description="Instance fields (private)"&gt;&lt;ElementKinds&gt;&lt;Kind Name="FIELD" /&gt;&lt;Kind Name="READONLY_FIELD" /&gt;&lt;/ElementKinds&gt;&lt;/Descriptor&gt;&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;&lt;/Policy&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/UserRules/=8b8504e3_002Df0be_002D4c14_002D9103_002Dc732f2bddc15/@EntryIndexedValue">&lt;Policy&gt;&lt;Descriptor Staticness="Any" AccessRightKinds="Any" Description="Enum members"&gt;&lt;ElementKinds&gt;&lt;Kind Name="ENUM_MEMBER" /&gt;&lt;/ElementKinds&gt;&lt;/Descriptor&gt;&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /&gt;&lt;/Policy&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/UserRules/=f9fce829_002De6f4_002D4cb2_002D80f1_002D5497c44f51df/@EntryIndexedValue">&lt;Policy&gt;&lt;Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static fields (private)"&gt;&lt;ElementKinds&gt;&lt;Kind Name="FIELD" /&gt;&lt;/ElementKinds&gt;&lt;/Descriptor&gt;&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;&lt;/Policy&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=JS_005FBLOCK_005FSCOPE_005FCONSTANT/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=JS_005FBLOCK_005FSCOPE_005FVARIABLE/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=JS_005FCONSTRUCTOR/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /&gt;</s:String>
Expand Down Expand Up @@ -438,6 +442,7 @@
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002ECSharpPlaceAttributeOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateThisQualifierSettings/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EPredefinedNamingRulesToUserRulesUpgrade/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002EJavaScript_002ECodeStyle_002ESettingsUpgrade_002EJsParsFormattingSettingsUpgrader/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002EJavaScript_002ECodeStyle_002ESettingsUpgrade_002EJsWrapperSettingsUpgrader/@EntryIndexedValue">True</s:Boolean>
<s:String x:Key="/Default/FilterSettingsManager/AttributeFilterXml/@EntryValue">&lt;data /&gt;</s:String>
Expand Down
14 changes: 14 additions & 0 deletions ModelBuilder/DefaultPropertyResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ public virtual bool IsIgnored(
}

var propertyValue = propertyInfo.GetValue(instance, null);

if (AreEqual(instance, propertyValue))
{
// This property is a self reference to the owning instance
// For example, Hashtable.SyncRoot is a reference to Hashtable itself
return true;
}

var defaultValue = GetDefaultValue(propertyInfo.PropertyType);

if (AreEqual(propertyValue, defaultValue))
Expand Down Expand Up @@ -202,6 +210,12 @@ private static bool AreEqual(object? first, object? second)
return false;
}

if (ReferenceEquals(first, second))
{
// These reference the same instance in memory
return true;
}

if (first.Equals(second))
{
return true;
Expand Down
Loading