Skip to content

Fix SA1023 for C# 9 function pointer parameters #3252

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.

Sign up for GitHub

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

Merged
sharwell merged 4 commits into from
Dec 1, 2020
Merged

Fix SA1023 for C# 9 function pointer parameters #3252

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d403d78
Fix SA1023 for C# 9 function pointer parameters
nxtn Feb 13, 2019
4a59836
Update SA1023 for function pointer types
nxtn Feb 24, 2019
00b9141
Group tests
nxtn Apr 16, 2019
56fa929
Add test cases for unmanaged keywork
nxtn Feb 26, 2020
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
65 changes: 65 additions & 0 deletions StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1023CSharp9UnitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,74 @@

namespace StyleCop.Analyzers.Test.CSharp9.SpacingRules
{
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using StyleCop.Analyzers.Test.CSharp8.SpacingRules;
using Xunit;
using static StyleCop.Analyzers.SpacingRules.SA1023DereferenceAndAccessOfSymbolsMustBeSpacedCorrectly;
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
StyleCop.Analyzers.SpacingRules.SA1023DereferenceAndAccessOfSymbolsMustBeSpacedCorrectly,
StyleCop.Analyzers.SpacingRules.TokenSpacingCodeFixProvider>;

public class SA1023CSharp9UnitTests : SA1023CSharp8UnitTests
{
[Fact]
public async Task TestFunctionPointerParameterInvalidSpacingAsync()
{
var testCode = @"public class TestClass
{
unsafe delegate*<int {|#0:*|}> FuncPtr1;
unsafe delegate*<int{|#1:*|} > FuncPtr2;
}
";

var fixedCode = @"public class TestClass
{
unsafe delegate*<int*> FuncPtr1;
unsafe delegate*<int*> FuncPtr2;
}
";

var expected = new[]
{
Diagnostic(DescriptorNotPreceded).WithLocation(0),
Diagnostic(DescriptorNotFollowed).WithLocation(1),
};

await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
public async Task TestFunctionPointerTypeInvalidSpacingAsync()
{
var testCode = @"public class TestClass
{
unsafe delegate {|#0:*|}<int*> FuncPtr1;
unsafe delegate{|#1:*|} <int*> FuncPtr2;
unsafe delegate {|#2:*|} managed<int*> FuncPtr3;
unsafe delegate{|#3:*|}managed<int*> FuncPtr4;

@sharwell sharwell Nov 30, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 IIRC, the unmanaged keyword only works for .NET 5 and newer. I believe there are some instances of tests in the repository using ReferenceAssemblies.Net.Net50 explicitly. Considering function pointers are only officially supported for .NET 5 and newer, it would be fine to just update this test to always use it and test both managed and unmanaged cases. I'm happy to leave this for a follow-up PR though.

@nxtn nxtn Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... error CS8889: The target runtime doesn't support extensible or runtime-environment default calling conventions.

@sharwell sharwell Nov 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler is looking for this field, which is only available in .NET 5 and newer:
https://github.com/dotnet/runtime/blob/2445324a501efaca19e136b7a7ca46d8ab5d95ec/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeFeature.cs#L18-L21

@nxtn nxtn Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I specify a calling convention to make the build pass as in #3254?

@sharwell sharwell Nov 30, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the test be updated to this form instead:

StyleCopAnalyzers/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/NamingRules/SA1300CSharp9UnitTests.cs

Lines 41 to 54 in 6399f49

await new CSharpTest(LanguageVersion.CSharp9)
{
ReferenceAssemblies = GenericAnalyzerTest.ReferenceAssembliesNet50,
TestCode = testCode,
ExpectedDiagnostics =
{
// /0/Test0.cs(2,15): warning SA1300: Element 'r' should begin with an uppercase letter
Diagnostic().WithLocation(0).WithArguments("r"),
// /0/Test0.cs(2,15): warning SA1300: Element 'r' should begin with an uppercase letter
Diagnostic().WithLocation(0).WithArguments("r"),
},
FixedCode = fixedCode,
}.RunAsync(CancellationToken.None).ConfigureAwait(false);

}
";

var fixedCode = @"public class TestClass
{
unsafe delegate*<int*> FuncPtr1;
unsafe delegate*<int*> FuncPtr2;
unsafe delegate* managed<int*> FuncPtr3;
unsafe delegate* managed<int*> FuncPtr4;
}
";

var expected = new[]
{
Diagnostic(DescriptorNotPreceded).WithLocation(0),
Diagnostic(DescriptorNotFollowed).WithLocation(1),
Diagnostic(DescriptorNotPreceded).WithLocation(2),
Diagnostic(DescriptorFollowed).WithLocation(3),
};

await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
}
}
}
4 changes: 4 additions & 0 deletions StyleCop.Analyzers/StyleCop.Analyzers/Lightup/SyntaxKindEx.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ internal static class SyntaxKindEx
{
public const SyntaxKind DotDotToken = (SyntaxKind)8222;
public const SyntaxKind QuestionQuestionEqualsToken = (SyntaxKind)8284;
public const SyntaxKind ManagedKeyword = (SyntaxKind)8445;
public const SyntaxKind UnmanagedKeyword = (SyntaxKind)8446;
public const SyntaxKind NullableKeyword = (SyntaxKind)8486;
public const SyntaxKind EnableKeyword = (SyntaxKind)8487;
public const SyntaxKind WarningsKeyword = (SyntaxKind)8488;
Expand Down Expand Up @@ -49,6 +51,8 @@ internal static class SyntaxKindEx
public const SyntaxKind ImplicitStackAllocArrayCreationExpression = (SyntaxKind)9053;
public const SyntaxKind SuppressNullableWarningExpression = (SyntaxKind)9054;
public const SyntaxKind NullableDirectiveTrivia = (SyntaxKind)9055;
public const SyntaxKind FunctionPointerType = (SyntaxKind)9056;
public const SyntaxKind FunctionPointerParameter = (SyntaxKind)9057;
public const SyntaxKind WithInitializerExpression = (SyntaxKind)9062;
public const SyntaxKind RecordDeclaration = (SyntaxKind)9063;
}
Expand Down
29 changes: 28 additions & 1 deletion ...yleCop.Analyzers/SpacingRules/SA1023DereferenceAndAccessOfSymbolsMustBeSpacedCorrectly.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace StyleCop.Analyzers.SpacingRules
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
using StyleCop.Analyzers.Helpers;
using StyleCop.Analyzers.Lightup;

/// <summary>
/// A dereference symbol or an access-of symbol within a C# element is not spaced correctly.
Expand Down Expand Up @@ -113,11 +114,37 @@ private static void HandleAsteriskToken(SyntaxTreeAnalysisContext context, Synta
bool allowTrailingSpace;
switch (token.Parent.Kind())
{
case SyntaxKindEx.FunctionPointerType:
allowAtLineStart = true;
allowAtLineEnd = true;
allowPrecedingSpace = false;
var nextToken = token.GetNextToken();
switch (nextToken.Kind())
{
case SyntaxKindEx.ManagedKeyword:
case SyntaxKindEx.UnmanagedKeyword:
allowTrailingSpace = true;
break;

default:
allowTrailingSpace = false;
break;
}

break;

case SyntaxKind.PointerType when token.Parent.Parent.IsKind(SyntaxKindEx.FunctionPointerParameter):
allowAtLineStart = true;
allowAtLineEnd = true;
allowPrecedingSpace = false;
allowTrailingSpace = false;
break;

case SyntaxKind.PointerType:
allowAtLineStart = false;
allowAtLineEnd = true;
allowPrecedingSpace = false;
var nextToken = token.GetNextToken();
Comment thread
nxtn marked this conversation as resolved.
Resolved
nextToken = token.GetNextToken();
switch (nextToken.Kind())
{
case SyntaxKind.OpenBracketToken:
Expand Down