Skip to content

Commit

Permalink
Allow unnecessary [In, Out] attributes on blittable arrays and report…
Browse files Browse the repository at this point in the history
… them as unnecessary (#87774)

* Change our '[In, Out]' attribute handling to have a concept of 'unnecessary but not invalid' to enable us in the future to report a non-fatal diagnostic.

* WIP enable unnecessary diagnostic in LibraryImportGenerator'

* Hook up diagnostics and add tests for LibraryImportGenerator

* Hook up diagnostics for ComInterfaceGenerator as well

* Update XLF

* Don't reference the live LibraryImportGenerator when building .NET 7 assets. Use the inbox instead.

* PR feedback

* Fix merge
  • Loading branch information
jkoritzinsky authored Jun 27, 2023
1 parent bc70fec commit 6d6fbe0
Show file tree
Hide file tree
Showing 70 changed files with 823 additions and 69 deletions.
8 changes: 2 additions & 6 deletions eng/generators.targets
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<EnabledGenerators Include="LibraryImportGenerator" Condition="'$(EnableLibraryImportGenerator)' == 'true'" />
<!-- If the current project is not System.Private.CoreLib, we enable the LibraryImportGenerator source generator
when the project is a C# source project that:
- doesn't target the latest TFM or
- doesn't target the a TFM that includes LibraryImportGenerator or
- doesn't reference the live targeting pack (i.e. when inbox) and
- references System.Private.CoreLib, or
- references System.Runtime.InteropServices -->
Expand All @@ -22,7 +22,7 @@
'$(IsSourceProject)' == 'true' and
'$(MSBuildProjectExtension)' == '.csproj' and
(
'$(TargetFrameworkMoniker)' != '$(NetCoreAppCurrentTargetFrameworkMoniker)' or
!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0')) or
(
'$(DisableImplicitFrameworkReferences)' == 'true' and
(
Expand All @@ -33,17 +33,13 @@
)" />
<!-- We enable the ComInterfaceGenerator source generator
when the project is a C# source project that:
- doesn't target the latest TFM or
- references System.Runtime.InteropServices directly and not through the live targeting pack (i.e. when inbox) -->
<EnabledGenerators Include="ComInterfaceGenerator"
Condition="'$(IsSourceProject)' == 'true' and
'$(MSBuildProjectExtension)' == '.csproj' and
(
'$(TargetFrameworkMoniker)' != '$(NetCoreAppCurrentTargetFrameworkMoniker)' or
(
'$(DisableImplicitFrameworkReferences)' == 'true' and
'@(Reference->AnyHaveMetadataValue('Identity', 'System.Runtime.InteropServices'))' == 'true'
)
)" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protected BaseJSGenerator(MarshalerType marshalerType, IMarshallingGenerator inn
public virtual bool UsesNativeIdentifier(TypePositionInfo info, StubCodeContext context) => _inner.UsesNativeIdentifier(info, context);
public SignatureBehavior GetNativeSignatureBehavior(TypePositionInfo info) => _inner.GetNativeSignatureBehavior(info);
public ValueBoundaryBehavior GetValueBoundaryBehavior(TypePositionInfo info, StubCodeContext context) => _inner.GetValueBoundaryBehavior(info, context);
public bool SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => _inner.SupportsByValueMarshalKind(marshalKind, context);
public ByValueMarshalKindSupport SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => _inner.SupportsByValueMarshalKind(marshalKind, context);

public virtual IEnumerable<ExpressionSyntax> GenerateBind(TypePositionInfo info, StubCodeContext context)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal sealed class EmptyJSGenerator : IJSMarshallingGenerator
public SignatureBehavior GetNativeSignatureBehavior(TypePositionInfo info) => SignatureBehavior.ManagedTypeAndAttributes;
public ValueBoundaryBehavior GetValueBoundaryBehavior(TypePositionInfo info, StubCodeContext context) => ValueBoundaryBehavior.ManagedIdentifier;
public bool IsSupported(TargetFramework target, Version version) => false;
public bool SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => false;
public ByValueMarshalKindSupport SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => ByValueMarshalKindSupport.NotSupported;
public bool UsesNativeIdentifier(TypePositionInfo info, StubCodeContext context) => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ internal sealed class DiagnosticDescriptorProvider : IDiagnosticDescriptorProvid
GeneratorDiagnostic.NotSupported { NotSupportedDetails: null, TypePositionInfo: { IsManagedReturnPosition: false, MarshallingAttributeInfo: MarshalAsInfo } } => GeneratorDiagnostics.MarshalAsParameterConfigurationNotSupported,
GeneratorDiagnostic.NotSupported { NotSupportedDetails: not null, TypePositionInfo.IsManagedReturnPosition: true } => GeneratorDiagnostics.ReturnTypeNotSupportedWithDetails,
GeneratorDiagnostic.NotSupported { NotSupportedDetails: not null, TypePositionInfo.IsManagedReturnPosition: false } => GeneratorDiagnostics.ParameterTypeNotSupportedWithDetails,
GeneratorDiagnostic.UnnecessaryData { TypePositionInfo.IsManagedReturnPosition: false } => GeneratorDiagnostics.UnnecessaryParameterMarshallingInfo,
GeneratorDiagnostic.UnnecessaryData { TypePositionInfo.IsManagedReturnPosition: true } => GeneratorDiagnostics.UnnecessaryReturnMarshallingInfo,
{ IsFatal: false } => null,
{ TypePositionInfo.IsManagedReturnPosition: true } => GeneratorDiagnostics.ReturnTypeNotSupported,
{ TypePositionInfo.IsManagedReturnPosition: false } => GeneratorDiagnostics.ParameterTypeNotSupported,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public class Ids
public const string TypeNotSupported = Prefix + "1051";
public const string ConfigurationNotSupported = Prefix + "1052";
public const string RequiresAllowUnsafeBlocks = Prefix + "1062";
public const string UnnecessaryMarshallingInfo = Prefix + "1063";
public const string InvalidGeneratedComInterfaceAttributeUsage = Prefix + "1090";
public const string MemberWillNotBeSourceGenerated = Prefix + "1091";
public const string MultipleComInterfaceBaseTypes = Prefix + "1092";
Expand Down Expand Up @@ -415,6 +416,34 @@ public class Ids
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.ClassDoesNotImplementAnyGeneratedComInterfacesDescription)));

