From 834380f897841f8f970c11a2dd5193f432c0d79c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Hellander?= Date: Tue, 2 Jul 2024 19:32:05 +0200 Subject: [PATCH 1/2] Update SA1649CodeFixProvider to first remove the file in other projects which reference the same file before adding them there (to not end up with two identical files in those projects) and also to re-add files with correct path (to not get a copy where there was a link before) #1693 #3866 --- .../SA1649CodeFixProvider.cs | 26 +++++--- .../DocumentationRules/SA1649UnitTests.cs | 63 +++++++++++++++++++ 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs index 09089b456..6a37daaeb 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs @@ -53,24 +53,30 @@ private static async Task GetTransformedSolutionAsync(Document documen { var solution = document.Project.Solution; var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var expectedFileName = diagnostic.Properties[SA1649FileNameMustMatchTypeName.ExpectedFileNameKey]; - var newPath = document.FilePath != null ? Path.Combine(Path.GetDirectoryName(document.FilePath), expectedFileName) : null; - - var newDocumentId = DocumentId.CreateNewId(document.Id.ProjectId); - var newSolution = solution - .RemoveDocument(document.Id) - .AddDocument(newDocumentId, expectedFileName, syntaxRoot, document.Folders, newPath); + var newSolution = ReplaceDocument(solution, document, document.Id, syntaxRoot, expectedFileName); - // Make sure to also add the file to linked projects + // Make sure to also update other projects which reference the same file foreach (var linkedDocumentId in document.GetLinkedDocumentIds()) { - DocumentId linkedExtractedDocumentId = DocumentId.CreateNewId(linkedDocumentId.ProjectId); - newSolution = newSolution.AddDocument(linkedExtractedDocumentId, expectedFileName, syntaxRoot, document.Folders); + newSolution = ReplaceDocument(newSolution, null, linkedDocumentId, syntaxRoot, expectedFileName); } return newSolution; } + + private static Solution ReplaceDocument(Solution solution, Document document, DocumentId documentId, SyntaxNode syntaxRoot, string expectedFileName) + { + document ??= solution.GetDocument(documentId); + + var newDocumentFilePath = document.FilePath != null ? Path.Combine(Path.GetDirectoryName(document.FilePath), expectedFileName) : null; + var newDocumentId = DocumentId.CreateNewId(documentId.ProjectId); + + var newSolution = solution + .RemoveDocument(documentId) + .AddDocument(newDocumentId, expectedFileName, syntaxRoot, document.Folders, newDocumentFilePath); + return newSolution; + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1649UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1649UnitTests.cs index 0ec463ea3..35e84fbea 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1649UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1649UnitTests.cs @@ -5,6 +5,7 @@ namespace StyleCop.Analyzers.Test.DocumentationRules { + using System.IO; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Testing; @@ -487,6 +488,59 @@ public class Class2 await VerifyCSharpDiagnosticAsync("Class1.cs", testCode, testSettings: null, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); } + [Fact] + [WorkItem(1693, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1693")] + [WorkItem(3866, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3866")] + public async Task VerifyWithLinkedFileAsync() + { + var dirName = "0"; + var testCode = "public class [|Type1|] { }"; + + await new StyleCopCodeFixVerifier.CSharpTest() + { + TestState = + { + Sources = + { + (BuildPath(dirName, "TestFile.cs"), testCode), + }, + AdditionalProjects = + { + ["Project2"] = + { + Sources = + { + (BuildPath(dirName, "TestFile.cs"), testCode), + }, + }, + }, + }, + FixedState = + { + Sources = + { + (BuildPath(dirName, "Type1.cs"), testCode), + }, + AdditionalProjects = + { + ["Project2"] = + { + Sources = + { + (BuildPath(dirName, "Type1.cs"), testCode), + }, + }, + }, + }, + + // Fails without this. Hard to be sure why this is needed, since the error message is not so good, + // but one guess could be that the test framework does not respect the fact that both projects + // point to the same file, and only inserts '#pragma warning disable' in the primary project's file. + // Then we would still get a diagnostic in the additional project. + TestBehaviors = TestBehaviors.SkipSuppressionCheck, + }.RunAsync().ConfigureAwait(false); + } + protected static string GetTypeDeclaration(string typeKind, string typeName, int? diagnosticKey = null) { if (diagnosticKey is not null) @@ -550,5 +604,14 @@ protected static Task VerifyCSharpFixAsync(string oldFileName, string source, st test.ExpectedDiagnostics.AddRange(expected); return test.RunAsync(cancellationToken); } + + // NOTE: Added to simplify the tests. After the fix has executed, + // the file paths will contain backslashes when running tests on Windows. + // Not really needed when setting up the test state, but handy in the fixed state. + // Might make tests pass on Linux if anyone is developing there. + private static string BuildPath(string part1, string part2) + { + return Path.Combine(part1, part2); + } } } From b8587375d995a50c2a53d67679b5ffb2d9f9f2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Hellander?= Date: Thu, 27 Nov 2025 22:03:06 +0100 Subject: [PATCH 2/2] Update SA1649CodeFixProvider to use method WithDocumentName in Solution if it is available #2277 --- .../SA1649CodeFixProvider.cs | 22 +++-- .../Lightup/SolutionExtensions.cs | 23 +++++ .../Lightup/LightupHelpers.cs | 92 +++++++++++++++++++ 3 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Lightup/SolutionExtensions.cs diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs index 6a37daaeb..fe5fb784f 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs @@ -1,8 +1,6 @@ // Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. // Licensed under the MIT License. See LICENSE in the project root for license information. -#nullable disable - namespace StyleCop.Analyzers.DocumentationRules { using System.Collections.Immutable; @@ -14,6 +12,7 @@ namespace StyleCop.Analyzers.DocumentationRules using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using StyleCop.Analyzers.Helpers; + using StyleCop.Analyzers.Lightup; /// /// Implements a code fix for . @@ -27,7 +26,7 @@ internal class SA1649CodeFixProvider : CodeFixProvider ImmutableArray.Create(SA1649FileNameMustMatchTypeName.DiagnosticId); /// - public override FixAllProvider GetFixAllProvider() + public override FixAllProvider? GetFixAllProvider() { // The batch fixer can't handle code fixes that create new files return null; @@ -55,25 +54,32 @@ private static async Task GetTransformedSolutionAsync(Document documen var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var expectedFileName = diagnostic.Properties[SA1649FileNameMustMatchTypeName.ExpectedFileNameKey]; - var newSolution = ReplaceDocument(solution, document, document.Id, syntaxRoot, expectedFileName); + var newSolution = RenameDocument(solution, document, document.Id, syntaxRoot, expectedFileName); // Make sure to also update other projects which reference the same file foreach (var linkedDocumentId in document.GetLinkedDocumentIds()) { - newSolution = ReplaceDocument(newSolution, null, linkedDocumentId, syntaxRoot, expectedFileName); + newSolution = RenameDocument(newSolution, null, linkedDocumentId, syntaxRoot, expectedFileName); } return newSolution; } - private static Solution ReplaceDocument(Solution solution, Document document, DocumentId documentId, SyntaxNode syntaxRoot, string expectedFileName) + private static Solution RenameDocument(Solution solution, Document? document, DocumentId documentId, SyntaxNode syntaxRoot, string expectedFileName) { - document ??= solution.GetDocument(documentId); + // First try to use the "new" WithDocumentName method. This will return null if it is not available in the current Roslyn version. + var newSolution = solution.WithDocumentName(documentId, expectedFileName); + if (newSolution != null) + { + return newSolution; + } + // Continue by instead removing and re-adding the file again + document ??= solution.GetDocument(documentId); var newDocumentFilePath = document.FilePath != null ? Path.Combine(Path.GetDirectoryName(document.FilePath), expectedFileName) : null; var newDocumentId = DocumentId.CreateNewId(documentId.ProjectId); - var newSolution = solution + newSolution = solution .RemoveDocument(documentId) .AddDocument(newDocumentId, expectedFileName, syntaxRoot, document.Folders, newDocumentFilePath); return newSolution; diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Lightup/SolutionExtensions.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Lightup/SolutionExtensions.cs new file mode 100644 index 000000000..191841a70 --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Lightup/SolutionExtensions.cs @@ -0,0 +1,23 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +namespace StyleCop.Analyzers.Lightup +{ + using System; + using Microsoft.CodeAnalysis; + + internal static class SolutionExtensions + { + private static readonly Func WithDocumentNameAccessor; + + static SolutionExtensions() + { + WithDocumentNameAccessor = LightupHelpers.CreateSyntaxPropertyAccessor(typeof(Solution), typeof(DocumentId), typeof(string), nameof(WithDocumentName)); + } + + public static Solution WithDocumentName(this Solution solution, DocumentId documentId, string name) + { + return WithDocumentNameAccessor(solution, documentId, name); + } + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs index ef27b7cdd..13a0e2a39 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs @@ -394,6 +394,98 @@ static TProperty FallbackAccessor(TSyntax syntax, TArg argument) return expression.Compile(); } + internal static Func CreateSyntaxPropertyAccessor(Type type, Type argumentType1, Type argumentType2, string accessorMethodName) + { + static TProperty FallbackAccessor(TSyntax syntax, TArg1 argument1, TArg2 argument2) + { + if (syntax == null) + { + // Unlike an extension method which would throw ArgumentNullException here, the light-up + // behavior needs to match behavior of the underlying property. + throw new NullReferenceException(); + } + + return default; + } + + if (type == null) + { + return FallbackAccessor; + } + + if (!typeof(TSyntax).GetTypeInfo().IsAssignableFrom(type.GetTypeInfo())) + { + throw new InvalidOperationException(); + } + + if (!typeof(TArg1).GetTypeInfo().IsAssignableFrom(argumentType1.GetTypeInfo())) + { + throw new InvalidOperationException(); + } + + if (!typeof(TArg2).GetTypeInfo().IsAssignableFrom(argumentType2.GetTypeInfo())) + { + throw new InvalidOperationException(); + } + + var methods = type.GetTypeInfo().GetDeclaredMethods(accessorMethodName); + MethodInfo method = null; + foreach (var candidate in methods) + { + var parameters = candidate.GetParameters(); + if (parameters.Length != 2) + { + continue; + } + + if (!Equals(argumentType1, parameters[0].ParameterType)) + { + continue; + } + + if (!Equals(argumentType2, parameters[1].ParameterType)) + { + continue; + } + + method = candidate; + } + + if (method == null) + { + return FallbackAccessor; + } + + if (!typeof(TProperty).GetTypeInfo().IsAssignableFrom(method.ReturnType.GetTypeInfo())) + { + throw new InvalidOperationException(); + } + + var syntaxParameter = Expression.Parameter(typeof(TSyntax), "syntax"); + var arg1Parameter = Expression.Parameter(typeof(TArg1), "arg1"); + var arg2Parameter = Expression.Parameter(typeof(TArg2), "arg2"); + Expression instance = + type.GetTypeInfo().IsAssignableFrom(typeof(TSyntax).GetTypeInfo()) + ? (Expression)syntaxParameter + : Expression.Convert(syntaxParameter, type); + Expression argument1 = + argumentType1.GetTypeInfo().IsAssignableFrom(typeof(TArg1).GetTypeInfo()) + ? (Expression)arg1Parameter + : Expression.Convert(arg1Parameter, argumentType1); + Expression argument2 = + argumentType2.GetTypeInfo().IsAssignableFrom(typeof(TArg2).GetTypeInfo()) + ? (Expression)arg2Parameter + : Expression.Convert(arg2Parameter, argumentType2); + + Expression> expression = + Expression.Lambda>( + Expression.Call(instance, method, argument1, argument2), + syntaxParameter, + arg1Parameter, + arg2Parameter); + return expression.Compile(); + } + internal static TryGetValueAccessor CreateTryGetValueAccessor(Type type, Type keyType, string methodName) { static bool FallbackAccessor(TSyntax syntax, TKey key, out TValue value)