Merge main to features/improved-definite-assignment #52900
Merged
Conversation
This file contains 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
This is just the invocation of the extract interface refactoring and then some immediate follow-ups to trivially retarget types.
…documents The overall approach here is to make GetOpenDocumentInCurrentContextWithChanges work to return SourceGeneratedDocuments like anything else. The strange bit is we have to ensure the source generated document matches the text that's currently in the buffer. Consider a case where: 1. The user makes a change in another file which means the generator will produce new results. That generator is running async. 2. The user switches back to the generated file and immediately invokes a command on it before we refresh the buffer contents. In that case, the buffer contents are stale, but we need to ensure the SourceGeneratedDocument given to all our features is in sync with the actual text buffer, or otherwise spans and everything won't align. This is very similar to the general concept of the "with changes" portion of that API: you may always get a forked version of the world that doesn't represent an entirely consistent view of the entire world, but it's at least going to ensure your document matches your starting point. In the "regular case" that the generator has already ran and we're able to confirm the contents of the text buffer still matches the generated output, this (like for regular documents) doesn't fork anything at all -- you end up with the Solution object that matches Workspace.CurrentSolution. The implementation approach here is to ensure that when we do fork a snapshot the final Compilation has the tree matching the text present no matter what. One approach would have been to fork the compilation tracker with some extra special state that remembers to fix that up in the end but I had two concerns with that approach: 1. The compilation tracker implementation is already crazy complicated. 2. Forking the compilation tracker while a generator is running potentially now running generators twice depending on the timing. I decided to take the approach that CompilationTracker has an interface extracted for it's actual surface area, and then when we do the forked solution we create a different implementation of that interface that forwards to the underlying implementation and then replaces out the tree at the very end. This means we don't ever have generators running twice, and the magic of swapping out the tree is all contained in the special implementation and the core implementation is untouched.
…uleOrDynamic works This is a naive implementation that simply ensures we don't get different behavior from the main implementation. Most of this change is just moving the creation and matching logic into UnrootedSymbolSet.cs itself.
This works the same as the GetOpenDocumentInCurrentContextWithChanges. This fixes navigation bars specifically which use this.
We were holding onto the ISourceGenerator instance and passing it around for various reasons; this decomposes it into holding onto the assembly name and type name strings, so we can serialize this across processes.
We don't know up-front whether a generated file exists in the workspace, and our APIs to grab a document from a buffer are intended to complete quickly. If it turns out later the file isn't in the workspace anymore we will fake it and add it back so that way features aren't surprised by this. However, that means semantics in that file may be inaccurate. Disconnecting the document means that once the file is gone, then we won't be in this incorrect state for long.
It's a checksum of the attributes, not info, which can be confusing here.
This isn't actually used by anything.
We already had an assert that we shouldn't read one, but this adds the same assert on the sending side where it's easier to debug the source of the problem.
There are a number of asserts which try to ensure that we don't try to sync a null across the wire during solution sync. If you wanted to try using Checksum.Null as a placeholder for an optional value during synchronization, you'd hit these asserts because not everything would filter them out. This change filters them out during some parts of the synchronization process, allowing it to be used as an optional value.
The solution has the concept of "frozen" source generated documents where we force a solution snapshot to have a source generated document of a certain content, even if the generator is producing something new. This allows us to isolate features operating on a source generated document open in the editor that hasn't been updated yet -- we freeze the contents of the source generated file to match the open buffer, so taggers and the like can still function normally. This commit ensures that we synchronize those to the OOP process so everything stays in sync. The primary counter-intuitive bit is why we're holding onto this information at the SolutionState level, but that's because the place where the information is ultimately needed is the compilation trackers, which are held by the Solution object and nowhere else. This allows us to reuse the underlying project states for maximum efficiency.
Since we want to put this into 16.10 but we're past the feature cutoff, we'll put this behind an experimental flag.
…nerated-files-to-the-workspace Connect open source generated files to the workspace
|
Apologies, I am afraid I am encountering technical difficulties that might have hampered my ability to assist with merging this pull request. I will continue to try to assist if there are further changes to this pull request. |
RikkiGibson
deleted the
merges/main-to-features/improved-definite-assignment
branch
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.
This is an automatically generated pull request from main into features/improved-definite-assignment.
Once all conflicts are resolved and all the tests pass, you are free to merge the pull request. 🐯
Troubleshooting conflicts
Identify authors of changes which introduced merge conflicts
Scroll to the bottom, then for each file containing conflicts copy its path into the following searches:
Usually the most recent change to a file between the two branches is considered to have introduced the conflicts, but sometimes it will be necessary to look for the conflicting lines and check the blame in each branch. Generally the author whose change introduced the conflicts should pull down this PR, fix the conflicts locally, then push up a commit resolving the conflicts.
Resolve merge conflicts using your local repo
Sometimes merge conflicts may be present on GitHub but merging locally will work without conflicts. This is due to differences between the merge algorithm used in local git versus the one used by GitHub.
git fetch --all git checkout merges/main-to-features/improved-definite-assignment git reset --hard upstream/features/improved-definite-assignment git merge upstream/main # Fix merge conflicts git commit git push upstream merges/main-to-features/improved-definite-assignment --force