Skip to content

Commit

Permalink
Merge pull request #1172 from microsoft/fix1158
Browse files Browse the repository at this point in the history
Drop the implicit `System.Memory` reference from the package
  • Loading branch information
AArnott committed May 4, 2024
2 parents 29bba46 + 6d6fc60 commit 9b7a024
Show file tree
Hide file tree
Showing 10 changed files with 224 additions and 106 deletions.
13 changes: 7 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,16 @@ Install the `Microsoft.Windows.CsWin32` package:
dotnet add package Microsoft.Windows.CsWin32 --prerelease
```

**Tip**: Remove the `IncludeAssets` metadata from the package reference so that you get better code generation by allowing nuget to bring in the `System.Memory` package as a transitive dependency.
You should also install the `System.Memory` package when targeting .NET Framework 4.5+ or .NET Standard 2.0,
as that adds APIs that significantly improve much of the code generated by CsWin32:

```diff
<PackageReference Include="Microsoft.Windows.CsWin32" Version="0.1.647-beta">
<PrivateAssets>all</PrivateAssets>
- <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
```ps1
dotnet add package System.Memory
```

Projects targeting .NET Core 2.1+ or .NET 5+ do *not* need to add the `System.Memory` package reference,
although it is harmless to do so.

Your project must allow unsafe code to support the generated code that will likely use pointers.
This does *not* automatically make all your code *unsafe*.
Use of the `unsafe` keyword is required anywhere you use pointers.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,21 @@
<dependency id="Microsoft.Windows.SDK.Win32Metadata" version="$MetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.WDK.Win32Metadata" version="$WDKMetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.SDK.Win32Docs" version="$ApiDocsVersion$" include="buildTransitive" />
<dependency id="System.Memory" version="4.5.5" include="All" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="5.0.0" include="All" />
</group>
<group targetFramework="net461">
<dependency id="Microsoft.Windows.SDK.Win32Metadata" version="$MetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.WDK.Win32Metadata" version="$WDKMetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.SDK.Win32Docs" version="$ApiDocsVersion$" include="buildTransitive" />
<dependency id="System.Memory" version="4.5.5" include="All" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="6.0.0" include="All" />
</group>
<group targetFramework=".NETStandard1.1">
<dependency id="Microsoft.Windows.SDK.Win32Metadata" version="$MetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.WDK.Win32Metadata" version="$WDKMetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.SDK.Win32Docs" version="$ApiDocsVersion$" include="buildTransitive" />
<dependency id="System.Memory" version="4.5.5" include="All" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="5.0.0" include="All" />
</group>
<group targetFramework=".NETStandard2.0">
<dependency id="Microsoft.Windows.SDK.Win32Metadata" version="$MetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.WDK.Win32Metadata" version="$WDKMetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.SDK.Win32Docs" version="$ApiDocsVersion$" include="buildTransitive" />
<dependency id="System.Memory" version="4.5.5" include="All" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="6.0.0" include="All" />
</group>
</dependencies>
</metadata>
Expand Down
13 changes: 13 additions & 0 deletions src/Microsoft.Windows.CsWin32/SourceGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,14 @@ public class SourceGenerator : ISourceGenerator
"Configuration",
DiagnosticSeverity.Error,
isEnabledByDefault: true);

public static readonly DiagnosticDescriptor MissingRecommendedReference = new(
"PInvoke009",
"Missing package reference",
"Missing reference to recommended package: \"{0}\"",
"Configuration",
DiagnosticSeverity.Warning,
isEnabledByDefault: true);
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

private const string NativeMethodsTxtAdditionalFileName = "NativeMethods.txt";
Expand Down Expand Up @@ -185,6 +193,11 @@ public void Execute(GeneratorExecutionContext context)
context.ReportDiagnostic(Diagnostic.Create(UnsafeCodeRequired, location: null));
}

if (compilation.GetTypeByMetadataName("System.Memory`1") is null)
{
context.ReportDiagnostic(Diagnostic.Create(MissingRecommendedReference, location: null, "System.Memory"));
}

