KnownReference: Add support for testing for referenced libraries #6726

martin-strecker-sonarsource · 2023-02-06T19:12:37Z

A precondition for #6662

For the Why see also: https://sonarsource.slack.com/archives/C012KBFFYD6/p1675686569055439

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.cs

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.Predicates.cs

martin-strecker-sonarsource · 2023-02-07T15:40:34Z

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.Predicates.cs

+    internal static Func<AssemblyIdentity, bool> OptionalPublicKeyTokenIs(string key) =>
+        x => !x.HasPublicKey || PublicKeyEqualHex(x, key);


This is for cases like StackExchange.Redis which comes with a strong name or without (others are e.g. Dapper or NitoAsyncEx).

I don't think we'll ever need to be that precise.

Suggested change

internal static Func<AssemblyIdentity, bool> OptionalPublicKeyTokenIs(string key) =>

x => !x.HasPublicKey || PublicKeyEqualHex(x, key);

pavel-mikula-sonarsource

First round, I didn't look into the UTs yet as things will change a bit. I asked for some simplifications

analyzers/src/SonarAnalyzer.Common/Helpers/CompilationExtensions.cs

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.Predicates.cs

pavel-mikula-sonarsource · 2023-02-08T08:09:36Z

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.Predicates.cs

+    internal static Func<AssemblyIdentity, bool> StartsWith(string name) =>
+        x => x.Name.StartsWith(name);
+
+    internal static Func<AssemblyIdentity, bool> EndsWith(string name) =>
+        x => x.Name.EndsWith(name);


Do you have a clear example in mind? Experience with KnownType shows that we typically exactly know what we're looking for.

Suggested change

internal static Func<AssemblyIdentity, bool> StartsWith(string name) =>

x => x.Name.StartsWith(name);

internal static Func<AssemblyIdentity, bool> EndsWith(string name) =>

x => x.Name.EndsWith(name);

For StartsWith I was thinking of something like Azure.Storage, Azure.ResourceManager, AWSSDK or Microsoft.Extensions and for Contains something like IdentityModel.Tokens or SqlClient or Logger

We will typically need to work with a price KnownType to do this check. And that one comes from a precisely known assembly => we should not need these.

There are also checks like "IsTestProject" or "Is this targeting .Net Core" or "Is this a Web/WinForms/Console/WPF project" or "Is this a cloud app". These shortcuts can come in handy for such questions, especially for frameworks that have different implementations on different platforms like entity framework, SqlClient, or Asp.Net MVC.

pavel-mikula-sonarsource · 2023-02-08T08:11:13Z

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.Predicates.cs

+    internal static Func<AssemblyIdentity, bool> OptionalPublicKeyTokenIs(string key) =>
+        x => !x.HasPublicKey || PublicKeyEqualHex(x, key);


I don't think we'll ever need to be that precise.

Suggested change

internal static Func<AssemblyIdentity, bool> OptionalPublicKeyTokenIs(string key) =>

x => !x.HasPublicKey || PublicKeyEqualHex(x, key);

pavel-mikula-sonarsource · 2023-02-08T08:11:29Z

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.Predicates.cs

+    internal static Func<AssemblyIdentity, bool> PublicKeyTokenIs(string key) =>
+        x => x.HasPublicKey && PublicKeyEqualHex(x, key);


Same as above

Suggested change

internal static Func<AssemblyIdentity, bool> PublicKeyTokenIs(string key) =>

x => x.HasPublicKey && PublicKeyEqualHex(x, key);

If there is a strong name it is the most reliable source of identity. Also if we don't add the possibility to check for the strong name no one will ever add the functionality even in cases where it would be useful. The functionality is now there and fully covered by tests with our own public key.

We don't need to be that precise at all. We'll likely use these for an early bailout, as you plan to use them. And there will be no harm if we trigger on something that is a false match.

Let's remove it.

I also highly doubt that we would ever track a library that updates it's public key between versions.

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.cs

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.Predicates.cs

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReferenceExtensions.cs

pavel-mikula-sonarsource

LGTM to unblock the sprint.
I left further improvement comments.
Please also remove the methods from the previous review. We won't need them any time soon.

pavel-mikula-sonarsource · 2023-02-10T12:18:52Z

analyzers/src/SonarAnalyzer.Common/Extensions/CompilationExtensions.cs

