Skip to content

Commit

Permalink
[wasm] JSImport generator AllowUnsafeBlocks diagnostic (#72996)
Browse files Browse the repository at this point in the history
- added `JSImportAttribute requires unsafe code` + unit test
- fixed Diagnostic IDs + reservation
- moved System.Runtime.InteropServices.JavaScript.UnitTests to subfolder
  • Loading branch information
pavelsavara authored Jul 29, 2022
1 parent 992cf8c commit 222da17
Show file tree
Hide file tree
Showing 14 changed files with 119 additions and 15 deletions.
12 changes: 12 additions & 0 deletions docs/project/list-of-diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,18 @@ The diagnostic id values reserved for .NET Libraries analyzer warnings are `SYSL
| __`SYSLIB1067`__ | *_`SYSLIB1063`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
| __`SYSLIB1068`__ | *_`SYSLIB1063`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
| __`SYSLIB1069`__ | *_`SYSLIB1063`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
| __`SYSLIB1070`__ | Invalid 'JSImportAttribute' usage |
| __`SYSLIB1071`__ | Invalid 'JSExportAttribute' usage |
| __`SYSLIB1072`__ | Specified type is not supported by source-generated JavaScript interop |
| __`SYSLIB1073`__ | Specified configuration is not supported by source-generated JavaScript interop |
| __`SYSLIB1074`__ | JSImportAttribute requires unsafe code |
| __`SYSLIB1075`__ | JSExportAttribute requires unsafe code |
| __`SYSLIB1076`__ | *_`SYSLIB1070`-`SYSLIB1089` reserved for System.Runtime.InteropServices.JavaScript.JSImportGenerator.* |
| __`SYSLIB1077`__ | *_`SYSLIB1070`-`SYSLIB1089` reserved for System.Runtime.InteropServices.JavaScript.JSImportGenerator.* |
| __`SYSLIB1078`__ | *_`SYSLIB1070`-`SYSLIB1089` reserved for System.Runtime.InteropServices.JavaScript.JSImportGenerator.* |
| __`SYSLIB1079`__ | *_`SYSLIB1070`-`SYSLIB1089` reserved for System.Runtime.InteropServices.JavaScript.JSImportGenerator.* |
| __`SYSLIB1080`__ | *_`SYSLIB1070`-`SYSLIB1089` reserved for System.Runtime.InteropServices.JavaScript.JSImportGenerator.* |


### Diagnostic Suppressions (`SYSLIBSUPPRESS****`)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<Compile Include="System\Runtime\InteropServices\JavaScript\Http\HttpRequestMessageTest.cs" />
<Compile Include="System\Runtime\InteropServices\JavaScript\ParallelTests.cs" />
<Compile Include="System\Runtime\InteropServices\JavaScript\TimerTests.cs" />
<Compile Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\tests\System\Runtime\InteropServices\JavaScript\Utils.cs" Link="System\Runtime\InteropServices\JavaScript\Utils.cs" />
<Compile Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\tests\System.Runtime.InteropServices.JavaScript.UnitTests\System\Runtime\InteropServices\JavaScript\Utils.cs" Link="System\Runtime\InteropServices\JavaScript\Utils.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Runtime.InteropServi
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "JSImportGenerator.Unit.Tests", "tests\JSImportGenerator.UnitTest\JSImportGenerator.Unit.Tests.csproj", "{BFED925C-18F2-4C98-833E-66F205234598}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Runtime.InteropServices.JavaScript.Tests", "tests\System.Runtime.InteropServices.JavaScript.Tests.csproj", "{765B4AA5-723A-44FF-BC4E-EB0F03103F6D}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Runtime.InteropServices.JavaScript.Tests", "tests\System.Runtime.InteropServices.JavaScript.UnitTests\System.Runtime.InteropServices.JavaScript.Tests.csproj", "{765B4AA5-723A-44FF-BC4E-EB0F03103F6D}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "LibraryImportGenerator", "..\System.Runtime.InteropServices\gen\LibraryImportGenerator\LibraryImportGenerator.csproj", "{09AA6758-0BD3-4312-9C07-AE9F1D50A3AD}"
EndProject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ public class GeneratorDiagnostics : IGeneratorDiagnostics
{
public class Ids
{
// SYSLIB1050-SYSLIB1059 are reserved for JSImportGenerator
public const string Prefix = "JS";
public const string InvalidJSImportAttributeUsage = Prefix + "1050";
public const string TypeNotSupported = Prefix + "1051";
public const string ConfigurationNotSupported = Prefix + "1052";
public const string CannotForwardToDllImport = Prefix + "1053";
// SYSLIB1070-SYSLIB1089 are reserved for JSImportGenerator
public const string Prefix = "SYSLIB";
public const string InvalidJSImportAttributeUsage = Prefix + "1070";
public const string InvalidJSExportAttributeUsage = Prefix + "1071";
public const string TypeNotSupported = Prefix + "1072";
public const string ConfigurationNotSupported = Prefix + "1073";
public const string JSImportRequiresAllowUnsafeBlocks = Prefix + "1074";
public const string JSExportRequiresAllowUnsafeBlocks = Prefix + "1075";
}

private const string Category = "JSImportGenerator";
Expand Down Expand Up @@ -268,7 +270,7 @@ public void ReportMarshallingNotSupported(

public static readonly DiagnosticDescriptor InvalidExportAttributedMethodSignature =
new DiagnosticDescriptor(
Ids.InvalidJSImportAttributeUsage,
Ids.InvalidJSExportAttributeUsage,
GetResourceString(nameof(SR.InvalidJSExportAttributeUsageTitle)),
GetResourceString(nameof(SR.InvalidJSExportAttributedMethodSignatureMessage)),
Category,
Expand All @@ -288,14 +290,34 @@ public void ReportMarshallingNotSupported(

public static readonly DiagnosticDescriptor InvalidExportAttributedMethodContainingTypeMissingModifiers =
new DiagnosticDescriptor(
Ids.InvalidJSImportAttributeUsage,
GetResourceString(nameof(SR.InvalidJSImportAttributeUsageTitle)),
Ids.InvalidJSExportAttributeUsage,
GetResourceString(nameof(SR.InvalidJSExportAttributeUsageTitle)),
GetResourceString(nameof(SR.InvalidAttributedMethodContainingTypeMissingModifiersMessage)),
Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.InvalidJSExportAttributedMethodDescription)));

public static readonly DiagnosticDescriptor JSImportRequiresAllowUnsafeBlocks =
new DiagnosticDescriptor(
Ids.JSImportRequiresAllowUnsafeBlocks,
GetResourceString(nameof(SR.JSImportRequiresAllowUnsafeBlocksTitle)),
GetResourceString(nameof(SR.JSImportRequiresAllowUnsafeBlocksMessage)),
Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.JSImportRequiresAllowUnsafeBlocksDescription)));

public static readonly DiagnosticDescriptor JSExportRequiresAllowUnsafeBlocks =
new DiagnosticDescriptor(
Ids.JSExportRequiresAllowUnsafeBlocks,
GetResourceString(nameof(SR.JSExportRequiresAllowUnsafeBlocksTitle)),
GetResourceString(nameof(SR.JSExportRequiresAllowUnsafeBlocksMessage)),
Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.JSExportRequiresAllowUnsafeBlocksDescription)));

private static LocalizableResourceString GetResourceString(string resourceName)
{
return new LocalizableResourceString(resourceName, SR.ResourceManager, typeof(FxResources.Microsoft.Interop.JavaScript.JSImportGenerator.SR));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public static class StepNames

public void Initialize(IncrementalGeneratorInitializationContext context)
{
// Collect all methods adorned with JSExportAttribute
var attributedMethods = context.SyntaxProvider
.CreateSyntaxProvider(
static (node, ct) => ShouldVisitNode(node),
Expand All @@ -70,6 +71,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
.Where(
static modelData => modelData is not null);

// Validate if attributed methods can have source generated
var methodsWithDiagnostics = attributedMethods.Select(static (data, ct) =>
{
Diagnostic? diagnostic = GetDiagnosticIfInvalidMethodForGeneration(data.Syntax, data.Symbol);
Expand All @@ -79,16 +81,32 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
var methodsToGenerate = methodsWithDiagnostics.Where(static data => data.Diagnostic is null);
var invalidMethodDiagnostics = methodsWithDiagnostics.Where(static data => data.Diagnostic is not null);

// Report diagnostics for invalid methods
context.RegisterSourceOutput(invalidMethodDiagnostics, static (context, invalidMethod) =>
{
context.ReportDiagnostic(invalidMethod.Diagnostic);
});

// Compute generator options
IncrementalValueProvider<JSGeneratorOptions> stubOptions = context.AnalyzerConfigOptionsProvider
.Select(static (options, ct) => new JSGeneratorOptions(options.GlobalOptions));

IncrementalValueProvider<StubEnvironment> stubEnvironment = context.CreateStubEnvironmentProvider();

// Validate environment that is being used to generate stubs.
context.RegisterDiagnostics(stubEnvironment.Combine(attributedMethods.Collect()).SelectMany((data, ct) =>
{
if (data.Right.IsEmpty // no attributed methods
|| data.Left.Compilation.Options is CSharpCompilationOptions { AllowUnsafe: true }) // Unsafe code enabled
{
return ImmutableArray<Diagnostic>.Empty;
}

return ImmutableArray.Create(Diagnostic.Create(GeneratorDiagnostics.JSExportRequiresAllowUnsafeBlocks, null));
}));

IncrementalValuesProvider<(MemberDeclarationSyntax, ImmutableArray<Diagnostic>)> generateSingleStub = methodsToGenerate
.Combine(context.CreateStubEnvironmentProvider())
.Combine(stubEnvironment)
.Combine(stubOptions)
.Select(static (data, ct) => new
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public static class StepNames

public void Initialize(IncrementalGeneratorInitializationContext context)
{
// Collect all methods adorned with JSImportAttribute
var attributedMethods = context.SyntaxProvider
.CreateSyntaxProvider(
static (node, ct) => ShouldVisitNode(node),
Expand All @@ -75,6 +76,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
.Where(
static modelData => modelData is not null);

// Validate if attributed methods can have source generated
var methodsWithDiagnostics = attributedMethods.Select(static (data, ct) =>
{
Diagnostic? diagnostic = GetDiagnosticIfInvalidMethodForGeneration(data.Syntax, data.Symbol);
Expand All @@ -84,16 +86,32 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
var methodsToGenerate = methodsWithDiagnostics.Where(static data => data.Diagnostic is null);
var invalidMethodDiagnostics = methodsWithDiagnostics.Where(static data => data.Diagnostic is not null);

// Report diagnostics for invalid methods
context.RegisterSourceOutput(invalidMethodDiagnostics, static (context, invalidMethod) =>
{
context.ReportDiagnostic(invalidMethod.Diagnostic);
});

// Compute generator options
IncrementalValueProvider<JSGeneratorOptions> stubOptions = context.AnalyzerConfigOptionsProvider
.Select(static (options, ct) => new JSGeneratorOptions(options.GlobalOptions));

IncrementalValueProvider<StubEnvironment> stubEnvironment = context.CreateStubEnvironmentProvider();

// Validate environment that is being used to generate stubs.
context.RegisterDiagnostics(stubEnvironment.Combine(attributedMethods.Collect()).SelectMany((data, ct) =>
{
if (data.Right.IsEmpty // no attributed methods
|| data.Left.Compilation.Options is CSharpCompilationOptions { AllowUnsafe: true }) // Unsafe code enabled
{
return ImmutableArray<Diagnostic>.Empty;
}

return ImmutableArray.Create(Diagnostic.Create(GeneratorDiagnostics.JSImportRequiresAllowUnsafeBlocks, null));
}));

IncrementalValuesProvider<(MemberDeclarationSyntax, ImmutableArray<Diagnostic>)> generateSingleStub = methodsToGenerate
.Combine(context.CreateStubEnvironmentProvider())
.Combine(stubEnvironment)
.Combine(stubOptions)
.Select(static (data, ct) => new
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,4 +208,22 @@
<value>Please annotate the argument with 'JSMarshalAsAttribute' to specify marshaling of {0}.</value>
<comment>{0} is a type of the argument</comment>
</data>
<data name="JSImportRequiresAllowUnsafeBlocksDescription" xml:space="preserve">
<value>JSImportAttribute requires unsafe code. Project must be updated with '&lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt;'.</value>
</data>
<data name="JSImportRequiresAllowUnsafeBlocksMessage" xml:space="preserve">
<value>JSImportAttribute requires unsafe code. Project must be updated with '&lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt;'.</value>
</data>
<data name="JSImportRequiresAllowUnsafeBlocksTitle" xml:space="preserve">
<value>JSImportAttribute requires unsafe code.</value>
</data>
<data name="JSExportRequiresAllowUnsafeBlocksDescription" xml:space="preserve">
<value>JSExportAttribute requires unsafe code. Project must be updated with '&lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt;'.</value>
</data>
<data name="JSExportRequiresAllowUnsafeBlocksMessage" xml:space="preserve">
<value>JSExportAttribute requires unsafe code. Project must be updated with '&lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt;'.</value>
</data>
<data name="JSExportRequiresAllowUnsafeBlocksTitle" xml:space="preserve">
<value>JSExportAttribute requires unsafe code.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static IEnumerable<object[]> CodeSnippetsToCompile()
[MemberData(nameof(CodeSnippetsToCompile))]
public async Task ValidateSnippets(string source)
{
Compilation comp = await TestUtils.CreateCompilation(source);
Compilation comp = await TestUtils.CreateCompilation(source, allowUnsafe: true);
TestUtils.AssertPreSourceGeneratorCompilation(comp);

var newComp = TestUtils.RunGenerators(comp, out var generatorDiags,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public class Fails
[MemberData(nameof(CodeSnippetsToFail))]
public async Task ValidateFailSnippets(string source, string[]? generatorMessages, string[]? compilerMessages)
{
Compilation comp = await TestUtils.CreateCompilation(source);
Compilation comp = await TestUtils.CreateCompilation(source, allowUnsafe: true);
TestUtils.AssertPreSourceGeneratorCompilation(comp);

var newComp = TestUtils.RunGenerators(comp, out var generatorDiags,
Expand All @@ -73,5 +73,21 @@ public async Task ValidateFailSnippets(string source, string[]? generatorMessage
JSTestUtils.AssertMessages(compilationDiags, compilerMessages);
}
}

[Fact]
public async Task ValidateRequireAllowUnsafeBlocksDiagnostic()
{
string source = CodeSnippets.TrivialClassDeclarations;
Compilation comp = await TestUtils.CreateCompilation(new[] { source }, allowUnsafe: false);
TestUtils.AssertPreSourceGeneratorCompilation(comp);

var newComp = TestUtils.RunGenerators(comp, out var generatorDiags,
new Microsoft.Interop.JavaScript.JSImportGenerator(),
new Microsoft.Interop.JavaScript.JSExportGenerator());

// The errors should indicate the AllowUnsafeBlocks is required.
Assert.True(generatorDiags.Single(d => d.Id == "SYSLIB1074") != null);
Assert.True(generatorDiags.Single(d => d.Id == "SYSLIB1075") != null);
}
}
}

0 comments on commit 222da17

Please sign in to comment.