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

Add feature flag to turn on the new Roslyn tokenizer #11185

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Add feature flag to turn on the new Roslyn tokenizer #11185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05ca733
Add feature flag to turn on the new Roslyn tokenizer
davidwengier Nov 8, 2024
4b7e93d
PR feedback
davidwengier Nov 11, 2024
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
2 changes: 2 additions & 0 deletions ...azor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultLanguageServerFeatureOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ internal class DefaultLanguageServerFeatureOptions : LanguageServerFeatureOption
public override bool DisableRazorLanguageServer => false;

public override bool ForceRuntimeCodeGeneration => false;

public override bool UseRoslynTokenizer => false;
}
3 changes: 3 additions & 0 deletions ...osoft.AspNetCore.Razor.LanguageServer/Hosting/ConfigurableLanguageServerFeatureOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
private readonly bool? _useRazorCohostServer;
private readonly bool? _disableRazorLanguageServer;
private readonly bool? _forceRuntimeCodeGeneration;
private readonly bool? _useRoslynTokenizer;

public override bool SupportsFileManipulation => _supportsFileManipulation ?? _defaults.SupportsFileManipulation;
public override string CSharpVirtualDocumentSuffix => _csharpVirtualDocumentSuffix ?? DefaultLanguageServerFeatureOptions.DefaultCSharpVirtualDocumentSuffix;
Expand All @@ -37,6 +38,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
public override bool UseRazorCohostServer => _useRazorCohostServer ?? _defaults.UseRazorCohostServer;
public override bool DisableRazorLanguageServer => _disableRazorLanguageServer ?? _defaults.DisableRazorLanguageServer;
public override bool ForceRuntimeCodeGeneration => _forceRuntimeCodeGeneration ?? _defaults.ForceRuntimeCodeGeneration;
public override bool UseRoslynTokenizer => _useRoslynTokenizer ?? _defaults.UseRoslynTokenizer;

public ConfigurableLanguageServerFeatureOptions(string[] args)
{
Expand All @@ -60,6 +62,7 @@ public ConfigurableLanguageServerFeatureOptions(string[] args)
TryProcessBoolOption(nameof(UseRazorCohostServer), ref _useRazorCohostServer, option, args, i);
TryProcessBoolOption(nameof(DisableRazorLanguageServer), ref _disableRazorLanguageServer, option, args, i);
TryProcessBoolOption(nameof(ForceRuntimeCodeGeneration), ref _forceRuntimeCodeGeneration, option, args, i);
TryProcessBoolOption(nameof(UseRoslynTokenizer), ref _useRoslynTokenizer, option, args, i);
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the options result in UseRoslynTokenizer && !ForceRuntimeCodeGeneration being true? Is that a configuration that we support?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't aware of any relationship between the two features?

It would definitely be possibly to hit though, as both of those flags are controlled by VS Code user settings.

Copy link
Member

Choose a reason for hiding this comment

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

I thought that we tried to simplify the overall scenarios a bit by saying that users could only use the new tokenizer if FUSE was enabled. This is more a simplification for the entire product experience vs. say just the compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

News to me, but makes sense. I guess we need @333fred to confirm, and I'll make the appropriate updates to the logic in the IDE. Or is it automatically handled by the compiler?

Copy link
Member

Choose a reason for hiding this comment

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

I thought that we tried to simplify the overall scenarios a bit by saying that users could only use the new tokenizer if FUSE was enabled.

Sorry, that's news to me too. I don't remember us saying that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the two features should be unrelated, and you can have one on without the other.

}
}

Expand Down
4 changes: 3 additions & 1 deletion src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/LanguageServerFeatureOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ internal abstract class LanguageServerFeatureOptions
public abstract bool DisableRazorLanguageServer { get; }

/// <summary>
/// When enabled, design time code will not be generated. All tooling will be using runtime code generation.
/// When enabled, design time code will not be generated. All tooling, except formatting, will be using runtime code generation.
/// </summary>
public abstract bool ForceRuntimeCodeGeneration { get; }

public abstract bool UseRoslynTokenizer { get; }
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
}
3 changes: 3 additions & 0 deletions src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.CodeAnalysis.Text;
using Microsoft.NET.Sdk.Razor.SourceGenerators;

namespace Microsoft.CodeAnalysis.Razor.ProjectSystem;