public static readonly DiagnosticDescriptor UnnecessaryParameterMarshallingInfo =
new DiagnosticDescriptor(
Ids.UnnecessaryMarshallingInfo,
GetResourceString(nameof(SR.UnnecessaryMarshallingInfoTitle)),
GetResourceString(nameof(SR.UnnecessaryParameterMarshallingInfoMessage)),
Category,
DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.UnnecessaryMarshallingInfoDescription)),
customTags: new[]
{
WellKnownDiagnosticTags.Unnecessary
});

public static readonly DiagnosticDescriptor UnnecessaryReturnMarshallingInfo =
new DiagnosticDescriptor(
Ids.UnnecessaryMarshallingInfo,
GetResourceString(nameof(SR.UnnecessaryMarshallingInfoTitle)),
GetResourceString(nameof(SR.UnnecessaryReturnMarshallingInfoMessage)),
Category,
DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.UnnecessaryMarshallingInfoDescription)),
customTags: new[]
{
WellKnownDiagnosticTags.Unnecessary
});

/// <summary>
/// Report diagnostic for invalid configuration for string marshalling.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public IEnumerable<StatementSyntax> Generate(TypePositionInfo info, StubCodeCont
public ValueBoundaryBehavior GetValueBoundaryBehavior(TypePositionInfo info, StubCodeContext context) => ValueBoundaryBehavior.NativeIdentifier;
public bool IsSupported(TargetFramework target, Version version)
=> target == TargetFramework.Net && version >= new Version(5, 0);
public bool SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => false;
public ByValueMarshalKindSupport SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => ByValueMarshalKindSupport.NotSupported;
public bool UsesNativeIdentifier(TypePositionInfo info, StubCodeContext context) => true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public IEnumerable<StatementSyntax> Generate(TypePositionInfo info, StubCodeCont
public SignatureBehavior GetNativeSignatureBehavior(TypePositionInfo info) => SignatureBehavior.NativeType;
public ValueBoundaryBehavior GetValueBoundaryBehavior(TypePositionInfo info, StubCodeContext context) => ValueBoundaryBehavior.ManagedIdentifier;
public bool IsSupported(TargetFramework target, Version version) => true;
public bool SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => false;
public ByValueMarshalKindSupport SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => ByValueMarshalKindSupport.NotSupported;
public bool UsesNativeIdentifier(TypePositionInfo info, StubCodeContext context) => false;
}

