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

Deprecate FieldSignature.CreateXXX Factory methods #254

Merged
merged 4 commits into from
Feb 22, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ private FieldDefinition AddPlaceHolderField(TypeDefinition placeHolderType, Meta
var placeHolderField = new FieldDefinition(
$"PlaceHolderField_{token.Rid.ToString()}",
FieldPlaceHolderAttributes,
FieldSignature.CreateStatic(_module.CorLibTypeFactory.Object));
_module.CorLibTypeFactory.Object);

// Add the field to the type.
placeHolderType.Fields.Add(placeHolderField);
Expand Down
22 changes: 17 additions & 5 deletions src/AsmResolver.DotNet/FieldDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using AsmResolver.Collections;
using AsmResolver.DotNet.Signatures;
using AsmResolver.DotNet.Signatures.Marshal;
using AsmResolver.DotNet.Signatures.Types;
using AsmResolver.PE.DotNet.Metadata.Tables;
using AsmResolver.PE.DotNet.Metadata.Tables.Rows;

Expand Down Expand Up @@ -56,11 +57,6 @@ protected FieldDefinition(MetadataToken token)
/// <param name="name">The name of the field.</param>
/// <param name="attributes">The attributes.</param>
/// <param name="signature">The signature of the field.</param>
/// <remarks>
/// For a valid .NET image, if <see cref="CallingConventionSignature.HasThis"/> of the signature referenced by
/// <paramref name="signature"/> is set, the <see cref="FieldAttributes.Static"/> bit should be unset in
/// <paramref name="attributes"/> and vice versa.
/// </remarks>
public FieldDefinition(string? name, FieldAttributes attributes, FieldSignature? signature)
: this(new MetadataToken(TableIndex.Field, 0))
{
Expand All @@ -69,6 +65,22 @@ public FieldDefinition(string? name, FieldAttributes attributes, FieldSignature?
Signature = signature;
}

/// <summary>
/// Creates a new field definition.
/// </summary>
/// <param name="name">The name of the field.</param>
/// <param name="attributes">The attributes.</param>
/// <param name="fieldType">The type of values the field contains.</param>
public FieldDefinition(string? name, FieldAttributes attributes, TypeSignature? fieldType)
: this(new MetadataToken(TableIndex.Field, 0))
{
Name = name;
Attributes = attributes;
Signature = fieldType is not null
? new FieldSignature(fieldType)
: null;
}

/// <summary>
/// Gets or sets the name of the field.
/// </summary>
Expand Down
10 changes: 8 additions & 2 deletions src/AsmResolver.DotNet/Signatures/FieldSignature.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System;
using AsmResolver.DotNet.Signatures.Types;
using AsmResolver.IO;
using AsmResolver.PE.DotNet.Metadata.Tables.Rows;

namespace AsmResolver.DotNet.Signatures
{
Expand All @@ -14,6 +14,9 @@ public class FieldSignature : MemberSignature
/// </summary>
/// <param name="fieldType">The value type of the field.</param>
/// <returns>The signature.</returns>
[Obsolete("The HasThis bit in field signatures is ignored by the CLR. Use the constructor instead,"
+ " or when this call is used in an argument of a FieldDefinition constructor, use the overload"
+ " taking TypeSignature instead.")]
public static FieldSignature CreateStatic(TypeSignature fieldType)
=> new(CallingConventionAttributes.Field, fieldType);

Expand All @@ -22,6 +25,9 @@ public static FieldSignature CreateStatic(TypeSignature fieldType)
/// </summary>
/// <param name="fieldType">The value type of the field.</param>
/// <returns>The signature.</returns>
[Obsolete("The HasThis bit in field signatures is ignored by the CLR. Use the constructor instead,"
+ " or when this call is used in an argument of a FieldDefinition constructor, use the overload"
+ " taking TypeSignature instead.")]
public static FieldSignature CreateInstance(TypeSignature fieldType)
=> new(CallingConventionAttributes.Field | CallingConventionAttributes.HasThis, fieldType);

Expand Down Expand Up @@ -61,7 +67,7 @@ public FieldSignature(CallingConventionAttributes attributes, TypeSignature fiel
}

/// <summary>
/// Gets the type of the object that the field stores.
/// Gets the type of the value that the field contains.
/// </summary>
public TypeSignature FieldType
{
Expand Down
4 changes: 2 additions & 2 deletions test/AsmResolver.DotNet.Tests/Builder/TokenMappingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void NewFieldDefinition()
var field = new FieldDefinition(
"MyField",
FieldAttributes.Public | FieldAttributes.Static,
FieldSignature.CreateStatic(module.CorLibTypeFactory.Object));
module.CorLibTypeFactory.Object);
module.GetOrCreateModuleType().Fields.Add(field);

// Rebuild.
Expand Down Expand Up @@ -107,7 +107,7 @@ public void NewTypeReference()
module.GetOrCreateModuleType().Fields.Add(new FieldDefinition(
"MyField",
FieldAttributes.Public | FieldAttributes.Static,
FieldSignature.CreateStatic(reference.ToTypeSignature())));
reference.ToTypeSignature()));

