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

Roslyn Tokenizer #11086

Merged
merged 44 commits into from
Oct 25, 2024
Merged

Roslyn Tokenizer #11086

merged 44 commits into from
Oct 25, 2024

Conversation

333fred
Copy link
Member

@333fred 333fred commented Oct 24, 2024

This adds the new roslyn-based tokenizer, off by default, which uses Roslyn, instead of a native tokenizer, to tokenize C# code. This is a major change and will be enabled by putting <Features>use-roslyn-tokenizer</Features> in the project file.

Closes #10737
Closes #10568
Closes #7084

This is some of the infrastructure that will be needed to get the Roslyn
tokenizer in. We'll have the new tokenizer, and an option to switch back
to the legacy tokenizer. To make reviewing easier, I would highly
suggest going commit-by-commit, to make sure that the renames show up as
actual renames.
* upstream/main:
  Disable nullable warnings on .NET Standard and Framework (#10677)
Merge main into the feature branch
… about the individual differences between most C# operators, as well the difference between C# numeric types, we'll just use a single kind in the new parser to keep things simpler.
… to just be from the back of the results, as that's the most common order for the parser to reset in. I've also refactored a common advance loop to reduce duplication.
This is the big one: using the Roslyn tokenizer during Razor parsing.
I've done my best to separate out various pieces into separate commits
to make the review a bit simpler, but there's no getting around the
lexer change being complicated. I would recommend commit-by-commit to
make it as simple as possible. Fixes
#10568, fixes
#7084.
…enizer

* upstream/main: (53 commits)
  Move to central package pinning (#10716)
  Try to fix rename tests
  Unskip rename tests
  I spent 10 minutes looking up cool Mr Freeze catch phrases for this commit message, and I didn't like any of them.
  Clean up CompilationTagHelperResolver
  Clean up all ITagHelperDescriptorProviders a bit (and found a bug!)
  Make ExcludeHidden and IncludeDocumentation init-only properties
  Swap TagHelperDescriptorProviderContext.Create methods for constructors
  Remove TagHelperDescriptorProviderContext.Items property
  Make TargetSymbol a TagHelperDescriptorProviderContext property
  Make Compilation a TagHelperDescriptorProviderContext property
  Merge TagHelperDescriptorProviderContext and DefaultContext
  Don't pass code document and source text around in diagnostics translator, plus some cleanup
  Remove unnecessary parameter, because it can be trivially retrieved
  Find razor document correctly in RemoveDocumentMappingService
  Add extension methods that convert URIs to Roslyn file paths
  Use Uri.LocalPath rather than GetAbsoluteOrUNCPath()
  Move MapToHostDocuementUriAndRangeAsync to extension methods
  Fix small typo in comment
  Remove unused DocumentState method
  ...
Clean merge, no manual changes required
Does as the tin says. Prerequisite to handling directives, as we'll need the parse options to know what preprocessor symbols are enabled.
* upstream/main: (71 commits)
  Fix after merge
  PR feedback
  Bump Roslyn version
  Moving formatting service to common layer (#10761)
  Move GetSyntaxTree to document snapshot
  Inject file path service into the document snapshot
  Remove code document parameter and just use document snapshot
  Update NOTICE.txt (#10768)
  Allow @@ as a fallback (#10752)
  Rework how we get generated documents
  Directly test the component definition service in cohosting
  Add missing test case
  Defer to C# for component attribute GTD in cohosting
  Allow LSP and cohosting to provide specialized methods to get a syntax tree for a C# document
  Dev Container (#10751)
  Use a proper Try pattern
  Add tests for co-hosted GTD
  Rework IDocumentPositionInfoStrategy and use correctly in co-hosted GTD
  Add DocumentMappingSerice to RazorDocumentServiceBase
  Move IDocumentPositionInfoStrategy and friends to Workspaces layer
  ...
Minor touchup of usings and baselines necessary, no functional changes.
333fred and others added 11 commits October 11, 2024 12:26
* Add new preprocessor tests for the old parser, and copy their baselines over to the new parser for comparison.

* Add line whitespace tracking for erroring when a preprocessor token isn't the first thing on a line.

* Add initial support for parsing directives in the new lexer.

* Initial work on plumbing through parser changes to correctly support directives. Many more tests are required.

* Add more tests for other control flow blocks

* Several misc refactors

* Error on def and undef

* Start testing beginning-of-line enforcement.

* Report a warning when we see potential misplaced directives in disabled text.

* Add more tests, include the correct location in warnings.

* Usings, recomment GenerateBaselines

* Nullable annotations

* Update baselines, skip design time verification where tracked by #10981.

* Feedback.

* More feedback.
* Enable the new tokenizer for tooling tests

* Use the new lexer in the integration tests as well.

* Move to new lexer in a lot more places
Also ensure that another IDE test is run with the new lexer.
* upstream/main: (290 commits)
  Add breaking changes document (#11064)
  Do not extract component into code block (#11069)
  Fix invalid setttings json (#11062)
  update MicrosoftSourceBuildIntermediatearcadePackageVersion
  Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2566512
  Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2566512
  Update source-build team references (#11032)
  Handle EditorRequired *Changed/*Expression parameters (#11043)
  Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2566213
  Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2566213
  Avoid ambiguous `object` reference in generic component recovery (#11053)
  Move culture info check (#11057)
  Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20241015.1
  Fix code actions integration tests
  Add option for format on paste (#11039)
  Update src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentDocumentClassifierPass.cs
  Fix merge to 17.12 version
  Update src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentDocumentClassifierPass.cs
  Ensure model directives are mapped at runtime (#11007)
  Fix @inherits mapping for fuse (#10985)
  ...
Merge main into the feature branch. Only trivial merge conflicts and
baseline updates were needed.
* Switch the new lexer off-by-default.

* Add a new document for the lexer breaking changes around pragmas.

* Update comment.

* Feedback

Co-authored-by: Jan Jones <[email protected]>

---------

Co-authored-by: Jan Jones <[email protected]>
* upstream/main:
  Fix resource string (#11079)
  Update Roslyn.Diagnostics.Analyzers package to latest version
  Update MS.CA.Analyzers package to 3.11.0
  Don't reference MS.CA.BannedApiAnalyzers package directly
  Version Roslyn.Diagnostics.Analzyers package separately
  Update dependencies from https://github.com/dotnet/arcade build 20241016.1
  Update dependencies from https://github.com/dotnet/arcade build 20241016.1
  Update dependencies from https://github.com/dotnet/arcade build 20241016.1
Trivial conflict resolutions only.
@333fred 333fred requested review from a team as code owners October 24, 2024 19:03
@ady109
Copy link
Contributor

ady109 commented Oct 24, 2024

Does this also close #9975 ?

@davidwengier
Copy link
Contributor

This is a major change and will be enabled by putting <Features>use-roslyn-tokenizer</Features> in the project file.

How is this hooked up in the IDE? Or is the "will" there because it's not yet?

@333fred
Copy link
Member Author

333fred commented Oct 25, 2024

Does this also close #9975 ?

No. Not sure why I said the new lexer would handle that.

@333fred
Copy link
Member Author

333fred commented Oct 25, 2024

This is a major change and will be enabled by putting <Features>use-roslyn-tokenizer</Features> in the project file.

How is this hooked up in the IDE? Or is the "will" there because it's not yet?

Followed up offline. We'll need to plumb the option through for this, but I don't think this needs to hold up the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants