Skip to content

Commit

Permalink
(RSRP-499137) CLI: require --omit-non-api-members when appropriate
Browse files Browse the repository at this point in the history
  • Loading branch information
ForNeVeR committed Nov 2, 2024
1 parent bf1a19d commit b1e507e
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 108 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ Mock assembly throws `System.NotImplementedException` in each imported method.

Reference assembly contains only type definition and method signatures with no method bodies.

By default, if you don't specify any of `--public`, `--internals`, or `--all`, Refasmer will try to detect the refasming mode from the input assembly. If the assembly has an `InternalsVisibleTo` attribute applied to it, then `--internals` will be implicitly applied; otherwise, `--public` will.

> [!IMPORTANT]
> Note that `--omit-non-api-members` performs a nontrivial transformation on the resulting assembly. Normally, a reference assembly should include any types, including private and internal ones, because this is up to the spec. However, in some cases, it is possible to omit private and internal types from the reference assembly, because they are not part of the public API, while preserving some of the value type semantics. In these cases, Refasmer is able to remove these types from the assembly, sometimes emitting synthetic fields in the output type. This will preserve the difference of empty and non-empty struct types, but will not preserve the type blittability (i.e. some types after refasming might obtain the ability to follow the `unmanaged` constraint, even if this wasn't possible before refasming).
If you didn't specify the `--all` option, you must pass either `--omit-non-api-members true` or `--omit-non-api-members false`, to exactly identify the required behavior of refasming.

## Examples:

```refasmer -v -O ref -c a.dll b.dll c.dll```
Expand Down
15 changes: 9 additions & 6 deletions src/Refasmer/Importer/MetadataImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@ public MetadataImporter( MetadataReader reader, MetadataBuilder builder, BlobBui
_ilStream = ilStream;
}


public static byte[] MakeRefasm(
MetadataReader metaReader,
PEReader peReader,
LoggerBase logger,
IImportFilter filter,
bool omitNonApiTypes,
bool? omitNonApiMembers,
bool makeMock = false,
bool omitReferenceAssemblyAttr = false)
{
Expand All @@ -54,12 +53,16 @@ public static byte[] MakeRefasm(
}
else if (importer.IsInternalsVisible())
{
importer.Filter = new AllowPublicAndInternals(omitNonApiTypes);
importer.Filter = new AllowPublicAndInternals(
omitNonApiMembers ?? throw new Exception(
$"{nameof(omitNonApiMembers)} should be specified for the current filter mode."));
logger.Info?.Invoke("InternalsVisibleTo attributes found, using AllowPublicAndInternals entity filter");
}
else
{
importer.Filter = new AllowPublic(omitNonApiTypes);
importer.Filter = new AllowPublic(
omitNonApiMembers ?? throw new Exception(
$"{nameof(omitNonApiMembers)} should be specified for the current filter mode."));
logger.Info?.Invoke("Using AllowPublic entity filter");
}

Expand Down Expand Up @@ -120,7 +123,7 @@ public static void MakeRefasm( string inputPath, string outputPath, LoggerBase l
if (!metaReader.IsAssembly)
throw new Exception("File format is not supported");

var result = MakeRefasm(metaReader, peReader, logger, filter, omitNonApiTypes: false, makeMock);
var result = MakeRefasm(metaReader, peReader, logger, filter, omitNonApiMembers: false, makeMock);

logger.Debug?.Invoke($"Writing result to {outputPath}");
if (File.Exists(outputPath))
Expand All @@ -129,4 +132,4 @@ public static void MakeRefasm( string inputPath, string outputPath, LoggerBase l
File.WriteAllBytes(outputPath, result);
}
}
}
}
16 changes: 11 additions & 5 deletions src/RefasmerExe/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ enum Operation
private static bool _public;
private static bool _internals;
private static bool _all;
private static bool _omitNonApiMembers;
private static bool? _omitNonApiMembers;

private static bool _makeMock;
private static bool _omitReferenceAssemblyAttr;
Expand Down Expand Up @@ -87,7 +87,7 @@ public static int Main(string[] args)
{ "p|public", "drop non-public types even with InternalsVisibleTo", v => _public = v != null },
{ "i|internals", "import public and internal types", v => _internals = v != null },
{ "all", "ignore visibility and import all", v => _all = v != null },
{ "omit-non-api-members", "omit private members and types not participating in the public API (will preserve the empty vs non-empty struct semantics, but might affect unmanaged struct constraint)", x => _omitNonApiMembers = x != null },
{ "omit-non-api-members=", "omit private members and types not participating in the public API (will preserve the empty vs non-empty struct semantics, but might affect unmanaged struct constraint)", x => _omitNonApiMembers = string.Equals(x, "true", StringComparison.OrdinalIgnoreCase) },