Expand Down Expand Up @@ -171,12 +172,14 @@ RazorProjectEngine CreateProjectEngine()
{
var configuration = HostProject.Configuration;
var rootDirectoryPath = Path.GetDirectoryName(HostProject.FilePath).AssumeNotNull();
var useRoslynTokenizer = LanguageServerFeatureOptions.UseRoslynTokenizer;
davidwengier marked this conversation as resolved.
Show resolved Hide resolved

return _projectEngineFactoryProvider.Create(configuration, rootDirectoryPath, builder =>
{
builder.SetRootNamespace(HostProject.RootNamespace);
builder.SetCSharpLanguageVersion(CSharpLanguageVersion);
builder.SetSupportLocalizedComponentNames();
builder.Features.Add(new ConfigureRazorParserOptions(useRoslynTokenizer, CSharpParseOptions.Default));
});
}
}
Expand Down
3 changes: 3 additions & 0 deletions ...r/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RemoteClientInitializationOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,7 @@ internal struct RemoteClientInitializationOptions

[JsonPropertyName("showAllCSharpCodeActions")]
public required bool ShowAllCSharpCodeActions { get; set; }

[JsonPropertyName("useRoslynTokenizer")]
public required bool UseRoslynTokenizer { get; set; }
}
2 changes: 2 additions & 0 deletions .../Microsoft.CodeAnalysis.Remote.Razor/Initialization/RemoteLanguageServerFeatureOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,6 @@ internal class RemoteLanguageServerFeatureOptions : LanguageServerFeatureOptions
public override bool DisableRazorLanguageServer => throw new InvalidOperationException("This option has not been synced to OOP.");

public override bool ForceRuntimeCodeGeneration => _options.ForceRuntimeCodeGeneration;

public override bool UseRoslynTokenizer => _options.UseRoslynTokenizer;
}
3 changes: 3 additions & 0 deletions src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteProjectSnapshot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Compiler.CSharp;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.NET.Sdk.Razor.SourceGenerators;
using Microsoft.VisualStudio.Threading;

namespace Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem;
Expand Down Expand Up @@ -51,6 +52,7 @@ public RemoteProjectSnapshot(Project project, RemoteSolutionSnapshot solutionSna
_lazyProjectEngine = new AsyncLazy<RazorProjectEngine>(async () =>
{
var configuration = await _lazyConfiguration.GetValueAsync();
var useRoslynTokenizer = SolutionSnapshot.SnapshotManager.LanguageServerFeatureOptions.UseRoslynTokenizer;
return ProjectEngineFactories.DefaultProvider.Create(
configuration,
rootDirectoryPath: Path.GetDirectoryName(FilePath).AssumeNotNull(),
Expand All @@ -59,6 +61,7 @@ public RemoteProjectSnapshot(Project project, RemoteSolutionSnapshot solutionSna
builder.SetRootNamespace(RootNamespace);
builder.SetCSharpLanguageVersion(CSharpLanguageVersion);
builder.SetSupportLocalizedComponentNames();
builder.Features.Add(new ConfigureRazorParserOptions(useRoslynTokenizer, CSharpParseOptions.Default));
});
},
joinableTaskFactory: null);
Expand Down
1 change: 1 addition & 0 deletions src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/Remote/RemoteServiceInvoker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ private async Task InitializeRemoteClientAsync(RazorRemoteHostClient remoteClien
ForceRuntimeCodeGeneration = _languageServerFeatureOptions.ForceRuntimeCodeGeneration,
SupportsFileManipulation = _languageServerFeatureOptions.SupportsFileManipulation,
ShowAllCSharpCodeActions = _languageServerFeatureOptions.ShowAllCSharpCodeActions,
UseRoslynTokenizer = _languageServerFeatureOptions.UseRoslynTokenizer,
};

_logger.LogDebug($"First OOP call, so initializing OOP service.");
Expand Down
11 changes: 10 additions & 1 deletion ...Microsoft.VisualStudio.LanguageServices.Razor/VisualStudioLanguageServerFeatureOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ internal class VisualStudioLanguageServerFeatureOptions : LanguageServerFeatureO
private readonly Lazy<bool> _useRazorCohostServer;
private readonly Lazy<bool> _disableRazorLanguageServer;
private readonly Lazy<bool> _forceRuntimeCodeGeneration;
private readonly Lazy<bool> _useRoslynTokenizer;

[ImportingConstructor]
public VisualStudioLanguageServerFeatureOptions(ILspEditorFeatureDetector lspEditorFeatureDetector)
Expand Down Expand Up @@ -71,12 +72,18 @@ public VisualStudioLanguageServerFeatureOptions(ILspEditorFeatureDetector lspEdi
var forceRuntimeCodeGeneration = featureFlags.IsFeatureEnabled(WellKnownFeatureFlagNames.ForceRuntimeCodeGeneration, defaultValue: false);
return forceRuntimeCodeGeneration;
});

