From 184b92e95cca5bb9f19634d75c6c749ff3db75fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20L=C3=BCbe=C3=9F?= Date: Sun, 3 Aug 2025 11:58:27 +0200 Subject: [PATCH] StyleChecker works, tests dont --- .../MakeConstUnitTests.cs | 49 ------- .../CatchUnmanagedCallersOnlyTest.cs | 120 ++++++++++++++++++ .../MakeConstCodeFixProvider.cs | 79 ------------ ScriptCoreGenerator/Strings.Designer.cs | 36 ++++++ ScriptCoreGenerator/Strings.resx | 12 ++ ScriptCoreGenerator/StyleChecker.cs | 63 --------- .../CatchUnmanagedCallersOnlyAnalyzer.cs | 109 ++++++++++++++++ .../CatchUnmanagedCallersOnlyFixProvider.cs | 88 +++++++++++++ ScriptCoreGenerator/SyntaxNodeExtensions.cs | 29 +++++ 9 files changed, 394 insertions(+), 191 deletions(-) delete mode 100644 ScriptCoreGenerator.Test/MakeConstUnitTests.cs create mode 100644 ScriptCoreGenerator.Test/StyleCheckers/CatchUnmanagedCallersOnlyTest.cs delete mode 100644 ScriptCoreGenerator/MakeConstCodeFixProvider.cs delete mode 100644 ScriptCoreGenerator/StyleChecker.cs create mode 100644 ScriptCoreGenerator/StyleCheckers/CatchUnmanagedCallersOnlyAnalyzer.cs create mode 100644 ScriptCoreGenerator/StyleCheckers/CatchUnmanagedCallersOnlyFixProvider.cs diff --git a/ScriptCoreGenerator.Test/MakeConstUnitTests.cs b/ScriptCoreGenerator.Test/MakeConstUnitTests.cs deleted file mode 100644 index 9f85c09..0000000 --- a/ScriptCoreGenerator.Test/MakeConstUnitTests.cs +++ /dev/null @@ -1,49 +0,0 @@ -using Microsoft.VisualStudio.TestTools.UnitTesting; -using System.Threading.Tasks; -using VerifyCS = MakeConst.Test.CSharpCodeFixVerifier< - ScriptCoreGenerator.StyleChecker, - ScriptCoreGenerator.MakeConstCodeFixProvider>; - -namespace MakeConst.Test -{ - [TestClass] - public class MakeConstUnitTest - { - //No diagnostics expected to show up - [TestMethod] - public async Task TestEmpty() - { - var test = @""; - - await VerifyCS.VerifyAnalyzerAsync(test); - } - - [TestMethod] - public async Task LocalIntCouldBeConstant_Diagnostic() - { - await VerifyCS.VerifyCodeFixAsync(@" -using System; - -class Program -{ - static void Main() - { - [|int i = 0;|] - Console.WriteLine(i); - } -} -", @" -using System; - -class Program -{ - static void Main() - { - const int i = 0; - Console.WriteLine(i); - } -} -"); - } - } -} diff --git a/ScriptCoreGenerator.Test/StyleCheckers/CatchUnmanagedCallersOnlyTest.cs b/ScriptCoreGenerator.Test/StyleCheckers/CatchUnmanagedCallersOnlyTest.cs new file mode 100644 index 0000000..6c7d5ee --- /dev/null +++ b/ScriptCoreGenerator.Test/StyleCheckers/CatchUnmanagedCallersOnlyTest.cs @@ -0,0 +1,120 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Threading.Tasks; + +using VerifyCS = MakeConst.Test.CSharpCodeFixVerifier< + ScriptCoreGenerator.StyleCheckers.CatchUnmanagedCallersOnlyAnalyzer, + ScriptCoreGenerator.StyleCheckers.CatchUnmanagedCallersOnlyFixProvider>; + +namespace ScriptCoreGenerator.Test.StyleCheckers +{ + [TestClass] + public class CatchUnmanagedCallersOnlyTest + { + //No diagnostics expected to show up + [TestMethod] + public async Task TestEmpty() + { + var test = @""; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [TestMethod] + public async Task TestNoDiagnosticsMethodWithStatementBlock() + { + var test = @""" + using System; + + class Program + { + static void Main() + { + Console.WriteLine(i); + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [TestMethod] + public async Task TestNoDiagnosticsWithArrowFunction() + { + var test = @""" + using System; + + class Program + { + static void Main() => Console.WriteLine(i); + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [TestMethod] + public async Task TestNoDiagnosticWithCorrectMethod() + { + await VerifyCS.VerifyAnalyzerAsync(@" +using System; +using System.Runtime.InteropServices; + +class Program +{ + [UnmanagedCallersOnly] + static void Main() + { + try + { + const int i = 0; + Console.WriteLine(i); + } + catch (Exception e) + { + } + } +} +"); + } + + [TestMethod] + public async Task LocalIntCouldBeConstant_Diagnostic() + { + await VerifyCS.VerifyCodeFixAsync( + @""" + using System; + using System.Runtime.InteropServices; + + class Program + { + [[|UnmanagedCallersOnly|]] + static void Main() + { + int i = 0; + Console.WriteLine(i); + } + } + """, + @""" + using System; + using System.Runtime.InteropServices; + + class Program + { + [UnmanagedCallersOnly] + static void Main() + { + try + { + const int i = 0; + Console.WriteLine(i); + } + catch (Exception e) + { + } + } + } + """); + } + } +} diff --git a/ScriptCoreGenerator/MakeConstCodeFixProvider.cs b/ScriptCoreGenerator/MakeConstCodeFixProvider.cs deleted file mode 100644 index 0ad9bec..0000000 --- a/ScriptCoreGenerator/MakeConstCodeFixProvider.cs +++ /dev/null @@ -1,79 +0,0 @@ -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Rename; -using Microsoft.CodeAnalysis.Text; -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Composition; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Formatting; - -namespace ScriptCoreGenerator -{ - [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MakeConstCodeFixProvider)), Shared] - public class MakeConstCodeFixProvider : CodeFixProvider - { - public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(StyleChecker.DiagnosticId); - - public sealed override FixAllProvider GetFixAllProvider() - { - // See https://github.com/dotnet/roslyn/blob/main/docs/analyzers/FixAllProvider.md for more information on Fix All Providers - return WellKnownFixAllProviders.BatchFixer; - } - - public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - - // TODO: Replace the following code with your own analysis, generating a CodeAction for each fix to suggest - var diagnostic = context.Diagnostics.First(); - var diagnosticSpan = diagnostic.Location.SourceSpan; - - // Find the type declaration identified by the diagnostic. - var declaration = root.FindToken(diagnosticSpan.Start).Parent.AncestorsAndSelf().OfType().First(); - - // Register a code action that will invoke the fix. - context.RegisterCodeFix( - CodeAction.Create( - title: Strings.CodeFixTitle, - createChangedDocument: c => MakeConstAsync(context.Document, declaration, c), - equivalenceKey: nameof(Strings.CodeFixTitle)), - diagnostic); - } - - private async Task MakeConstAsync(Document contextDocument, LocalDeclarationStatementSyntax declaration, CancellationToken cancellationToken) - { - // Remove the leading trivia from the local declaration. - SyntaxToken firstToken = declaration.GetFirstToken(); - SyntaxTriviaList leadingTrivia = firstToken.LeadingTrivia; - LocalDeclarationStatementSyntax trimmedLocal = - declaration.ReplaceToken(firstToken, firstToken.WithLeadingTrivia(SyntaxTriviaList.Empty)); - - // Create a const token with the leading trivia. - SyntaxToken constToken = - SyntaxFactory.Token(leadingTrivia, SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); - - // Insert the const token into the modifier list, creating a new modifiers list. - SyntaxTokenList newModifiers = trimmedLocal.Modifiers.Insert(0, constToken); - // Produce the new local dclaration. - LocalDeclarationStatementSyntax newLocal = - trimmedLocal.WithModifiers(newModifiers).WithDeclaration(declaration.Declaration); - - // Add an annotation to format the new local declaration. - LocalDeclarationStatementSyntax formattedLocal = newLocal.WithAdditionalAnnotations(Formatter.Annotation); - - // Replace the old local declaration with the new local declaration. - SyntaxNode oldRoot = await contextDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - SyntaxNode newRoot = oldRoot.ReplaceNode(declaration, formattedLocal); - - // Return document with transformed tree. - return contextDocument.WithSyntaxRoot(newRoot); - } - } -} diff --git a/ScriptCoreGenerator/Strings.Designer.cs b/ScriptCoreGenerator/Strings.Designer.cs index 031c140..6b75fc5 100644 --- a/ScriptCoreGenerator/Strings.Designer.cs +++ b/ScriptCoreGenerator/Strings.Designer.cs @@ -87,6 +87,42 @@ namespace ScriptCoreGenerator { } } + /// + /// Looks up a localized string similar to Methods marked with UnmanagedCallersOnlyAttribute must wrap their logic in a try-catch statement, to ensure that no exception is leaks out of the C# runtime.. + /// + internal static string CatchUnmanagedCallers_Description { + get { + return ResourceManager.GetString("CatchUnmanagedCallers_Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Wrap in try-catch-Statement. + /// + internal static string CatchUnmanagedCallers_FixTitle { + get { + return ResourceManager.GetString("CatchUnmanagedCallers_FixTitle", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Method '{0}' must wrap it's logic in a try-catch-Statement. + /// + internal static string CatchUnmanagedCallers_MessageFormat { + get { + return ResourceManager.GetString("CatchUnmanagedCallers_MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to UnmanagedCallersOnly-Methods must catch all exceptions.. + /// + internal static string CatchUnmanagedCallers_Title { + get { + return ResourceManager.GetString("CatchUnmanagedCallers_Title", resourceCulture); + } + } + /// /// Looks up a localized string similar to Make constant. /// diff --git a/ScriptCoreGenerator/Strings.resx b/ScriptCoreGenerator/Strings.resx index 0a6bc7c..0fc044d 100644 --- a/ScriptCoreGenerator/Strings.resx +++ b/ScriptCoreGenerator/Strings.resx @@ -126,6 +126,18 @@ Variable can be made const + + Methods marked with UnmanagedCallersOnlyAttribute must wrap their logic in a try-catch statement, to ensure that no exception is leaks out of the C# runtime. + + + Wrap in try-catch-Statement + + + Method '{0}' must wrap it's logic in a try-catch-Statement + + + UnmanagedCallersOnly-Methods must catch all exceptions. + Make constant diff --git a/ScriptCoreGenerator/StyleChecker.cs b/ScriptCoreGenerator/StyleChecker.cs deleted file mode 100644 index 8739bb1..0000000 --- a/ScriptCoreGenerator/StyleChecker.cs +++ /dev/null @@ -1,63 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Linq; -using System.Text; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Diagnostics; - -namespace ScriptCoreGenerator; - -[DiagnosticAnalyzer(LanguageNames.CSharp)] -public class StyleChecker : DiagnosticAnalyzer -{ - public const string DiagnosticId = "MakeConst"; - - private static readonly LocalizableString Title = - new LocalizableResourceString(nameof(Strings.AnalyzerTitle), Strings.ResourceManager, typeof(Strings)); - private static readonly LocalizableString MessageFormat = - new LocalizableResourceString(nameof(Strings.AnalyzerMessageFormat), Strings.ResourceManager, typeof(Strings)); - private static readonly LocalizableString Description = - new LocalizableResourceString(nameof(Strings.AnalyzerDescription), Strings.ResourceManager, typeof(Strings)); - - private const string Category = "Usage"; - - private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, - Category, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description); - - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); - - public override void Initialize(AnalysisContext context) - { - context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); - context.EnableConcurrentExecution(); - - context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.LocalDeclarationStatement); - } - - private void AnalyzeNode(SyntaxNodeAnalysisContext context) - { - var localDeclaration = (LocalDeclarationStatementSyntax)context.Node; - - if (localDeclaration.Modifiers.Any(SyntaxKind.ConstKeyword)) - { - return; - } - - // Perform data flow analysis on the local declaration. - DataFlowAnalysis dataFlowAnalysis = context.SemanticModel.AnalyzeDataFlow(localDeclaration); - - // Retrieve the local symbol for each variable in the local declaration - // and ensure that it is not written outside of the data flow analysis region. - VariableDeclaratorSyntax variable = localDeclaration.Declaration.Variables.Single(); - ISymbol variableSymbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken); - if (dataFlowAnalysis.WrittenOutside.Contains(variableSymbol)) - { - return; - } - - context.ReportDiagnostic(Diagnostic.Create(Rule, context.Node.GetLocation(), localDeclaration.Declaration.Variables.First().Identifier.ValueText)); - } -} diff --git a/ScriptCoreGenerator/StyleCheckers/CatchUnmanagedCallersOnlyAnalyzer.cs b/ScriptCoreGenerator/StyleCheckers/CatchUnmanagedCallersOnlyAnalyzer.cs new file mode 100644 index 0000000..606d1fb --- /dev/null +++ b/ScriptCoreGenerator/StyleCheckers/CatchUnmanagedCallersOnlyAnalyzer.cs @@ -0,0 +1,109 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Text; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace ScriptCoreGenerator.StyleCheckers; + +/// +/// Checks that methods with UnmanagedCallersOnly attribute always have a try-catch statement to ensure that exceptions never escape out of the C# code - as this would crash the application. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class CatchUnmanagedCallersOnlyAnalyzer : DiagnosticAnalyzer +{ + public const string DiagnosticId = "GE0001"; + + private static readonly LocalizableString Title = + new LocalizableResourceString(nameof(Strings.CatchUnmanagedCallers_Title), Strings.ResourceManager, + typeof(Strings)); + + private static readonly LocalizableString MessageFormat = + new LocalizableResourceString(nameof(Strings.CatchUnmanagedCallers_MessageFormat), Strings.ResourceManager, + typeof(Strings)); + + private static readonly LocalizableString Description = + new LocalizableResourceString(nameof(Strings.CatchUnmanagedCallers_Description), Strings.ResourceManager, + typeof(Strings)); + + private const string Category = "Usage"; + + private static readonly DiagnosticDescriptor Rule = new(DiagnosticId, Title, MessageFormat, + Category, DiagnosticSeverity.Error, isEnabledByDefault: true, description: Description); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.Attribute); + } + + private void AnalyzeNode(SyntaxNodeAnalysisContext context) + { + var attributeNode = (AttributeSyntax)context.Node; + + if (SyntaxNodeExtensions.ExtractName(attributeNode.Name) is not ("UnmanagedCallersOnly" or "UnmanagedCallersOnlyAttribute")) + return; + + MethodDeclarationSyntax? method = attributeNode.GetParentOrNull(); + + + if (method is null) + return; + + void ReportDiagnostic() + { + context.ReportDiagnostic(Diagnostic.Create(Rule, attributeNode.GetLocation(), method.Identifier.ValueText)); + } + + if (method.Body is not null) + { + // Check if the outer most statement is a try-catch block. + var tryStatement = method.Body.Statements.FirstOrDefault(s => s is TryStatementSyntax) as TryStatementSyntax; + if (tryStatement is null) + { + // If there is no try-catch block, report a diagnostic. + ReportDiagnostic(); + return; + } + else + { + // Check if the try block has a catch clause. + if (tryStatement.Catches.Count == 0) + { + ReportDiagnostic(); + return; + } + + // Report a diagnostic if there is no catch block that catches System.Exception. + if (tryStatement.Catches.All(c => + { + TypeSyntax? typeSyntax = c.Declaration?.Type; + + if (typeSyntax is null) + return true; + + TypeInfo typeInfo = context.SemanticModel.GetTypeInfo(typeSyntax); + return typeInfo.Type?.Name != "Exception"; + })) + { + ReportDiagnostic(); + return; + } + } + } + else if (method.ExpressionBody is not null) + { + // Expression-bodied methods cannot have an outer-most try-catch block. + ReportDiagnostic(); + return; + } + } +} diff --git a/ScriptCoreGenerator/StyleCheckers/CatchUnmanagedCallersOnlyFixProvider.cs b/ScriptCoreGenerator/StyleCheckers/CatchUnmanagedCallersOnlyFixProvider.cs new file mode 100644 index 0000000..48e691a --- /dev/null +++ b/ScriptCoreGenerator/StyleCheckers/CatchUnmanagedCallersOnlyFixProvider.cs @@ -0,0 +1,88 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Rename; +using Microsoft.CodeAnalysis.Text; +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Linq.Expressions; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Formatting; + +namespace ScriptCoreGenerator.StyleCheckers +{ + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(CatchUnmanagedCallersOnlyAnalyzer)), Shared] + public class CatchUnmanagedCallersOnlyFixProvider : CodeFixProvider + { + public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(CatchUnmanagedCallersOnlyAnalyzer.DiagnosticId); + + public sealed override FixAllProvider GetFixAllProvider() + { + // See https://github.com/dotnet/roslyn/blob/main/docs/analyzers/FixAllProvider.md for more information on Fix All Providers + return WellKnownFixAllProviders.BatchFixer; + } + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + var diagnostic = context.Diagnostics.First(); + var diagnosticSpan = diagnostic.Location.SourceSpan; + + // Find the type declaration identified by the diagnostic. + var declaration = root.FindToken(diagnosticSpan.Start).Parent.AncestorsAndSelf().OfType().First(); + + // Register a code action that will invoke the fix. + context.RegisterCodeFix( + CodeAction.Create( + title: Strings.CatchUnmanagedCallers_FixTitle, + createChangedDocument: c => AddTryCatchAsync(context.Document, declaration, c), + equivalenceKey: nameof(Strings.CatchUnmanagedCallers_FixTitle)), + diagnostic); + } + + private async Task AddTryCatchAsync(Document contextDocument, MethodDeclarationSyntax method, CancellationToken cancellationToken) + { + if (method.Body is not null) + { + // Create a try-catch block + var tryBlock = SyntaxFactory.Block(method.Body.Statements); + var catchClause = SyntaxFactory.CatchClause() + .WithDeclaration(SyntaxFactory.CatchDeclaration(SyntaxFactory.IdentifierName("Exception")) + .WithIdentifier(SyntaxFactory.Identifier("ex"))) + .WithBlock(SyntaxFactory.Block( + SyntaxFactory.SingletonList( + SyntaxFactory.ExpressionStatement( + SyntaxFactory.InvocationExpression( + SyntaxFactory.IdentifierName("Console.WriteLine")) + .WithArgumentList( + SyntaxFactory.ArgumentList( + SyntaxFactory.SingletonSeparatedList( + SyntaxFactory.Argument( + SyntaxFactory.IdentifierName("ex.Message"))))))))); + + var tryStatement = SyntaxFactory.TryStatement() + .WithBlock(tryBlock) + .WithCatches(SyntaxFactory.SingletonList(catchClause)); + + // Replace the method body with the new try-catch block + var newMethodBody = method.Body.WithStatements(SyntaxFactory.SingletonList(tryStatement)); + var newMethod = method.WithBody(newMethodBody); + + // Update the syntax tree + var oldRoot = await contextDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var newRoot = oldRoot.ReplaceNode(method, newMethod); + + return contextDocument.WithSyntaxRoot(newRoot); + } + + return contextDocument; + } + } +} diff --git a/ScriptCoreGenerator/SyntaxNodeExtensions.cs b/ScriptCoreGenerator/SyntaxNodeExtensions.cs index c386fb7..bba45b4 100644 --- a/ScriptCoreGenerator/SyntaxNodeExtensions.cs +++ b/ScriptCoreGenerator/SyntaxNodeExtensions.cs @@ -8,6 +8,16 @@ namespace ScriptCoreGenerator; public static class SyntaxNodeExtensions { + public static string? ExtractName(NameSyntax? name) + { + return name switch + { + SimpleNameSyntax ins => ins.Identifier.Text, + QualifiedNameSyntax qns => qns.Right.Identifier.Text, + _ => null + }; + } + public static T GetParent(this SyntaxNode node) { var parent = node.Parent; @@ -27,6 +37,25 @@ public static class SyntaxNodeExtensions } } + public static T? GetParentOrNull(this SyntaxNode node) where T : SyntaxNode + { + SyntaxNode? parent = node.Parent; + + while (true) + { + switch (parent) + { + case null: + return null; + case T t: + return t; + default: + parent = parent.Parent; + break; + } + } + } + /// /// determine the namespace the class/enum/struct is declared in, if any ///