Add ParserOptions and CodeGenerationOptions properties to RazorCodeDocument and rationalize options configuration #11526
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove unused usings - Use ArgHelper.ThrowIfNull throughout - Annotate for nullability
There are two RazorProjectEngineBuilderExtensions classes in the compiler in different namespaces. This merges the two, which may be a breaking change for the RazorSDK, which uses the SetCSharpLanguageVersion extension method. This change also merges the RazorProjectEngineBuilderExtensionsTest files and tweaks the SetCSharpLanguageVersion test to be a bit more useful. Finally, a few using directives have been removed since SetCSharpLanguageVersion now resolves from a different namespace.
Adds ConfigureParserOptions and ConfigureCodeGenerationOptions extension methods for RazorProjectEngineBuilder
Replaces uses of ConfigureRazorCodeGenerationOptions class with call to ConfigureCodeGenerationOptions(...) helper.
Replaces uses of ConfigureRazorParserOptions class with call to ConfigureParserOptions(...) helper. This is a much larger change because there are many more uses of this class in both product and test code. I've updated each of these mechanically and removed "#nullable disable" when it wouldn't introduce warnings.
Most IConfigureRazorParserOptionsFeature implementations are trivial and can be replaced with calls to the RazorProjectEngineBuilder.ConfigureParserOptions(...) helper method.
Most IConfigureRazorCodeGenerationOptionsFeature implementations are trivial and can be replaced with calls to the RazorProjectEngineBuilder.ConfigureCodeGenerationOptions(...) helper method.
The goal of this rewrite is to avoid exposing collection interfaces and making consumers understand the semantics of the logic. Instead, it now has a simple API to add and get directives.
Change the signature of AddDirective to take a ReadOnlySpan<string> and make it internal. This API is not used by the RazorSDK, so it shouldn't be considered a breaking change. In addition, I've removed some code duplication for features that need to get a feature or create one if it doesn't exist.
AddDefaultImports is only used by tests and isn't consumed by the RazorSDK. So, it isn't a breaking change to move out of product code.
Three methods on RazorProjectEngineBuilderExtensions aren't actually used by anything in Razor or the RazorSDK: ConfigureClass, SetBaseType, and SetNamespace. These have been removed. In addition, AddTargetExtension is only used in Razor and not the RazorSDK. So, I've changed its accessibility to internal.
Both IConfigureRazorParserOptionsFeature and IConfigureRazorCodeGenerationOptionsFeature support Order properties that should be used. ConfigureDirectivesFeature sets Order to 100 to ensure that it runs last. So, this change honors that.
- "designTime" is always false for DefaultRazorCodeGenerationOptionsFeature - "designTime" is always false for DefaultRazorParserOptionsFeature - "fileKind" is always null for DefaultRazorParserOptionsFeature
This is a larger change that rationalizes the RazorCodeGenerationOptions and its builder APIs.
RazorCodeGenerationOptions:
- Added "Wither" methods
- Removed Create(...) methods
- Removed ToBuilder() method
RazorCodeGenerationOptionsBuilder:
- Replaced Configuration property with LanguageVersion.
- Converted SetDesignTime method to property.
- Removed excess constructors.
- Renamed Build() method to ToOptions()
The remaining files changed are largely mechanical changes responding to the API changes.
This is a larger change that rationalizes the RazorParserOptions and its builder APIs.
RazorParserOptions :
- Introduced RazorParserOptionsFlags to hold boolean values.
- Convert CreateDefault() method to a Default property.
- Added "Wither" methods
- Removed Create(...) methods
RazorParserOptionsBuilder:
- Remove Configuration property.
- Converted SetDesignTime method to property.
- Removed excess constructors.
- Renamed Build() method to ToOptions()
- Use RazorParserOptionsFlags for all boolean values.
- Add FeatureFlags property to hold RazorParserFeatureFlags.
The remaining files changed are largely mechanical changes responding to the API changes.
RazorProjectOptionsBuilder is initialized with CSharpParseOptions.Default, so it's unnecesary to set that value.
This change nests RazorParserOptions' builder and flags within it. The flags have been made private along with the RazorParserOptions constructor.
This change nests RazorCodeGenerationOptions' builder and flags within it. The flags have been made private along with the RazorCodeGenerationOptions constructor.
This change pulls the RazorParserFeatureFlags values into RazorParserOptions.
This change removes DefaultRazorParserOptionsFactoryProjectFeature and DefaultRazorCodeGenerationOptionsFactoryProjectFeature and their associated interfaces. The purpose of these features can be achieved within RazorProjectEngine.
This makes RazorCodeGenerationOptions consistent with RazorParserOptions (and their builders).
This change converts the following extension methods to RazorCodeDocument instance methods: - GetParserOptions() - GetCodeGenerationOptions() - SetParserOptions() - SetCodeGenerationOptions() The options instances are now stored directly as fields on RazorCodeDocument rather than in the Items collection.
This method simply calls RazorCodeDocument.Create(Source, Imports). So, the test helper is unnecessary.
Because RazorCodeDocument now owns its ParserOptions and CodeGenerationOptions, it is generally important to create a RazorCodeDocument by calling RazorProjectEngine.CreateCodeDocument(...) in order to configure the options correctly.
…r passes Call RazroProjectEngine.CreateCodeDocument(...) rather than RazorCodeDocument.Create(...) within the compiler to ensure in is created with the configured options.
There's a RazorProjectEngineTestBase.CreateEngine() method that returns a RazorEngine and is only used by a handful of tests. These tests should be using RazorProjectEngine to create RazorCodeDocuments. So, this change updates those tests and removes CreateEngine(). In addition, several helper methods have been moved to MS.ANC.Razor.Test.Common as extension methods to prepare for further test clean up.
This change updates MS.ANC.Mvc.Razor.Extensions.Test\ModelDirectiveTest to create and use RazorCodeDocuments from the RazorProjectEngine. In addition, this change adds helpers for creating design-time RazorCodeDocuments.
Both of the design-time tests fail after being updated to use code documents from the project engine. The reason is that the test was initially buggy. Previously, the design-time tests created a RazorProjectEngine with options configured for design-time. However, the RazorCodeDocument was created without those options. So, DocumentIntermediateNode.Options.DesignTime wouldn't be set, and ModelDirective.Pass.ExecuteCore(...) would follow a runtime code path. Now that the RazorCodeDocument is correctly configured, it takes the correct design-time code path and the assertions need to be updated. IOW, the test was wrong and now it's correct.
DustinCampbell
changed the title
chsienki
reviewed
Feb 21, 2025
src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorCodeGenerationOptions.cs
Show resolved
Hide resolved
davidwengier
approved these changes
Feb 21, 2025
333fred
reviewed
Feb 22, 2025
src/Shared/Microsoft.AspNetCore.Razor.Test.Common/Language/RazorProjectEngineTestBase.cs
Outdated
Show resolved
Hide resolved
ryzngard
approved these changes
Feb 22, 2025
chsienki
approved these changes
Feb 24, 2025
A normal null-coalescing expression works just fine. Co-authored-by: Fred Silberberg <[email protected]>
333fred
approved these changes
Feb 25, 2025
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
This was referenced Feb 26, 2025
This was referenced Mar 4, 2025
This was referenced Mar 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
This contains RazorSDK breaking changes! I've prepared a fix for these changes that can applied when the SDK build fails: DustinCampbell/sdk@4e955e1
Warning
This is a big change with a lot of commits. To facilitate code reviews, I've tried to keep the commit history informative. Most commits have explanatory messages and product and pure test changes are in separate commits.
The majority of the changes are in the compiler, so this will require two compiler reviews from @chsienki, @333fred, @jjonescz, or @jaredpar.
CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2647163&view=results
Test Insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/612761
Summary
RazorParserOptionsAPI and its builder pattern.RazorCodeGenerationOptionsAPI and its builder pattern.RazorProjectEngineBuilderextension methods to configure parser and code generation options.IConfigureRazorParserOptionsFeatureandIConfigureRazorCodeGenerationOptionsFeatureimplementations.ICollection<T>andIReadOnlyDictionary<TKey, TValue>.ParserOptionsandCodeGenerationOptionsproperties toRazorCodeDocument(and remove them fromRazorCodeDocument.Items)RazorCodeDocumentis always created with valid parser and code generation options.IRazorParserOptionsFeatureandIRazorCodeGenerationOptionsFeatureinterfaces and their implementations. These served as fallbacks when aRazorCodeDocumentdidn't have parser or code gen options. However, that's no longer the case, so the fallbacks can be removed.IRazorParserOptionsFactoryProjectFeatureandIRazorCodeGenerationOptionsFactoryProjectFeatureinterfaces and their implementations. These were only used byRazorProjectEnginewhen creating aRazorCodeDocumentand their functionality has been rolled intoRazorProjectEngine.RazorCodeDocumentfrom aRazorProjectEngine.RazorProjectEngine-based tests. The same test infrastructure is shared across the Language and "Extensions" testsRazorCodeDocumentProcessorhelper class for executing particular compiler phases and passes with aRazorCodeDocument.RazorCodeDocumentsfrom aRazorProjectEngine. This ensures that the options are always configured correctly.RazorCodeDocument.Create(...)API. Realistically, this should rarely be used. It's always best to acquire aRazorCodeDocumentfrom aRazorProjectEngine.#nullable disablewhere it's easy to do. 😄Tip
If you wish to only review product changes, you can skip the following commits. Note that 1, 2, and 15 are strays, but 3-14 are grouped together in the commit history.