// Rebuild.
var builder = new ManagedPEImageBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ public class FieldTokenPreservationTest : TokenPreservationTestBase
private static ModuleDefinition CreateSampleFieldDefsModule(int typeCount, int fieldsPerType)
{
var module = ModuleDefinition.FromBytes(Properties.Resources.HelloWorld_NetCore);

for (int i = 0; i < typeCount; i++)
{
var dummyType = new TypeDefinition("Namespace", $"Type{i.ToString()}",
TypeAttributes.Public | TypeAttributes.Abstract | TypeAttributes.Sealed);

module.TopLevelTypes.Add(dummyType);
for (int j = 0; j < fieldsPerType; j++)
dummyType.Fields.Add(CreateDummyField(module, $"Field{j}"));
Expand All @@ -28,18 +28,16 @@ private static ModuleDefinition CreateSampleFieldDefsModule(int typeCount, int f
return RebuildAndReloadModule(module, MetadataBuilderFlags.None);
}

private static FieldDefinition CreateDummyField(ModuleDefinition module, string name)
{
return new FieldDefinition(name,
FieldAttributes.Public | FieldAttributes.Static,
FieldSignature.CreateStatic(module.CorLibTypeFactory.Int32));
}

private static FieldDefinition CreateDummyField(ModuleDefinition module, string name) => new(
name,
FieldAttributes.Public | FieldAttributes.Static,
module.CorLibTypeFactory.Int32);

[Fact]
public void PreserveFieldDefsNoChange()
{
var module = CreateSampleFieldDefsModule(10, 10);

var newModule = RebuildAndReloadModule(module,MetadataBuilderFlags.PreserveFieldDefinitionIndices);

AssertSameTokens(module, newModule, t => t.Fields);
Expand All @@ -49,12 +47,12 @@ public void PreserveFieldDefsNoChange()
public void PreserveFieldDefsChangeOrderOfTypes()
{
var module = CreateSampleFieldDefsModule(10, 10);

const int swapIndex = 3;
var type = module.TopLevelTypes[swapIndex];
module.TopLevelTypes.RemoveAt(swapIndex);
module.TopLevelTypes.Insert(swapIndex + 1, type);

var newModule = RebuildAndReloadModule(module,MetadataBuilderFlags.PreserveFieldDefinitionIndices);

AssertSameTokens(module, newModule, t => t.Fields);
Expand All @@ -70,7 +68,7 @@ public void PreserveFieldDefsChangeOrderOfFieldsInType()
var field = type.Fields[swapIndex];
type.Fields.RemoveAt(swapIndex);
type.Fields.Insert(swapIndex + 1, field);

var newModule = RebuildAndReloadModule(module,MetadataBuilderFlags.PreserveFieldDefinitionIndices);

AssertSameTokens(module, newModule, t => t.Fields);
Expand All @@ -84,7 +82,7 @@ public void PreserveFieldDefsAddExtraField()
var type = module.TopLevelTypes[2];
var field = CreateDummyField(module, "ExtraField");
type.Fields.Insert(3, field);

var newModule = RebuildAndReloadModule(module,MetadataBuilderFlags.PreserveFieldDefinitionIndices);

AssertSameTokens(module, newModule, t => t.Fields);
Expand All @@ -99,10 +97,10 @@ public void PreserveFieldDefsRemoveField()
const int indexToRemove = 3;
var field = type.Fields[indexToRemove];
type.Fields.RemoveAt(indexToRemove);

var newModule = RebuildAndReloadModule(module,MetadataBuilderFlags.PreserveFieldDefinitionIndices);

AssertSameTokens(module, newModule, t => t.Fields, field.MetadataToken);
}
}
}
}
2 changes: 1 addition & 1 deletion test/AsmResolver.DotNet.Tests/FieldDefinitionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void PersistentFieldSignature()
var field = (FieldDefinition) module.LookupMember(
typeof(SingleField).GetField(nameof(SingleField.IntField)).MetadataToken);

