Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update filenames in file banners when moving a type to a new file. #76270

Merged
merged 3 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,10 @@ namespace Microsoft.CodeAnalysis.CSharp.FileHeaders;
/// <summary>
/// Helper class used for working with file headers.
/// </summary>
internal sealed class CSharpFileHeaderHelper : AbstractFileHeaderHelper
internal sealed class CSharpFileHeaderHelper() : AbstractFileHeaderHelper(CSharpSyntaxKinds.Instance)
{
public static readonly CSharpFileHeaderHelper Instance = new();

private CSharpFileHeaderHelper()
: base(CSharpSyntaxKinds.Instance)
{
}

public override string CommentPrefix => "//";

protected override ReadOnlyMemory<char> GetTextContextOfComment(SyntaxTrivia commentTrivia)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,8 @@

namespace Microsoft.CodeAnalysis.FileHeaders;

internal abstract class AbstractFileHeaderHelper
internal abstract class AbstractFileHeaderHelper(ISyntaxKinds syntaxKinds)
{
protected AbstractFileHeaderHelper(ISyntaxKinds syntaxKinds)
{
SingleLineCommentTriviaKind = syntaxKinds.SingleLineCommentTrivia;
MultiLineCommentTriviaKind = syntaxKinds.MultiLineCommentTrivia;
WhitespaceTriviaKind = syntaxKinds.WhitespaceTrivia;
EndOfLineTriviaKind = syntaxKinds.EndOfLineTrivia;
}

/// <summary>
/// Gets the text prefix indicating a single-line comment.
/// </summary>
Expand All @@ -27,16 +19,16 @@ protected AbstractFileHeaderHelper(ISyntaxKinds syntaxKinds)
protected abstract ReadOnlyMemory<char> GetTextContextOfComment(SyntaxTrivia commentTrivia);

/// <inheritdoc cref="ISyntaxKinds.SingleLineCommentTrivia"/>
private int SingleLineCommentTriviaKind { get; }
private int SingleLineCommentTriviaKind { get; } = syntaxKinds.SingleLineCommentTrivia;

/// <inheritdoc cref="ISyntaxKinds.MultiLineCommentTrivia"/>
private int? MultiLineCommentTriviaKind { get; }
private int? MultiLineCommentTriviaKind { get; } = syntaxKinds.MultiLineCommentTrivia;

/// <inheritdoc cref="ISyntaxKinds.WhitespaceTrivia"/>
private int WhitespaceTriviaKind { get; }
private int WhitespaceTriviaKind { get; } = syntaxKinds.WhitespaceTrivia;

/// <inheritdoc cref="ISyntaxKinds.EndOfLineTrivia"/>
private int EndOfLineTriviaKind { get; }
private int EndOfLineTriviaKind { get; } = syntaxKinds.EndOfLineTrivia;

public FileHeader ParseFileHeader(SyntaxNode root)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class Class1 { }

await TestMoveTypeToNewFileAsync(
code, codeAfterMove, expectedDocumentName,
destinationDocumentText, destinationDocumentContainers: ImmutableArray.Create("A", "B"));
destinationDocumentText, destinationDocumentContainers: ["A", "B"]);
}

[WpfFact]
Expand Down Expand Up @@ -2012,4 +2012,37 @@ partial interface I
""";
await TestMoveTypeToNewFileAsync(code, codeAfterMove, expectedDocumentName, destinationDocumentText);
}

[WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/74703")]
public async Task TestUpdateFileName()
{
var code =
"""

<Workspace>
<Project Language="C#" AssemblyName="Assembly1" CommonReferences="true">
<Document Folders="A\B" FilePath="Goo.cs">// This is a banner referencing Goo.cs
[||]class Class1 { }
class Class2 { }
</Document>
</Project>
</Workspace>
""";
var codeAfterMove = """
// This is a banner referencing Goo.cs
class Class2 { }

""";

var expectedDocumentName = "Class1.cs";
var destinationDocumentText = """
// This is a banner referencing Class1.cs
class Class1 { }

