diff --git a/Analyzers/Refactoring/TypeClassParameter/Analyzer.cs b/Analyzers/Refactoring/TypeClassParameter/Analyzer.cs index c961122..e09abf3 100644 --- a/Analyzers/Refactoring/TypeClassParameter/Analyzer.cs +++ b/Analyzers/Refactoring/TypeClassParameter/Analyzer.cs @@ -1,6 +1,7 @@ namespace StyleChecker.Analyzers.Refactoring.TypeClassParameter; using System; +using System.Collections.Frozen; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -90,7 +91,9 @@ static Func> } var toCalls = NewCallsSupplier(global.GetAllOperations()); + var referenceSymbolSet = global.GetAllReferenceSymbols(); var all = global.GetAllSymbols() + .Where(s => !referenceSymbolSet.Contains(s)) .SelectMany(toCalls) .SelectMany(c => c.ToDiagnostics()) .ToList(); @@ -158,10 +161,20 @@ static Func> NewCallsSupplier( var root = context.GetCompilationUnitRoot(); var allNodes = root.DescendantNodes(); + var methodReferenceSet = Enumerable.Empty() + .Concat(allNodes.OfType()) + .Concat(allNodes.OfType()) + .Select(toOperation) + .OfType() + .Select(o => o.Method) + .ToFrozenSet(); + global.AddReferenceSymbols(methodReferenceSet); + var localFunctions = allNodes.OfType() .Select(toOperation) .OfType() .Select(o => o.Symbol) + .Where(m => !methodReferenceSet.Contains(m)) .Where(HasTypeClassParameter); var methodGroups = allNodes.OfType() @@ -171,7 +184,8 @@ static Func> NewCallsSupplier( && !m.IsExtern && m.PartialDefinitionPart is null && m.PartialImplementationPart is null - && m.ContainingType.TypeKind is not TypeKind.Interface) + && m.ContainingType.TypeKind is not TypeKind.Interface + && !methodReferenceSet.Contains(m)) .Where(HasTypeClassParameter) .GroupBy(IsPrivate); var (privateMethods, unitMethods) = Split(methodGroups); diff --git a/Analyzers/Refactoring/TypeClassParameter/MethodInvocationBank.cs b/Analyzers/Refactoring/TypeClassParameter/MethodInvocationBank.cs index 4f998b9..8c2f59f 100644 --- a/Analyzers/Refactoring/TypeClassParameter/MethodInvocationBank.cs +++ b/Analyzers/Refactoring/TypeClassParameter/MethodInvocationBank.cs @@ -1,5 +1,6 @@ namespace StyleChecker.Analyzers.Refactoring.TypeClassParameter; +using System.Collections.Frozen; using System.Collections.Generic; using System.Collections.Immutable; using Microsoft.CodeAnalysis; @@ -16,10 +17,6 @@ namespace StyleChecker.Analyzers.Refactoring.TypeClassParameter; /// public sealed class MethodInvocationBank { - private readonly List symbols = []; - - private readonly List operations = []; - /// /// Initializes a new instance of the /// class. @@ -28,6 +25,12 @@ public MethodInvocationBank() { } + private List Symbols { get; } = []; + + private List Operations { get; } = []; + + private HashSet ReferenceSymbolSet { get; } = []; + /// /// Adds a collection of method symbols to the bank. /// @@ -36,7 +39,7 @@ public MethodInvocationBank() /// public void AddSymbols(IEnumerable all) { - AddRange(symbols, all); + AddRange(Symbols, all); } /// @@ -47,7 +50,21 @@ public void AddSymbols(IEnumerable all) /// public void AddOperations(IEnumerable all) { - AddRange(operations, all); + AddRange(Operations, all); + } + + /// + /// Adds a collection of Method Reference symbols to the bank. + /// + /// + /// The collection of method reference symbols to add. + /// + public void AddReferenceSymbols(IEnumerable all) + { + lock (ReferenceSymbolSet) + { + ReferenceSymbolSet.UnionWith(all); + } } /// @@ -56,9 +73,9 @@ public void AddOperations(IEnumerable all) /// /// An immutable array of all method symbols in the bank. /// - public ImmutableArray GetAllSymbols() + public FrozenSet GetAllSymbols() { - return ToImmutableArray(symbols); + return ToFrozenSet(Symbols); } /// @@ -69,7 +86,18 @@ public ImmutableArray GetAllSymbols() /// public ImmutableArray GetAllOperations() { - return ToImmutableArray(operations); + return ToImmutableArray(Operations); + } + + /// + /// Gets all Method Reference symbols in the bank. + /// + /// + /// A frozen set of all Method Reference symbols in the bank. + /// + public FrozenSet GetAllReferenceSymbols() + { + return ToFrozenSet(ReferenceSymbolSet); } private static void AddRange(List a, IEnumerable all) @@ -87,4 +115,12 @@ private static ImmutableArray ToImmutableArray(IEnumerable a) return [.. a]; } } + + private static FrozenSet ToFrozenSet(IEnumerable a) + { + lock (a) + { + return a.ToFrozenSet(); + } + } } diff --git a/TestSuite/Refactoring/TypeClassParameter/AnalyzerTest.cs b/TestSuite/Refactoring/TypeClassParameter/AnalyzerTest.cs index edcd226..43ef9ea 100644 --- a/TestSuite/Refactoring/TypeClassParameter/AnalyzerTest.cs +++ b/TestSuite/Refactoring/TypeClassParameter/AnalyzerTest.cs @@ -26,6 +26,19 @@ public AnalyzerTest() public void Okay() => VerifyDiagnostic(ReadText("Okay"), Atmosphere.Default); + [TestMethod] + public void LocalFunctionOkay() + => VerifyDiagnostic(ReadText("LocalFunctionOkay"), Atmosphere.Default); + + [TestMethod] + public void GlobalOkay() + => VerifyDiagnostic( + [ + ReadText("ReferencedOkay"), + ReadText("ReferencingOkay"), + ], + Atmosphere.Default); + [TestMethod] public void Code() => Check("Code"); diff --git a/TestSuite/Refactoring/TypeClassParameter/LocalFunctionOkay.cs b/TestSuite/Refactoring/TypeClassParameter/LocalFunctionOkay.cs new file mode 100644 index 0000000..5d3ae07 --- /dev/null +++ b/TestSuite/Refactoring/TypeClassParameter/LocalFunctionOkay.cs @@ -0,0 +1,54 @@ +#pragma warning disable CS8321 + +namespace StyleChecker.Test.Refactoring.TypeClassParameter; + +using System; + +public sealed class LocalFunctionOkay +{ + public void IncludesStaticClass() + { + void Print(Type t) + { + Console.WriteLine(t.FullName); + } + + Print(typeof(string)); + Print(typeof(StaticClass)); + } + + public void NotAllArgumentsAreTypeof() + { + void Print(Type t) + { + Console.WriteLine(t.FullName); + } + + Print(typeof(string)); + Print(GetType()); + } + + public void NobodyInvokes() + { + void NeverUsed(Type t) + => Console.WriteLine(t.FullName); + } + + public void MethodRederence() + { + string ToName(Type t) => t.FullName; + + void Print(Func toName, Type[] array) + { + foreach (var i in array) + { + Console.Write(toName(i)); + } + } + + var name = ToName(typeof(object)); + Print(ToName, [typeof(string), typeof(int)]); + } + + public static class StaticClass; +} diff --git a/TestSuite/Refactoring/TypeClassParameter/Okay.cs b/TestSuite/Refactoring/TypeClassParameter/Okay.cs index b35d2d0..6ecd658 100644 --- a/TestSuite/Refactoring/TypeClassParameter/Okay.cs +++ b/TestSuite/Refactoring/TypeClassParameter/Okay.cs @@ -6,49 +6,85 @@ namespace StyleChecker.Test.Refactoring.TypeClassParameter; public sealed class Okay { - public void IncludesStaticClass() + public class IncludesStaticClass { - void Print(Type t) + public void Print(Type t) { Console.WriteLine(t.FullName); } - Print(typeof(string)); - Print(typeof(StaticClass)); + public void Invoke() + { + Print(typeof(string)); + Print(typeof(StaticClass)); + } } - public void NotAllArgumentsAreTypeofLocalFunction() + public class NotAllArgumentsAreTypeof { - void Print(Type t) + public void Print(Type t) { Console.WriteLine(t.FullName); } - Print(typeof(string)); - Print(GetType()); + public void Invoke() + { + Print(typeof(string)); + Print(GetType()); + } } - public void NobodyInvokesLocalFunction() + public void NobodyInvokes(Type type) { - void NeverUsed(Type t) - => Console.WriteLine(t.FullName); + Console.WriteLine(type.FullName); } - public void NotAllArgumentsAreTypeof(Type type) + public class MethodReference { - Console.WriteLine(type.FullName); + public string ToName(Type t) + => t.FullName; + + void Print(Func toName, Type[] array) + { + foreach (var i in array) + { + Console.Write(toName(i)); + } + } + + public void Invoke() + { + var name = ToName(typeof(object)); + Print(ToName, [typeof(string), typeof(int)]); + } } - public void NobodyInvokes(Type type) + public class InstanceMethodReference { - Console.WriteLine(type.FullName); + public string ToName(Type t) + => t.FullName; + + public void Invoke() + { + var name = ToName(typeof(object)); + } } - public void Invoke() + private Func instanceAction + = new InstanceMethodReference().ToName; + + public class StaticMethodReference { - NotAllArgumentsAreTypeof(typeof(string)); - NotAllArgumentsAreTypeof(GetType()); + public static string ToName(Type t) + => t.FullName; + + public void Invoke() + { + var name = ToName(typeof(object)); + } } + private Func staticAction = StaticMethodReference.ToName; + public static class StaticClass; } diff --git a/TestSuite/Refactoring/TypeClassParameter/ReferencedOkay.cs b/TestSuite/Refactoring/TypeClassParameter/ReferencedOkay.cs new file mode 100644 index 0000000..4d6ae48 --- /dev/null +++ b/TestSuite/Refactoring/TypeClassParameter/ReferencedOkay.cs @@ -0,0 +1,21 @@ +namespace StyleChecker.Test.Refactoring.TypeClassParameter; + +using System; + +public sealed class ReferencedOkay +{ + private static void Log(string message) + { + } + + /// + /// Print the specified type. + /// + /// + /// The type to be printed. + /// + public static void PrintMethod(Type type) + { + Log(type.FullName); + } +} diff --git a/TestSuite/Refactoring/TypeClassParameter/ReferencingOkay.cs b/TestSuite/Refactoring/TypeClassParameter/ReferencingOkay.cs new file mode 100644 index 0000000..27bd455 --- /dev/null +++ b/TestSuite/Refactoring/TypeClassParameter/ReferencingOkay.cs @@ -0,0 +1,19 @@ +namespace StyleChecker.Test.Refactoring.TypeClassParameter; + +public sealed class ReferencingOkay +{ + public void CallWithStringType() + { + ReferencedOkay.PrintMethod(typeof(string)); + } + + public void CallWithIntType() + { + ReferencedOkay.PrintMethod(typeof(int)); + } + + public void MethodReference() + { + var method = ReferencedOkay.PrintMethod; + } +} diff --git a/TestSuite/TestSuite.csproj b/TestSuite/TestSuite.csproj index 7d55092..028f69b 100644 --- a/TestSuite/TestSuite.csproj +++ b/TestSuite/TestSuite.csproj @@ -105,11 +105,14 @@ + + + @@ -451,6 +454,15 @@ PreserveNewest + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + PreserveNewest diff --git a/doc/rules/TypeClassParameter.md b/doc/rules/TypeClassParameter.md index c635362..17b7106 100644 --- a/doc/rules/TypeClassParameter.md +++ b/doc/rules/TypeClassParameter.md @@ -11,8 +11,8 @@ ## Summary -Replace the parameter of methods or local functions with a type parameter -if possible. +Replace a `System.Type` parameter in a method or local function with a generic +type parameter when all arguments passed to it are `typeof(…)` expressions. ## Default severity @@ -20,12 +20,12 @@ Warning ## Description -The parameter of methods or local functions can be replaced with a type -parameter if its type is `System.Type` and every argument for it is a -`typeof()` operator. For example, the local function `Print` has a single -parameter `type`, whose type is `System.Type`, and *all* the invocations of it -are performed with an argument of the `typeof()` operator whose operand is -not a `static` class, as follows: +This analyzer identifies method or local function parameters of type +`System.Type` that are _always_ invoked with `typeof(…)` expressions. In such +cases, the parameter can be safely replaced with a generic type parameter `T`, +which improves type safety and readability. + +For example, consider the following code: ```csharp public void PrintTypes() @@ -40,8 +40,8 @@ public void PrintTypes() ⋮ ``` -The following code shows the revised version of `Print` where the -parameter `type` is removed but a type parameter `T` is added instead: +All calls to `Print` use `typeof(…)`. Therefore, you can refactor it using a +generic type parameter: ```csharp public void PrintTypes() @@ -57,24 +57,62 @@ public void PrintTypes() ⋮ ``` -Note that this analyzer doesn't report diagnostics if at least one caller -invokes the original version of `Print` with an argument other than the -`typeof()` operator whose operand is not a `static` class, because it is -unable to replace the parameter `type` with a type parameter `T`. - > 🚧 **Restriction** > -> This analyzer can only diagnose local functions and private methods -> with the Visual Studio 2019 editor. -> To diagnose non-private methods with Visual Studio 2019, -> perform Build Solution or Analysis ➜ Run Code Analysis. +> In Visual Studio IDE, this analyzer only reports diagnostics for: +> +> - Local functions +> - Private methods +> +> To analyze non-private methods, you must build the solution or run Analyze ➜ +> Run Code Analysis. + +## Excluded Cases + +### Static classes cannot be used with type parameters + +This analyzer does **not** report diagnostics if any of the `typeof(…)` +arguments refer to a `static` class. Static classes cannot be used as type +arguments in C#, so replacing `System.Type` with a type parameter would be +invalid. + +```csharp +public static class SomeStaticClass; + +// ❌ Skipped — static classes are not allowed as type arguments +Print(typeof(SomeStaticClass)); +``` + +### Method references prevent replacement + +If the method or local function is passed as a method group (method reference) +to a delegate (e.g., `Action`), the analyzer does **not** report a +diagnostic. + +```csharp +void DoAction(Action action) +{ + ⋮ +} + +// ❌ Skipped — cannot convert generic method to Action +DoAction(Print); +``` + +Replacing `Print(Type)` with a generic method `Print()` would +break this usage, so the analyzer conservatively ignores such cases. ## Code fix -The code fix provides the option of replacing the parameter with a type -parameter and inserting a local variable declaration to the top of the -method or the local function. The variable name of the inserted declaration -is the same as the name of the removed parameter. +The code fix will: + +- Replace the `Type` parameter with a generic type parameter. +- Insert a local variable declaration `var … = typeof(T);` at the beginning of + the method or local function. +- Update all call sites to use generic method syntax (e.g., + `DoSomething()`). + +The new local variable will reuse the original parameter name for consistency. ## Example @@ -109,10 +147,12 @@ public void Invoke() > 🚨 **Remarks** > -> If a type has both `DoSomething()` and `DoSomething(Type)` methods -> at the same time, the code fix provider renames `DoSomething` -> (to `DoSomething_0`, for example) at first, and then replaces -> `DoSomething(Type)` with `DoSomething()`. +> If the type already contains both `DoSomething(Type)` and `DoSomething()`, +> the code fix first **renames the existing `DoSomething()` method (e.g., to +> `DoSomething_0`) to avoid a name conflict**. Then it replaces the +> `DoSomething(Type)` method with the new generic version using the original +> name (`DoSomething()`). This ensures that call sites referring to +> `DoSomething(Type)` can be safely updated to use the generic method. [fig-TypeClassParameter]: https://maroontress.github.io/StyleChecker/images/TypeClassParameter.png