Skip to content

Commit 07fded2

Browse files
RikkiGibsonRikkiGibson
committed
Address feedback from dotnet#78648 
1 parent 656c077 commit 07fded2

File tree

4 files changed

+25
-25
lines changed

4 files changed

+25
-25
lines changed

‎src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/VirtualProjectXmlProviderTests.cs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public VirtualProjectXmlProviderTests(ITestOutputHelper testOutputHelper) : base
3232

3333
private class EnableRunApiTests : ExecutionCondition
3434
{
35+
// https://github.com/dotnet/roslyn/issues/78879: Enable these tests unconditionally
3536
public override bool ShouldSkip =>
3637
#if RoslynTestRunApi
3738
false;
@@ -69,7 +70,7 @@ await appFile.WriteAllTextAsync("""
6970
}
7071
"""));
7172

72-
var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, CancellationToken.None);
73+
var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, LoggerFactory.CreateLogger<VirtualProjectXmlProviderTests>(), CancellationToken.None);
7374
Assert.Null(contentNullable);
7475
}
7576

@@ -93,10 +94,11 @@ await globalJsonFile.WriteAllTextAsync("""
9394
}
9495
""");
9596

96-
var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, CancellationToken.None);
97+
var logger = LoggerFactory.CreateLogger<VirtualProjectXmlProviderTests>();
98+
var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, logger, CancellationToken.None);
9799
var content = contentNullable.Value;
98100
var virtualProjectXml = content.VirtualProjectXml;
99-
LoggerFactory.CreateLogger<VirtualProjectXmlProviderTests>().LogTrace(virtualProjectXml);
101+
logger.LogTrace(virtualProjectXml);
100102

101103
Assert.Contains("<TargetFramework>net10.0</TargetFramework>", virtualProjectXml);
102104
Assert.Contains("<ArtifactsPath>", virtualProjectXml);
@@ -125,7 +127,7 @@ await globalJsonFile.WriteAllTextAsync("""
125127
}
126128
""");
127129

128-
var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, CancellationToken.None);
130+
var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, LoggerFactory.CreateLogger<VirtualProjectXmlProviderTests>(), CancellationToken.None);
129131
var content = contentNullable.Value;
130132
LoggerFactory.CreateLogger<VirtualProjectXmlProviderTests>().LogTrace(content.VirtualProjectXml);
131133

@@ -150,7 +152,7 @@ await globalJsonFile.WriteAllTextAsync("""
150152
}
151153
""");
152154

153-
var content = await projectProvider.GetVirtualProjectContentAsync(Path.Combine(tempDir.Path, "BAD"), CancellationToken.None);
155+
var content = await projectProvider.GetVirtualProjectContentAsync(Path.Combine(tempDir.Path, "BAD"), LoggerFactory.CreateLogger<VirtualProjectXmlProviderTests>(), CancellationToken.None);
154156
Assert.Null(content);
155157
}
156158

@@ -176,7 +178,7 @@ await globalJsonFile.WriteAllTextAsync("""
176178
}
177179
""");
178180

179-
var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, CancellationToken.None);
181+
var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, LoggerFactory.CreateLogger<VirtualProjectXmlProviderTests>(), CancellationToken.None);
180182
var content = contentNullable.Value;
181183
var diagnostic = content.Diagnostics.Single();
182184
Assert.Contains("Unrecognized directive 'BAD'", diagnostic.Message);

‎src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/DotnetCliHelper.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,15 @@ private async Task<string> GetDotnetSdkFolderFromDotnetExecutableAsync(string pr
6969
return dotnetSdkFolderPath;
7070
}
7171

