From d9749bdce8e49322503c8b7c426456393e639d8f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 15 May 2020 11:21:18 -0700 Subject: [PATCH] NRT much of the rename core. --- .../Portable/Rename/ConflictResolution.cs | 4 +- .../Core/Portable/Rename/IRemoteRenamer.cs | 78 +++++++++++-------- .../Rename/IRenameRewriterLanguageService.cs | 2 + .../RenameLocation.ReferenceProcessing.cs | 45 +++++++---- .../Core/Portable/Rename/RenameLocation.cs | 4 +- .../Rename/RenameLocations.SearchResult.cs | 4 + .../Core/Portable/Rename/RenameLocations.cs | 4 +- .../Core/Portable/Rename/Renamer.cs | 14 ++-- 8 files changed, 96 insertions(+), 59 deletions(-) diff --git a/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs b/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs index b73182d59d6..0904d3081df 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System.Collections.Immutable; using Microsoft.CodeAnalysis.Rename.ConflictEngine; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -12,7 +14,7 @@ namespace Microsoft.CodeAnalysis.Rename { internal readonly partial struct ConflictResolution { - public readonly string ErrorMessage; + public readonly string? ErrorMessage; private readonly Solution _newSolutionWithoutRenamedDocument; private readonly (DocumentId documentId, string newName) _renamedDocument; diff --git a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs index 78cc28d03af..695df0893de 100644 --- a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs +++ b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs @@ -2,7 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -10,6 +13,7 @@ using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Rename.ConflictEngine; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -68,36 +72,35 @@ internal class SerializableSearchResult { // We use arrays so we can represent default immutable arrays. - public SerializableRenameLocation[] Locations; - public SerializableReferenceLocation[] ImplicitLocations; - public SerializableSymbolAndProjectId[] ReferencedSymbols; + public SerializableRenameLocation[]? Locations; + public SerializableReferenceLocation[]? ImplicitLocations; + public SerializableSymbolAndProjectId[]? ReferencedSymbols; - public static SerializableSearchResult Dehydrate(Solution solution, RenameLocations.SearchResult result, CancellationToken cancellationToken) + [return: NotNullIfNotNull("result")] + public static SerializableSearchResult? Dehydrate(Solution solution, RenameLocations.SearchResult? result, CancellationToken cancellationToken) => result == null ? null : new SerializableSearchResult { - Locations = result.Locations?.Select(loc => SerializableRenameLocation.Dehydrate(loc)).ToArray(), + Locations = result.Locations.Select(loc => SerializableRenameLocation.Dehydrate(loc)).ToArray(), ImplicitLocations = result.ImplicitLocations.IsDefault ? null : result.ImplicitLocations.Select(loc => SerializableReferenceLocation.Dehydrate(loc, cancellationToken)).ToArray(), ReferencedSymbols = result.ReferencedSymbols.IsDefault ? null : result.ReferencedSymbols.Select(s => SerializableSymbolAndProjectId.Dehydrate(solution, s, cancellationToken)).ToArray(), }; public async Task RehydrateAsync(Solution solution, CancellationToken cancellationToken) { - ImmutableHashSet locations = null; ImmutableArray implicitLocations = default; ImmutableArray referencedSymbols = default; - if (Locations != null) - { - using var _ = ArrayBuilder.GetInstance(Locations.Length, out var builder); - foreach (var loc in Locations) - builder.Add(await loc.RehydrateAsync(solution, cancellationToken).ConfigureAwait(false)); + Contract.ThrowIfNull(Locations); - locations = builder.ToImmutableHashSet(); - } + using var _1 = ArrayBuilder.GetInstance(Locations.Length, out var locBuilder); + foreach (var loc in Locations) + locBuilder.Add(await loc.RehydrateAsync(solution, cancellationToken).ConfigureAwait(false)); + + var locations = locBuilder.ToImmutableHashSet(); if (ImplicitLocations != null) { - using var _ = ArrayBuilder.GetInstance(ImplicitLocations.Length, out var builder); + using var _2 = ArrayBuilder.GetInstance(ImplicitLocations.Length, out var builder); foreach (var loc in ImplicitLocations) builder.Add(await loc.RehydrateAsync(solution, cancellationToken).ConfigureAwait(false)); @@ -106,7 +109,7 @@ public async Task RehydrateAsync(Solution solution if (ReferencedSymbols != null) { - using var _ = ArrayBuilder.GetInstance(ReferencedSymbols.Length, out var builder); + using var _3 = ArrayBuilder.GetInstance(ReferencedSymbols.Length, out var builder); foreach (var symbol in ReferencedSymbols) builder.AddIfNotNull(await symbol.TryRehydrateAsync(solution, cancellationToken).ConfigureAwait(false)); @@ -141,8 +144,8 @@ public static SerializableRenameLocation Dehydrate(RenameLocation location) public async Task RehydrateAsync(Solution solution, CancellationToken cancellation) { - var document = solution.GetDocument(DocumentId); - var tree = await document.GetSyntaxTreeAsync(cancellation).ConfigureAwait(false); + var document = solution.GetRequiredDocument(DocumentId); + var tree = await document.GetRequiredSyntaxTreeAsync(cancellation).ConfigureAwait(false); return new RenameLocation( CodeAnalysis.Location.Create(tree, Location), @@ -169,11 +172,14 @@ public SerializableRenameLocations Dehydrate(Solution solution, CancellationToke CommentsResult = _commentsResult.IsDefault ? null : _commentsResult.Select(r => SerializableRenameLocation.Dehydrate(r)).ToArray(), }; - internal static async Task RehydrateAsync(Solution solution, SerializableRenameLocations locations, CancellationToken cancellationToken) + internal static async Task TryRehydrateAsync(Solution solution, SerializableRenameLocations locations, CancellationToken cancellationToken) { if (locations == null) return null; + if (locations.Symbol == null) + return null; + var symbol = await locations.Symbol.TryRehydrateAsync(solution, cancellationToken).ConfigureAwait(false); if (symbol == null) return null; @@ -209,11 +215,17 @@ internal static async Task RehydrateAsync(Solution solution, Se commentsResult = builder.ToImmutable(); } + var originalSymbolResult = locations.OriginalSymbolResult == null + ? null + : await locations.OriginalSymbolResult.RehydrateAsync(solution, cancellationToken).ConfigureAwait(false); + + Contract.ThrowIfNull(locations.MergedResult); + return new RenameLocations( symbol, solution, locations.Options.Rehydrate(), - await locations.OriginalSymbolResult.RehydrateAsync(solution, cancellationToken).ConfigureAwait(false), + originalSymbolResult, await locations.MergedResult.RehydrateAsync(solution, cancellationToken).ConfigureAwait(false), overloadsResult, stringsResult, @@ -223,15 +235,15 @@ internal static async Task RehydrateAsync(Solution solution, Se internal class SerializableRenameLocations { - public SerializableSymbolAndProjectId Symbol; + public SerializableSymbolAndProjectId? Symbol; public SerializableRenameOptionSet Options; - public SerializableSearchResult OriginalSymbolResult; - public SerializableSearchResult MergedResult; + public SerializableSearchResult? OriginalSymbolResult; + public SerializableSearchResult? MergedResult; // We use arrays so we can represent default immutable arrays. - public SerializableSearchResult[] OverloadsResult; - public SerializableRenameLocation[] StringsResult; - public SerializableRenameLocation[] CommentsResult; + public SerializableSearchResult[]? OverloadsResult; + public SerializableRenameLocation[]? StringsResult; + public SerializableRenameLocation[]? CommentsResult; } internal class SerializableComplexifiedSpan @@ -257,7 +269,7 @@ internal class SerializableRelatedLocation public TextSpan ConflictCheckSpan; public RelatedLocationType Type; public bool IsReference; - public DocumentId DocumentId; + public DocumentId? DocumentId; public TextSpan ComplexifiedTargetSpan; public RelatedLocation Rehydrate() @@ -276,7 +288,7 @@ public static SerializableRelatedLocation Dehydrate(RelatedLocation location) internal class SerializableConflictResolution { - public string ErrorMessage; + public string? ErrorMessage; public bool ReplacementTextValid; @@ -286,12 +298,12 @@ internal class SerializableConflictResolution // // We also flatten dictionaries into key/value tuples because jsonrpc only supports dictionaries with string keys. - public DocumentId[] DocumentIds; - public SerializableRelatedLocation[] RelatedLocations; - public (DocumentId, TextChange[])[] DocumentTextChanges; - public (DocumentId, ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)>)[] DocumentToModifiedSpansMap; - public (DocumentId, ImmutableArray)[] DocumentToComplexifiedSpansMap; - public (DocumentId, ImmutableArray)[] DocumentToRelatedLocationsMap; + public DocumentId[]? DocumentIds; + public SerializableRelatedLocation[]? RelatedLocations; + public (DocumentId, TextChange[])[]? DocumentTextChanges; + public (DocumentId, ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)>)[]? DocumentToModifiedSpansMap; + public (DocumentId, ImmutableArray)[]? DocumentToComplexifiedSpansMap; + public (DocumentId, ImmutableArray)[]? DocumentToRelatedLocationsMap; public async Task RehydrateAsync(Solution oldSolution, CancellationToken cancellationToken) { diff --git a/src/Workspaces/Core/Portable/Rename/IRenameRewriterLanguageService.cs b/src/Workspaces/Core/Portable/Rename/IRenameRewriterLanguageService.cs index a68b6f0b0f6..2d4b0c49e1b 100644 --- a/src/Workspaces/Core/Portable/Rename/IRenameRewriterLanguageService.cs +++ b/src/Workspaces/Core/Portable/Rename/IRenameRewriterLanguageService.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System; using System.Collections.Generic; using System.Collections.Immutable; diff --git a/src/Workspaces/Core/Portable/Rename/RenameLocation.ReferenceProcessing.cs b/src/Workspaces/Core/Portable/Rename/RenameLocation.ReferenceProcessing.cs index 0628d4e288c..13c8cccf4c8 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameLocation.ReferenceProcessing.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameLocation.ReferenceProcessing.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System; using System.Collections.Generic; using System.Diagnostics; @@ -31,14 +33,12 @@ internal static class ReferenceProcessing /// Given a symbol in a document, returns the "right" symbol that should be renamed in /// the case the name binds to things like aliases _and_ the underlying type at once. /// - public static async Task GetRenamableSymbolAsync( + public static async Task TryGetRenamableSymbolAsync( Document document, int position, CancellationToken cancellationToken) { var symbol = await SymbolFinder.FindSymbolAtPositionAsync(document, position, cancellationToken: cancellationToken).ConfigureAwait(false); if (symbol == null) - { return null; - } var definitionSymbol = await FindDefinitionSymbolAsync(symbol, document.Project.Solution, cancellationToken).ConfigureAwait(false); Contract.ThrowIfNull(definitionSymbol); @@ -115,7 +115,7 @@ internal static class ReferenceProcessing } // in case this is e.g. an overridden property accessor, we'll treat the property itself as the definition symbol - var property = await GetPropertyFromAccessorOrAnOverrideAsync(bestSymbol, solution, cancellationToken).ConfigureAwait(false); + var property = await TryGetPropertyFromAccessorOrAnOverrideAsync(bestSymbol, solution, cancellationToken).ConfigureAwait(false); return property ?? bestSymbol; } @@ -214,7 +214,7 @@ static bool IsConstructorForType(ISymbol possibleConstructor, ISymbol possibleTy } } - internal static async Task GetPropertyFromAccessorOrAnOverrideAsync( + internal static async Task TryGetPropertyFromAccessorOrAnOverrideAsync( ISymbol symbol, Solution solution, CancellationToken cancellationToken) { if (symbol.IsPropertyAccessor()) @@ -229,7 +229,7 @@ static bool IsConstructorForType(ISymbol possibleConstructor, ISymbol possibleTy if (originalSourceSymbol != null) { - return await GetPropertyFromAccessorOrAnOverrideAsync(originalSourceSymbol, solution, cancellationToken).ConfigureAwait(false); + return await TryGetPropertyFromAccessorOrAnOverrideAsync(originalSourceSymbol, solution, cancellationToken).ConfigureAwait(false); } } @@ -241,7 +241,7 @@ static bool IsConstructorForType(ISymbol possibleConstructor, ISymbol possibleTy foreach (var methodImplementor in methodImplementors) { - var propertyAccessorOrAnOverride = await GetPropertyFromAccessorOrAnOverrideAsync(methodImplementor, solution, cancellationToken).ConfigureAwait(false); + var propertyAccessorOrAnOverride = await TryGetPropertyFromAccessorOrAnOverrideAsync(methodImplementor, solution, cancellationToken).ConfigureAwait(false); if (propertyAccessorOrAnOverride != null) { return propertyAccessorOrAnOverride; @@ -255,7 +255,7 @@ static bool IsConstructorForType(ISymbol possibleConstructor, ISymbol possibleTy private static async Task IsPropertyAccessorOrAnOverrideAsync( ISymbol symbol, Solution solution, CancellationToken cancellationToken) { - var result = await GetPropertyFromAccessorOrAnOverrideAsync( + var result = await TryGetPropertyFromAccessorOrAnOverrideAsync( symbol, solution, cancellationToken).ConfigureAwait(false); return result != null; } @@ -300,7 +300,8 @@ private static string TrimNameToAfterLastDot(string name) if (originalSymbol.Kind == SymbolKind.Alias) { var location = originalSymbol.Locations.Single(); - results.Add(new RenameLocation(location, solution.GetDocument(location.SourceTree).Id)); + Contract.ThrowIfNull(location.SourceTree); + results.Add(new RenameLocation(location, solution.GetRequiredDocument(location.SourceTree).Id)); return results.ToImmutableAndFree(); } @@ -309,9 +310,10 @@ private static string TrimNameToAfterLastDot(string name) { if (location.IsInSource) { + Contract.ThrowIfNull(location.SourceTree); results.Add(new RenameLocation( location, - solution.GetDocument(location.SourceTree).Id, + solution.GetRequiredDocument(location.SourceTree).Id, isRenamableAccessor: isRenamableAccessor)); } } @@ -320,7 +322,10 @@ private static string TrimNameToAfterLastDot(string name) // destructors declarations that match the name if (referencedSymbol.Kind == SymbolKind.NamedType && referencedSymbol.Locations.All(l => l.IsInSource)) { - var syntaxFacts = solution.GetDocument(referencedSymbol.Locations[0].SourceTree).GetLanguageService(); + var firstLocation = referencedSymbol.Locations[0]; + Contract.ThrowIfNull(firstLocation.SourceTree); + var syntaxFacts = solution.GetRequiredDocument(firstLocation.SourceTree) + .GetRequiredLanguageService(); var namedType = (INamedTypeSymbol)referencedSymbol; foreach (var method in namedType.GetMembers().OfType()) @@ -337,7 +342,8 @@ private static string TrimNameToAfterLastDot(string name) if (!syntaxFacts.IsReservedOrContextualKeyword(token) && token.ValueText == referencedSymbol.Name) { - results.Add(new RenameLocation(location, solution.GetDocument(location.SourceTree).Id)); + Contract.ThrowIfNull(location.SourceTree); + results.Add(new RenameLocation(location, solution.GetRequiredDocument(location.SourceTree).Id)); } } } @@ -376,7 +382,8 @@ internal static async Task> GetRenamableReferenceLoc // We also need to add the location of the alias // itself var aliasLocation = location.Alias.Locations.Single(); - results.Add(new RenameLocation(aliasLocation, solution.GetDocument(aliasLocation.SourceTree).Id)); + Contract.ThrowIfNull(aliasLocation.SourceTree); + results.Add(new RenameLocation(aliasLocation, solution.GetRequiredDocument(aliasLocation.SourceTree).Id)); } } else @@ -394,7 +401,8 @@ internal static async Task> GetRenamableReferenceLoc // We also need to add the location of the alias itself var aliasLocation = location.Alias.Locations.Single(); - results.Add(new RenameLocation(aliasLocation, solution.GetDocument(aliasLocation.SourceTree).Id)); + Contract.ThrowIfNull(aliasLocation.SourceTree); + results.Add(new RenameLocation(aliasLocation, solution.GetRequiredDocument(aliasLocation.SourceTree).Id)); } } else @@ -459,7 +467,7 @@ internal static async Task> GetRenamableReferenceLoc Document document, string renameText, ISyntaxFactsService syntaxFactsService, ArrayBuilder renameLocations, CancellationToken cancellationToken) { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var renameTextLength = renameText.Length; var renameStringsAndPositions = root @@ -477,7 +485,7 @@ internal static async Task> GetRenamableReferenceLoc private static async Task AddLocationsToRenameInCommentsAsync( Document document, string renameText, ArrayBuilder renameLocations, CancellationToken cancellationToken) { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var renameTextLength = renameText.Length; var renameStringsAndPositions = root @@ -508,8 +516,11 @@ internal static async Task> GetRenamableReferenceLoc var matches = regex.Matches(renameString); - foreach (Match match in matches) + foreach (Match? match in matches) { + if (match == null) + continue; + var start = renameStringPosition + match.Index; Debug.Assert(renameText.Length == match.Length); var matchTextSpan = new TextSpan(start, renameText.Length); diff --git a/src/Workspaces/Core/Portable/Rename/RenameLocation.cs b/src/Workspaces/Core/Portable/Rename/RenameLocation.cs index 2922c57f76f..6062296a346 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameLocation.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameLocation.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System; using Microsoft.CodeAnalysis.FindSymbols; using Microsoft.CodeAnalysis.Text; @@ -48,7 +50,7 @@ public RenameLocation(ReferenceLocation referenceLocation, DocumentId documentId public bool Equals(RenameLocation other) => Location == other.Location; - public override bool Equals(object obj) + public override bool Equals(object? obj) { return obj is RenameLocation loc && Equals(loc); diff --git a/src/Workspaces/Core/Portable/Rename/RenameLocations.SearchResult.cs b/src/Workspaces/Core/Portable/Rename/RenameLocations.SearchResult.cs index ce700b5f82f..1953f8e1cb0 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameLocations.SearchResult.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameLocations.SearchResult.cs @@ -2,8 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System.Collections.Immutable; using Microsoft.CodeAnalysis.FindSymbols; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Rename { @@ -20,6 +23,7 @@ public class SearchResult ImmutableArray implicitLocations, ImmutableArray referencedSymbols) { + Contract.ThrowIfNull(locations); this.Locations = locations; this.ImplicitLocations = implicitLocations; this.ReferencedSymbols = referencedSymbols; diff --git a/src/Workspaces/Core/Portable/Rename/RenameLocations.cs b/src/Workspaces/Core/Portable/Rename/RenameLocations.cs index 3b6f38c086b..378a995b173 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameLocations.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameLocations.cs @@ -150,8 +150,10 @@ internal sealed partial class RenameLocations if (result.HasValue) { - return await RenameLocations.RehydrateAsync( + var rehydrated = await RenameLocations.TryRehydrateAsync( solution, result.Value, cancellationToken).ConfigureAwait(false); + if (rehydrated != null) + return rehydrated; } } } diff --git a/src/Workspaces/Core/Portable/Rename/Renamer.cs b/src/Workspaces/Core/Portable/Rename/Renamer.cs index e909bf6ad91..f3cc281740b 100644 --- a/src/Workspaces/Core/Portable/Rename/Renamer.cs +++ b/src/Workspaces/Core/Portable/Rename/Renamer.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System; using System.Collections.Generic; using System.Collections.Immutable; @@ -35,7 +37,7 @@ public static partial class Renamer RenameOptionSet.From(solution, optionSet), nonConflictSymbols: null, cancellationToken).ConfigureAwait(false); - // This is a public entrypoint. So if rename failed to resolve conflicts, we report that back to caller as + // This is a public entry-point. So if rename failed to resolve conflicts, we report that back to caller as // an exception. if (resolution.ErrorMessage != null) throw new ArgumentException(resolution.ErrorMessage); @@ -66,8 +68,8 @@ public static partial class Renamer public static async Task RenameDocumentAsync( Document document, string newDocumentName, - IReadOnlyList newDocumentFolders = null, - OptionSet optionSet = null, + IReadOnlyList? newDocumentFolders = null, + OptionSet? optionSet = null, CancellationToken cancellationToken = default) { if (document is null) @@ -109,7 +111,7 @@ internal static Task FindRenameLocationsAsync(Solution solution ISymbol symbol, string newName, RenameOptionSet optionSet, - ImmutableHashSet nonConflictSymbols, + ImmutableHashSet? nonConflictSymbols, CancellationToken cancellationToken) { Contract.ThrowIfNull(solution); @@ -130,7 +132,7 @@ internal static Task FindRenameLocationsAsync(Solution solution WellKnownServiceHubServices.CodeAnalysisService, nameof(IRemoteRenamer.RenameSymbolAsync), solution, - new object[] + new object?[] { SerializableSymbolAndProjectId.Create(symbol, project, cancellationToken), newName, @@ -156,7 +158,7 @@ internal static Task FindRenameLocationsAsync(Solution solution ISymbol symbol, string newName, RenameOptionSet optionSet, - ImmutableHashSet nonConflictSymbols, + ImmutableHashSet? nonConflictSymbols, CancellationToken cancellationToken) { Contract.ThrowIfNull(solution); -- GitLab