Docs? docs = ParseDocs(context);
SuperGenerator superGenerator = SuperGenerator.Combine(CollectMetadataPaths(context).Select(path => new Generator(path, docs, options, compilation, parseOptions)));
try
Expand Down
17 changes: 7 additions & 10 deletions src/Microsoft.Windows.CsWin32/readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,16 @@ methods and supporting types to a C# project.
To get started, create a "NativeMethods.txt" file in your project directory
that lists the names of Win32 APIs for which you need to have generated, one per line.

Tips
----
You should also install the `System.Memory` package when targeting .NET Framework 4.5+ or .NET Standard 2.0,
as that adds APIs that significantly improve much of the code generated by CsWin32:

Remove the `IncludeAssets` metadata from the package reference so that you get better code generation
by allowing nuget to bring in the `System.Memory` package as a transitive dependency.

```diff
<PackageReference Include="Microsoft.Windows.CsWin32" Version="0.1.647-beta">
<PrivateAssets>all</PrivateAssets>
- <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
```ps1
dotnet add package System.Memory
```

Projects targeting .NET Core 2.1+ or .NET 5+ do *not* need to add the `System.Memory` package reference,
although it is harmless to do so.

Your project must allow unsafe code to support the generated code that will likely use pointers.

Learn more from our README on GitHub: https://github.com/microsoft/CsWin32#readme
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Testing;

internal static class CSharpSourceGeneratorVerifier
{
internal class Test : CSharpSourceGeneratorTest<SourceGenerator, DefaultVerifier>
{
private readonly string testFile;
private readonly string testMethod;

public Test([CallerFilePath] string? testFile = null, [CallerMemberName] string? testMethod = null)
{
this.testFile = testFile ?? throw new ArgumentNullException(nameof(testFile));
this.testMethod = testMethod ?? throw new ArgumentNullException(nameof(testMethod));

// We don't mean to use record/playback verification.
this.TestBehaviors |= TestBehaviors.SkipGeneratedSourcesCheck;

this.ReferenceAssemblies = MyReferenceAssemblies.NetStandard20;
this.TestState.Sources.Add(string.Empty);
this.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", ConstructGlobalConfigString()));
}

public LanguageVersion LanguageVersion { get; set; } = LanguageVersion.CSharp9;

public string? NativeMethodsTxt { get; set; }

[StringSyntax(StringSyntaxAttribute.Json)]
public string? NativeMethodsJson { get; set; }

protected override IEnumerable<Type> GetSourceGenerators()
{
yield return typeof(SourceGenerator);
}

protected override ParseOptions CreateParseOptions()
{
return ((CSharpParseOptions)base.CreateParseOptions()).WithLanguageVersion(this.LanguageVersion);
}

protected override CompilationOptions CreateCompilationOptions()
{
var compilationOptions = (CSharpCompilationOptions)base.CreateCompilationOptions();
return compilationOptions
.WithAllowUnsafe(true)
.WithWarningLevel(99)
.WithSpecificDiagnosticOptions(compilationOptions.SpecificDiagnosticOptions.SetItem("CS1591", ReportDiagnostic.Suppress));
}

protected override Task RunImplAsync(CancellationToken cancellationToken)
{
if (this.NativeMethodsTxt is not null)
{
this.TestState.AdditionalFiles.Add(("NativeMethods.txt", this.NativeMethodsTxt));
}

if (this.NativeMethodsJson is not null)
{
this.TestState.AdditionalFiles.Add(("NativeMethods.json", this.NativeMethodsJson));
}

return base.RunImplAsync(cancellationToken);
}

private static string ConstructGlobalConfigString(bool omitDocs = false)
{
StringBuilder globalConfigBuilder = new();
globalConfigBuilder.AppendLine("is_global = true");
globalConfigBuilder.AppendLine();
globalConfigBuilder.AppendLine($"build_property.CsWin32InputMetadataPaths = {JoinAssemblyMetadata("ProjectionMetadataWinmd")}");
if (!omitDocs)
{
globalConfigBuilder.AppendLine($"build_property.CsWin32InputDocPaths = {JoinAssemblyMetadata("ProjectionDocs")}");
}

return globalConfigBuilder.ToString();

static string JoinAssemblyMetadata(string name)
{
return string.Join(";", typeof(GeneratorTests).Assembly.GetCustomAttributes<AssemblyMetadataAttribute>().Where(metadata => metadata.Key == name).Select(metadata => metadata.Value));
}
}
}
}
38 changes: 8 additions & 30 deletions test/Microsoft.Windows.CsWin32.Tests/GeneratorTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,15 @@ protected async Task<CSharpCompilation> CreateCompilationAsync(ReferenceAssembli

// Workaround for https://github.com/dotnet/roslyn-sdk/issues/699
const string winRTPackageId = "Microsoft.Windows.SDK.Contracts";
metadataReferences = metadataReferences.AddRange(
Directory.GetFiles(Path.Combine(Path.GetTempPath(), "test-packages", $"{winRTPackageId}.{references.Packages.Single(id => string.Equals(id.Id, winRTPackageId, StringComparison.OrdinalIgnoreCase)).Version}", "ref", "netstandard2.0"), "*.winmd").Select(p => MetadataReference.CreateFromFile(p)));
var winRTPackage = references.Packages.SingleOrDefault(id => string.Equals(id.Id, winRTPackageId, StringComparison.OrdinalIgnoreCase));
if (winRTPackage is not null)
{
metadataReferences = metadataReferences.AddRange(
Directory.GetFiles(Path.Combine(Path.GetTempPath(), "test-packages", $"{winRTPackageId}.{winRTPackage.Version}", "ref", "netstandard2.0"), "*.winmd").Select(p => MetadataReference.CreateFromFile(p)));
}

// CONSIDER: How can I pass in the source generator itself, with AdditionalFiles, so I'm exercising that code too?
// QUESTION: How can I pass in the source generator itself, with AdditionalFiles, so I'm exercising that code too?
// ANSWER: Follow the pattern now used in SourceGeneratorTests.cs
var compilation = CSharpCompilation.Create(
assemblyName: "test",
references: metadataReferences,
Expand Down Expand Up @@ -365,31 +370,4 @@ private static void AssertConsistentLineEndings(SyntaxTree syntaxTree)
lineCount++;
}
}

protected static class MyReferenceAssemblies
{
#pragma warning disable SA1202 // Elements should be ordered by access - because field initializer depend on each other
private static readonly ImmutableArray<PackageIdentity> AdditionalLegacyPackages = ImmutableArray.Create(
new PackageIdentity("Microsoft.Windows.SDK.Contracts", "10.0.22621.2"));

private static readonly ImmutableArray<PackageIdentity> AdditionalModernPackages = AdditionalLegacyPackages.AddRange(ImmutableArray.Create(
new PackageIdentity("System.Runtime.CompilerServices.Unsafe", "6.0.0"),
new PackageIdentity("System.Memory", "4.5.5"),
new PackageIdentity("Microsoft.Win32.Registry", "5.0.0")));

internal static readonly ReferenceAssemblies NetStandard20 = ReferenceAssemblies.NetStandard.NetStandard20.AddPackages(AdditionalModernPackages);
#pragma warning restore SA1202 // Elements should be ordered by access

internal static class NetFramework
{
internal static readonly ReferenceAssemblies Net35 = ReferenceAssemblies.NetFramework.Net35.WindowsForms.AddPackages(AdditionalLegacyPackages);
internal static readonly ReferenceAssemblies Net472 = ReferenceAssemblies.NetFramework.Net472.WindowsForms.AddPackages(AdditionalModernPackages);
}

internal static class Net
{
internal static readonly ReferenceAssemblies Net60 = ReferenceAssemblies.Net.Net60.AddPackages(AdditionalModernPackages);
internal static readonly ReferenceAssemblies Net70 = ReferenceAssemblies.Net.Net70.AddPackages(AdditionalModernPackages);
}
}
}
51 changes: 0 additions & 51 deletions test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using VerifyTest = Microsoft.CodeAnalysis.CSharp.Testing.CSharpSourceGeneratorTest<Microsoft.Windows.CsWin32.SourceGenerator, Microsoft.CodeAnalysis.Testing.Verifiers.XUnitVerifier>;

public class GeneratorTests : GeneratorTestBase
{
public GeneratorTests(ITestOutputHelper logger)
Expand Down Expand Up @@ -928,36 +926,6 @@ void LogProject(string name)
}
}

[Fact]
public async Task UnparseableNativeMethodsJson()
{
await new VerifyTest
{
TestState =
{
ReferenceAssemblies = MyReferenceAssemblies.NetStandard20,
Sources = { string.Empty },
AdditionalFiles =
{
("NativeMethods.txt", "CreateFile"),
("NativeMethods.json", @"{ ""allowMarshaling"": f }"), // the point where the user is typing "false"
},
AnalyzerConfigFiles =
{
("/.globalconfig", ConstructGlobalConfigString()),
},
GeneratedSources =
{
// Nothing generated, but no exceptions thrown that would lead Roslyn to disable the source generator in the IDE either.
},
ExpectedDiagnostics =
{
new DiagnosticResult(SourceGenerator.OptionsParsingError.Id, DiagnosticSeverity.Error),
},
},
}.RunAsync();
}

[Fact]
public void OpensMetadataForSharedReading()
{
Expand Down Expand Up @@ -1059,23 +1027,4 @@ public void SeekOriginEnumPreferred()
QualifiedNameSyntax seekParamType = Assert.IsType<QualifiedNameSyntax>(seekMethod.ParameterList.Parameters[1].Type);
Assert.Equal(nameof(SeekOrigin), seekParamType.Right.Identifier.ValueText);
}

private static string ConstructGlobalConfigString(bool omitDocs = false)
{
StringBuilder globalConfigBuilder = new();
globalConfigBuilder.AppendLine("is_global = true");
globalConfigBuilder.AppendLine();
globalConfigBuilder.AppendLine($"build_property.CsWin32InputMetadataPaths = {JoinAssemblyMetadata("ProjectionMetadataWinmd")}");
if (!omitDocs)
{
globalConfigBuilder.AppendLine($"build_property.CsWin32InputDocPaths = {JoinAssemblyMetadata("ProjectionDocs")}");
}

return globalConfigBuilder.ToString();

static string JoinAssemblyMetadata(string name)
{
return string.Join(";", typeof(GeneratorTests).Assembly.GetCustomAttributes<AssemblyMetadataAttribute>().Where(metadata => metadata.Key == name).Select(metadata => metadata.Value));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>net6.0</TargetFrameworks>
<TargetFrameworks>net8.0</TargetFrameworks>
<RootNamespace />
</PropertyGroup>

Expand Down
29 changes: 29 additions & 0 deletions test/Microsoft.Windows.CsWin32.Tests/MyReferenceAssemblies.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

internal static class MyReferenceAssemblies
{
#pragma warning disable SA1202 // Elements should be ordered by access - because field initializer depend on each other
private static readonly ImmutableArray<PackageIdentity> AdditionalLegacyPackages = ImmutableArray.Create(
new PackageIdentity("Microsoft.Windows.SDK.Contracts", "10.0.22621.2"));

private static readonly ImmutableArray<PackageIdentity> AdditionalModernPackages = AdditionalLegacyPackages.AddRange(ImmutableArray.Create(
new PackageIdentity("System.Runtime.CompilerServices.Unsafe", "6.0.0"),
new PackageIdentity("System.Memory", "4.5.5"),
new PackageIdentity("Microsoft.Win32.Registry", "5.0.0")));

internal static readonly ReferenceAssemblies NetStandard20 = ReferenceAssemblies.NetStandard.NetStandard20.AddPackages(AdditionalModernPackages);
#pragma warning restore SA1202 // Elements should be ordered by access

internal static class NetFramework
{
internal static readonly ReferenceAssemblies Net35 = ReferenceAssemblies.NetFramework.Net35.WindowsForms.AddPackages(AdditionalLegacyPackages);
internal static readonly ReferenceAssemblies Net472 = ReferenceAssemblies.NetFramework.Net472.WindowsForms.AddPackages(AdditionalModernPackages);
}

internal static class Net
{
internal static readonly ReferenceAssemblies Net60 = ReferenceAssemblies.Net.Net60.AddPackages(AdditionalModernPackages);
internal static readonly ReferenceAssemblies Net70 = ReferenceAssemblies.Net.Net70.AddPackages(AdditionalModernPackages);
}
}
Loading

0 comments on commit 9b7a024

Please sign in to comment.