72-
public Process Run(string[] arguments, string? workingDirectory, bool shouldLocalizeOutput, bool redirectStandardInput = false)
72+
public Process Run(string[] arguments, string? workingDirectory, bool shouldLocalizeOutput)
7373
{
7474
_logger.LogDebug($"Running dotnet CLI command at {_dotnetExecutablePath.Value} in directory {workingDirectory} with arguments {arguments}");
7575

7676
var startInfo = new ProcessStartInfo(_dotnetExecutablePath.Value)
7777
{
7878
CreateNoWindow = true,
7979
UseShellExecute = false,
80-
RedirectStandardInput = redirectStandardInput,
80+
RedirectStandardInput = true,
8181
RedirectStandardOutput = true,
8282
RedirectStandardError = true,
8383
};

‎src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/FileBasedProgramsProjectSystem.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public async ValueTask TryRemoveMiscellaneousDocumentAsync(DocumentUri uri, bool
132132
protected override async Task<RemoteProjectLoadResult?> TryLoadProjectInMSBuildHostAsync(
133133
BuildHostProcessManager buildHostProcessManager, string documentPath, CancellationToken cancellationToken)
134134
{
135-
var content = await _projectXmlProvider.GetVirtualProjectContentAsync(documentPath, cancellationToken);
135+
var content = await _projectXmlProvider.GetVirtualProjectContentAsync(documentPath, _logger, cancellationToken);
136136
if (content is not var (virtualProjectContent, diagnostics))
137137
{
138138
// https://github.com/dotnet/roslyn/issues/78618: falling back to this until dotnet run-api is more widely available

‎src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,12 @@ namespace Microsoft.CodeAnalysis.LanguageServer.FileBasedPrograms;
2121
[Export(typeof(VirtualProjectXmlProvider)), Shared]
2222
[method: ImportingConstructor]
2323
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
24-
internal class VirtualProjectXmlProvider(DotnetCliHelper dotnetCliHelper, ILoggerFactory loggerFactory)
24+
internal class VirtualProjectXmlProvider(DotnetCliHelper dotnetCliHelper)
2525
{
26-
private readonly ILogger<VirtualProjectXmlProvider> _logger = loggerFactory.CreateLogger<VirtualProjectXmlProvider>();
27-
28-
internal async Task<(string VirtualProjectXml, ImmutableArray<SimpleDiagnostic> Diagnostics)?> GetVirtualProjectContentAsync(string documentFilePath, CancellationToken cancellationToken)
26+
internal async Task<(string VirtualProjectXml, ImmutableArray<SimpleDiagnostic> Diagnostics)?> GetVirtualProjectContentAsync(string documentFilePath, ILogger logger, CancellationToken cancellationToken)
2927
{
3028
var workingDirectory = Path.GetDirectoryName(documentFilePath);
31-
var process = dotnetCliHelper.Run(["run-api"], workingDirectory, shouldLocalizeOutput: true, redirectStandardInput: true);
29+
var process = dotnetCliHelper.Run(["run-api"], workingDirectory, shouldLocalizeOutput: true);
3230

3331
cancellationToken.Register(() =>
3432
{
@@ -42,21 +40,21 @@ internal class VirtualProjectXmlProvider(DotnetCliHelper dotnetCliHelper, ILogge
4240

4341
// Debug severity is used for these because we think it will be common for the user environment to have too old of an SDK for the call to work.
4442
// Rather than representing a hard error condition, it represents a condition where we need to gracefully downgrade the experience.
45-
process.ErrorDataReceived += (sender, args) => _logger.LogDebug($"dotnet run-api: {args.Data}");
43+
process.ErrorDataReceived += (sender, args) => logger.LogDebug($"[stderr] dotnet run-api: {args.Data}");
4644
process.BeginErrorReadLine();
4745

4846
var responseJson = await process.StandardOutput.ReadLineAsync(cancellationToken);
4947
await process.WaitForExitAsync(cancellationToken);
5048

5149
if (process.ExitCode != 0)
5250
{
53-
_logger.LogDebug($"dotnet run-api exited with exit code '{process.ExitCode}'.");
51+
logger.LogDebug($"dotnet run-api exited with exit code '{process.ExitCode}'.");
5452
return null;
5553
}
5654

5755
if (string.IsNullOrWhiteSpace(responseJson))
5856
{
59-
_logger.LogError($"dotnet run-api exited with exit code 0, but did not return any response.");
57+
logger.LogError($"dotnet run-api exited with exit code 0, but did not return any response.");
6058
return null;
6159
}
6260

@@ -65,17 +63,13 @@ internal class VirtualProjectXmlProvider(DotnetCliHelper dotnetCliHelper, ILogge
6563
var response = JsonSerializer.Deserialize(responseJson, RunFileApiJsonSerializerContext.Default.RunApiOutput);
6664
if (response is RunApiOutput.Error error)
6765
{
68-
_logger.LogError($"dotnet run-api version: {error.Version}. Latest known version: {RunApiOutput.LatestKnownVersion}");
69-
_logger.LogError($"dotnet run-api returned error: '{error.Message}'");
66+
logger.LogError($"dotnet run-api version: {error.Version}. Latest known version: {RunApiOutput.LatestKnownVersion}");
67+
logger.LogError($"dotnet run-api returned error: '{error.Message}'");
7068
return null;
7169
}
7270

7371
if (response is RunApiOutput.Project project)
7472
{
75-
if (project.Version > RunApiOutput.LatestKnownVersion)
76-
{
77-
_logger.LogWarning($"'dotnet run-api' version '{project.Version}' is newer than latest known version {RunApiOutput.LatestKnownVersion}");
78-
}
7973

8074
return (project.Content, project.Diagnostics);
8175
}
@@ -85,7 +79,11 @@ internal class VirtualProjectXmlProvider(DotnetCliHelper dotnetCliHelper, ILogge
8579
catch (JsonException ex)
8680
{
8781
// In this case, run-api returned 0 exit code, but gave us back JSON that we don't know how to parse.
88-
_logger.LogError(ex, "Could not deserialize run-api response.");
82+
logger.LogError(ex, "Could not deserialize run-api response.");
83+
logger.LogTrace($"""
84+
Full run-api response:
85+
{responseJson}
86+
""");
8987
return null;
9088
}
9189
}
@@ -99,7 +97,7 @@ internal static string GetVirtualProjectPath(string documentFilePath)
9997

10098
internal static bool IsFileBasedProgram(string documentFilePath, SourceText text)
10199
{
102-
// TODO: this needs to be adjusted to be more sustainable.
100+
// https://github.com/dotnet/roslyn/issues/78878: this needs to be adjusted to be more sustainable.
103101
// When we adopt the dotnet run-api, we need to get rid of this or adjust it to be more sustainable (e.g. using the appropriate document to get a syntax tree)
104102
var tree = CSharpSyntaxTree.ParseText(text, options: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.Preview), path: documentFilePath);
105103
var root = tree.GetRoot();

0 commit comments

Comments
 (0)