""";

await TestMoveTypeToNewFileAsync(
code, codeAfterMove, expectedDocumentName,
destinationDocumentText, destinationDocumentContainers: ["A", "B"]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,10 @@ namespace Microsoft.CodeAnalysis.CSharp.AddFileBanner;

[ExportCodeRefactoringProvider(LanguageNames.CSharp,
Name = PredefinedCodeRefactoringProviderNames.AddFileBanner), Shared]
internal class CSharpAddFileBannerCodeRefactoringProvider : AbstractAddFileBannerCodeRefactoringProvider
[method: ImportingConstructor]
[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
internal sealed class CSharpAddFileBannerCodeRefactoringProvider() : AbstractAddFileBannerCodeRefactoringProvider
{
[ImportingConstructor]
[SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
public CSharpAddFileBannerCodeRefactoringProvider()
{
}

protected override bool IsCommentStartCharacter(char ch)
=> ch == '/';

protected override SyntaxTrivia CreateTrivia(SyntaxTrivia trivia, string text)
{
switch (trivia.Kind())
{
case SyntaxKind.SingleLineCommentTrivia:
case SyntaxKind.MultiLineCommentTrivia:
case SyntaxKind.SingleLineDocumentationCommentTrivia:
case SyntaxKind.MultiLineDocumentationCommentTrivia:
return SyntaxFactory.Comment(text);
}

return trivia;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,10 @@
namespace Microsoft.CodeAnalysis.CSharp.AddFileBanner;

[ExportNewDocumentFormattingProvider(LanguageNames.CSharp), Shared]
internal class CSharpAddFileBannerNewDocumentFormattingProvider : AbstractAddFileBannerNewDocumentFormattingProvider
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed class CSharpAddFileBannerNewDocumentFormattingProvider() : AbstractAddFileBannerNewDocumentFormattingProvider
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public CSharpAddFileBannerNewDocumentFormattingProvider()
{
}

protected override SyntaxGenerator SyntaxGenerator => CSharpSyntaxGenerator.Instance;
protected override SyntaxGeneratorInternal SyntaxGeneratorInternal => CSharpSyntaxGeneratorInternal.Instance;
protected override AbstractFileHeaderHelper FileHeaderHelper => CSharpFileHeaderHelper.Instance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ internal abstract class AbstractAddFileBannerCodeRefactoringProvider : SyntaxEdi

protected abstract bool IsCommentStartCharacter(char ch);

protected abstract SyntaxTrivia CreateTrivia(SyntaxTrivia trivia, string text);

protected sealed override ImmutableArray<FixAllScope> SupportedFixAllScopes { get; }
= [FixAllScope.Project, FixAllScope.Solution];

Expand All @@ -53,15 +51,10 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
var position = span.Start;
var firstToken = root.GetFirstToken();
if (!firstToken.FullSpan.IntersectsWith(position))
{
return;
}

if (HasExistingBanner(document, root))
{
// Already has a banner.
return;
}

// Process the other documents in this document's project. Look at the
// ones that we can get a root from (without having to parse). Then
Expand All @@ -86,9 +79,10 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
context.RegisterRefactoring(
CodeAction.Create(
CodeFixesResources.Add_file_header,
_ => AddBannerAsync(document, root, siblingDocument, siblingBanner),
cancellationToken => AddFileBannerHelpers.CopyBannerAsync(
destinationDocument: document, document.FilePath, sourceDocument: siblingDocument, cancellationToken),
equivalenceKey: GetEquivalenceKey(siblingDocument, siblingBanner)),
new Text.TextSpan(position, length: 0));
new TextSpan(position, length: 0));
return;
}
}
Expand Down Expand Up @@ -125,38 +119,6 @@ private static ImmutableArray<SyntaxTrivia> GetBannerFromEquivalenceKey(string e
return bannerService.GetFileBanner(token);
}

private Task<Document> AddBannerAsync(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to a common location so it can be used in MoveType.

Document document, SyntaxNode root,
Document siblingDocument, ImmutableArray<SyntaxTrivia> banner)
{
banner = UpdateEmbeddedFileNames(siblingDocument, document, banner);

var newRoot = root.WithPrependedLeadingTrivia(new SyntaxTriviaList(banner));
return Task.FromResult(document.WithSyntaxRoot(newRoot));
}

/// <summary>
/// Looks at <paramref name="banner"/> to see if it contains the name of <paramref name="sourceDocument"/>
/// in it. If so, those names will be replaced with <paramref name="destinationDocument"/>'s name.
/// </summary>
private ImmutableArray<SyntaxTrivia> UpdateEmbeddedFileNames(
Document sourceDocument, Document destinationDocument, ImmutableArray<SyntaxTrivia> banner)
{
var sourceName = IOUtilities.PerformIO(() => Path.GetFileName(sourceDocument.FilePath));
var destinationName = IOUtilities.PerformIO(() => Path.GetFileName(destinationDocument.FilePath));
if (string.IsNullOrEmpty(sourceName) || string.IsNullOrEmpty(destinationName))
return banner;

var result = new FixedSizeArrayBuilder<SyntaxTrivia>(banner.Length);
foreach (var trivia in banner)
{
var updated = CreateTrivia(trivia, trivia.ToFullString().Replace(sourceName, destinationName));
result.Add(updated);
}

return result.MoveToImmutable();
}

private async Task<ImmutableArray<SyntaxTrivia>> TryGetBannerAsync(
Document document, SyntaxNode? root, CancellationToken cancellationToken)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ public async Task<Document> FormatNewDocumentAsync(Document document, Document?
{
var newLineTrivia = SyntaxGeneratorInternal.EndOfLine(options.FormattingOptions.NewLine);
var rootWithFileHeader = await AbstractFileHeaderCodeFixProvider.GetTransformedSyntaxRootAsync(
SyntaxGenerator.SyntaxFacts,
FileHeaderHelper,
newLineTrivia,
document,
fileHeaderTemplate,
cancellationToken).ConfigureAwait(false);
SyntaxGenerator.SyntaxFacts,
FileHeaderHelper,
newLineTrivia,
document,
fileHeaderTemplate,
cancellationToken).ConfigureAwait(false);

return document.WithSyntaxRoot(rootWithFileHeader);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Utilities;

namespace Microsoft.CodeAnalysis.AddFileBanner;

internal static class AddFileBannerHelpers
{
public static async Task<Document> CopyBannerAsync(
Document destinationDocument,
string? destinationFilePath,
Document sourceDocument,
CancellationToken cancellationToken)
{
var bannerService = destinationDocument.GetRequiredLanguageService<IFileBannerFactsService>();

var fromRoot = await sourceDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var sourceBanner = bannerService.GetFileBanner(fromRoot);

sourceBanner = UpdateEmbeddedFileNames(
sourceDocument, destinationFilePath, sourceBanner, bannerService.CreateTrivia);

var destinationRoot = await destinationDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var destinationBanner = bannerService.GetFileBanner(destinationRoot);

var firstToken = destinationRoot.GetFirstToken();
var newRoot = destinationRoot.ReplaceToken(
firstToken,
firstToken.WithLeadingTrivia(
sourceBanner.Concat(firstToken.LeadingTrivia.Skip(destinationBanner.Length))));
return destinationDocument.WithSyntaxRoot(newRoot);
}

/// <summary>
/// Looks at <paramref name="banner"/> to see if it contains the name of <paramref name="sourceDocument"/>
/// in it. If so, those names will be replaced with <paramref name="destinationFilePath"/>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with

nit: with -> "with the file name from "

/// </summary>
private static ImmutableArray<SyntaxTrivia> UpdateEmbeddedFileNames(
Document sourceDocument,
string? destinationFilePath,
ImmutableArray<SyntaxTrivia> banner,
Func<SyntaxTrivia, string, SyntaxTrivia> createTrivia)
{
var sourceName = IOUtilities.PerformIO(() => Path.GetFileName(sourceDocument.FilePath));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path.GetFileName

didn't change in this PR, but I think Path.GetFileName doesn't do any IO, and won't throw those exceptions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do a pass later to address these

var destinationName = IOUtilities.PerformIO(() => Path.GetFileName(destinationFilePath));
if (string.IsNullOrEmpty(sourceName) || string.IsNullOrEmpty(destinationName))
return banner;

var result = new FixedSizeArrayBuilder<SyntaxTrivia>(banner.Length);
foreach (var trivia in banner)
{
var updated = createTrivia(trivia, trivia.ToFullString().Replace(sourceName, destinationName));
result.Add(updated);
}

return result.MoveToImmutable();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.AddFileBanner;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.LanguageService;
Expand All @@ -27,7 +28,6 @@ private sealed class MoveTypeEditor(
string fileName,
CancellationToken cancellationToken) : Editor(service, state, fileName, cancellationToken)
{

/// <summary>
/// Given a document and a type contained in it, moves the type
/// out to its own document. The new document's name typically
Expand Down Expand Up @@ -111,8 +111,7 @@ private async Task<Solution> RemoveUnnecessaryImportsAsync(
private async Task<Document> AddNewDocumentWithSingleTypeDeclarationAsync(DocumentId newDocumentId)
{
var document = SemanticDocument.Document;
Debug.Assert(document.Name != FileName,
$"New document name is same as old document name:{FileName}");
Debug.Assert(document.Name != FileName, $"New document name is same as old document name:{FileName}");

var root = SemanticDocument.Root;
var projectToBeUpdated = document.Project;
Expand Down Expand Up @@ -146,7 +145,10 @@ private async Task<Document> AddNewDocumentWithSingleTypeDeclarationAsync(Docume
// get the updated document, give it the minimal set of imports that the type
// inside it needs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment confuses me, it doesn't look like imports are handled in this method

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Seems incorrect. Was already like that, so I'll move to another pr in this space to cleanup.

var newDocument = solutionWithNewDocument.GetRequiredDocument(newDocumentId);
return newDocument;
var newDocumentWithUpdatedBanner = await AddFileBannerHelpers.CopyBannerAsync(
newDocument, FileName, document, this.CancellationToken).ConfigureAwait(false);

return newDocumentWithUpdatedBanner;
}

/// <summary>
Expand Down Expand Up @@ -194,6 +196,7 @@ private async Task<Solution> RemoveTypeFromSourceDocumentAsync(Document sourceDo
documentEditor.RemoveNode(State.TypeNode, SyntaxRemoveOptions.KeepUnbalancedDirectives);

var updatedDocument = documentEditor.GetChangedDocument();
updatedDocument = await AddFileBannerHelpers.CopyBannerAsync(updatedDocument, sourceDocument.FilePath, sourceDocument, this.CancellationToken).ConfigureAwait(false);

return updatedDocument.Project.Solution;
}
Expand Down
Loading
Loading