@@ -24,4 +24,17 @@ internal static class CompilationExtensions
 {
    public static INamedTypeSymbol GetTypeByMetadataName(this Compilation compilation, KnownType knownType) =>
        compilation.GetTypeByMetadataName(knownType.FullName);
+
+    public static IMethodSymbol GetTypeMethod(this Compilation compilation, SpecialType type, string methodName) =>


Suggested change

public static IMethodSymbol GetTypeMethod(this Compilation compilation, SpecialType type, string methodName) =>

public static IMethodSymbol SpecialTypeMethod(this Compilation compilation, SpecialType type, string methodName) =>

pavel-mikula-sonarsource · 2023-02-10T12:19:07Z

analyzers/src/SonarAnalyzer.Common/Extensions/CompilationExtensions.cs

+        (IMethodSymbol)compilation.GetSpecialType(type)
+            .GetMembers(methodName)
+            .SingleOrDefault();


This could be a single line

pavel-mikula-sonarsource · 2023-02-10T12:22:38Z

analyzers/src/SonarAnalyzer.Common/Extensions/CompilationExtensions.cs

+    public static bool IsNetFrameworkTarget(this Compilation compilation) =>
+        // There's no direct way of checking compilation target framework yet (09/2020).
+        // See https://github.com/dotnet/roslyn/issues/3798
+        compilation.ObjectType.ContainingAssembly.Name == "mscorlib";


This is somewhat overlapping with NetFrameworkVersionProvider class
https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/src/SonarAnalyzer.Common/Helpers/NetFrameworkVersionProvider.cs

While this extension can live here, the implementation should be in that class to have this kind of ugly detection should live on one place.

The implementations can not easily be merged. I created #6751

pavel-mikula-sonarsource · 2023-02-10T12:26:41Z

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.cs

+            NameIs("xunit.assert"),
+            NameIs("xunit").And(VersionLowerThen("2.0")));
+
+        internal KnownReference(Func<AssemblyIdentity, bool> predicate, params Func<AssemblyIdentity, bool>[] or)


This will simplify it quite a lot. As this is supposed to be instantiated only from within this class (and therefore it would be beneficial to have it private), we don't need to enforce it syntactically. You can throw in case it's empty

Suggested change

internal KnownReference(Func<AssemblyIdentity, bool> predicate, params Func<AssemblyIdentity, bool>[] or)

internal KnownReference(params Func<AssemblyIdentity, bool>[] orPredicates)

This would be a bad API design. We can enforce proper usage at compile time and should do so.

pavel-mikula-sonarsource · 2023-02-10T12:27:06Z

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.cs

+
+using static SonarAnalyzer.Helpers.KnownReference.Predicates;
+
+namespace SonarAnalyzer.Helpers


Could be file scoped namespace

pavel-mikula-sonarsource · 2023-02-10T12:30:00Z

analyzers/tests/SonarAnalyzer.UnitTest/Extensions/KnownReferenceTests.cs

+using static SonarAnalyzer.Helpers.KnownReference;
+using static SonarAnalyzer.Helpers.KnownReference.Predicates;
+
+namespace SonarAnalyzer.UnitTest;


Thsi file should live in Helpers directory, same as the class it tests.
And have .Helpers namespace

Suggested change

namespace SonarAnalyzer.UnitTest;

namespace SonarAnalyzer.UnitTest.Helpers;

pavel-mikula-sonarsource · 2023-02-10T12:32:55Z

analyzers/tests/SonarAnalyzer.UnitTest/Extensions/KnownReferenceTests.cs

+        var compilation = new Mock<Compilation>("compilationName", ImmutableArray<MetadataReference>.Empty, new Dictionary<string, string>(), false, null, null);
+        var identity = new AssemblyIdentity(name);
+        compilation.SetupGet(x => x.ReferencedAssemblyNames).Returns(new[] { identity });
+        sut.IsReferencedBy(compilation.Object).Should().Be(expected);


Let's extract the repetitive scaffodling into a static method

Suggested change

var compilation = new Mock<Compilation>("compilationName", ImmutableArray<MetadataReference>.Empty, new Dictionary<string, string>(), false, null, null);

var identity = new AssemblyIdentity(name);

compilation.SetupGet(x => x.ReferencedAssemblyNames).Returns(new[] { identity });

sut.IsReferencedBy(compilation.Object).Should().Be(expected);

