Merge pull request #695 from JoeRobich/refactor-documentwithoptions

Remove DocumentWithOptions and pass around DocumentIds
dotnet · Jun 1, 2020 · 61e5536 · 61e5536
2 parents 15d4777 + 36ca2c0
commit 61e5536
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 65 deletions.
diff --git a/src/CodeFormatter.cs b/src/CodeFormatter.cs
@@ -233,7 +233,7 @@ static void LogWorkspaceDiagnostics(ILogger logger, bool logWorkspaceWarnings, I
 
         private static async Task<Solution> RunCodeFormattersAsync(
             Solution solution,
-            ImmutableArray<DocumentWithOptions> formattableDocuments,
+            ImmutableArray<DocumentId> formattableDocuments,
             FormatOptions options,
             ILogger logger,
             List<FormattedFile> formattedFiles,
@@ -249,7 +249,7 @@ static void LogWorkspaceDiagnostics(ILogger logger, bool logWorkspaceWarnings, I
             return formattedSolution;
         }
 
-        internal static async Task<(int, ImmutableArray<DocumentWithOptions>)> DetermineFormattableFilesAsync(
+        internal static async Task<(int, ImmutableArray<DocumentId>)> DetermineFormattableFilesAsync(
             Solution solution,
             string projectPath,
             Matcher fileMatcher,
@@ -258,10 +258,10 @@ static void LogWorkspaceDiagnostics(ILogger logger, bool logWorkspaceWarnings, I
             CancellationToken cancellationToken)
         {
             var totalFileCount = solution.Projects.Sum(project => project.DocumentIds.Count);
-            int projectFileCount = 0;
+            var projectFileCount = 0;
 
-            var documentsCoveredByEditorConfig = ImmutableArray.CreateBuilder<DocumentWithOptions>(totalFileCount);
-            var documentsNotCoveredByEditorConfig = ImmutableArray.CreateBuilder<DocumentWithOptions>(totalFileCount);
+            var documentsCoveredByEditorConfig = ImmutableArray.CreateBuilder<DocumentId>(totalFileCount);
+            var documentsNotCoveredByEditorConfig = ImmutableArray.CreateBuilder<DocumentId>(totalFileCount);
 
             var addedFilePaths = new HashSet<string>(totalFileCount);
 
@@ -317,19 +317,15 @@ await GeneratedCodeUtilities.IsGeneratedCodeAsync(syntaxTree, cancellationToken)
                         continue;
                     }
 
-                    var analyzerConfigOptions = document.Project.AnalyzerOptions.AnalyzerConfigOptionsProvider.GetOptions(syntaxTree);
-                    var optionSet = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
-
-                    var formattableDocument = new DocumentWithOptions(document, optionSet, analyzerConfigOptions);
-
                     // Track files covered by an editorconfig separately from those not covered.
+                    var analyzerConfigOptions = document.Project.AnalyzerOptions.AnalyzerConfigOptionsProvider.GetOptions(syntaxTree);
                     if (analyzerConfigOptions is object)
                     {
-                        documentsCoveredByEditorConfig.Add(formattableDocument);
+                        documentsCoveredByEditorConfig.Add(document.Id);
                     }
                     else
                     {
-                        documentsNotCoveredByEditorConfig.Add(formattableDocument);
+                        documentsNotCoveredByEditorConfig.Add(document.Id);
                     }
                 }
             }

diff --git a/src/DocumentWithOptions.cs b/src/DocumentWithOptions.cs
diff --git a/src/Formatters/DocumentFormatter.cs b/src/Formatters/DocumentFormatter.cs
@@ -25,7 +25,7 @@ internal abstract class DocumentFormatter : ICodeFormatter
         /// </summary>
         public async Task<Solution> FormatAsync(
             Solution solution,
-            ImmutableArray<DocumentWithOptions> formattableDocuments,
+            ImmutableArray<DocumentId> formattableDocuments,
             FormatOptions formatOptions,
             ILogger logger,
             List<FormattedFile> formattedFiles,
@@ -52,21 +52,32 @@ internal abstract class DocumentFormatter : ICodeFormatter
         /// </summary>
         private ImmutableArray<(Document, Task<(SourceText originalText, SourceText? formattedText)>)> FormatFiles(
             Solution solution,
-            ImmutableArray<DocumentWithOptions> formattableDocuments,
+            ImmutableArray<DocumentId> formattableDocuments,
             FormatOptions formatOptions,
             ILogger logger,
             CancellationToken cancellationToken)
         {
             var formattedDocuments = ImmutableArray.CreateBuilder<(Document, Task<(SourceText originalText, SourceText? formattedText)>)>(formattableDocuments.Length);
 
-            foreach (var formattableDocument in formattableDocuments)
+            foreach (var documentId in formattableDocuments)
             {
-                var document = solution.GetDocument(formattableDocument.Document.Id);
+                var document = solution.GetDocument(documentId);
                 if (document is null)
                     continue;
 
-                var formatTask = Task.Run(async ()
-                    => await GetFormattedSourceTextAsync(document, formattableDocument.OptionSet, formattableDocument.AnalyzerConfigOptions, formatOptions, logger, cancellationToken).ConfigureAwait(false), cancellationToken);
+                var formatTask = Task.Run(async () =>
+                {
+                    var originalSourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
+
+                    var syntaxTree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
+                    if (syntaxTree is null)
+                        return (originalSourceText, null);
+
+                    var analyzerConfigOptions = document.Project.AnalyzerOptions.AnalyzerConfigOptionsProvider.GetOptions(syntaxTree);
+                    var optionSet = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
+
+                    return await GetFormattedSourceTextAsync(document, optionSet, analyzerConfigOptions, formatOptions, logger, cancellationToken).ConfigureAwait(false);
+                }, cancellationToken);
 
                 formattedDocuments.Add((document, formatTask));
             }

diff --git a/src/Formatters/ICodeFormatter.cs b/src/Formatters/ICodeFormatter.cs
@@ -15,7 +15,7 @@ internal interface ICodeFormatter
         /// </summary>
         Task<Solution> FormatAsync(
             Solution solution,
-            ImmutableArray<DocumentWithOptions> formattableDocuments,
+            ImmutableArray<DocumentId> formattableDocuments,
             FormatOptions options,
             ILogger logger,
             List<FormattedFile> formattedFiles,

diff --git a/tests/Formatters/AbstractFormatterTests.cs b/tests/Formatters/AbstractFormatterTests.cs
@@ -125,7 +125,7 @@ private protected async Task<SourceText> TestAsync(string testCode, string expec
                 reportPath: string.Empty,
                 includeGeneratedFiles: false);
 
-            var pathsToFormat = await GetOnlyFileToFormatAsync(solution);
+            var pathsToFormat = GetOnlyFileToFormat(solution);
 
             var formattedSolution = await Formatter.FormatAsync(solution, pathsToFormat, formatOptions, Logger, new List<FormattedFile>(), default);
             var formattedDocument = GetOnlyDocument(formattedSolution);
@@ -137,21 +137,11 @@ private protected async Task<SourceText> TestAsync(string testCode, string expec
         }
 
         /// <summary>
-        /// Gets the only <see cref="Document"/> along with related options and conventions.
+        /// Gets the only <see cref="DocumentId"/>.
         /// </summary>
         /// <param name="solution">A Solution containing a single Project containing a single Document.</param>
-        /// <param name="editorConfig">The editorconfig to apply to the documents options set.</param>
-        /// <returns>The document contained within along with option set and coding conventions.</returns>
-        internal async Task<ImmutableArray<DocumentWithOptions>> GetOnlyFileToFormatAsync(Solution solution)
-        {
-            var document = GetOnlyDocument(solution);
-            var options = (OptionSet)await document.GetOptionsAsync();
-
-            var syntaxTree = await document.GetSyntaxTreeAsync();
-            var analyzerConfigOptions = document.Project.AnalyzerOptions.AnalyzerConfigOptionsProvider.GetOptions(syntaxTree);
-
-            return ImmutableArray.Create(new DocumentWithOptions(document, options, analyzerConfigOptions));
-        }
+        /// <returns>The only document id.</returns>
+        internal ImmutableArray<DocumentId> GetOnlyFileToFormat(Solution solution) => ImmutableArray.Create(GetOnlyDocument(solution).Id);
 
         /// <summary>
         /// Gets the only <see cref="Document"/> contained within the only <see cref="Project"/> within the <see cref="Solution"/>.

diff --git a/tests/Formatters/FormattedFilesTests.cs b/tests/Formatters/FormattedFilesTests.cs
@@ -64,7 +64,7 @@ private async Task<List<FormattedFile>> TestFormattedFiles(string testCode)
                 reportPath: string.Empty,
                 includeGeneratedFiles: false);
 
-            var pathsToFormat = await GetOnlyFileToFormatAsync(solution);
+            var pathsToFormat = GetOnlyFileToFormat(solution);
 
             var formattedFiles = new List<FormattedFile>();
             await Formatter.FormatAsync(solution, pathsToFormat, formatOptions, new TestLogger(), formattedFiles, default);