_useRoslynTokenizer = new Lazy<bool>(() =>
{
var featureFlags = (IVsFeatureFlags)Package.GetGlobalService(typeof(SVsFeatureFlags));
var useRoslynTokenizer = featureFlags.IsFeatureEnabled(WellKnownFeatureFlagNames.UseRoslynTokenizer, defaultValue: false);
return useRoslynTokenizer;
});
}

// We don't currently support file creation operations on VS Codespaces or VS Liveshare
public override bool SupportsFileManipulation => !IsCodespacesOrLiveshare;


public override string CSharpVirtualDocumentSuffix => ".ide.g.cs";

public override string HtmlVirtualDocumentSuffix => "__virtual.html";
Expand All @@ -103,4 +110,6 @@ public VisualStudioLanguageServerFeatureOptions(ILspEditorFeatureDetector lspEdi

/// <inheritdoc />
public override bool ForceRuntimeCodeGeneration => _forceRuntimeCodeGeneration.Value;

public override bool UseRoslynTokenizer => _useRoslynTokenizer.Value;
}
1 change: 1 addition & 0 deletions src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WellKnownFeatureFlagNames.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ internal static class WellKnownFeatureFlagNames
public const string UseRazorCohostServer = "Razor.LSP.UseRazorCohostServer";
public const string DisableRazorLanguageServer = "Razor.LSP.DisableRazorLanguageServer";
public const string ForceRuntimeCodeGeneration = "Razor.LSP.ForceRuntimeCodeGeneration";
public const string UseRoslynTokenizer = "Razor.LSP.UseRoslynTokenizer";
}
7 changes: 7 additions & 0 deletions ...Microsoft.VisualStudio.RazorExtension/Microsoft.VisualStudio.RazorExtension.Custom.pkgdef
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@
"PreviewPaneChannels"="*"
"VisibleToInternalUsersOnlyChannels"="*"

[$RootKey$\FeatureFlags\Razor\LSP\UseRoslynTokenizer]
"Description"="Enables some new C# features, like interpolated and raw strings, in Razor files opened in Visual Studio. This matches using '<features>use-roslyn-tokenizer</feature>' in a project file for command line builds, and may result in inconsistencies if this option and your project files do not match."
"Value"=dword:00000000
"Title"="Use the C# tokenizer for Razor files in the IDE (requires restart)"
"PreviewPaneChannels"="*"
"VisibleToInternalUsersOnlyChannels"="*"

// CacheTag value should be changed when registration file changes
// See https://devdiv.visualstudio.com/DevDiv/_wiki/wikis/DevDiv.wiki/39345/Manifest-Build-Deployment-and-Setup-Authoring-In-Depth?anchor=example-pkgdef-key for more infomation
[$RootKey$\SettingsManifests\{13b72f58-279e-49e0-a56d-296be02f0805}]
Expand Down
2 changes: 2 additions & 0 deletions ...osoft.AspNetCore.Razor.Test.Common.Tooling/Workspaces/TestLanguageServerFeatureOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,6 @@ internal class TestLanguageServerFeatureOptions(
public override bool DisableRazorLanguageServer => false;

public override bool ForceRuntimeCodeGeneration => forceRuntimeCodeGeneration;

public override bool UseRoslynTokenizer => false;
}
1 change: 1 addition & 0 deletions .../test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostEndpointTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ protected override async Task InitializeAsync()
ForceRuntimeCodeGeneration = false,
SupportsFileManipulation = true,
ShowAllCSharpCodeActions = false,
UseRoslynTokenizer = false,
};
UpdateClientInitializationOptions(c => c);

Expand Down