sut.IsReferencedBy(CreateCompilation(name)).Should().Be(expected);

Version can be an optional parameter

pavel-mikula-sonarsource · 2023-02-10T12:34:25Z

analyzers/tests/SonarAnalyzer.UnitTest/Extensions/KnownReferenceTests.cs

+        var compilation = SolutionBuilder.Create()
+            .AddProject(AnalyzerLanguage.CSharp)
+            .AddReferences(NuGetMetadataReference.XunitFramework("2.4.2"))
+            .AddSnippet("// Empty file")
+            .GetCompilation();


There's a helper for this

Suggested change

var compilation = SolutionBuilder.Create()

.AddProject(AnalyzerLanguage.CSharp)

.AddReferences(NuGetMetadataReference.XunitFramework("2.4.2"))

.AddSnippet("// Empty file")

.GetCompilation();

var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.XunitFramework("2.4.2")).Model.Compilation

pavel-mikula-sonarsource · 2023-02-10T12:34:40Z

analyzers/tests/SonarAnalyzer.UnitTest/Extensions/KnownReferenceTests.cs

+        var compilation = SolutionBuilder.Create()
+            .AddProject(AnalyzerLanguage.CSharp)
+            .AddReferences(NuGetMetadataReference.XunitFrameworkV1)
+            .AddSnippet("// Empty file")
+            .GetCompilation();


pavel-mikula-sonarsource · 2023-02-10T12:34:46Z

analyzers/tests/SonarAnalyzer.UnitTest/Extensions/KnownReferenceTests.cs

+        var compilation = SolutionBuilder.Create()
+            .AddProject(AnalyzerLanguage.CSharp)
+            .AddSnippet("// Empty file")
+            .GetCompilation();


sonarcloud · 2023-02-14T10:52:42Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

sonarcloud · 2023-02-14T10:58:56Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

Add KnowReference and tests

695358b

martin-strecker-sonarsource added Type: Improvement Making existing code better. Area: VB.NET VB.NET rules related issues. Area: C# C# rules related issues. labels Feb 6, 2023

github-actions bot assigned martin-strecker-sonarsource Feb 6, 2023

github-actions bot added this to In progress in Best Kanban Feb 6, 2023

martin-strecker-sonarsource added 8 commits February 6, 2023 21:06

Fix code smell (parameter names)

7547b03

Add overloads for version passed as string.

6b5d8f4

Add support for public key checks

5f39f96

Move predicates to own file

c739408

Rename to IsReferencedBy

d3842bd

Clean-up

92deb06

Add argument null checks

8422676

Clean-up

ec96228

martin-strecker-sonarsource commented Feb 7, 2023

View reviewed changes

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.cs Outdated Show resolved Hide resolved

analyzers/src/SonarAnalyzer.Common/Helpers/KnownReference.Predicates.cs Outdated Show resolved Hide resolved

martin-strecker-sonarsource marked this pull request as ready for review February 7, 2023 15:31

martin-strecker-sonarsource commented Feb 7, 2023

View reviewed changes

pavel-mikula-sonarsource self-requested a review February 8, 2023 08:07

github-actions bot assigned pavel-mikula-sonarsource and unassigned martin-strecker-sonarsource Feb 8, 2023

github-actions bot moved this from In progress to Review in progress in Best Kanban Feb 8, 2023

pavel-mikula-sonarsource requested changes Feb 8, 2023

View reviewed changes

github-actions bot assigned martin-strecker-sonarsource and unassigned pavel-mikula-sonarsource Feb 8, 2023

github-actions bot moved this from Review in progress to In progress in Best Kanban Feb 8, 2023

martin-strecker-sonarsource added 5 commits February 8, 2023 15:58

Remove Or and replace with params

ab95c9b

argument naming

4caa689

test Naming

af2cdc8

Move extension methods to the right file ine the right folder

c4ae3a4

Add suing after move

657c32e

martin-strecker-sonarsource added 5 commits February 10, 2023 08:43

Use nested class

886b451

Move to extensions folder

98364c7

Remove "Any" extension method

abed89f

Rename parameter

d619346

Exception message

6d341b1

martin-strecker-sonarsource requested a review from pavel-mikula-sonarsource February 10, 2023 11:29

github-actions bot assigned pavel-mikula-sonarsource and unassigned martin-strecker-sonarsource Feb 10, 2023