field.Signature = FieldSignature.CreateInstance(module.CorLibTypeFactory.Byte);
field.Signature = new FieldSignature(module.CorLibTypeFactory.Byte);

var newField = RebuildAndLookup(field);

Expand Down
6 changes: 4 additions & 2 deletions test/AsmResolver.DotNet.Tests/MetadataResolverTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,10 @@ public void ResolveStringEmptyField()
var module = new ModuleDefinition("SomeModule.dll");

var stringType = new TypeReference(module.CorLibTypeFactory.CorLibScope, "System", "String");
var emptyField = new MemberReference(stringType, "Empty",
FieldSignature.CreateStatic(module.CorLibTypeFactory.String));
var emptyField = new MemberReference(
stringType,
"Empty",
new FieldSignature(module.CorLibTypeFactory.String));

var definition = _fwResolver.ResolveField(emptyField);

Expand Down
39 changes: 22 additions & 17 deletions test/AsmResolver.DotNet.Tests/ReferenceImporterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ public class ReferenceImporterTest
private readonly AssemblyReference _dummyAssembly = new AssemblyReference("SomeAssembly", new Version(1, 2, 3, 4));
private readonly ModuleDefinition _module;
private readonly ReferenceImporter _importer;

public ReferenceImporterTest()
{
_module = new ModuleDefinition("SomeModule.dll");
_importer = new ReferenceImporter(_module);
}

[Fact]
public void ImportNewAssemblyShouldAddToModule()
{
var result = _importer.ImportScope(_dummyAssembly);

Assert.Equal(_dummyAssembly, result, _comparer);
Assert.Contains(result, _module.AssemblyReferences);
}
Expand All @@ -38,10 +38,10 @@ public void ImportExistingAssemblyShouldUseExistingAssembly()
_module.AssemblyReferences.Add(_dummyAssembly);

int count = _module.AssemblyReferences.Count;

var copy = new AssemblyReference(_dummyAssembly);
var result = _importer.ImportScope(copy);

Assert.Same(_dummyAssembly, result);
Assert.Equal(count, _module.AssemblyReferences.Count);
}
Expand All @@ -61,7 +61,7 @@ public void ImportAlreadyImportedTypeShouldUseSameInstance()
{
var type = new TypeReference(_dummyAssembly, "SomeNamespace", "SomeName");
var importedType = _importer.ImportType(type);

var result = _importer.ImportType(importedType);

Assert.Same(importedType, result);
Expand All @@ -80,13 +80,13 @@ public void ImportTypeDefFromDifferentModuleShouldReturnTypeRef()
Assert.IsAssignableFrom<TypeReference>(result);
Assert.Equal(definition, result, _comparer);
}

[Fact]
public void ImportTypeDefInSameModuleShouldReturnSameInstance()
{
var definition = new TypeDefinition("SomeNamespace", "SomeName", TypeAttributes.Public);
_module.TopLevelTypes.Add(definition);

var importedType = _importer.ImportType(definition);

Assert.Same(definition, importedType);
Expand Down Expand Up @@ -127,7 +127,7 @@ public void ImportArrayTypeShouldResultInTypeSpecWithSzArray()
Assert.IsAssignableFrom<TypeSpecification>(result);
Assert.IsAssignableFrom<SzArrayTypeSignature>(((TypeSpecification) result).Signature);
}