Expand Down Expand Up @@ -108,7 +108,7 @@ public IEnumerable<StatementSyntax> Generate(TypePositionInfo info, StubCodeCont
public SignatureBehavior GetNativeSignatureBehavior(TypePositionInfo info) => SignatureBehavior.NativeType;
public ValueBoundaryBehavior GetValueBoundaryBehavior(TypePositionInfo info, StubCodeContext context) => ValueBoundaryBehavior.ManagedIdentifier;
public bool IsSupported(TargetFramework target, Version version) => true;
public bool SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => false;
public ByValueMarshalKindSupport SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => ByValueMarshalKindSupport.NotSupported;
public bool UsesNativeIdentifier(TypePositionInfo info, StubCodeContext context) => false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public IEnumerable<StatementSyntax> Generate(TypePositionInfo info, StubCodeCont
public SignatureBehavior GetNativeSignatureBehavior(TypePositionInfo info) => SignatureBehavior.NativeType;
public ValueBoundaryBehavior GetValueBoundaryBehavior(TypePositionInfo info, StubCodeContext context) => ValueBoundaryBehavior.NativeIdentifier;
public bool IsSupported(TargetFramework target, Version version) => true;
public bool SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => false;
public ByValueMarshalKindSupport SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, StubCodeContext context) => ByValueMarshalKindSupport.NotSupported;
public bool UsesNativeIdentifier(TypePositionInfo info, StubCodeContext context) => true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,4 +395,16 @@
<data name="MarshalAsConfigurationNotSupportedMessageReturn" xml:space="preserve">
<value>The specified 'MarshalAsAttribute' configuration for the return value of method '{1}' is not supported by source-generated COM. If the specified configuration is required, use `ComImport` instead.</value>
</data>
<data name="UnnecessaryMarshallingInfoDescription" xml:space="preserve">
<value>Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</value>
</data>
<data name="UnnecessaryMarshallingInfoTitle" xml:space="preserve">
<value>Unnecessary marshalling info was provided and can be removed.</value>
</data>
<data name="UnnecessaryParameterMarshallingInfoMessage" xml:space="preserve">
<value>Unnecessary marshalling info '{0}' was provided for parameter '{1}'</value>
</data>
<data name="UnnecessaryReturnMarshallingInfoMessage" xml:space="preserve">
<value>Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,26 @@
<target state="translated">Zadaný typ není podporován modelem COM generovaným zdrojem.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryMarshallingInfoDescription">
<source>Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</source>
<target state="new">Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryMarshallingInfoTitle">
<source>Unnecessary marshalling info was provided and can be removed.</source>
<target state="new">Unnecessary marshalling info was provided and can be removed.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryParameterMarshallingInfoMessage">
<source>Unnecessary marshalling info '{0}' was provided for parameter '{1}'</source>
<target state="new">Unnecessary marshalling info '{0}' was provided for parameter '{1}'</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryReturnMarshallingInfoMessage">
<source>Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</source>
<target state="new">Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,26 @@
<target state="translated">Der angegebene Typ wird vom quellgenerierten COM nicht unterstützt.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryMarshallingInfoDescription">
<source>Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</source>
<target state="new">Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryMarshallingInfoTitle">
<source>Unnecessary marshalling info was provided and can be removed.</source>
<target state="new">Unnecessary marshalling info was provided and can be removed.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryParameterMarshallingInfoMessage">
<source>Unnecessary marshalling info '{0}' was provided for parameter '{1}'</source>
<target state="new">Unnecessary marshalling info '{0}' was provided for parameter '{1}'</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryReturnMarshallingInfoMessage">
<source>Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</source>
<target state="new">Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,26 @@
<target state="translated">El tipo especificado no es compatible con COM generado por el origen</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryMarshallingInfoDescription">
<source>Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</source>
<target state="new">Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryMarshallingInfoTitle">
<source>Unnecessary marshalling info was provided and can be removed.</source>
<target state="new">Unnecessary marshalling info was provided and can be removed.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryParameterMarshallingInfoMessage">
<source>Unnecessary marshalling info '{0}' was provided for parameter '{1}'</source>
<target state="new">Unnecessary marshalling info '{0}' was provided for parameter '{1}'</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryReturnMarshallingInfoMessage">
<source>Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</source>
<target state="new">Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,26 @@
<target state="translated">Le type spécifié n’est pas pris en charge par com généré par la source</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryMarshallingInfoDescription">
<source>Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</source>
<target state="new">Unnecesssary marshalling info was provided. This marshalling information can be removed without any change in behavior to the application.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryMarshallingInfoTitle">
<source>Unnecessary marshalling info was provided and can be removed.</source>
<target state="new">Unnecessary marshalling info was provided and can be removed.</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryParameterMarshallingInfoMessage">
<source>Unnecessary marshalling info '{0}' was provided for parameter '{1}'</source>
<target state="new">Unnecessary marshalling info '{0}' was provided for parameter '{1}'</target>
<note />
</trans-unit>
<trans-unit id="UnnecessaryReturnMarshallingInfoMessage">
<source>Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</source>
<target state="new">Unnecessary marshalling info '{0}' was provided for the return type of method '{1}'</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Loading

0 comments on commit 6d6fbe0

Please sign in to comment.