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.

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

Fix vscode fuse feature flag #10169

Merged
merged 13 commits into from
Mar 29, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -991,10 +991,7 @@ public void OmitsMinimizedAttributeValueParameter()
public void IncludesMinimizedAttributeValueParameterBeforeLanguageVersion5()
{
// Arrange
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

// Act
var generated = CompileToCSharp(@"
Expand Down Expand Up @@ -9178,10 +9175,7 @@ @preservewhitespace true
public void Legacy_3_1_LeadingWhiteSpace_WithDirective()
{
// Arrange/Act
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

var generated = CompileToCSharp(@"

Expand All @@ -9199,10 +9193,7 @@ @using System
public void Legacy_3_1_LeadingWhiteSpace_WithCSharpExpression()
{
// Arrange/Act
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

var generated = CompileToCSharp(@"

Expand All @@ -9220,10 +9211,7 @@ public void Legacy_3_1_LeadingWhiteSpace_WithCSharpExpression()
public void Legacy_3_1_LeadingWhiteSpace_WithComponent()
{
// Arrange
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

AdditionalSyntaxTrees.Add(Parse(@"
using Microsoft.AspNetCore.Components;
Expand Down Expand Up @@ -9256,10 +9244,7 @@ public class SomeOtherComponent : ComponentBase
public void Legacy_3_1_TrailingWhiteSpace_WithDirective()
{
// Arrange/Act
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

var generated = CompileToCSharp(@"
<h1>Hello</h1>
Expand All @@ -9278,10 +9263,7 @@ public void Legacy_3_1_TrailingWhiteSpace_WithDirective()
public void Legacy_3_1_TrailingWhiteSpace_WithCSharpExpression()
{
// Arrange/Act
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

var generated = CompileToCSharp(@"
<h1>Hello</h1>
Expand All @@ -9300,10 +9282,7 @@ public void Legacy_3_1_TrailingWhiteSpace_WithCSharpExpression()
public void Legacy_3_1_TrailingWhiteSpace_WithComponent()
{
// Arrange
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

AdditionalSyntaxTrees.Add(Parse(@"
using Microsoft.AspNetCore.Components;
Expand Down Expand Up @@ -9334,10 +9313,7 @@ public class SomeOtherComponent : ComponentBase
public void Legacy_3_1_Whitespace_BetweenElementAndFunctions()
{
// Arrange
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

// Act
var generated = CompileToCSharp(@"
Expand All @@ -9357,10 +9333,7 @@ public void Legacy_3_1_Whitespace_BetweenElementAndFunctions()
public void Legacy_3_1_WhiteSpace_InsideAttribute_InMarkupBlock()
{
// Arrange
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

// Act
var generated = CompileToCSharp(@"<div class=""first second"">Hello</div>");
Expand All @@ -9375,10 +9348,7 @@ public void Legacy_3_1_WhiteSpace_InsideAttribute_InMarkupBlock()
public void Legacy_3_1_WhiteSpace_InMarkupInFunctionsBlock()
{
// Arrange
_configuration = new(
RazorLanguageVersion.Version_3_0,
base.Configuration.ConfigurationName,
base.Configuration.Extensions);
_configuration = base.Configuration with { LanguageVersion = RazorLanguageVersion.Version_3_0 };

// Act
var generated = CompileToCSharp(@"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Razor.Language;

/// <summary>
/// This class represents flags provided by the language server instead of project configuration.
/// They should not be serialized as part of the configuration, as they instead change runtime behavior
/// impacted by LSP configuration rather than any project configuration
/// </summary>
public sealed record class LanguageServerFlags(bool ForceRuntimeCodeGeneration);
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,22 @@ public sealed record class RazorConfiguration(
RazorLanguageVersion LanguageVersion,
string ConfigurationName,
ImmutableArray<RazorExtension> Extensions,
bool UseConsolidatedMvcViews = false,
bool ForceRuntimeCodeGeneration = false)
LanguageServerFlags? LanguageServerFlags = null,
bool UseConsolidatedMvcViews = false)
{
public static readonly RazorConfiguration Default = new(
RazorLanguageVersion.Latest,
ConfigurationName: "unnamed",
Extensions: [],
UseConsolidatedMvcViews: false,
ForceRuntimeCodeGeneration: false);
LanguageServerFlags: null,
UseConsolidatedMvcViews: false);

public bool Equals(RazorConfiguration? other)
=> other is not null &&
LanguageVersion == other.LanguageVersion &&
ConfigurationName == other.ConfigurationName &&
LanguageServerFlags == other.LanguageServerFlags &&
UseConsolidatedMvcViews == other.UseConsolidatedMvcViews &&
ForceRuntimeCodeGeneration == other.ForceRuntimeCodeGeneration &&
Extensions.SequenceEqual(other.Extensions);

public override int GetHashCode()
Expand All @@ -36,7 +36,7 @@ public override int GetHashCode()
hash.Add(ConfigurationName);
hash.Add(Extensions);
hash.Add(UseConsolidatedMvcViews);
hash.Add(ForceRuntimeCodeGeneration);
hash.Add(LanguageServerFlags);
return hash;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public RazorCodeDocument ProcessDesignTime(RazorProjectItem projectItem)
throw new ArgumentNullException(nameof(projectItem));
}

var codeDocument = Configuration.ForceRuntimeCodeGeneration
var codeDocument = Configuration.LanguageServerFlags?.ForceRuntimeCodeGeneration == true
? CreateCodeDocumentCore(projectItem)
: CreateCodeDocumentDesignTimeCore(projectItem);
ProcessCore(codeDocument);
Expand All @@ -127,7 +127,7 @@ public RazorCodeDocument ProcessDesignTime(
throw new ArgumentNullException(nameof(source));
}

var codeDocument = Configuration.ForceRuntimeCodeGeneration
var codeDocument = Configuration.LanguageServerFlags?.ForceRuntimeCodeGeneration == true
? CreateCodeDocumentCore(source, fileKind, importSources, tagHelpers, configureParser: null, configureCodeGeneration: null)
: CreateCodeDocumentDesignTimeCore(source, fileKind, importSources, tagHelpers, configureParser: null, configureCodeGeneration: null);
ProcessCore(codeDocument);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.AspNetCore.Razor.ProjectSystem;
using Microsoft.AspNetCore.Razor.Utilities;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Workspaces;

namespace Microsoft.AspNetCore.Razor.LanguageServer;

Expand All @@ -32,7 +33,7 @@ public ProjectConfigurationFileChangeEventArgs(
_gate = new object();
}

public bool TryDeserialize([NotNullWhen(true)] out RazorProjectInfo? projectInfo)
public bool TryDeserialize(LanguageServerFeatureOptions languageServerFeatureOptions, [NotNullWhen(true)] out RazorProjectInfo? projectInfo)
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
{
if (Kind == RazorFileChangeKind.Removed)
{
Expand All @@ -59,6 +60,15 @@ public bool TryDeserialize([NotNullWhen(true)] out RazorProjectInfo? projectInfo
var normalizedDetectedFilePath = FilePathNormalizer.Normalize(ConfigurationFilePath);
if (string.Equals(normalizedSerializedFilePath, normalizedDetectedFilePath, FilePathComparison.Instance))
{
// Modify the feature flags on the configuration before storing
deserializedProjectInfo = deserializedProjectInfo with
{
Configuration = deserializedProjectInfo.Configuration with
{
LanguageServerFlags = languageServerFeatureOptions.ToLanguageServerFlags()
}
};

_projectInfo = deserializedProjectInfo;
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNetCore.Razor.LanguageServer;
Expand All @@ -22,17 +23,20 @@ internal class ProjectConfigurationStateSynchronizer : IProjectConfigurationFile
{
private readonly ProjectSnapshotManagerDispatcher _projectSnapshotManagerDispatcher;
private readonly IRazorProjectService _projectService;
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions;
private readonly ILogger _logger;
private readonly Dictionary<string, ProjectKey> _configurationToProjectMap;
internal readonly Dictionary<ProjectKey, DelayedProjectInfo> ProjectInfoMap;

public ProjectConfigurationStateSynchronizer(
ProjectSnapshotManagerDispatcher projectSnapshotManagerDispatcher,
IRazorProjectService projectService,
IRazorLoggerFactory loggerFactory)
IRazorLoggerFactory loggerFactory,
LanguageServerFeatureOptions languageServerFeatureOptions)
{
_projectSnapshotManagerDispatcher = projectSnapshotManagerDispatcher;
_projectService = projectService;
_languageServerFeatureOptions = languageServerFeatureOptions;
_logger = loggerFactory.CreateLogger<ProjectConfigurationStateSynchronizer>();
_configurationToProjectMap = new Dictionary<string, ProjectKey>(FilePathComparer.Instance);
ProjectInfoMap = new Dictionary<ProjectKey, DelayedProjectInfo>();
Expand All @@ -54,7 +58,7 @@ public void ProjectConfigurationFileChanged(ProjectConfigurationFileChangeEventA
case RazorFileChangeKind.Changed:
{
var configurationFilePath = FilePathNormalizer.Normalize(args.ConfigurationFilePath);
if (!args.TryDeserialize(out var projectInfo))
if (!args.TryDeserialize(_languageServerFeatureOptions, out var projectInfo))
{
if (!_configurationToProjectMap.TryGetValue(configurationFilePath, out var lastAssociatedProjectKey))
{
Expand Down Expand Up @@ -90,7 +94,7 @@ public void ProjectConfigurationFileChanged(ProjectConfigurationFileChangeEventA
case RazorFileChangeKind.Added:
{
var configurationFilePath = FilePathNormalizer.Normalize(args.ConfigurationFilePath);
if (!args.TryDeserialize(out var projectInfo))
if (!args.TryDeserialize(_languageServerFeatureOptions, out var projectInfo))
{
// Given that this is the first time we're seeing this configuration file if we can't deserialize it
// then we have to noop.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@

namespace Microsoft.AspNetCore.Razor.ProjectSystem;

internal sealed class RazorProjectInfo
internal sealed record class RazorProjectInfo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does making this a record help in some way? Note that the equality of this record won't be correct because it will need to use SequenceEquals for the Documents property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It made it cleaner to add a with for modifying readonly properties than having to write out new(...)

{
private static readonly MessagePackSerializerOptions s_options = MessagePackSerializerOptions.Standard
.WithResolver(CompositeResolver.Create(
RazorProjectInfoResolver.Instance,
StandardResolver.Instance));

public string SerializedFilePath { get; }
public string FilePath { get; }
public RazorConfiguration Configuration { get; }
public string? RootNamespace { get; }
public string DisplayName { get; }
public ProjectWorkspaceState ProjectWorkspaceState { get; }
public ImmutableArray<DocumentSnapshotHandle> Documents { get; }
public string SerializedFilePath { get; init; }
public string FilePath { get; init; }
public RazorConfiguration Configuration { get; init; }
public string? RootNamespace { get; init; }
public string DisplayName { get; init; }
public ProjectWorkspaceState ProjectWorkspaceState { get; init; }
public ImmutableArray<DocumentSnapshotHandle> Documents { get; init; }

public RazorProjectInfo(
string serializedFilePath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public override RazorConfiguration Deserialize(ref MessagePackReader reader, Ser
? version
: RazorLanguageVersion.Version_2_1;

return new(languageVersion, configurationName, extensions, ForceRuntimeCodeGeneration: forceRuntimeCodeGeneration);
return new(languageVersion, configurationName, extensions);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we bothering to read forceRuntimeCodeGeneration above if it's not used any longer? It looks like the read was removed from the JSON serialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is potential overlap where we have it written to a file right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's super small though. Is it worth protecting between 17.10p2 and 17.10p3? (Was this even in 17.10p2?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only p3, which we could port to remove

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh actually, it's shipped in vs code already.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a flow chart!

}

public override void Serialize(ref MessagePackWriter writer, RazorConfiguration value, SerializerCachingOptions options)
Expand All @@ -69,9 +69,7 @@ public override void Serialize(ref MessagePackWriter writer, RazorConfiguration
CachedStringFormatter.Instance.Serialize(ref writer, value.LanguageVersion.ToString(), options);
}

writer.Write(value.ForceRuntimeCodeGeneration);

count -= 3;
count -= 2;

for (var i = 0; i < count; i++)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.AspNetCore.Razor.Language;

namespace Microsoft.CodeAnalysis.Razor.Workspaces;

internal abstract class LanguageServerFeatureOptions
Expand Down Expand Up @@ -52,4 +54,7 @@ internal abstract class LanguageServerFeatureOptions
/// When enabled, design time code will not be generated. All tooling will be using runtime code generation.
/// </summary>
public abstract bool ForceRuntimeCodeGeneration { get; }

public LanguageServerFlags ToLanguageServerFlags()
=> new(ForceRuntimeCodeGeneration);
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,13 @@ private async Task AddFallbackProjectAsync(ProjectId projectId, string filePath,

var rootNamespace = project.DefaultNamespace;

var configuration = FallbackRazorConfiguration.Latest with { LanguageServerFlags = _languageServerFeatureOptions.ToLanguageServerFlags() };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty change! 😄


// We create this as a fallback project so that other parts of the system can reason about them - eg we don't do code
// generation for closed files for documents in these projects. If these projects become "real", either because capabilities
// change or simply a timing difference between Roslyn and our CPS components, the HostProject instance associated with
// the project will be updated, and it will no longer be a fallback project.
var hostProject = new FallbackHostProject(project.FilePath, intermediateOutputPath, FallbackRazorConfiguration.Latest, rootNamespace, project.Name);
var hostProject = new FallbackHostProject(project.FilePath, intermediateOutputPath, configuration, rootNamespace, project.Name);

await UpdateProjectManagerAsync(
updater => updater.ProjectAdded(hostProject),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public DefaultWindowsRazorProjectHost(

protected override async Task HandleProjectChangeAsync(string sliceDimensions, IProjectVersionedValue<IProjectSubscriptionUpdate> update)
{
if (TryGetConfiguration(update.Value.CurrentState, _languageServerFeatureOptions.ForceRuntimeCodeGeneration, out var configuration) &&
if (TryGetConfiguration(update.Value.CurrentState, _languageServerFeatureOptions.ToLanguageServerFlags(), out var configuration) &&
TryGetIntermediateOutputPath(update.Value.CurrentState, out var intermediatePath))
{
TryGetRootNamespace(update.Value.CurrentState, out var rootNamespace);
Expand Down Expand Up @@ -94,8 +94,8 @@ await UpdateAsync(

var hostProject = new HostProject(CommonServices.UnconfiguredProject.FullPath, intermediatePath, configuration, rootNamespace, displayName);

var projectConfigurationFile = Path.Combine(intermediatePath, _languageServerFeatureOptions.ProjectConfigurationFileName);
ProjectConfigurationFilePathStore.Set(hostProject.Key, projectConfigurationFile);
var projectConfigurationFile = Path.Combine(intermediatePath, _languageServerFeatureOptions.ProjectConfigurationFileName);
ProjectConfigurationFilePathStore.Set(hostProject.Key, projectConfigurationFile);

UpdateProject(updater, hostProject);

Expand Down Expand Up @@ -134,7 +134,7 @@ await UpdateAsync(
// Internal for testing
internal static bool TryGetConfiguration(
IImmutableDictionary<string, IProjectRuleSnapshot> state,
bool forceRuntimeCodeGeneration,
LanguageServerFlags? languageServerFlags,
[NotNullWhen(returnValue: true)] out RazorConfiguration? configuration)
{
if (!TryGetDefaultConfiguration(state, out var defaultConfiguration))
Expand Down Expand Up @@ -162,7 +162,12 @@ internal static bool TryGetConfiguration(
return false;
}

configuration = new(languageVersion, configurationItem.Key, extensions, ForceRuntimeCodeGeneration: forceRuntimeCodeGeneration);
configuration = new(
languageVersion,
configurationItem.Key,
extensions,
languageServerFlags);

return true;
}

Expand Down
Loading
Loading