From 0361611cae4d863693245df7c3ba4ec579ae9a6c Mon Sep 17 00:00:00 2001 From: JohanLarsson Date: Fri, 17 Aug 2018 10:56:46 +0200 Subject: [PATCH] BUGFIX IDISP004 vs ApplicationInsights LoggerFactory. Fix #90. --- .../AssemblyAttributes.cs | 12 ++++++ .../Helpers/DisposableTests.IsCreation.cs | 35 ++++++++++++++++ ...sposableTests.IsPotentiallyAssignableTo.cs | 38 ++++++++++++++++++ .../HappyPath.cs | 40 +++++++++++++++++++ ...sposableTests.IsPotentiallyAssignableTo.cs | 5 +-- IDisposableAnalyzers.sln | 6 ++- .../Helpers/Disposable.IsCreation.cs | 11 +++++ .../Helpers/KnownSymbols/KnownSymbol.cs | 1 + .../Helpers/Temp/MethodSymbolExt.cs | 26 ++++++++++++ .../IDisposableAnalyzers.csproj.DotSettings | 1 + 10 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 IDisposableAnalyzers.NetCoreTests/Helpers/DisposableTests.IsCreation.cs create mode 100644 IDisposableAnalyzers.NetCoreTests/Helpers/DisposableTests.IsPotentiallyAssignableTo.cs create mode 100644 IDisposableAnalyzers.NetCoreTests/IDISP004DontIgnoreReturnValueOfTypeIDisposableTests/HappyPath.cs create mode 100644 IDisposableAnalyzers/Helpers/Temp/MethodSymbolExt.cs diff --git a/IDisposableAnalyzers.NetCoreTests/AssemblyAttributes.cs b/IDisposableAnalyzers.NetCoreTests/AssemblyAttributes.cs index 87c9756d..5a20b631 100644 --- a/IDisposableAnalyzers.NetCoreTests/AssemblyAttributes.cs +++ b/IDisposableAnalyzers.NetCoreTests/AssemblyAttributes.cs @@ -3,4 +3,16 @@ [assembly: MetadataReference(typeof(object), new[] { "global", "mscorlib" })] [assembly: MetadataReference(typeof(System.Diagnostics.Debug), new[] { "global", "System" })] [assembly: TransitiveMetadataReferences(typeof(Microsoft.CodeAnalysis.CSharp.CSharpCompilation))] +[assembly: TransitiveMetadataReferences(typeof(Microsoft.AspNetCore.Builder.IApplicationBuilder))] +[assembly: TransitiveMetadataReferences(typeof(Microsoft.AspNetCore.Hosting.IApplicationLifetime))] [assembly: TransitiveMetadataReferences(typeof(Microsoft.Extensions.Logging.ILoggerFactory))] +[assembly: TransitiveMetadataReferences(typeof(Microsoft.Extensions.Logging.ApplicationInsightsLoggerFactoryExtensions))] +[assembly: MetadataReferences( + typeof(System.Linq.Enumerable), + typeof(System.Net.WebClient), + typeof(System.Data.Common.DbConnection), + typeof(System.Threading.Tasks.ValueTask), + typeof(System.Xml.Serialization.XmlSerializer), + typeof(Gu.Roslyn.AnalyzerExtensions.SyntaxTokenExt), + typeof(Gu.Roslyn.CodeFixExtensions.Parse), + typeof(NUnit.Framework.Assert))] diff --git a/IDisposableAnalyzers.NetCoreTests/Helpers/DisposableTests.IsCreation.cs b/IDisposableAnalyzers.NetCoreTests/Helpers/DisposableTests.IsCreation.cs new file mode 100644 index 00000000..ffa81f6c --- /dev/null +++ b/IDisposableAnalyzers.NetCoreTests/Helpers/DisposableTests.IsCreation.cs @@ -0,0 +1,35 @@ +namespace IDisposableAnalyzers.NetCoreTests.Helpers +{ + using System.Threading; + using Gu.Roslyn.Asserts; + using Microsoft.CodeAnalysis.CSharp; + using NUnit.Framework; + + internal partial class DisposableTests + { + internal class IsCreation + { + [TestCase("Microsoft.Extensions.Logging.ApplicationInsightsLoggerFactoryExtensions.AddApplicationInsights(((Microsoft.Extensions.Logging.ILoggerFactory)o), null)")] + public void WhiteList(string code) + { + var testCode = @" +namespace RoslynSandbox +{ + internal class Foo + { + internal Foo(object o) + { + var value = PLACEHOLDER; + } + } +}"; + testCode = testCode.AssertReplace("PLACEHOLDER", code); + var syntaxTree = CSharpSyntaxTree.ParseText(testCode); + var compilation = CSharpCompilation.Create("test", new[] { syntaxTree }, MetadataReferences.FromAttributes()); + var semanticModel = compilation.GetSemanticModel(syntaxTree); + var value = syntaxTree.FindExpression(code); + Assert.AreEqual(Result.No, Disposable.IsCreation(value, semanticModel, CancellationToken.None)); + } + } + } +} diff --git a/IDisposableAnalyzers.NetCoreTests/Helpers/DisposableTests.IsPotentiallyAssignableTo.cs b/IDisposableAnalyzers.NetCoreTests/Helpers/DisposableTests.IsPotentiallyAssignableTo.cs new file mode 100644 index 00000000..d745d24c --- /dev/null +++ b/IDisposableAnalyzers.NetCoreTests/Helpers/DisposableTests.IsPotentiallyAssignableTo.cs @@ -0,0 +1,38 @@ +namespace IDisposableAnalyzers.NetCoreTests.Helpers +{ + using System.Threading; + using Gu.Roslyn.Asserts; + using Microsoft.CodeAnalysis.CSharp; + using NUnit.Framework; + + internal partial class DisposableTests + { + internal class IsPotentiallyAssignableTo + { + [TestCase("new string(' ', 1)", false)] + [TestCase("new System.Text.StringBuilder()", false)] + [TestCase("new System.IO.MemoryStream()", true)] + [TestCase("(Microsoft.Extensions.Logging.ILoggerFactory)o", true)] + public void Expression(string code, bool expected) + { + var testCode = @" +namespace RoslynSandbox +{ + internal class Foo + { + internal Foo(object o) + { + var value = PLACEHOLDER; + } + } +}"; + testCode = testCode.AssertReplace("PLACEHOLDER", code); + var syntaxTree = CSharpSyntaxTree.ParseText(testCode); + var compilation = CSharpCompilation.Create("test", new[] { syntaxTree }, MetadataReferences.FromAttributes()); + var semanticModel = compilation.GetSemanticModel(syntaxTree); + var value = syntaxTree.FindEqualsValueClause(code).Value; + Assert.AreEqual(expected, Disposable.IsPotentiallyAssignableFrom(value, semanticModel, CancellationToken.None)); + } + } + } +} diff --git a/IDisposableAnalyzers.NetCoreTests/IDISP004DontIgnoreReturnValueOfTypeIDisposableTests/HappyPath.cs b/IDisposableAnalyzers.NetCoreTests/IDISP004DontIgnoreReturnValueOfTypeIDisposableTests/HappyPath.cs new file mode 100644 index 00000000..3bd49ea4 --- /dev/null +++ b/IDisposableAnalyzers.NetCoreTests/IDISP004DontIgnoreReturnValueOfTypeIDisposableTests/HappyPath.cs @@ -0,0 +1,40 @@ +// ReSharper disable InconsistentNaming +#pragma warning disable SA1203 // Constants must appear before fields +namespace IDisposableAnalyzers.NetCoreTests.IDISP004DontIgnoreReturnValueOfTypeIDisposableTests +{ + using Gu.Roslyn.Asserts; + using Microsoft.CodeAnalysis.Diagnostics; + using NUnit.Framework; + + [TestFixture(typeof(IDISP004DontIgnoreReturnValueOfTypeIDisposable))] + [TestFixture(typeof(ObjectCreationAnalyzer))] + internal partial class HappyPath + where T : DiagnosticAnalyzer, new() + { + private static readonly DiagnosticAnalyzer Analyzer = new T(); + + [Test] + public void ILoggerFactoryAddApplicationInsights() + { + var testCode = @" +namespace RoslynSandbox +{ + using Microsoft.AspNetCore.Builder; + using Microsoft.AspNetCore.Hosting; + using Microsoft.Extensions.Logging; + + public class Foo + { + public void Configure( + IApplicationBuilder app, + IHostingEnvironment env, + ILoggerFactory loggerFactory) + { + loggerFactory.AddApplicationInsights(app.ApplicationServices, LogLevel.Warning); + } + } +}"; + AnalyzerAssert.Valid(Analyzer, testCode); + } + } +} diff --git a/IDisposableAnalyzers.Test/Helpers/DisposableTests.IsPotentiallyAssignableTo.cs b/IDisposableAnalyzers.Test/Helpers/DisposableTests.IsPotentiallyAssignableTo.cs index 2261217d..e494c204 100644 --- a/IDisposableAnalyzers.Test/Helpers/DisposableTests.IsPotentiallyAssignableTo.cs +++ b/IDisposableAnalyzers.Test/Helpers/DisposableTests.IsPotentiallyAssignableTo.cs @@ -2,9 +2,7 @@ namespace IDisposableAnalyzers.Test.Helpers { using System.Threading; using Gu.Roslyn.Asserts; - using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; - using Moq; using NUnit.Framework; internal partial class DisposableTests @@ -30,11 +28,12 @@ internal Foo() testCode = testCode.AssertReplace("PLACEHOLDER", code); var syntaxTree = CSharpSyntaxTree.ParseText(testCode); var value = syntaxTree.FindEqualsValueClause(code).Value; - Assert.AreEqual(expected, Disposable.IsPotentiallyAssignableFrom(value, new Mock(MockBehavior.Strict).Object, CancellationToken.None)); + Assert.AreEqual(expected, Disposable.IsPotentiallyAssignableFrom(value, null, CancellationToken.None)); } [TestCase("new string(' ', 1)", false)] [TestCase("new System.Text.StringBuilder()", false)] + [TestCase("new System.IO.MemoryStream()", true)] public void ObjectCreation(string code, bool expected) { var testCode = @" diff --git a/IDisposableAnalyzers.sln b/IDisposableAnalyzers.sln index 82781fd2..a9a29d7f 100644 --- a/IDisposableAnalyzers.sln +++ b/IDisposableAnalyzers.sln @@ -56,7 +56,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "IDisposableAnalyzers.Vsix", EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "IDisposableAnalyzers", "IDisposableAnalyzers\IDisposableAnalyzers.csproj", "{18EB2A1B-98EA-4DCF-A97C-D9E8A5DA747B}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "IDisposableAnalyzers.NetCoreTests", "IDisposableAnalyzers.NetCoreTests\IDisposableAnalyzers.NetCoreTests.csproj", "{8534B151-C831-4A31-B05B-7B2DAEA94033}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "IDisposableAnalyzers.NetCoreTests", "IDisposableAnalyzers.NetCoreTests\IDisposableAnalyzers.NetCoreTests.csproj", "{8534B151-C831-4A31-B05B-7B2DAEA94033}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -84,6 +84,10 @@ Global {8534B151-C831-4A31-B05B-7B2DAEA94033}.Debug|Any CPU.Build.0 = Debug|Any CPU {8534B151-C831-4A31-B05B-7B2DAEA94033}.Release|Any CPU.ActiveCfg = Release|Any CPU {8534B151-C831-4A31-B05B-7B2DAEA94033}.Release|Any CPU.Build.0 = Release|Any CPU + {8CC16990-2621-4854-8556-ED648A2D360F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {8CC16990-2621-4854-8556-ED648A2D360F}.Debug|Any CPU.Build.0 = Debug|Any CPU + {8CC16990-2621-4854-8556-ED648A2D360F}.Release|Any CPU.ActiveCfg = Release|Any CPU + {8CC16990-2621-4854-8556-ED648A2D360F}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/IDisposableAnalyzers/Helpers/Disposable.IsCreation.cs b/IDisposableAnalyzers/Helpers/Disposable.IsCreation.cs index 244082c6..ef7d40ba 100644 --- a/IDisposableAnalyzers/Helpers/Disposable.IsCreation.cs +++ b/IDisposableAnalyzers/Helpers/Disposable.IsCreation.cs @@ -373,6 +373,17 @@ private static Result IsCreationCore(ISymbol candidate, Compilation compilation) return Result.AssumeNo; } + if (method.TryGetThisParameter(out var thisParameter)&& + thisParameter.Type.Equals(method.ReturnType)) + { + if (method.ReturnType == KnownSymbol.ILoggerFactory) + { + return Result.No; + } + + return Result.AssumeNo; + } + return IsAssignableFrom(method.ReturnType, compilation) ? Result.AssumeYes : Result.No; diff --git a/IDisposableAnalyzers/Helpers/KnownSymbols/KnownSymbol.cs b/IDisposableAnalyzers/Helpers/KnownSymbols/KnownSymbol.cs index ef5bff72..99534a64 100644 --- a/IDisposableAnalyzers/Helpers/KnownSymbols/KnownSymbol.cs +++ b/IDisposableAnalyzers/Helpers/KnownSymbols/KnownSymbol.cs @@ -54,6 +54,7 @@ internal static class KnownSymbol internal static readonly QualifiedType NUnitOneTimeSetUpAttribute = new QualifiedType("NUnit.Framework.OneTimeSetUpAttribute"); internal static readonly QualifiedType NUnitOneTimeTearDownAttribute = new QualifiedType("NUnit.Framework.OneTimeTearDownAttribute"); internal static readonly QualifiedType NinjectStandardKernel = new QualifiedType("Ninject.StandardKernel"); + internal static readonly QualifiedType ILoggerFactory = new QualifiedType("Microsoft.Extensions.Logging.ILoggerFactory"); private static QualifiedType Create(string qualifiedName, string alias = null) { diff --git a/IDisposableAnalyzers/Helpers/Temp/MethodSymbolExt.cs b/IDisposableAnalyzers/Helpers/Temp/MethodSymbolExt.cs new file mode 100644 index 00000000..45f5aaa6 --- /dev/null +++ b/IDisposableAnalyzers/Helpers/Temp/MethodSymbolExt.cs @@ -0,0 +1,26 @@ +namespace IDisposableAnalyzers +{ + using System; + using Gu.Roslyn.AnalyzerExtensions; + using Microsoft.CodeAnalysis; + + internal static class MethodSymbolExt + { + [Obsolete("Move to Gu.Extensions")] + internal static bool TryGetThisParameter(this IMethodSymbol method, out IParameterSymbol parameter) + { + if (method.IsExtensionMethod) + { + if (method.ReducedFrom is IMethodSymbol reduced) + { + return reduced.Parameters.TryFirst(out parameter); + } + + return method.Parameters.TryFirst(out parameter); + } + + parameter = null; + return false; + } + } +} diff --git a/IDisposableAnalyzers/IDisposableAnalyzers.csproj.DotSettings b/IDisposableAnalyzers/IDisposableAnalyzers.csproj.DotSettings index dc434edf..65f4ab18 100644 --- a/IDisposableAnalyzers/IDisposableAnalyzers.csproj.DotSettings +++ b/IDisposableAnalyzers/IDisposableAnalyzers.csproj.DotSettings @@ -13,6 +13,7 @@ True True True + True True True True