github-actions bot moved this from In progress to Review in progress in Best Kanban Feb 10, 2023

pavel-mikula-sonarsource approved these changes Feb 10, 2023

View reviewed changes

Best Kanban automation moved this from Review in progress to Review approved Feb 10, 2023

github-actions bot assigned martin-strecker-sonarsource and unassigned pavel-mikula-sonarsource Feb 10, 2023

martin-strecker-sonarsource added 3 commits February 13, 2023 11:54

Rename KnownReference to KnownAssembly

078a782

Rename SpecialTypeMethod

f9bbb8a

Formatting

0cd2ee6

martin-strecker-sonarsource mentioned this pull request Feb 13, 2023

Merge NetFrameworkVersionProvider and CompilationExtensions.IsNetFrameworkTarget #6751

Open

martin-strecker-sonarsource added 7 commits February 13, 2023 12:26

Move file

88e2f90

File scoped namespace

d2794d2

Extract MockCompilation

98ddc1b

Simplify project setup for tests

0c21bd5

Rename method

54ed0dd

Remove EditorBrowsable

5117f76

Add public key token to XUnit

cb7633f

martin-strecker-sonarsource merged commit c4258e5 into master Feb 14, 2023

Best Kanban automation moved this from Review approved to Validate Peach Feb 14, 2023

martin-strecker-sonarsource deleted the Martin/KnownReferences branch February 14, 2023 12:59

martin-strecker-sonarsource moved this from Validate Peach to Done in Best Kanban Feb 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KnownReference: Add support for testing for referenced libraries #6726

KnownReference: Add support for testing for referenced libraries #6726

martin-strecker-sonarsource commented Feb 6, 2023 •

edited

martin-strecker-sonarsource Feb 7, 2023 •

edited

pavel-mikula-sonarsource Feb 8, 2023

pavel-mikula-sonarsource left a comment

pavel-mikula-sonarsource Feb 8, 2023

martin-strecker-sonarsource Feb 9, 2023

pavel-mikula-sonarsource Feb 10, 2023

martin-strecker-sonarsource Feb 13, 2023 •

edited

pavel-mikula-sonarsource Feb 8, 2023

pavel-mikula-sonarsource Feb 8, 2023

martin-strecker-sonarsource Feb 9, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource left a comment

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

martin-strecker-sonarsource Feb 13, 2023

pavel-mikula-sonarsource Feb 10, 2023

martin-strecker-sonarsource Feb 13, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

pavel-mikula-sonarsource Feb 10, 2023

sonarcloud bot commented Feb 14, 2023

sonarcloud bot commented Feb 14, 2023

		internal static Func<AssemblyIdentity, bool> OptionalPublicKeyTokenIs(string key) =>
		x => !x.HasPublicKey \|\| PublicKeyEqualHex(x, key);

		internal static Func<AssemblyIdentity, bool> PublicKeyTokenIs(string key) =>
		x => x.HasPublicKey && PublicKeyEqualHex(x, key);

	public static IMethodSymbol GetTypeMethod(this Compilation compilation, SpecialType type, string methodName) =>
	public static IMethodSymbol SpecialTypeMethod(this Compilation compilation, SpecialType type, string methodName) =>

	internal KnownReference(Func<AssemblyIdentity, bool> predicate, params Func<AssemblyIdentity, bool>[] or)
	internal KnownReference(params Func<AssemblyIdentity, bool>[] orPredicates)


		using static SonarAnalyzer.Helpers.KnownReference.Predicates;

		namespace SonarAnalyzer.Helpers

	namespace SonarAnalyzer.UnitTest;
	namespace SonarAnalyzer.UnitTest.Helpers;

KnownReference: Add support for testing for referenced libraries #6726

KnownReference: Add support for testing for referenced libraries #6726

Conversation

martin-strecker-sonarsource commented Feb 6, 2023 • edited

martin-strecker-sonarsource Feb 7, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavel-mikula-sonarsource left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martin-strecker-sonarsource Feb 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavel-mikula-sonarsource left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Feb 14, 2023

sonarcloud bot commented Feb 14, 2023

martin-strecker-sonarsource commented Feb 6, 2023 •

edited

martin-strecker-sonarsource Feb 7, 2023 •

edited

martin-strecker-sonarsource Feb 13, 2023 •

edited