{ "m|mock", "make mock assembly instead of reference assembly", p => _makeMock = p != null },
{ "n|noattr", "omit reference assembly attribute", p => _omitReferenceAssemblyAttr = p != null },
Expand Down Expand Up @@ -134,6 +134,12 @@ public static int Main(string[] args)
return 2;
}

if (!_all && _omitNonApiMembers == null)
{
Console.Error.WriteLine("Either specify --all to emit all private types, or specify --omit-non-api-members to either true or false.");
return 2;
}

_logger = new LoggerBase(new VerySimpleLogger(Console.Error, verbosity));

try
Expand Down Expand Up @@ -255,9 +261,9 @@ private static void MakeRefasm((string Path, string RelativeForOutput) input)
IImportFilter filter = null;

if (_public)
filter = new AllowPublic(_omitNonApiMembers);
filter = new AllowPublic(_omitNonApiMembers ?? throw new Exception("--omit-non-api-members should be specified for the passed filter type."));
else if (_internals)
filter = new AllowPublicAndInternals(_omitNonApiMembers);
filter = new AllowPublicAndInternals(_omitNonApiMembers ?? throw new Exception("--omit-non-api-members should be specified for the passed filter type."));
else if (_all)
filter = new AllowAll();

Expand Down Expand Up @@ -357,4 +363,4 @@ private static PEReader ReadAssembly(string input, out MetadataReader metaReader
return expanded;
}
}
}
}
29 changes: 29 additions & 0 deletions tests/Refasmer.Tests/ExitCodeTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
namespace JetBrains.Refasmer.Tests;

public class ExitCodeTests : IntegrationTestBase
{
[TestCase]
public Task OmitNonApiTypesTrue() =>
DoTest(0, "--omit-non-api-members", "true");

[TestCase]
public Task OmitNonApiTypesFalse() =>
DoTest(0, "--omit-non-api-members", "false");

[TestCase]
public Task OmitNonApiTypesMissing() =>
DoTest(2);

private async Task DoTest(int expectedCode, params string[] additionalArgs)
{
var assemblyPath = await BuildTestAssembly();
var outputPath = Path.GetTempFileName();
using var collector = CollectConsoleOutput();
var exitCode = ExecuteRefasmAndGetExitCode(assemblyPath, outputPath, additionalArgs);
Assert.That(
exitCode,
Is.EqualTo(expectedCode),
$"Refasmer returned exit code {exitCode}, while {expectedCode} was expected." +
$" StdOut:\n{collector.StdOut}\nStdErr: {collector.StdErr}");
}
}
106 changes: 106 additions & 0 deletions tests/Refasmer.Tests/IntegrationTestBase.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
using System.Globalization;
using System.Text;
using Medallion.Shell;
using Mono.Cecil;

namespace JetBrains.Refasmer.Tests;

public abstract class IntegrationTestBase
{
protected static async Task<string> BuildTestAssembly()
{
var root = FindSourceRoot();
var testProject = Path.Combine(root, "tests/RefasmerTestAssembly/RefasmerTestAssembly.csproj");
Console.WriteLine($"Building project {testProject}…");
var result = await Command.Run("dotnet", "build", testProject, "--configuration", "Release").Task;

Assert.That(
result.ExitCode,
Is.EqualTo(0),
$"Failed to build test assembly, exit code {result.ExitCode}. StdOut:\n{result.StandardOutput}\nStdErr: {result.StandardError}");

return Path.Combine(root, "tests/RefasmerTestAssembly/bin/Release/net6.0/RefasmerTestAssembly.dll");
}

private static string FindSourceRoot()
{
var current = Directory.GetCurrentDirectory();
while (current != null)
{
if (File.Exists(Path.Combine(current, "README.md")))
return current;
current = Path.GetDirectoryName(current);
}
throw new Exception("Cannot find source root.");
}

protected static int ExecuteRefasmAndGetExitCode(
string assemblyPath,
string outputPath,
params string[] additionalOptions)
{
var args = new List<string>
{
"-v",
$"--output={outputPath}"
};
args.AddRange(additionalOptions);
args.Add(assemblyPath);
return Program.Main(args.ToArray());
}

protected static string RefasmTestAssembly(string assemblyPath, bool omitNonApiMembers = false)
{
var outputPath = Path.GetTempFileName();
var options = new[]{ "--omit-non-api-members", omitNonApiMembers.ToString(CultureInfo.InvariantCulture) };
using var collector = CollectConsoleOutput();
var exitCode = ExecuteRefasmAndGetExitCode(assemblyPath, outputPath, options);
Assert.That(
exitCode,
Is.EqualTo(0),
$"Refasmer returned exit code {exitCode}. StdOut:\n{collector.StdOut}\nStdErr: {collector.StdErr}");

return outputPath;
}

protected static Task VerifyTypeContent(string assemblyPath, string typeName)
{
var assembly = AssemblyDefinition.ReadAssembly(assemblyPath);
var type = assembly.MainModule.GetType(typeName);
Assert.That(assembly.MainModule.GetType(typeName), Is.Not.Null);

var printout = new StringBuilder();
Printer.PrintType(type, printout);

var verifySettings = new VerifySettings();
verifySettings.DisableDiff();
verifySettings.UseDirectory("data");
verifySettings.UseParameters(typeName);
return Verify(printout, verifySettings);
}

protected class Outputs : IDisposable
{
public StringBuilder StdOut { get; } = new();
public StringBuilder StdErr { get; } = new();

private readonly TextWriter _oldStdOut;
private readonly TextWriter _oldStdErr;

public Outputs()
{
_oldStdOut = Console.Out;
_oldStdErr = Console.Error;
Console.SetOut(new StringWriter(StdOut));
Console.SetError(new StringWriter(StdErr));
}

public void Dispose()
{
Console.SetError(_oldStdErr);
Console.SetOut(_oldStdOut);
}
}

protected static Outputs CollectConsoleOutput() => new();
}
99 changes: 2 additions & 97 deletions tests/Refasmer.Tests/IntegrationTests.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
using System.Text;
using Medallion.Shell;
using Mono.Cecil;

