From c746b74d424e0152068fd6de5182dc5989306348 Mon Sep 17 00:00:00 2001 From: Antonio Aversa Date: Thu, 16 Nov 2023 10:15:39 +0100 Subject: [PATCH] S1144: Add repro for #8342 (#8354) --- .../TestCases/UnusedPrivateMember.CSharp7.cs | 314 +++++++++--------- .../TestCases/UnusedPrivateMember.CSharp8.cs | 16 +- .../UnusedPrivateMember.Fixed.Batch.cs | 9 + .../TestCases/UnusedPrivateMember.Fixed.cs | 9 + .../TestCases/UnusedPrivateMember.cs | 14 + 5 files changed, 203 insertions(+), 159 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp7.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp7.cs index c768095456a..3d1d9e0e6ef 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp7.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp7.cs @@ -4,218 +4,216 @@ using System.Runtime.Serialization; using System.Threading.Tasks; -namespace Tests.Diagnostics +public class ExpressionBodyProperties { - public class ExpressionBodyProperties - { - private int field; + private int field; - private int Property01 - { - get => field; - set => field = value; // Noncompliant - } + private int Property01 + { + get => field; + set => field = value; // Noncompliant + } - private int Property02 - { - get => field; // Noncompliant - set => field = value; - } + private int Property02 + { + get => field; // Noncompliant + set => field = value; + } - public void Method() - { - int x; + public void Method() + { + int x; - x = Property01; - Property02 = x; - } + x = Property01; + Property02 = x; } +} - // https://github.com/SonarSource/sonar-dotnet/issues/2478 - public class ReproIssue2478 +// https://github.com/SonarSource/sonar-dotnet/issues/2478 +public class ReproIssue2478 +{ + public void SomeMethod() { - public void SomeMethod() - { - var (a, (barA, barB)) = new PublicDeconstructWithInnerType(); + var (a, (barA, barB)) = new PublicDeconstructWithInnerType(); - var (_, _, c) = new PublicDeconstruct(); + var (_, _, c) = new PublicDeconstruct(); - var qix = new MultipleDeconstructors(); - object b; - (a, b, c) = qix; + var qix = new MultipleDeconstructors(); + object b; + (a, b, c) = qix; - (a, b) = ReturnFromMethod(); + (a, b) = ReturnFromMethod(); - (a, b) = new ProtectedInternalDeconstruct(); + (a, b) = new ProtectedInternalDeconstruct(); - (a, b, c) = new Ambiguous(); // Error [CS0121] - (a, b) = new NotUsedDifferentArgumentCount(); // Error [CS7036,CS8129] - (a, b) = new NotUsedNotVisible(); // Error [CS7036,CS8129] - } - - internal void InternalMethod(InternalDeconstruct bar) - { - var (a, b) = bar; - } + (a, b, c) = new Ambiguous(); // Error [CS0121] + (a, b) = new NotUsedDifferentArgumentCount(); // Error [CS7036,CS8129] + (a, b) = new NotUsedNotVisible(); // Error [CS7036,CS8129] + } - private sealed class PublicDeconstructWithInnerType - { - public void Deconstruct(out object a, out InternalDeconstruct b) { a = b = null; } + internal void InternalMethod(InternalDeconstruct bar) + { + var (a, b) = bar; + } - // deconstructors must be public, internal or protected internal - private void Deconstruct(out object a, out object b) { a = b = null; } // Noncompliant - } + private sealed class PublicDeconstructWithInnerType + { + public void Deconstruct(out object a, out InternalDeconstruct b) { a = b = null; } - internal sealed class InternalDeconstruct - { - internal void Deconstruct(out object a, out object b) { a = b = null; } + // deconstructors must be public, internal or protected internal + private void Deconstruct(out object a, out object b) { a = b = null; } // Noncompliant + } - // deconstructors must be public, internal or protected internal - private void Deconstruct(out object a, out string b, out string c) { a = b = c = null; } // Noncompliant - } + internal sealed class InternalDeconstruct + { + internal void Deconstruct(out object a, out object b) { a = b = null; } - private class PublicDeconstruct - { - public void Deconstruct(out object a, out object b, out object c) { a = b = c = null; } + // deconstructors must be public, internal or protected internal + private void Deconstruct(out object a, out string b, out string c) { a = b = c = null; } // Noncompliant + } - // deconstructors must be public, internal or protected internal - protected void Deconstruct(out string a, out string b, out string c) { a = b = c = null; } // Noncompliant - private void Deconstruct(out object a, out string b, out string c) { a = b = c = null; } // Noncompliant - } + private class PublicDeconstruct + { + public void Deconstruct(out object a, out object b, out object c) { a = b = c = null; } - private sealed class MultipleDeconstructors - { - public void Deconstruct(out object a, out object b, out object c) { a = b = c = null; } + // deconstructors must be public, internal or protected internal + protected void Deconstruct(out string a, out string b, out string c) { a = b = c = null; } // Noncompliant + private void Deconstruct(out object a, out string b, out string c) { a = b = c = null; } // Noncompliant + } - public void Deconstruct(out object a, out object b) // Noncompliant - { - a = b = null; - } - } + private sealed class MultipleDeconstructors + { + public void Deconstruct(out object a, out object b, out object c) { a = b = c = null; } - private class ProtectedInternalDeconstruct + public void Deconstruct(out object a, out object b) // Noncompliant { - protected internal void Deconstruct(out object a, out object b) { a = b = null; } - - protected internal void Deconstruct(out object a, out object b, out object c) { a = b = c = null; } // Noncompliant + a = b = null; } + } - private class Ambiguous - { - public void Deconstruct(out string a, out string b, out string c) { a = b = c = null; } - public void Deconstruct(out object a, out object b, out object c) { a = b = c = null; } // Noncompliant FP, actually the one above is not used - } + private class ProtectedInternalDeconstruct + { + protected internal void Deconstruct(out object a, out object b) { a = b = null; } - private class NotUsedDifferentArgumentCount - { - public void Deconstruct(out string a, out string b, out string c) { a = b = c = null; } // Noncompliant - public void Deconstruct(out string a, out string b, out string c, out string d) { a = b = c = d = null; } // Noncompliant - } + protected internal void Deconstruct(out object a, out object b, out object c) { a = b = c = null; } // Noncompliant + } - private class NotUsedNotVisible - { - protected void Deconstruct(out object a, out object b) { a = b = null; } // Noncompliant - private void Deconstruct(out string a, out string b) { a = b = null; } // Noncompliant - } + private class Ambiguous + { + public void Deconstruct(out string a, out string b, out string c) { a = b = c = null; } + public void Deconstruct(out object a, out object b, out object c) { a = b = c = null; } // Noncompliant FP, actually the one above is not used + } - private ForMethod ReturnFromMethod() => null; - private sealed class ForMethod - { - public void Deconstruct(out object a, out object b) { a = b = null; } - } + private class NotUsedDifferentArgumentCount + { + public void Deconstruct(out string a, out string b, out string c) { a = b = c = null; } // Noncompliant + public void Deconstruct(out string a, out string b, out string c, out string d) { a = b = c = d = null; } // Noncompliant } - public class ReproIssue2333 + private class NotUsedNotVisible { - public void Method() - { - PrivateNestedClass x = new PrivateNestedClass(); - (x.ReadAndWrite, x.OnlyWriteNoBody, x.OnlyWrite) = ("A", "B", "C"); - var tuple = (x.ReadAndWrite, x.OnlyRead); - } + protected void Deconstruct(out object a, out object b) { a = b = null; } // Noncompliant + private void Deconstruct(out string a, out string b) { a = b = null; } // Noncompliant + } - private class PrivateNestedClass - { - private string hasOnlyWrite; - - public string ReadAndWrite { get; set; } // Setters are compliant, they are used in tuple assignment - public string OnlyWriteNoBody { get; set; } // Compliant, we don't raise on get without body - - public string OnlyRead - { - get; - set; // Noncompliant - } - - public string OnlyWrite - { - get => hasOnlyWrite; // Noncompliant - set => hasOnlyWrite = value; - } - } + private ForMethod ReturnFromMethod() => null; + private sealed class ForMethod + { + public void Deconstruct(out object a, out object b) { a = b = null; } + } +} + +public class ReproIssue2333 +{ + public void Method() + { + PrivateNestedClass x = new PrivateNestedClass(); + (x.ReadAndWrite, x.OnlyWriteNoBody, x.OnlyWrite) = ("A", "B", "C"); + var tuple = (x.ReadAndWrite, x.OnlyRead); } - // https://github.com/SonarSource/sonar-dotnet/issues/2752 - public class ReproIssue2752 + private class PrivateNestedClass { - private struct PrivateStructRef + private string hasOnlyWrite; + + public string ReadAndWrite { get; set; } // Setters are compliant, they are used in tuple assignment + public string OnlyWriteNoBody { get; set; } // Compliant, we don't raise on get without body + + public string OnlyRead { - public uint part1; // Noncompliant FP. Type is communicated an external call. + get; + set; // Noncompliant } - private class PrivateClassRef + public string OnlyWrite { - public uint part1; // Noncompliant FP. Type is communicated an external call. + get => hasOnlyWrite; // Noncompliant + set => hasOnlyWrite = value; } - - [DllImport("user32.dll")] - private static extern bool ExternalMethodWithStruct(ref PrivateStructRef reference); - - [DllImport("user32.dll")] - private static extern bool ExternalMethodWithClass(ref PrivateClassRef reference); } +} - public class EmptyCtor +// https://github.com/SonarSource/sonar-dotnet/issues/2752 +public class ReproIssue2752 +{ + private struct PrivateStructRef { - // That's invalid syntax, but it is still empty ctor and we should not raise for it, even if it is not used - public EmptyCtor() => // Error [CS1525,CS1002] + public uint part1; // Noncompliant FP. Type is communicated an external call. } - public class WithEnums + private class PrivateClassRef { - private enum X // Noncompliant - { - A - } + public uint part1; // Noncompliant FP. Type is communicated an external call. + } - public void UseEnum() - { - var b = Y.B; - } + [DllImport("user32.dll")] + private static extern bool ExternalMethodWithStruct(ref PrivateStructRef reference); - private enum Y - { - A, - B - } + [DllImport("user32.dll")] + private static extern bool ExternalMethodWithClass(ref PrivateClassRef reference); +} + +public class EmptyCtor +{ + // That's invalid syntax, but it is still empty ctor and we should not raise for it, even if it is not used + public EmptyCtor() => // Error [CS1525,CS1002] +} + +public class WithEnums +{ + private enum X // Noncompliant + { + A } - // https://github.com/SonarSource/sonar-dotnet/issues/6699 - public class Repro_6699 + public void UseEnum() { - public void MethodUsingLocalMethod() - { - void LocalMethod() { } // FN - } + var b = Y.B; } - // https://github.com/SonarSource/sonar-dotnet/issues/6724 - public class Repro_6724 + private enum Y { - public int PrivateGetter { private get; set; } // FN - public int PrivateSetter { get; private set; } // FN + A, + B + } +} - public int ExpressionBodiedPropertyWithPrivateGetter { private get => 1; set => _ = value; } // FN - public int ExpressionBodiedPropertyWithPrivateSetter { get => 1; private set => _ = value; } // FN +// https://github.com/SonarSource/sonar-dotnet/issues/6699 +public class Repro_6699 +{ + public void MethodUsingLocalMethod() + { + void LocalMethod() { } // FN } } + +// https://github.com/SonarSource/sonar-dotnet/issues/6724 +public class Repro_6724 +{ + public int PrivateGetter { private get; set; } // FN + public int PrivateSetter { get; private set; } // FN + + public int ExpressionBodiedPropertyWithPrivateGetter { private get => 1; set => _ = value; } // FN + public int ExpressionBodiedPropertyWithPrivateSetter { get => 1; private set => _ = value; } // FN +} + diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp8.cs index a79ce49aa65..f3227a150f5 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp8.cs @@ -1,4 +1,6 @@ -namespace Tests.Diagnostics +using System; + +namespace Tests.Diagnostics { public interface MyInterface1 { @@ -147,3 +149,15 @@ public void SomeMethod() } } +// https://github.com/SonarSource/sonar-dotnet/issues/8342 +public class Repro_8342 +{ + [Private1] private protected void APrivateProtectedMethod() { } + [Public1, Private2] public void APublicMethodWithMultipleAttributes1() { } + [Public1][Private2] public void APublicMethodWithMultipleAttributes2() { } + + private class Private1Attribute : Attribute { } // Noncompliant: FP: attribute used on a private protected method + private class Private2Attribute : Attribute { } // Noncompliant: FP: attribute used on a public method + private class Private3Attribute : Attribute { } // Noncompliant: FP: attribute used on a public method + public class Public1Attribute : Attribute { } // Compliant: public +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.Fixed.Batch.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.Fixed.Batch.cs index 27c3d1485e4..4958aef07f7 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.Fixed.Batch.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.Fixed.Batch.cs @@ -341,3 +341,12 @@ class Repro_8348 { [MyAttribute] void PrivateMethodWithAttribute() { } // FN: due to the attribute } + +// https://github.com/SonarSource/sonar-dotnet/issues/8342 +class Repro_8342 +{ + [Private1] public void APublicMethod() => APrivateMethodCalledByAPublicMethod(); + [Private2] internal void AnInternalMethod() { } + [Private3] protected void AProtectedMethod() { } + [Private4] private void APrivateMethodCalledByAPublicMethod() { } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.Fixed.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.Fixed.cs index c48cfd71790..65cdf492223 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.Fixed.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.Fixed.cs @@ -325,3 +325,12 @@ class Repro_8348 { [MyAttribute] void PrivateMethodWithAttribute() { } // FN: due to the attribute } + +// https://github.com/SonarSource/sonar-dotnet/issues/8342 +class Repro_8342 +{ + [Private1] public void APublicMethod() => APrivateMethodCalledByAPublicMethod(); + [Private2] internal void AnInternalMethod() { } + [Private3] protected void AProtectedMethod() { } + [Private4] private void APrivateMethodCalledByAPublicMethod() { } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.cs index 5d3671c82c8..e76f5bb6295 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.cs @@ -420,3 +420,17 @@ class Repro_8348 { [MyAttribute] void PrivateMethodWithAttribute() { } // FN: due to the attribute } + +// https://github.com/SonarSource/sonar-dotnet/issues/8342 +class Repro_8342 +{ + [Private1] public void APublicMethod() => APrivateMethodCalledByAPublicMethod(); + [Private2] internal void AnInternalMethod() { } + [Private3] protected void AProtectedMethod() { } + [Private4] private void APrivateMethodCalledByAPublicMethod() { } + + private class Private1Attribute : Attribute { } // Noncompliant: FP: attribute used on a public method + private class Private2Attribute : Attribute { } // Noncompliant: FP: attribute used on an internal method + private class Private3Attribute : Attribute { } // Noncompliant: FP: attribute used on a protected method + private class Private4Attribute : Attribute { } // Noncompliant: FP: attribute used on a private method used by a public method +}