[Fact]
public void ImportCorLibTypeAsSignatureShouldResultInCorLibTypeSignature()
{
Expand Down Expand Up @@ -179,7 +179,7 @@ public void ImportMethodFromSameModuleShouldResultInSameInstance()
{
var type = new TypeDefinition(null, "Type", TypeAttributes.Public);
_module.TopLevelTypes.Add(type);

var method = new MethodDefinition("Method", MethodAttributes.Public | MethodAttributes.Static,
MethodSignature.CreateStatic(_module.CorLibTypeFactory.Void));
type.Methods.Add(method);
Expand Down Expand Up @@ -224,8 +224,10 @@ public void ImportGenericMethodFromReflectionShouldResultInMethodSpec()
public void ImportFieldFromExternalModuleShouldResultInMemberRef()
{
var type = new TypeReference(_dummyAssembly, null, "Type");
var field = new MemberReference(type, "Field",
FieldSignature.CreateStatic(_module.CorLibTypeFactory.String));
var field = new MemberReference(
type,
"Field",
new FieldSignature(_module.CorLibTypeFactory.String));

var result = _importer.ImportField(field);

Expand All @@ -238,9 +240,12 @@ public void ImportFieldFromSameModuleShouldResultInSameInstance()
{
var type = new TypeDefinition(null, "Type", TypeAttributes.Public);
_module.TopLevelTypes.Add(type);

var field = new FieldDefinition("Method", FieldAttributes.Public | FieldAttributes.Static,
FieldSignature.CreateStatic(_module.CorLibTypeFactory.Void));

var field = new FieldDefinition(
"Field",
FieldAttributes.Public | FieldAttributes.Static,
_module.CorLibTypeFactory.Int32);

type.Fields.Add(field);

var result = _importer.ImportField(field);
Expand All @@ -254,10 +259,10 @@ public void ImportFieldFromReflectionShouldResultInMemberRef()
var field = typeof(string).GetField("Empty");

var result = _importer.ImportField(field);

Assert.Equal(field.Name, result.Name);
Assert.Equal(field.DeclaringType.FullName, result.DeclaringType.FullName);
Assert.Equal(field.FieldType.FullName, ((FieldSignature) result.Signature).FieldType.FullName);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,7 @@ public void ParseGenericFromField()

var genericParameter = new GenericParameterSignature(GenericParameterType.Type, 0);

var field = new FieldDefinition("Field", FieldAttributes.Private,
FieldSignature.CreateStatic(genericParameter));
var field = new FieldDefinition("Field", FieldAttributes.Private, genericParameter);

var member = new MemberReference(typeSpecification, field.Name, field.Signature);

Expand All @@ -292,8 +291,7 @@ public void ParseGenericFromNotGenericField()
var type = new TypeDefinition("", "Test type", TypeAttributes.Public);
var notGenericSignature = new TypeDefOrRefSignature(type);

var field = new FieldDefinition("Field", FieldAttributes.Private,
FieldSignature.CreateStatic(notGenericSignature));
var field = new FieldDefinition("Field", FieldAttributes.Private, notGenericSignature);

var member = new MemberReference(type, field.Name, field.Signature);

Expand Down
6 changes: 2 additions & 4 deletions test/AsmResolver.DotNet.Tests/TokenAllocatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,8 @@ public void AssignTokenOfNextMemberShouldPreserve()

// Create two dummy fields.
var fieldType = module.CorLibTypeFactory.Object;
var field1 = new FieldDefinition("NonAssignedField", FieldAttributes.Static,
FieldSignature.CreateStatic(fieldType));
var field2 = new FieldDefinition("AssignedField", FieldAttributes.Static,
FieldSignature.CreateStatic(fieldType));
var field1 = new FieldDefinition("NonAssignedField", FieldAttributes.Static, fieldType);
var field2 = new FieldDefinition("AssignedField", FieldAttributes.Static, fieldType);

// Add both.
var moduleType = module.GetOrCreateModuleType();
Expand Down