namespace JetBrains.Refasmer.Tests;

public class IntegrationTests
public class IntegrationTests : IntegrationTestBase
{
[TestCase("RefasmerTestAssembly.PublicClassWithPrivateFields")]
[TestCase("RefasmerTestAssembly.PublicStructWithPrivateFields")]
Expand Down Expand Up @@ -37,95 +33,4 @@ public async Task CheckRefasmedTypeOmitNonApi(string typeName)
var resultAssembly = RefasmTestAssembly(assemblyPath, omitNonApiMembers: true);
await VerifyTypeContent(resultAssembly, typeName);
}

private static async Task<string> BuildTestAssembly()
{
var root = FindSourceRoot();
var testProject = Path.Combine(root, "tests/RefasmerTestAssembly/RefasmerTestAssembly.csproj");
Console.WriteLine($"Building project {testProject}…");
var result = await Command.Run("dotnet", "build", testProject, "--configuration", "Release").Task;

Assert.That(
result.ExitCode,
Is.EqualTo(0),
$"Failed to build test assembly, exit code {result.ExitCode}. StdOut:\n{result.StandardOutput}\nStdErr: {result.StandardError}");

return Path.Combine(root, "tests/RefasmerTestAssembly/bin/Release/net6.0/RefasmerTestAssembly.dll");
}

private static string FindSourceRoot()
{
var current = Directory.GetCurrentDirectory();
while (current != null)
{
if (File.Exists(Path.Combine(current, "README.md")))
return current;
current = Path.GetDirectoryName(current);
}
throw new Exception("Cannot find source root.");
}

private static string RefasmTestAssembly(string assemblyPath, bool omitNonApiMembers = false)
{
var tempLocation = Path.GetTempFileName();
var args = new List<string>
{
"-v",
$"--output={tempLocation}"
};
if (omitNonApiMembers)
{
args.Add("--omit-non-api-members");
}
args.Add(assemblyPath);
using var collector = CollectConsoleOutput();
var exitCode = Program.Main(args.ToArray());
Assert.That(
exitCode,
Is.EqualTo(0),
$"Refasmer returned exit code {exitCode}. StdOut:\n{collector.StdOut}\nStdErr: {collector.StdErr}");

return tempLocation;
}

private static Task VerifyTypeContent(string assemblyPath, string typeName)
{
var assembly = AssemblyDefinition.ReadAssembly(assemblyPath);
var type = assembly.MainModule.GetType(typeName);
Assert.That(assembly.MainModule.GetType(typeName), Is.Not.Null);

var printout = new StringBuilder();
Printer.PrintType(type, printout);

var verifySettings = new VerifySettings();
verifySettings.DisableDiff();
verifySettings.UseDirectory("data");
verifySettings.UseParameters(typeName);
return Verify(printout, verifySettings);
}

private class Outputs : IDisposable
{
public StringBuilder StdOut { get; } = new();
public StringBuilder StdErr { get; } = new();

private readonly TextWriter _oldStdOut;
private readonly TextWriter _oldStdErr;

public Outputs()
{
_oldStdOut = Console.Out;
_oldStdErr = Console.Error;
Console.SetOut(new StringWriter(StdOut));
Console.SetError(new StringWriter(StdErr));
}

public void Dispose()
{
Console.SetError(_oldStdErr);
Console.SetOut(_oldStdOut);
}
}

private static Outputs CollectConsoleOutput() => new();
}
}

0 comments on commit b1e507e

Please sign in to comment.