diff --git a/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.Impl.cs b/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.Impl.cs index 9dc03d62da..4737d0cc97 100644 --- a/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.Impl.cs +++ b/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.Impl.cs @@ -257,12 +257,12 @@ private void OnSymbolActionCore(ISymbol symbol, Action reportDiagnos } else { - reportAnnotateApi(symbol, isImplicitlyDeclaredConstructor, publicApiName, foundApiLine.IsShippedApi); + reportAnnotateApi(symbol, isImplicitlyDeclaredConstructor, publicApiName, foundApiLine.IsShippedApi, foundApiLine.Path); } } else if (hasPublicApiEntryWithNullability && symbolUsesOblivious) { - reportAnnotateApi(symbol, isImplicitlyDeclaredConstructor, publicApiName, foundApiLine.IsShippedApi); + reportAnnotateApi(symbol, isImplicitlyDeclaredConstructor, publicApiName, foundApiLine.IsShippedApi, foundApiLine.Path); } } else @@ -395,7 +395,7 @@ void reportDeclareNewApi(ISymbol symbol, bool isImplicitlyDeclaredConstructor, s reportDiagnosticAtLocations(DeclareNewApiRule, propertyBag, errorMessageName); } - void reportAnnotateApi(ISymbol symbol, bool isImplicitlyDeclaredConstructor, ApiName publicApiName, bool isShipped) + void reportAnnotateApi(ISymbol symbol, bool isImplicitlyDeclaredConstructor, ApiName publicApiName, bool isShipped, string filename) { // Public API missing annotations in public API file - report diagnostic. string errorMessageName = GetErrorMessageName(symbol, isImplicitlyDeclaredConstructor); @@ -403,7 +403,8 @@ void reportAnnotateApi(ISymbol symbol, bool isImplicitlyDeclaredConstructor, Api .Add(PublicApiNamePropertyBagKey, publicApiName.Name) .Add(PublicApiNameWithNullabilityPropertyBagKey, withObliviousIfNeeded(publicApiName.NameWithNullability)) .Add(MinimalNamePropertyBagKey, errorMessageName) - .Add(PublicApiIsShippedPropertyBagKey, isShipped ? "true" : "false"); + .Add(PublicApiIsShippedPropertyBagKey, isShipped ? "true" : "false") + .Add(FileName, filename); reportDiagnosticAtLocations(AnnotateApiRule, propertyBag, errorMessageName); } diff --git a/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs b/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs index 781e76b88a..d6eea8fd97 100644 --- a/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs +++ b/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs @@ -5,7 +5,6 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.IO; using System.Linq; using System.Reflection; using System.Threading; @@ -21,8 +20,11 @@ namespace Microsoft.CodeAnalysis.PublicApiAnalyzers [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] public sealed partial class DeclarePublicApiAnalyzer : DiagnosticAnalyzer { + internal const string ShippedFileNamePrefix = "PublicAPI.Shipped"; internal const string ShippedFileName = "PublicAPI.Shipped.txt"; - internal const string UnshippedFileName = "PublicAPI.Unshipped.txt"; + internal const string UnshippedFileNamePrefix = "PublicAPI.Unshipped"; + internal const string Extension = ".txt"; + internal const string UnshippedFileName = UnshippedFileNamePrefix + Extension; internal const string PublicApiNamePropertyBagKey = "PublicAPIName"; internal const string PublicApiNameWithNullabilityPropertyBagKey = "PublicAPINameWithNullability"; internal const string MinimalNamePropertyBagKey = "MinimalName"; @@ -33,6 +35,7 @@ public sealed partial class DeclarePublicApiAnalyzer : DiagnosticAnalyzer internal const string InvalidReasonShippedCantHaveRemoved = "The shipped API file can't have removed members"; internal const string InvalidReasonMisplacedNullableEnable = "The '#nullable enable' marker can only appear as the first line in the shipped API file"; internal const string PublicApiIsShippedPropertyBagKey = "PublicAPIIsShipped"; + internal const string FileName = "FileName"; private const char ObliviousMarker = '~'; @@ -258,38 +261,42 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte compilationContext.RegisterCompilationEndAction(impl.OnCompilationEnd); } - private static ApiData ReadApiData(string path, SourceText sourceText, bool isShippedApi) + private static ApiData ReadApiData(List<(string path, SourceText sourceText)> data, bool isShippedApi) { var apiBuilder = ImmutableArray.CreateBuilder(); var removedBuilder = ImmutableArray.CreateBuilder(); var maxNullableRank = -1; - int rank = -1; - foreach (TextLine line in sourceText.Lines) + foreach (var (path, sourceText) in data) { - string text = line.ToString(); - if (string.IsNullOrWhiteSpace(text)) + int rank = -1; + + foreach (TextLine line in sourceText.Lines) { - continue; - } + string text = line.ToString(); + if (string.IsNullOrWhiteSpace(text)) + { + continue; + } - rank++; + rank++; - if (text == NullableEnable) - { - maxNullableRank = rank; - continue; - } + if (text == NullableEnable) + { + maxNullableRank = Math.Max(rank, maxNullableRank); + continue; + } - var apiLine = new ApiLine(text, line.Span, sourceText, path, isShippedApi); - if (text.StartsWith(RemovedApiPrefix, StringComparison.Ordinal)) - { - string removedtext = text[RemovedApiPrefix.Length..]; - removedBuilder.Add(new RemovedApiLine(removedtext, apiLine)); - } - else - { - apiBuilder.Add(apiLine); + var apiLine = new ApiLine(text, line.Span, sourceText, path, isShippedApi); + if (text.StartsWith(RemovedApiPrefix, StringComparison.Ordinal)) + { + string removedtext = text[RemovedApiPrefix.Length..]; + removedBuilder.Add(new RemovedApiLine(removedtext, apiLine)); + } + else + { + apiBuilder.Add(apiLine); + } } } @@ -322,8 +329,8 @@ private static bool TryGetApiData(AnalyzerOptions analyzerOptions, Compilation c return false; } - shippedData = ReadApiData(shippedText.Value.path, shippedText.Value.text, isShippedApi: true); - unshippedData = ReadApiData(unshippedText.Value.path, unshippedText.Value.text, isShippedApi: false); + shippedData = ReadApiData(shippedText, isShippedApi: true); + unshippedData = ReadApiData(unshippedText, isShippedApi: false); return true; } @@ -380,42 +387,47 @@ private static bool TryGetEditorConfigOptionForMissingFiles(AnalyzerOptions anal private static bool TryGetApiText( ImmutableArray additionalTexts, CancellationToken cancellationToken, - [NotNullWhen(returnValue: true)] out (string path, SourceText text)? shippedText, - [NotNullWhen(returnValue: true)] out (string path, SourceText text)? unshippedText) + [NotNullWhen(returnValue: true)] out List<(string path, SourceText text)>? shippedText, + [NotNullWhen(returnValue: true)] out List<(string path, SourceText text)>? unshippedText) { shippedText = null; unshippedText = null; - StringComparer comparer = StringComparer.Ordinal; foreach (AdditionalText additionalText in additionalTexts) { cancellationToken.ThrowIfCancellationRequested(); - string fileName = Path.GetFileName(additionalText.Path); + var file = new PublicApiFile(additionalText.Path); - bool isShippedFile = comparer.Equals(fileName, ShippedFileName); - bool isUnshippedFile = comparer.Equals(fileName, UnshippedFileName); - - if (isShippedFile || isUnshippedFile) + if (file.IsApiFile) { SourceText text = additionalText.GetText(cancellationToken); - if (text == null) + if (text is null) { continue; } var data = (additionalText.Path, text); - if (isShippedFile) + + if (file.IsShipping) { - shippedText = data; - } + if (shippedText is null) + { + shippedText = new(); + } - if (isUnshippedFile) + shippedText.Add(data); + } + else { - unshippedText = data; + if (unshippedText is null) + { + unshippedText = new(); + } + + unshippedText.Add(data); } - continue; } } diff --git a/src/PublicApiAnalyzers/Core/Analyzers/PublicApiFile.cs b/src/PublicApiAnalyzers/Core/Analyzers/PublicApiFile.cs new file mode 100644 index 0000000000..a11477daae --- /dev/null +++ b/src/PublicApiAnalyzers/Core/Analyzers/PublicApiFile.cs @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.IO; + +namespace Microsoft.CodeAnalysis.PublicApiAnalyzers +{ + public readonly struct PublicApiFile + { + public PublicApiFile(string path) + { + var fileName = Path.GetFileName(path); + + IsShipping = IsFile(fileName, DeclarePublicApiAnalyzer.ShippedFileNamePrefix); + var isUnshippedFile = IsFile(fileName, DeclarePublicApiAnalyzer.UnshippedFileNamePrefix); + + IsApiFile = IsShipping || isUnshippedFile; + } + + public bool IsShipping { get; } + + public bool IsApiFile { get; } + + private static bool IsFile(string path, string prefix) + => path.StartsWith(prefix, StringComparison.Ordinal) && path.EndsWith(DeclarePublicApiAnalyzer.Extension, StringComparison.Ordinal); + } +} diff --git a/src/PublicApiAnalyzers/Core/CodeFixes/AnnotatePublicApiFix.cs b/src/PublicApiAnalyzers/Core/CodeFixes/AnnotatePublicApiFix.cs index ecee97d4db..baf7fd352e 100644 --- a/src/PublicApiAnalyzers/Core/CodeFixes/AnnotatePublicApiFix.cs +++ b/src/PublicApiAnalyzers/Core/CodeFixes/AnnotatePublicApiFix.cs @@ -8,10 +8,10 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using Analyzer.Utilities.PooledObjects; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Text; + using DiagnosticIds = Roslyn.Diagnostics.Analyzers.RoslynDiagnosticIds; #nullable enable @@ -38,15 +38,16 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) string minimalSymbolName = diagnostic.Properties[DeclarePublicApiAnalyzer.MinimalNamePropertyBagKey]; string publicSymbolName = diagnostic.Properties[DeclarePublicApiAnalyzer.PublicApiNamePropertyBagKey]; string publicSymbolNameWithNullability = diagnostic.Properties[DeclarePublicApiAnalyzer.PublicApiNameWithNullabilityPropertyBagKey]; - bool isShippedDocument = diagnostic.Properties[DeclarePublicApiAnalyzer.PublicApiIsShippedPropertyBagKey] == "true"; + string fileName = diagnostic.Properties[DeclarePublicApiAnalyzer.FileName]; - TextDocument? document = isShippedDocument ? PublicApiFixHelpers.GetShippedDocument(project) : PublicApiFixHelpers.GetUnshippedDocument(project); + TextDocument? document = project.GetPublicApiDocument(fileName); if (document != null) { context.RegisterCodeFix( new DeclarePublicApiFix.AdditionalDocumentChangeAction( $"Annotate {minimalSymbolName} in public API", + document.Id, c => GetFix(document, publicSymbolName, publicSymbolNameWithNullability, c)), diagnostic); } @@ -81,8 +82,8 @@ private static SourceText AnnotateSymbolNamesInSourceText(SourceText sourceText, } } - var endOfLine = PublicApiFixHelpers.GetEndOfLine(sourceText); - SourceText newSourceText = sourceText.Replace(new TextSpan(0, sourceText.Length), string.Join(endOfLine, lines) + PublicApiFixHelpers.GetEndOfFileText(sourceText, endOfLine)); + var endOfLine = sourceText.GetEndOfLine(); + SourceText newSourceText = sourceText.Replace(new TextSpan(0, sourceText.Length), string.Join(endOfLine, lines) + sourceText.GetEndOfFileText(endOfLine)); return newSourceText; } @@ -102,53 +103,22 @@ public FixAllAdditionalDocumentChangeAction(string title, Solution solution, Lis protected override async Task GetChangedSolutionAsync(CancellationToken cancellationToken) { - var updatedPublicSurfaceAreaText = new List>(); - - using var uniqueShippedDocuments = PooledHashSet.GetInstance(); - using var uniqueUnshippedDocuments = PooledHashSet.GetInstance(); + var updatedPublicSurfaceAreaText = new List<(DocumentId, SourceText)>(); - foreach (KeyValuePair> pair in _diagnosticsToFix) + foreach (var (project, diagnostics) in _diagnosticsToFix) { - Project project = pair.Key; - ImmutableArray diagnostics = pair.Value; - - TextDocument? unshippedDocument = PublicApiFixHelpers.GetUnshippedDocument(project); - if (unshippedDocument?.FilePath != null && !uniqueUnshippedDocuments.Add(unshippedDocument.FilePath)) - { - // Skip past duplicate unshipped documents. - // Multi-tfm projects can likely share the same api files, and we want to avoid duplicate code fix application. - unshippedDocument = null; - } - - TextDocument? shippedDocument = PublicApiFixHelpers.GetShippedDocument(project); - if (shippedDocument?.FilePath != null && !uniqueShippedDocuments.Add(shippedDocument.FilePath)) - { - // Skip past duplicate shipped documents. - // Multi-tfm projects can likely share the same api files, and we want to avoid duplicate code fix application. - shippedDocument = null; - } - - if (unshippedDocument == null && shippedDocument == null) - { - continue; - } - - SourceText? unshippedSourceText = unshippedDocument is null ? null : await unshippedDocument.GetTextAsync(cancellationToken).ConfigureAwait(false); - SourceText? shippedSourceText = shippedDocument is null ? null : await shippedDocument.GetTextAsync(cancellationToken).ConfigureAwait(false); - IEnumerable> groupedDiagnostics = diagnostics .Where(d => d.Location.IsInSource) .GroupBy(d => d.Location.SourceTree); - var shippedChanges = new Dictionary(); - var unshippedChanges = new Dictionary(); + var allChanges = new Dictionary>(); foreach (IGrouping grouping in groupedDiagnostics) { Document document = project.GetDocument(grouping.Key); - if (document == null) + if (document is null) { continue; } @@ -166,29 +136,36 @@ protected override async Task GetChangedSolutionAsync(CancellationToke string oldName = diagnostic.Properties[DeclarePublicApiAnalyzer.PublicApiNamePropertyBagKey]; string newName = diagnostic.Properties[DeclarePublicApiAnalyzer.PublicApiNameWithNullabilityPropertyBagKey]; bool isShipped = diagnostic.Properties[DeclarePublicApiAnalyzer.PublicApiIsShippedPropertyBagKey] == "true"; - var mapToUpdate = isShipped ? shippedChanges : unshippedChanges; + string fileName = diagnostic.Properties[DeclarePublicApiAnalyzer.FileName]; + + if (!allChanges.TryGetValue(fileName, out var mapToUpdate)) + { + mapToUpdate = new(); + allChanges.Add(fileName, mapToUpdate); + } + mapToUpdate[oldName] = newName; } } - if (shippedSourceText is object) + foreach (var (path, changes) in allChanges) { - SourceText newShippedSourceText = AnnotateSymbolNamesInSourceText(shippedSourceText, shippedChanges); - updatedPublicSurfaceAreaText.Add(new KeyValuePair(shippedDocument!.Id, newShippedSourceText)); - } + var doc = project.GetPublicApiDocument(path); - if (unshippedSourceText is object) - { - SourceText newUnshippedSourceText = AnnotateSymbolNamesInSourceText(unshippedSourceText, unshippedChanges); - updatedPublicSurfaceAreaText.Add(new KeyValuePair(unshippedDocument!.Id, newUnshippedSourceText)); + if (doc is not null) + { + var text = await doc.GetTextAsync(cancellationToken).ConfigureAwait(false); + SourceText newShippedSourceText = AnnotateSymbolNamesInSourceText(text, changes); + updatedPublicSurfaceAreaText.Add((doc.Id, newShippedSourceText)); + } } } Solution newSolution = _solution; - foreach (KeyValuePair pair in updatedPublicSurfaceAreaText) + foreach (var (docId, text) in updatedPublicSurfaceAreaText) { - newSolution = newSolution.WithAdditionalDocumentText(pair.Key, pair.Value); + newSolution = newSolution.WithAdditionalDocumentText(docId, text); } return newSolution; diff --git a/src/PublicApiAnalyzers/Core/CodeFixes/DeclarePublicApiFix.cs b/src/PublicApiAnalyzers/Core/CodeFixes/DeclarePublicApiFix.cs index 1674969880..c65f35100c 100644 --- a/src/PublicApiAnalyzers/Core/CodeFixes/DeclarePublicApiFix.cs +++ b/src/PublicApiAnalyzers/Core/CodeFixes/DeclarePublicApiFix.cs @@ -31,7 +31,6 @@ public sealed override FixAllProvider GetFixAllProvider() public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) { var project = context.Document.Project; - var publicSurfaceAreaDocument = PublicApiFixHelpers.GetUnshippedDocument(project); foreach (Diagnostic diagnostic in context.Diagnostics) { @@ -41,16 +40,41 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) .Split(DeclarePublicApiAnalyzer.PublicApiNamesOfSiblingsToRemovePropertyBagValueSeparator.ToCharArray()) .ToImmutableHashSet(); - context.RegisterCodeFix( - new AdditionalDocumentChangeAction( - $"Add {minimalSymbolName} to public API", - c => GetFixAsync(publicSurfaceAreaDocument, project, publicSurfaceAreaSymbolName, siblingSymbolNamesToRemove, c)), - diagnostic); + foreach (var file in GetUnshippedPublicApiFiles(context.Document.Project)) + { + context.RegisterCodeFix( + new AdditionalDocumentChangeAction( + $"Add {minimalSymbolName} to public API file {file?.Name}", + file?.Id, + c => GetFixAsync(file, project, publicSurfaceAreaSymbolName, siblingSymbolNamesToRemove, c)), + diagnostic); + } } return Task.CompletedTask; } + private static IEnumerable GetUnshippedPublicApiFiles(Project project) + { + var count = 0; + + foreach (var additional in project.AdditionalDocuments) + { + var file = new PublicApiFile(additional.FilePath); + + if (file.IsApiFile && !file.IsShipping) + { + yield return additional; + count++; + } + } + + if (count == 0) + { + yield return null; + } + } + private static async Task GetFixAsync(TextDocument? publicSurfaceAreaDocument, Project project, string newSymbolName, ImmutableHashSet siblingSymbolNamesToRemove, CancellationToken cancellationToken) { if (publicSurfaceAreaDocument == null) @@ -97,9 +121,9 @@ private static SourceText AddSymbolNamesToSourceText(SourceText? sourceText, IEn insertInList(lines, name); } - var endOfLine = PublicApiFixHelpers.GetEndOfLine(sourceText); + var endOfLine = sourceText.GetEndOfLine(); - var newText = string.Join(endOfLine, lines) + PublicApiFixHelpers.GetEndOfFileText(sourceText, endOfLine); + var newText = string.Join(endOfLine, lines) + sourceText.GetEndOfFileText(endOfLine); return sourceText?.Replace(new TextSpan(0, sourceText.Length), newText) ?? SourceText.From(newText); // Insert name at the first suitable position @@ -128,8 +152,8 @@ private static SourceText RemoveSymbolNamesFromSourceText(SourceText sourceText, List lines = GetLinesFromSourceText(sourceText); IEnumerable newLines = lines.Where(line => !linesToRemove.Contains(line)); - var endOfLine = PublicApiFixHelpers.GetEndOfLine(sourceText); - SourceText newSourceText = sourceText.Replace(new TextSpan(0, sourceText.Length), string.Join(endOfLine, newLines) + PublicApiFixHelpers.GetEndOfFileText(sourceText, endOfLine)); + var endOfLine = sourceText.GetEndOfLine(); + SourceText newSourceText = sourceText.Replace(new TextSpan(0, sourceText.Length), string.Join(endOfLine, newLines) + sourceText.GetEndOfFileText(endOfLine)); return newSourceText; } @@ -158,15 +182,16 @@ internal class AdditionalDocumentChangeAction : CodeAction { private readonly Func> _createChangedAdditionalDocument; - public AdditionalDocumentChangeAction(string title, Func> createChangedAdditionalDocument) + public AdditionalDocumentChangeAction(string title, DocumentId? apiDocId, Func> createChangedAdditionalDocument) { this.Title = title; + EquivalenceKey = apiDocId.CreateEquivalenceKey(); _createChangedAdditionalDocument = createChangedAdditionalDocument; } public override string Title { get; } - public override string EquivalenceKey => Title; + public override string EquivalenceKey { get; } protected override Task GetChangedSolutionAsync(CancellationToken cancellationToken) { @@ -177,11 +202,13 @@ protected override Task GetChangedSolutionAsync(CancellationToken canc private class FixAllAdditionalDocumentChangeAction : CodeAction { private readonly List>> _diagnosticsToFix; + private readonly DocumentId? _apiDocId; private readonly Solution _solution; - public FixAllAdditionalDocumentChangeAction(string title, Solution solution, List>> diagnosticsToFix) + public FixAllAdditionalDocumentChangeAction(string title, DocumentId? apiDocId, Solution solution, List>> diagnosticsToFix) { this.Title = title; + _apiDocId = apiDocId; _solution = solution; _diagnosticsToFix = diagnosticsToFix; } @@ -198,8 +225,7 @@ protected override async Task GetChangedSolutionAsync(CancellationToke Project project = pair.Key; ImmutableArray diagnostics = pair.Value; - var publicSurfaceAreaAdditionalDocument = PublicApiFixHelpers.GetUnshippedDocument(project); - + var publicSurfaceAreaAdditionalDocument = _apiDocId is not null ? project.GetAdditionalDocument(_apiDocId) : null; var sourceText = publicSurfaceAreaAdditionalDocument != null ? await publicSurfaceAreaAdditionalDocument.GetTextAsync(cancellationToken).ConfigureAwait(false) : null; @@ -333,7 +359,7 @@ private class PublicSurfaceAreaFixAllProvider : FixAllProvider return null; } - return new FixAllAdditionalDocumentChangeAction(title, fixAllContext.Solution, diagnosticsToFix); + return new FixAllAdditionalDocumentChangeAction(title, fixAllContext.CreateDocIdFromEquivalenceKey(), fixAllContext.Solution, diagnosticsToFix); } } diff --git a/src/PublicApiAnalyzers/Core/CodeFixes/NullableEnablePublicApiFix.cs b/src/PublicApiAnalyzers/Core/CodeFixes/NullableEnablePublicApiFix.cs index 9c384712e1..cd69d6cd27 100644 --- a/src/PublicApiAnalyzers/Core/CodeFixes/NullableEnablePublicApiFix.cs +++ b/src/PublicApiAnalyzers/Core/CodeFixes/NullableEnablePublicApiFix.cs @@ -32,13 +32,14 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) foreach (Diagnostic diagnostic in context.Diagnostics) { - TextDocument? document = PublicApiFixHelpers.GetShippedDocument(project); + TextDocument? document = project.GetShippedDocument(); if (document != null) { context.RegisterCodeFix( new DeclarePublicApiFix.AdditionalDocumentChangeAction( $"Add '#nullable enable' to public API", + document.Id, c => GetFixAsync(document, c)), diagnostic); } @@ -57,7 +58,7 @@ private static async Task GetFixAsync(TextDocument publicSurfaceAreaDo private static SourceText AddNullableEnable(SourceText sourceText) { - string extraLine = "#nullable enable" + PublicApiFixHelpers.GetEndOfLine(sourceText); + string extraLine = "#nullable enable" + sourceText.GetEndOfLine(); SourceText newSourceText = sourceText.WithChanges(new TextChange(new TextSpan(0, 0), extraLine)); return newSourceText; } @@ -83,7 +84,7 @@ protected override async Task GetChangedSolutionAsync(CancellationToke using var uniqueShippedDocuments = PooledHashSet.GetInstance(); foreach (var project in _projectsToFix) { - TextDocument? shippedDocument = PublicApiFixHelpers.GetShippedDocument(project); + TextDocument? shippedDocument = project.GetShippedDocument(); if (shippedDocument == null || shippedDocument.FilePath != null && !uniqueShippedDocuments.Add(shippedDocument.FilePath)) { diff --git a/src/PublicApiAnalyzers/Core/CodeFixes/PublicApiFixHelpers.cs b/src/PublicApiAnalyzers/Core/CodeFixes/PublicApiFixHelpers.cs index c1fba13812..0d0fd67cb2 100644 --- a/src/PublicApiAnalyzers/Core/CodeFixes/PublicApiFixHelpers.cs +++ b/src/PublicApiAnalyzers/Core/CodeFixes/PublicApiFixHelpers.cs @@ -1,30 +1,69 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Linq; +using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.PublicApiAnalyzers { internal static class PublicApiFixHelpers { - internal static TextDocument? GetUnshippedDocument(Project project) + private static readonly char[] SemicolonSplit = new[] { ';' }; + + internal static void Deconstruct(this KeyValuePair kv, out TKey key, out TValue value) + { + key = kv.Key; + value = kv.Value; + } + + private const string ApiDocEquivalenceKeyPrefix = "__ApiDoc__"; + + internal static string CreateEquivalenceKey(this DocumentId? doc) + { + if (doc is null) + { + return $"{ApiDocEquivalenceKeyPrefix};;"; + } + else + { + return $"{ApiDocEquivalenceKeyPrefix};{doc.ProjectId.Id};{doc.Id}"; + + } + } + + internal static DocumentId? CreateDocIdFromEquivalenceKey(this FixAllContext fixAllContext) { - return project.AdditionalDocuments.FirstOrDefault(doc => doc.Name.Equals(DeclarePublicApiAnalyzer.UnshippedFileName, StringComparison.Ordinal)); + var split = fixAllContext.CodeActionEquivalenceKey.Split(SemicolonSplit, StringSplitOptions.RemoveEmptyEntries); + + if (split.Length == 3 && + string.Equals(split[0], ApiDocEquivalenceKeyPrefix, StringComparison.Ordinal) && + Guid.TryParse(split[1], out var projectGuid) && projectGuid != Guid.Empty && + Guid.TryParse(split[2], out var docGuid) && docGuid != Guid.Empty) + { + var projectId = ProjectId.CreateFromSerialized(projectGuid); + return DocumentId.CreateFromSerialized(projectId, docGuid); + } + + return null; } - internal static TextDocument? GetShippedDocument(Project project) + internal static TextDocument? GetPublicApiDocument(this Project project, string name) { - return project.AdditionalDocuments.FirstOrDefault(doc => doc.Name.Equals(DeclarePublicApiAnalyzer.ShippedFileName, StringComparison.Ordinal)); + return project.AdditionalDocuments.FirstOrDefault(doc => doc.Name.Equals(name, StringComparison.Ordinal)); } + internal static TextDocument? GetShippedDocument(this Project project) + => project.GetPublicApiDocument(DeclarePublicApiAnalyzer.ShippedFileName); + /// /// Returns the trailing newline from the end of , if one exists. /// /// The source text. /// if ends with a trailing newline; /// otherwise, . - internal static string GetEndOfFileText(SourceText? sourceText, string endOfLine) + internal static string GetEndOfFileText(this SourceText? sourceText, string endOfLine) { if (sourceText == null || sourceText.Length == 0) return string.Empty; @@ -33,7 +72,7 @@ internal static string GetEndOfFileText(SourceText? sourceText, string endOfLine return lastLine.Span.IsEmpty ? endOfLine : string.Empty; } - internal static string GetEndOfLine(SourceText? sourceText) + internal static string GetEndOfLine(this SourceText? sourceText) { if (sourceText?.Lines.Count > 1) { diff --git a/src/PublicApiAnalyzers/UnitTests/AnnotatePublicApiAnalyzerTests.cs b/src/PublicApiAnalyzers/UnitTests/AnnotatePublicApiAnalyzerTests.cs index 4af3f4a43a..b9514b9c95 100644 --- a/src/PublicApiAnalyzers/UnitTests/AnnotatePublicApiAnalyzerTests.cs +++ b/src/PublicApiAnalyzers/UnitTests/AnnotatePublicApiAnalyzerTests.cs @@ -99,6 +99,41 @@ public class C where T : class? await VerifyCSharpAsync(source, shippedText, unshippedText); } + [Fact] + public async Task NoObliviousWhenAnnotatedClassConstraintMultipleFiles() + { + var source = @" +#nullable enable +public class C where T : class? +{ +} +"; + + var shippedText = @"#nullable enable"; + var unshippedText1 = @"#nullable enable +C.C() -> void +"; + var unshippedText2 = @"#nullable enable +C +"; + + var test = new CSharpCodeFixTest + { + TestState = + { + Sources = { source }, + AdditionalFiles = + { + (DeclarePublicApiAnalyzer.ShippedFileName, shippedText), + (DeclarePublicApiAnalyzer.UnshippedFileName, unshippedText1), + (DeclarePublicApiAnalyzer.UnshippedFileNamePrefix + "test" + DeclarePublicApiAnalyzer.Extension, unshippedText2), + }, + }, + }; + + await test.RunAsync(); + } + [Fact, WorkItem(4040, "https://github.com/dotnet/roslyn-analyzers/issues/4040")] public async Task ObliviousWhenObliviousClassConstraintAsync() { @@ -312,6 +347,50 @@ public class C await VerifyCSharpAdditionalFileFixAsync(source, shippedText, unshippedText, shippedText, fixedUnshippedText); } + [Fact] + public async Task AnnotatedMemberInAnnotatedUnshippedAPI_EnabledViaMultipleUnshippedAsync() + { + var source = @" +#nullable enable +public class C +{ + public string? OldField; + public string? {|RS0036:Field|}; + public string {|RS0036:Field2|}; +} +"; + + var unshippedText = @"#nullable enable +C +C.C() -> void +C.OldField -> string? +C.Field -> string +C.Field2 -> string"; + + var shippedText = @""; + + var fixedUnshippedText = @"#nullable enable +C +C.C() -> void +C.OldField -> string? +C.Field -> string? +C.Field2 -> string!"; + + var test = new CSharpCodeFixTest(); + + test.TestState.Sources.Add(source); + + test.TestState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.ShippedFileName, shippedText)); + test.TestState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.UnshippedFileName, string.Empty)); + test.TestState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.UnshippedFileNamePrefix + "test" + DeclarePublicApiAnalyzer.Extension, unshippedText)); + + test.FixedState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.ShippedFileName, shippedText)); + test.FixedState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.UnshippedFileName, string.Empty)); + test.FixedState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.UnshippedFileNamePrefix + "test" + DeclarePublicApiAnalyzer.Extension, fixedUnshippedText)); + + await test.RunAsync(); + } + [Fact] public async Task AnnotatedMemberInAnnotatedUnshippedAPI_EnabledViaShippedAsync() { diff --git a/src/PublicApiAnalyzers/UnitTests/DeclarePublicApiAnalyzerTests.cs b/src/PublicApiAnalyzers/UnitTests/DeclarePublicApiAnalyzerTests.cs index 2bbf608c89..94832036c0 100644 --- a/src/PublicApiAnalyzers/UnitTests/DeclarePublicApiAnalyzerTests.cs +++ b/src/PublicApiAnalyzers/UnitTests/DeclarePublicApiAnalyzerTests.cs @@ -1798,6 +1798,58 @@ public class C await VerifyCSharpAdditionalFileFixAsync(source, shippedText, unshippedText, fixedUnshippedText); } + [InlineData(0)] + [InlineData(1)] + [Theory] + public async Task TestSimpleMissingMember_Fix_WithoutNullability_MultipleFilesAsync(int index) + { + var source = @" +#nullable enable +public class C +{ + public string? {|RS0037:{|RS0016:NewField|}|}; // Newly added field, not in current public API. +} +"; + + var shippedText = @""; + var unshippedText1 = @"C +C.C() -> void"; + var unshippedText2 = @""; + var fixedUnshippedText1_index0 = @"C +C.C() -> void +C.NewField -> string"; + var fixedUnshippedText1_index1 = "C.NewField -> string"; + + var unshippedTextName2 = DeclarePublicApiAnalyzer.UnshippedFileNamePrefix + "test" + DeclarePublicApiAnalyzer.Extension; + + var test = new CSharpCodeFixTest(); + + test.TestState.Sources.Add(source); + test.TestState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.ShippedFileName, shippedText)); + test.TestState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.UnshippedFileName, unshippedText1)); + test.TestState.AdditionalFiles.Add((unshippedTextName2, unshippedText2)); + + test.CodeActionIndex = index; + test.FixedState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.ShippedFileName, shippedText)); + + if (index == 0) + { + test.FixedState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.UnshippedFileName, fixedUnshippedText1_index0)); + test.FixedState.AdditionalFiles.Add((unshippedTextName2, unshippedText2)); + } + else if (index == 1) + { + test.FixedState.AdditionalFiles.Add((DeclarePublicApiAnalyzer.UnshippedFileName, unshippedText1)); + test.FixedState.AdditionalFiles.Add((unshippedTextName2, fixedUnshippedText1_index1)); + } + else + { + throw new NotSupportedException(); + } + + await test.RunAsync(); + } + [Fact] public async Task TestSimpleMissingMember_Fix_WithNullabilityAsync() {