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.

Sign up for GitHub

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

Allow PublicAPI analyzers to merge from multiple sources #5422

Merged
merged 6 commits into from
Nov 2, 2021
Merged

Allow PublicAPI analyzers to merge from multiple sources #5422

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c05922b
Merge PublicAPI from multiple sources
twsouthwick Aug 31, 2021
9ed2cdd
Add additional test case
twsouthwick Aug 31, 2021
7eb0fdd
Merge remote-tracking branch 'origin/main' into merge-multiple-public…
twsouthwick Oct 19, 2021
be5c172
Raise code fix for each applicable doc
twsouthwick Oct 25, 2021
5cdee2f
convert to extension methods
twsouthwick Oct 25, 2021
6b51a0d
Add using
twsouthwick Nov 2, 2021
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
9 changes: 5 additions & 4 deletions src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.Impl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,12 @@ private void OnSymbolActionCore(ISymbol symbol, Action<Diagnostic> 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
Expand Down Expand Up @@ -395,15 +395,16 @@ 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);
ImmutableDictionary<string, string> propertyBag = ImmutableDictionary<string, string>.Empty
.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);
}
Expand Down
83 changes: 52 additions & 31 deletions src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ 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 UnshippedFileNamePrefix = "PublicAPI.Unshipped";
internal const string Extension = ".txt";
internal const string UnshippedFileName = "PublicAPI.Unshipped.txt";
internal const string PublicApiNamePropertyBagKey = "PublicAPIName";
internal const string PublicApiNameWithNullabilityPropertyBagKey = "PublicAPINameWithNullability";
Expand All @@ -33,6 +36,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 = '~';

Expand Down Expand Up @@ -258,38 +262,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<ApiLine>();
var removedBuilder = ImmutableArray.CreateBuilder<RemovedApiLine>();
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);
}
}
}

Expand Down Expand Up @@ -322,8 +330,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;
}

Expand Down Expand Up @@ -377,24 +385,27 @@ private static bool TryGetEditorConfigOptionForMissingFiles(AnalyzerOptions anal
}
}

private static bool IsFile(string path, string prefix, StringComparison comparison)
=> path.StartsWith(prefix, comparison) && path.EndsWith(Extension, comparison);

private static bool TryGetApiText(
ImmutableArray<AdditionalText> 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;
StringComparison comparison = StringComparison.Ordinal;
foreach (AdditionalText additionalText in additionalTexts)
{
cancellationToken.ThrowIfCancellationRequested();

string fileName = Path.GetFileName(additionalText.Path);

bool isShippedFile = comparer.Equals(fileName, ShippedFileName);
bool isUnshippedFile = comparer.Equals(fileName, UnshippedFileName);
bool isShippedFile = IsFile(fileName, ShippedFileNamePrefix, comparison);
bool isUnshippedFile = IsFile(fileName, UnshippedFileNamePrefix, comparison);

if (isShippedFile || isUnshippedFile)
{
Expand All @@ -408,12 +419,22 @@ private static bool TryGetApiText(
var data = (additionalText.Path, text);
if (isShippedFile)
{
shippedText = data;
if (shippedText is null)
{
shippedText = new();
}

shippedText.Add(data);
}

if (isUnshippedFile)
{
unshippedText = data;
if (unshippedText is null)
{
unshippedText = new();
}

unshippedText.Add(data);
}
continue;
}
Expand Down
74 changes: 25 additions & 49 deletions src/PublicApiAnalyzers/Core/CodeFixes/AnnotatePublicApiFix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -38,9 +38,9 @@ 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];
jmarolf marked this conversation as resolved.
Show resolved Hide resolved

TextDocument? document = isShippedDocument ? PublicApiFixHelpers.GetShippedDocument(project) : PublicApiFixHelpers.GetUnshippedDocument(project);
TextDocument? document = PublicApiFixHelpers.GetPublicApiDocument(project, fileName);

if (document != null)
{
Expand Down Expand Up @@ -102,53 +102,22 @@ public FixAllAdditionalDocumentChangeAction(string title, Solution solution, Lis

protected override async Task<Solution> GetChangedSolutionAsync(CancellationToken cancellationToken)
{
var updatedPublicSurfaceAreaText = new List<KeyValuePair<DocumentId, SourceText>>();

using var uniqueShippedDocuments = PooledHashSet<string>.GetInstance();
using var uniqueUnshippedDocuments = PooledHashSet<string>.GetInstance();
var updatedPublicSurfaceAreaText = new List<(DocumentId, SourceText)>();

foreach (KeyValuePair<Project, ImmutableArray<Diagnostic>> pair in _diagnosticsToFix)
foreach (var (project, diagnostics) in _diagnosticsToFix)
{
Project project = pair.Key;
ImmutableArray<Diagnostic> 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<IGrouping<SyntaxTree, Diagnostic>> groupedDiagnostics =
diagnostics
.Where(d => d.Location.IsInSource)
.GroupBy(d => d.Location.SourceTree);

var shippedChanges = new Dictionary<string, string>();
var unshippedChanges = new Dictionary<string, string>();
var allChanges = new Dictionary<string, Dictionary<string, string>>();

foreach (IGrouping<SyntaxTree, Diagnostic> grouping in groupedDiagnostics)
{
Document document = project.GetDocument(grouping.Key);

if (document == null)
if (document is null)
{
continue;
}
Expand All @@ -166,29 +135,36 @@ protected override async Task<Solution> 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<DocumentId, SourceText>(shippedDocument!.Id, newShippedSourceText));
}
var doc = PublicApiFixHelpers.GetPublicApiDocument(project, path);

if (unshippedSourceText is object)
{
SourceText newUnshippedSourceText = AnnotateSymbolNamesInSourceText(unshippedSourceText, unshippedChanges);
updatedPublicSurfaceAreaText.Add(new KeyValuePair<DocumentId, SourceText>(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<DocumentId, SourceText> pair in updatedPublicSurfaceAreaText)
foreach (var (docId, text) in updatedPublicSurfaceAreaText)
{
newSolution = newSolution.WithAdditionalDocumentText(pair.Key, pair.Value);
newSolution = newSolution.WithAdditionalDocumentText(docId, text);
}

return newSolution;
Expand Down
16 changes: 12 additions & 4 deletions src/PublicApiAnalyzers/Core/CodeFixes/PublicApiFixHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,31 @@
// 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.Text;

namespace Microsoft.CodeAnalysis.PublicApiAnalyzers
{
internal static class PublicApiFixHelpers
{
internal static TextDocument? GetUnshippedDocument(Project project)
internal static void Deconstruct<TKey, TValue>(this KeyValuePair<TKey, TValue> kv, out TKey key, out TValue value)
{
return project.AdditionalDocuments.FirstOrDefault(doc => doc.Name.Equals(DeclarePublicApiAnalyzer.UnshippedFileName, StringComparison.Ordinal));
key = kv.Key;
value = kv.Value;
}

internal static TextDocument? GetShippedDocument(Project project)
internal static TextDocument? GetPublicApiDocument(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? GetUnshippedDocument(Project project)
=> GetPublicApiDocument(project, DeclarePublicApiAnalyzer.UnshippedFileName);

internal static TextDocument? GetShippedDocument(Project project)
=> GetPublicApiDocument(project, DeclarePublicApiAnalyzer.ShippedFileName);

/// <summary>
/// Returns the trailing newline from the end of <paramref name="sourceText"/>, if one exists.
/// </summary>
Expand Down
Loading