Provide IntelliSense for ~/ asset paths in Razor components - #85058
Draft
chsienki wants to merge 4 commits into
Draft
Provide IntelliSense for ~/ asset paths in Razor components#85058chsienki wants to merge 4 commits into
chsienki wants to merge 4 commits into
Conversation
Razor tooling has no awareness of static web assets. The SDK flows the set of keys that resolve through ResourceAssetCollection as an additional file tagged with IsStaticWebAssetsManifest, which is what makes this cheap to keep current: the manifest is part of the workspace snapshot, so a rebuild that changes it produces a new document version and invalidates the cached parse without a file watcher or any disk access from OOP. A manifest that cannot be understood costs completions rather than failing the request that asked for them, so a newer schema version or a file caught midway through being written degrades quietly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a8730b9d-cc1b-4c5d-b22e-b444c2917af2
ComponentTildePathPass rewrites a '~/' literal into Assets["..."] wherever an (element, attribute) pair opted in via [AcceptsAssetPath] or a component parameter is marked [AssetPath]. Nothing offered those paths, and the HTML language server filled the gap with relative file path completions, which are the wrong thing entirely for a value resolved against a manifest rather than the file system. Those items are now suppressed for an opted-in '~/' value. The allowlist comes from the project-wide tag helper set rather than the document's. The carriers produced from [AcceptsAssetPath] live on a type in the runtime's namespace, so scoping them by the document's @using directives would let completion and compilation disagree about which attributes opted in. Two gates keep the offer honest. The tag helper producer that discovers [AcceptsAssetPath] is registered from Razor 3.0 while expansion only happens from 11.0, so a project can hold the opt-in metadata and still have nothing rewritten. A value mixing a literal with a C# expression never expands either. Resolving the project's asset data is gated behind a textual check for '~/', because completion runs on most keystrokes and this data is project-scoped. Items carry an explicit TextEdit: asset keys contain '/' and '.', so the editor's word-boundary heuristic would replace only the last path segment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a8730b9d-cc1b-4c5d-b22e-b444c2917af2
Without these, the asset path list only appears on an explicit Ctrl+Space, because the Razor trigger set is limited to '@', '<', ':' and space. Both characters already reach the editor's registration through the C# set, so the set of characters the client triggers on is unchanged; this only widens which triggers are routed to the Razor providers. Providers that don't care return immediately. The accompanying test pins all three trigger sets. They flow straight into the LSP registration, where widening one changes how often the editor asks for completions across every Razor document and narrowing one silently stops a feature from ever being offered while typing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a8730b9d-cc1b-4c5d-b22e-b444c2917af2
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
The file-kind check moved into AssetPathCompletionFacts.IsSupported, leaving nothing in this file that needs Microsoft.AspNetCore.Razor.Language. IDE0005 is an error under the correctness leg's RoslynEnforceCodeStyle build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a8730b9d-cc1b-4c5d-b22e-b444c2917af2
Member
Author
|
/pr-validation |
Contributor
|
View PR Validation Run triggered by @chsienki Parameters
|
|
|
||
| namespace Microsoft.CodeAnalysis.Remote.Razor.Completion; | ||
|
|
||
| public class AssetPathCompletionItemProviderTest : RazorToolingIntegrationTestBase |
Member
There was a problem hiding this comment.
Rather than, or perhaps in addition to, these provider specific unit tests, it would be good to have some end-to-end tests in CohostDocumentCompletionEndpointTest so its easy to see that this all works together as a complete system.
| { | ||
| TestCode testCode = """<img src="~$$/" />"""; | ||
|
|
||
| Assert.Empty(GetCompletionItems(testCode)); |
Member
There was a problem hiding this comment.
I don't see why we couldn't offer completion here? Seems like it would be helpful. I also don't see a test for an explicit invocation in an empty attribute, but completion in there would be helpful too.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Adds IntelliSense for
~/asset paths in Razor components, the tooling half ofthe compile-time expansion that landed in #84796.
Fixes #84793
What this does
Inside an attribute that opted into asset-path expansion, typing
~/now offersthe project's static web assets:
The HTML language server currently fills this gap with relative file path
completions, which are the wrong thing entirely for a value resolved against the
static web assets manifest rather than the file system. Those are now suppressed
once the value starts with
~/.Where the data comes from
The set of keys that resolve through
ResourceAssetCollectionarrives as anAdditionalFiletaggedIsStaticWebAssetsManifest, emitted by a companionchange in dotnet/sdk. Riding the workspace snapshot means invalidation is by
document version, with no file watcher and no disk access from OOP.
This PR is inert until that SDK change ships — an absent manifest yields an
empty asset set and no completions — so it can merge independently.
Notes for reviewers
document's. The carriers produced from
[AcceptsAssetPath]live on a type inthe runtime's namespace, so scoping them by the document's
@usingdirectiveswould let completion and compilation disagree about which attributes opted in.
ComponentTildePathPassreads the full set for the same reason.[AcceptsAssetPath]is registered from Razor 3.0 while expansion only happensfrom 11.0, so a project can hold the opt-in metadata with nothing rewritten.
Note
RazorLanguageVersion.Latestis still 9.0, so today this only lights upunder
preview. Mixed literal/expression values are excluded too, matching thepass.
~and/are added to the Razor set. Both alreadyreach the client's registration through the C# set, so what the editor triggers
on is unchanged; this only widens what is routed to the Razor providers, all of
which early-out. Nothing pinned these sets before, so the last commit adds a
test that does — worth a look, since
/now reaches the providers on everyclosing tag.
TextEditon items. Asset keys contain/and., so theeditor's word-boundary heuristic would otherwise replace only the last segment
of a partially typed path.
Testing
AssetPathCompletionItemProviderTestcovers the HTML allowlist,[AssetPath]parameters, non-opted-in element/attribute, cursor inside the
~/prefix, cursorin the attribute name, empty asset set, mixed content, legacy documents, and the
language-version gate.
StaticWebAssetsManifestReaderTestincludes verbatim realSDK output so a shape change there fails loudly rather than silently costing
completions.
Green locally: Remote.Razor 519/519, Workspaces 423/423,
VisualStudioCode.RazorExtension 1525/1525.
Each of the three commits builds and tests independently.
Microsoft Reviewers: Open in CodeFlow