diff --git a/Directory.Packages.props b/Directory.Packages.props index 455735fc4..b52fd5d7c 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -20,6 +20,9 @@ + + + @@ -48,5 +51,7 @@ + + diff --git a/Ryujinx.sln b/Ryujinx.sln index b8304164d..84dd6e934 100644 --- a/Ryujinx.sln +++ b/Ryujinx.sln @@ -87,6 +87,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Ryujinx.Horizon", "src\Ryuj EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Ryujinx.Horizon.Kernel.Generators", "src\Ryujinx.Horizon.Kernel.Generators\Ryujinx.Horizon.Kernel.Generators.csproj", "{7F55A45D-4E1D-4A36-ADD3-87F29A285AA2}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Ryujinx.Analyzers", "src\Ryujinx.Analyzers\Ryujinx.Analyzers.csproj", "{C9523766-7101-442D-89F8-98A43B00267D}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Ryujinx.Tests.Analyzers", "src\Ryujinx.Tests.Analyzers\Ryujinx.Tests.Analyzers.csproj", "{F9D4CBAA-C63D-4062-94A8-F06299DC486B}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -249,6 +253,14 @@ Global {7F55A45D-4E1D-4A36-ADD3-87F29A285AA2}.Debug|Any CPU.Build.0 = Debug|Any CPU {7F55A45D-4E1D-4A36-ADD3-87F29A285AA2}.Release|Any CPU.ActiveCfg = Release|Any CPU {7F55A45D-4E1D-4A36-ADD3-87F29A285AA2}.Release|Any CPU.Build.0 = Release|Any CPU + {C9523766-7101-442D-89F8-98A43B00267D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {C9523766-7101-442D-89F8-98A43B00267D}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C9523766-7101-442D-89F8-98A43B00267D}.Release|Any CPU.ActiveCfg = Release|Any CPU + {C9523766-7101-442D-89F8-98A43B00267D}.Release|Any CPU.Build.0 = Release|Any CPU + {F9D4CBAA-C63D-4062-94A8-F06299DC486B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {F9D4CBAA-C63D-4062-94A8-F06299DC486B}.Debug|Any CPU.Build.0 = Debug|Any CPU + {F9D4CBAA-C63D-4062-94A8-F06299DC486B}.Release|Any CPU.ActiveCfg = Release|Any CPU + {F9D4CBAA-C63D-4062-94A8-F06299DC486B}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/src/Ryujinx.Analyzers/AnalyzerReleases.Shipped.md b/src/Ryujinx.Analyzers/AnalyzerReleases.Shipped.md new file mode 100644 index 000000000..dd5108b62 --- /dev/null +++ b/src/Ryujinx.Analyzers/AnalyzerReleases.Shipped.md @@ -0,0 +1,7 @@ +## Release 1.0 + +### New Rules + +| Rule ID | Category | Severity | Notes | +|---------|-----------------|----------|-------------------------------------| +| RYU0001 | Maintainability | Warning | Caught exceptions should be logged. | diff --git a/src/Ryujinx.Analyzers/CatchClauseAnalyzer.cs b/src/Ryujinx.Analyzers/CatchClauseAnalyzer.cs new file mode 100644 index 000000000..dbbf15961 --- /dev/null +++ b/src/Ryujinx.Analyzers/CatchClauseAnalyzer.cs @@ -0,0 +1,232 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; + +namespace Ryujinx.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class CatchClauseAnalyzer : DiagnosticAnalyzer + { + public const string LoggerIdentifier = "Logger"; + public const string LogClassIdentifier = "LogClass"; + + public const string DiagnosticId = "RYU0001"; + + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.RYU0001Title), + Resources.ResourceManager, typeof(Resources)); + + private static readonly LocalizableString MessageFormat = + new LocalizableResourceString(nameof(Resources.RYU0001MessageFormat), Resources.ResourceManager, + typeof(Resources)); + + private static readonly LocalizableString Description = + new LocalizableResourceString(nameof(Resources.RYU0001Description), Resources.ResourceManager, + typeof(Resources)); + + private const string Category = "Maintainability"; + + private static readonly DiagnosticDescriptor Rule = new(DiagnosticId, Title, MessageFormat, Category, + DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description); + + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.ReportDiagnostics); + context.EnableConcurrentExecution(); + + context.RegisterSyntaxNodeAction(AnalyzeSyntax, SyntaxKind.CatchClause); + } + + private static bool EndsWithExpressionText(ExpressionSyntax expression, string text) + { + if (expression is MemberAccessExpressionSyntax memberAccessExpression) + { + if (memberAccessExpression.Expression.ToString().EndsWith(text)) + { + return true; + } + } + + foreach (var childNode in expression.ChildNodes()) + { + if (childNode is not ExpressionSyntax childExpression) + { + continue; + } + + if (EndsWithExpressionText(childExpression, text)) + { + return true; + } + } + + return false; + } + + private static bool ContainsInvocationArgText(ExpressionSyntax expression, string text) + { + if (expression is InvocationExpressionSyntax invocationExpression) + { + if (invocationExpression.ArgumentList.Arguments.Count > 0) + { + var invocationArg = invocationExpression.ArgumentList.Arguments.First(); + + return invocationArg.ToString().StartsWith($"{text}.") || + invocationArg.ToString().Contains($".{text}."); + } + + return false; + } + + foreach (var childNode in expression.ChildNodes()) + { + if (childNode is not ExpressionSyntax childExpression) + { + continue; + } + + if (ContainsInvocationArgText(childExpression, text)) + { + return true; + } + } + + return false; + } + + private static bool ContainsIdentifier(ArgumentSyntax argument, string identifierText) + { + foreach (var argChild in argument.ChildNodes()) + { + switch (argChild) + { + case IdentifierNameSyntax identifierName when identifierName.ToString() == identifierText: + return true; + case InterpolatedStringExpressionSyntax interpolatedStringExpression: + { + foreach (var interpolatedStringChild in interpolatedStringExpression.ChildNodes()) + { + if (interpolatedStringChild is not InterpolationSyntax interpolation) + { + continue; + } + + foreach (var interpolationChild in interpolation.ChildNodes()) + { + if (interpolationChild is not IdentifierNameSyntax interpolationIdentifier) + { + continue; + } + + if (interpolationIdentifier.ToString() == identifierText) + { + return true; + } + } + } + break; + } + } + } + + return false; + } + + private static bool InvocationContainsIdentifier(ExpressionSyntax expression, string identifierText) + { + foreach (var expressionChild in expression.ChildNodes()) + { + if (expressionChild is not InvocationExpressionSyntax invocationExpression) + { + continue; + } + + foreach (var argument in invocationExpression.ArgumentList.Arguments) + { + if (ContainsIdentifier(argument, identifierText)) + { + return true; + } + } + } + + return false; + } + + /// + /// Executed for each Syntax Node with 'SyntaxKind.CatchClause'. + /// + /// Operation context. + private void AnalyzeSyntax(SyntaxNodeAnalysisContext context) + { + if (context.Node is not CatchClauseSyntax catchClauseSyntax) + return; + + var catchDeclaration = catchClauseSyntax.Declaration; + + // Find catch clauses without declaration. + if (catchDeclaration == null) + { + var diagnostic = Diagnostic.Create(Rule, + // The highlighted area in the analyzed source code. Keep it as specific as possible. + catchClauseSyntax.GetLocation(), + // The value is passed to 'MessageFormat' argument of your 'Rule'. + "Exception"); + + context.ReportDiagnostic(diagnostic); + } + // Find catch declarations without an identifier + else if (string.IsNullOrWhiteSpace(catchDeclaration.Identifier.Text)) + { + var diagnostic = Diagnostic.Create(Rule, + // The highlighted area in the analyzed source code. Keep it as specific as possible. + catchClauseSyntax.GetLocation(), + // The value is passed to 'MessageFormat' argument of your 'Rule'. + catchDeclaration.Type.ToString()); + + context.ReportDiagnostic(diagnostic); + } + // Check logging statements for a reference to the identifier of the catch declaration + else + { + var catchDeclarationIdentifier = catchDeclaration.Identifier; + bool exceptionLogged = false; + + // Iterate through all expression statements + foreach (var statement in catchClauseSyntax.Block.Statements) + { + if (statement is not ExpressionStatementSyntax expressionStatement) + { + continue; + } + + // Find Logger invocation + if (EndsWithExpressionText(expressionStatement.Expression, LoggerIdentifier) && ContainsInvocationArgText(expressionStatement.Expression, LogClassIdentifier)) + { + // Find catchDeclarationIdentifier in Logger invocation + if (InvocationContainsIdentifier(expressionStatement.Expression, catchDeclarationIdentifier.Text)) + { + exceptionLogged = true; + } + } + } + + // Create a diagnostic report if the exception was not logged + if (!exceptionLogged) + { + var diagnostic = Diagnostic.Create(Rule, + // The highlighted area in the analyzed source code. Keep it as specific as possible. + catchClauseSyntax.GetLocation(), + // The value is passed to 'MessageFormat' argument of your 'Rule'. + catchDeclaration.Type.ToString()); + + context.ReportDiagnostic(diagnostic); + } + } + } + } +} diff --git a/src/Ryujinx.Analyzers/CatchClauseCodeFixProvider.cs b/src/Ryujinx.Analyzers/CatchClauseCodeFixProvider.cs new file mode 100644 index 000000000..81a78fe33 --- /dev/null +++ b/src/Ryujinx.Analyzers/CatchClauseCodeFixProvider.cs @@ -0,0 +1,150 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Simplification; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using SyntaxFactory = Microsoft.CodeAnalysis.CSharp.SyntaxFactory; +using SyntaxKind = Microsoft.CodeAnalysis.CSharp.SyntaxKind; + +namespace Ryujinx.Analyzers +{ + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(CatchClauseCodeFixProvider)), Shared] + public class CatchClauseCodeFixProvider : CodeFixProvider + { + public sealed override ImmutableArray FixableDiagnosticIds { get; } = + ImmutableArray.Create(CatchClauseAnalyzer.DiagnosticId); + + public override FixAllProvider? GetFixAllProvider() => null; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var diagnostic = context.Diagnostics.Single(); + var diagnosticSpan = diagnostic.Location.SourceSpan; + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + // Find SyntaxNode corresponding to the diagnostic. + var diagnosticNode = root?.FindNode(diagnosticSpan); + + // To get the required metadata, we should match the Node to the specific type: 'CatchClauseSyntax'. + if (diagnosticNode is not CatchClauseSyntax catchClause) + return; + + // Register a code action that will invoke the fix. + context.RegisterCodeFix( + CodeAction.Create( + title: string.Format(Resources.RYU0001CodeFixTitle, GetExceptionType(catchClause)), + createChangedDocument: c => LogException(context.Document, catchClause, c), + equivalenceKey: nameof(Resources.RYU0001CodeFixTitle)), + diagnostic); + } + + private static string GetExceptionType(CatchClauseSyntax catchClause) + { + return catchClause.Declaration != null ? catchClause.Declaration.Type.ToString() : "Exception"; + } + + private static MemberAccessExpressionSyntax GetLoggingClass(string className) + { + return SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName("Ryujinx"), + SyntaxFactory.IdentifierName("Common")), + SyntaxFactory.IdentifierName("Logging")), + SyntaxFactory.IdentifierName(className)) + .WithAdditionalAnnotations(Simplifier.AddImportsAnnotation, Simplifier.Annotation); + } + + /// + /// Executed on the quick fix action raised by the user. + /// + /// Affected source file. + /// Highlighted catch clause syntax node. + /// Any fix is cancellable by the user, so we should support the cancellation token. + /// Clone of the document with the modified catch clause. + private async Task LogException(Document document, + CatchClauseSyntax catchClauseSyntax, CancellationToken cancellationToken) + { + CatchDeclarationSyntax catchDeclaration; + string catchDeclarationIdentifier = "exception"; + + // Add a catch declaration if it doesn't exist. + if (catchClauseSyntax.Declaration == null) + { + // System.Exception exception + catchDeclaration = + SyntaxFactory.CatchDeclaration( + SyntaxFactory.QualifiedName( + SyntaxFactory.IdentifierName("System"), SyntaxFactory.IdentifierName("Exception")), + SyntaxFactory.Identifier("exception") + ); + } + else + { + if (!string.IsNullOrWhiteSpace(catchClauseSyntax.Declaration.Identifier.Text)) + { + catchDeclaration = catchClauseSyntax.Declaration; + catchDeclarationIdentifier = catchDeclaration.Identifier.Text; + } + else + { + catchDeclaration = catchClauseSyntax.Declaration.WithIdentifier( + SyntaxFactory.Identifier(catchDeclarationIdentifier)); + } + } + + // Create logging statement. + // Ryujinx.Common.Logging.Logger.Error?.Print(LogClass.Application, $"Exception caught: {exception}"); + var newStatements = catchClauseSyntax.Block.Statements.Insert(0, SyntaxFactory.ExpressionStatement(SyntaxFactory.ConditionalAccessExpression( + SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, + GetLoggingClass("Logger"), + SyntaxFactory.IdentifierName("Error")), + SyntaxFactory.InvocationExpression( + SyntaxFactory.MemberBindingExpression(SyntaxFactory.IdentifierName("Print"))) + .AddArgumentListArguments( + SyntaxFactory.Argument(SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + GetLoggingClass("LogClass"), + SyntaxFactory.IdentifierName("Application"))), + SyntaxFactory.Argument(SyntaxFactory.InterpolatedStringExpression( + SyntaxFactory.Token(SyntaxKind.InterpolatedStringStartToken)).AddContents( + SyntaxFactory.InterpolatedStringText().WithTextToken( + SyntaxFactory.Token( + SyntaxTriviaList.Empty, + SyntaxKind.InterpolatedStringTextToken, + "Exception caught: ", + "Exception caught: ", + SyntaxTriviaList.Empty) + ), + SyntaxFactory.Interpolation( + SyntaxFactory.IdentifierName( + catchDeclarationIdentifier).WithAdditionalAnnotations( + RenameAnnotation.Create()) + ) + ) + ) + ) + ))); + + // Produce the new catch clause. + var newCatchClause = catchClauseSyntax + .WithCatchKeyword(catchClauseSyntax.CatchKeyword.WithTrailingTrivia(SyntaxFactory.Space)) + .WithDeclaration(catchDeclaration) + .WithBlock(catchClauseSyntax.Block.WithStatements(newStatements)) + .WithAdditionalAnnotations(Formatter.Annotation); + + // Replace the old local declaration with the new local declaration. + SyntaxNode oldRoot = (await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false))!; + SyntaxNode newRoot = oldRoot.ReplaceNode(catchClauseSyntax, newCatchClause); + + // Return document with the transformed tree. + return document.WithSyntaxRoot(newRoot); + } + } +} diff --git a/src/Ryujinx.Analyzers/Resources.Designer.cs b/src/Ryujinx.Analyzers/Resources.Designer.cs new file mode 100644 index 000000000..8b24d0e32 --- /dev/null +++ b/src/Ryujinx.Analyzers/Resources.Designer.cs @@ -0,0 +1,72 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by a tool. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + +namespace Ryujinx.Analyzers { + using System; + + + [System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "4.0.0.0")] + [System.Diagnostics.DebuggerNonUserCodeAttribute()] + [System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + internal class Resources { + + private static System.Resources.ResourceManager resourceMan; + + private static System.Globalization.CultureInfo resourceCulture; + + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + internal Resources() { + } + + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] + internal static System.Resources.ResourceManager ResourceManager { + get { + if (object.Equals(null, resourceMan)) { + System.Resources.ResourceManager temp = new System.Resources.ResourceManager("Ryujinx.Analyzers.Resources", typeof(Resources).Assembly); + resourceMan = temp; + } + return resourceMan; + } + } + + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] + internal static System.Globalization.CultureInfo Culture { + get { + return resourceCulture; + } + set { + resourceCulture = value; + } + } + + internal static string RYU0001Title { + get { + return ResourceManager.GetString("RYU0001Title", resourceCulture); + } + } + + internal static string RYU0001Description { + get { + return ResourceManager.GetString("RYU0001Description", resourceCulture); + } + } + + internal static string RYU0001MessageFormat { + get { + return ResourceManager.GetString("RYU0001MessageFormat", resourceCulture); + } + } + + internal static string RYU0001CodeFixTitle { + get { + return ResourceManager.GetString("RYU0001CodeFixTitle", resourceCulture); + } + } + } +} diff --git a/src/Ryujinx.Analyzers/Resources.resx b/src/Ryujinx.Analyzers/Resources.resx new file mode 100644 index 000000000..a73cc62fd --- /dev/null +++ b/src/Ryujinx.Analyzers/Resources.resx @@ -0,0 +1,33 @@ + + + + + + + + + + text/microsoft-resx + + + 1.3 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + Caught exception is not logged + + + Caught exceptions should be logged. + + + Caught exception '{0}' is not logged + + + Log the caught exception '{0}' + + \ No newline at end of file diff --git a/src/Ryujinx.Analyzers/Ryujinx.Analyzers.csproj b/src/Ryujinx.Analyzers/Ryujinx.Analyzers.csproj new file mode 100644 index 000000000..267ccf012 --- /dev/null +++ b/src/Ryujinx.Analyzers/Ryujinx.Analyzers.csproj @@ -0,0 +1,44 @@ + + + + netstandard2.0 + false + enable + latest + + true + true + + Ryujinx.Analyzers + Ryujinx.Analyzers + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + + + ResXFileCodeGenerator + Resources.Designer.cs + + + + + + True + True + Resources.resx + + + + + + + + diff --git a/src/Ryujinx.Tests.Analyzers/CatchClauseAnalyzerTests.cs b/src/Ryujinx.Tests.Analyzers/CatchClauseAnalyzerTests.cs new file mode 100644 index 000000000..9cce0d2bf --- /dev/null +++ b/src/Ryujinx.Tests.Analyzers/CatchClauseAnalyzerTests.cs @@ -0,0 +1,149 @@ +using System.IO; +using System.Reflection; +using System.Threading.Tasks; +using Xunit; +using Verifier = + Microsoft.CodeAnalysis.CSharp.Testing.XUnit.AnalyzerVerifier< + Ryujinx.Analyzers.CatchClauseAnalyzer>; + +namespace Ryujinx.Tests.Analyzers +{ + public class CatchClauseAnalyzerTests + { + private static readonly string _loggerTextPath = Path.Combine( + Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, + "Fixtures", "CatchClauseLogger.cs"); + + private static readonly string _loggerText = File.ReadAllText(_loggerTextPath); + + [Fact] + public async Task CatchWithoutDeclaration_WarningDiagnostic() + { + const string text = @" +using System; + +public class MyClass +{ + public void MyMethod1() + { + try + { + Console.WriteLine(""test""); + } + catch + { + throw; + } + } +} +"; + + var expected = Verifier.Diagnostic() + .WithSpan(12, 9, 15, 10) + .WithArguments("Exception"); + await Verifier.VerifyAnalyzerAsync(text, expected).ConfigureAwait(false); + } + + [Fact] + public async Task CatchWithoutIdentifier_WarningDiagnostic() + { + const string text = @" +using System; + +public class MyClass +{ + public void MyMethod2() + { + try + { + Console.WriteLine(""test""); + } + catch (NullReferenceException) + { + // testme + } + } +} +"; + + var expected = Verifier.Diagnostic() + .WithSpan(12, 9, 15, 10) + .WithArguments("NullReferenceException"); + await Verifier.VerifyAnalyzerAsync(text, expected).ConfigureAwait(false); + } + + [Fact] + public async Task LogWithoutCatchIdentifier_WarningDiagnostic() + { + string text = _loggerText + @" +public class MyClass +{ + public void MyMethod3() + { + try + { + Console.WriteLine(""test""); + } + catch (ArgumentException exception) + { + Ryujinx.Common.Logging.Logger.Info?.Print(Ryujinx.Common.Logging.LogClass.Application, ""test exception""); + } + } +} +"; + + var expected = Verifier.Diagnostic() + .WithSpan(89, 9, 92, 10) + .WithArguments("ArgumentException"); + await Verifier.VerifyAnalyzerAsync(text, expected).ConfigureAwait(false); + } + + [Fact] + public async Task LogWithIdentifierInString_NoDiagnostic() + { + string text = _loggerText + @" +public class MyClass +{ + public void MyMethod4() + { + try + { + Console.WriteLine(""test""); + } + catch (Exception abc) + { + Ryujinx.Common.Logging.Logger.Info?.Print(Ryujinx.Common.Logging.LogClass.Application, $""test: {abc}""); + Console.WriteLine(""Test""); + } + } +} +"; + + await Verifier.VerifyAnalyzerAsync(text).ConfigureAwait(false); + } + + [Fact] + public async Task LogWithIdentifierAsArg_NoDiagnostic() + { + string text = _loggerText + @" +public class MyClass +{ + public void MyMethod5() + { + try + { + Console.WriteLine(""test""); + } + catch (System.Exception exception) + { + Ryujinx.Common.Logging.Logger.Info?.Print(Ryujinx.Common.Logging.LogClass.Application, $""test"", exception); + Console.WriteLine(""Test""); + } + } +} +"; + + await Verifier.VerifyAnalyzerAsync(text).ConfigureAwait(false); + } + } +} diff --git a/src/Ryujinx.Tests.Analyzers/CatchClauseCodeFixProviderTests.cs b/src/Ryujinx.Tests.Analyzers/CatchClauseCodeFixProviderTests.cs new file mode 100644 index 000000000..b3a67fef7 --- /dev/null +++ b/src/Ryujinx.Tests.Analyzers/CatchClauseCodeFixProviderTests.cs @@ -0,0 +1,151 @@ +using System.IO; +using System.Reflection; +using System.Threading.Tasks; +using Xunit; +using Verifier = + Microsoft.CodeAnalysis.CSharp.Testing.XUnit.CodeFixVerifier; + +namespace Ryujinx.Tests.Analyzers +{ + public class CatchClauseCodeFixProviderTests + { + private static readonly string _loggerTextPath = Path.Combine( + Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, + "Fixtures", "CatchClauseLogger.cs"); + + private static readonly string _loggerText = File.ReadAllText(_loggerTextPath); + + [Fact] + public async Task CatchWithoutDeclaration_LogException() + { + string text = _loggerText + @" +public class MyClass +{ + public void MyMethod1() + { + try + { + Console.WriteLine(""test""); + } + catch + { + // ignored + } + } +} +"; + + string newText = _loggerText + @" +public class MyClass +{ + public void MyMethod1() + { + try + { + Console.WriteLine(""test""); + } + catch (System.Exception exception) + { + Ryujinx.Common.Logging.Logger.Error?.Print(Ryujinx.Common.Logging.LogClass.Application, $""Exception caught: {exception}""); + // ignored + } + } +} +"; + + var expected = Verifier.Diagnostic() + .WithSpan(89, 9, 92, 10) + .WithArguments("Exception"); + await Verifier.VerifyCodeFixAsync(text, expected, newText).ConfigureAwait(false); + } + + [Fact] + public async Task CatchWithoutIdentifier_LogException() + { + string text = _loggerText + @" +public class MyClass +{ + public void MyMethod2() + { + try + { + Console.WriteLine(""test""); + } + catch (NullReferenceException) + { + // ignored + } + } +} +"; + + string newText = _loggerText + @" +public class MyClass +{ + public void MyMethod2() + { + try + { + Console.WriteLine(""test""); + } + catch (NullReferenceException exception) + { + Ryujinx.Common.Logging.Logger.Error?.Print(Ryujinx.Common.Logging.LogClass.Application, $""Exception caught: {exception}""); + // ignored + } + } +} +"; + + var expected = Verifier.Diagnostic() + .WithSpan(89, 9, 92, 10) + .WithArguments("NullReferenceException"); + await Verifier.VerifyCodeFixAsync(text, expected, newText).ConfigureAwait(false); + } + + [Fact] + public async Task LogWithoutCatchIdentifier_LogException() + { + string text = _loggerText + @" +public class MyClass +{ + public void MyMethod3() + { + try + { + Console.WriteLine(""test""); + } + catch (ArgumentException ex) + { + Ryujinx.Common.Logging.Logger.Info?.Print(Ryujinx.Common.Logging.LogClass.Application, ""test""); + } + } +} +"; + + string newText = _loggerText + @" +public class MyClass +{ + public void MyMethod3() + { + try + { + Console.WriteLine(""test""); + } + catch (ArgumentException ex) + { + Ryujinx.Common.Logging.Logger.Error?.Print(Ryujinx.Common.Logging.LogClass.Application, $""Exception caught: {ex}""); + Ryujinx.Common.Logging.Logger.Info?.Print(Ryujinx.Common.Logging.LogClass.Application, ""test""); + } + } +} +"; + + var expected = Verifier.Diagnostic() + .WithSpan(89, 9, 92, 10) + .WithArguments("ArgumentException"); + await Verifier.VerifyCodeFixAsync(text, expected, newText).ConfigureAwait(false); + } + } +} diff --git a/src/Ryujinx.Tests.Analyzers/Fixtures/CatchClauseLogger.cs b/src/Ryujinx.Tests.Analyzers/Fixtures/CatchClauseLogger.cs new file mode 100644 index 000000000..2f40e8581 --- /dev/null +++ b/src/Ryujinx.Tests.Analyzers/Fixtures/CatchClauseLogger.cs @@ -0,0 +1,79 @@ +using System; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Threading; +// ReSharper disable All + +namespace Ryujinx.Common.Logging +{ + public enum LogClass + { + Application, + } + + public class LogEventArgs : EventArgs + { + public readonly int Level; + public readonly TimeSpan Time; + public readonly string ThreadName; + + public readonly string Message; + public readonly object Data; + + public LogEventArgs(int level, TimeSpan time, string threadName, string message, object data = null) + { + Level = level; + Time = time; + ThreadName = threadName; + Message = message; + Data = data; + } + } + + public static class Logger + { + private static readonly Stopwatch _time; + private static readonly bool[] _enabledClasses; + public static event EventHandler Updated; + public readonly struct Log + { + internal readonly int Level; + + internal Log(int level) + { + Level = level; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Print(LogClass logClass, string message, [CallerMemberName] string caller = "") + { + if (_enabledClasses[(int)logClass]) + { + Updated?.Invoke(null, new LogEventArgs(Level, _time.Elapsed, Thread.CurrentThread.Name, FormatMessage(logClass, caller, message))); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Print(LogClass logClass, string message, object data, [CallerMemberName] string caller = "") + { + if (_enabledClasses[(int)logClass]) + { + Updated?.Invoke(null, new LogEventArgs(Level, _time.Elapsed, Thread.CurrentThread.Name, FormatMessage(logClass, caller, message), data)); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static string FormatMessage(LogClass logClass, string caller, string message) => $"{logClass} {caller}: {message}"; + } + + public static Log? Debug { get; private set; } + public static Log? Info { get; private set; } + public static Log? Warning { get; private set; } + public static Log? Error { get; private set; } + public static Log? Guest { get; private set; } + public static Log? AccessLog { get; private set; } + public static Log? Stub { get; private set; } + public static Log? Trace { get; private set; } + public static Log Notice { get; } // Always enabled + } +} diff --git a/src/Ryujinx.Tests.Analyzers/Ryujinx.Tests.Analyzers.csproj b/src/Ryujinx.Tests.Analyzers/Ryujinx.Tests.Analyzers.csproj new file mode 100644 index 000000000..74f893463 --- /dev/null +++ b/src/Ryujinx.Tests.Analyzers/Ryujinx.Tests.Analyzers.csproj @@ -0,0 +1,35 @@ + + + + net8.0 + enable + + false + + + + + + + + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + + +