Parse partial events and constructors #76860

jjonescz · 2025-01-22T15:28:35Z

Test plan: #76859

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

CyrusNajmabadi

Done with pass.

CyrusNajmabadi · 2025-01-23T15:58:49Z

We should bring to ldm if we can just make partial a keyword in the next version. Like we've done for many other cases at this point. There's a simple workaround (using @partial).

jjonescz · 2025-01-23T16:19:16Z

We should bring to ldm if we can just make partial a keyword in the next version. Like we've done for many other cases at this point. There's a simple workaround (using @partial).

Sounds good to me. Added the question: dotnet/csharplang#9085

jjonescz · 2025-01-24T11:35:23Z

Went ahead with the breaking change. I can address any LDM feedback later, I wouldn't like to block on that since it shouldn't have any effect on subsequent PRs of the feature.

src/Compilers/CSharp/Test/Syntax/Parsing/PartialEventsAndConstructorsParsingTests.cs

docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md

src/Compilers/CSharp/Test/Syntax/Parsing/LocalFunctionParsingTests.cs

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

src/Compilers/CSharp/Test/Syntax/Parsing/PartialEventsAndConstructorsParsingTests.cs

RikkiGibson · 2025-01-27T23:54:37Z

src/Compilers/CSharp/Test/Syntax/Parsing/PartialEventsAndConstructorsParsingTests.cs

+    public void Event_Definition_CSharp13()
+    {
+        UsingDeclaration("""
+            partial event Action E;


I suspect we are in the same position we were in with records, where we can't really prompt for LangVersion with forms like record Rec() { } in old language versions.

I don't recall receiving much in the way of complaints about that, just something to be conscious of.

Yeah, I haven't investigated that but I imagine that would require some complicated changes in the parser.

You might do it by reporting a special error when a lookup fails for ‘record’ or ‘partial’ when binding a return type.

That would work for constructors, not events. I'm also not sure how the diagnostic would work exactly - if it was warning or error, it would be a breaking change even for older langversions. I guess that might be fine, it seems similar to the field breaking change diagnostics.

if it was warning or error, it would be a breaking change even for older langversions.

The scenario I am describing is one where an error already occurs, for example, partial is used in a type position, but no such type is in scope.

That would work for constructors, not events

I think events would work by simply parsing the partial modifier for them in any LangVersion, then reporting an error on the declaration.

But, to reiterate, I don't think there is a strong need to do these, it's probably something to only do if it is found to be convenient.

The scenario I am describing is one where an error already occurs, for example, partial is used in a type position, but no such type is in scope.

I see, thanks. I'm going to investigate that possibility in a future PR.

I think events would work by simply parsing the partial modifier for them in any LangVersion, then reporting an error on the declaration.

Yes, you're right, that's what actually happens in the next PR I have prepared.

src/Compilers/CSharp/Test/Syntax/Parsing/PartialEventsAndConstructorsParsingTests.cs

src/Compilers/CSharp/Test/Emit3/PartialEventsAndConstructorsTests.cs

src/Compilers/CSharp/Test/Syntax/Parsing/PartialEventsAndConstructorsParsingTests.cs

cston · 2025-01-28T04:15:26Z

src/Compilers/CSharp/Test/Emit3/PartialEventsAndConstructorsTests.cs

+            // (5,50): error CS1513: } expected
+            //         System.Console.Write(F().GetType().Name);
+            Diagnostic(ErrorCode.ERR_RbraceExpected, "").WithLocation(5, 50),
+            // (6,9): error CS0267: The 'partial' modifier can only appear immediately before 'class', 'record', 'struct', 'interface', or a method or property return type.


or a method or property return type

Should this error message be updated to include event or constructor?

Yes, it will be in the next PR (I already have that locally so I would skip adding a prototype comment for that if that's okay)

jjonescz · 2025-01-28T22:18:23Z

@RikkiGibson for a second review, thanks

RikkiGibson · 2025-01-29T03:25:37Z

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

+            //   partial event
+            if (this.PeekToken(1).Kind == SyntaxKind.EventKeyword)
+            {
+                return IsFeatureEnabled(MessageID.IDS_FeaturePartialEventsAndConstructors);


not to belabor it, but, it wasn't obvious to me why event parsing should be conditional on LangVersion. Maybe it's simpler to just be consistent? Perhaps there is some existing form we are trying to avoid breaking? Feel free to add as an open question for a future PR, if any further investigation/work is found to be needed here.

Good point, I did this for consistency but now I realize in parsing we want to avoid being conditional like this whenever possible. I don't think this can break anyone since event is a keyword anywhere (it's not contextual).

jjonescz · 2025-01-29T19:25:34Z

@RikkiGibson can you take another look? Thanks.

* Parse partial events and constructors (#76860) * Parse partial events and constructors * Update pre-existing tests * Explain why partial ctors are sometimes disallowed * Parse `partial` constructors unconditionally * Gate parsing on LangVersion * Extend tests and docs * Fix indentation * Rename combinatorial values * Test more lang versions * Parse partial events unconditionally * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250203.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.607901 -> To Version 10.0.610302 * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250204.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.607901 -> To Version 10.0.610402 * Partial events and constructors: merge and check symbols (#76970) * Partial events and constructors: merge and check symbols * Improve code * Move partial availability check * Improve code * Fix duplicate definition diagnostic * Test different ordering * Avoid reporting diagnostic for escaped `partial` * Clarify naming of AccessorsHaveImplementation property * Add event definition accessor symbol * Improve code * Test sequence points * Test extern IL * Fix interface container diagnostics * Fix sequence points issue in the test utility * Ensure the new accessor owner is a definition * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250210.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.607901 -> To Version 10.0.611002 * Remove async/await * Update dependencies from https://github.com/dotnet/arcade build 20250211.5 Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks From Version 9.0.0-beta.25077.4 -> To Version 9.0.0-beta.25111.5 * Partial events and constructors: check differences (#77118) * Partial events and constructors: check differences * Improve code and tests * Complete constructor implementation part * Report error for tuple differences in constructors * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250212.3 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.608004 -> To Version 10.0.611203 * Add test for override completion error * Fix override completion when attribute below * Partial events and constructors: doc comments (#77200) * Ensure generated operator methods have the format annotation * Update VB tests to assert new formatting behavior * Use raw strings in override tests * Ensure diagnostics refresh when source generators run in balanced mode * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250218.1 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.608004 -> To Version 10.0.611801 * Create missing options types * Fix Quick Info nullability display for backing fields (#77240) Co-authored-by: Cyrus Najmabadi <[email protected]> * Mix the source generator version info into the dependent checksum we get for projects * Make diagnostic checksum an extension * Move * simplify * Fix crash in 'introduce variable' when converting an object creation expression to an implicit object creation expression * Enable NRT * Update src/Tools/SemanticSearch/ReferenceAssemblies/Apis/Microsoft.CodeAnalysis.CSharp.txt * Partial events and constructors: attributes (#77182) * Partial events and constructors: attributes * Update pre-existing tests * Fixup expected PEVerify output * Check a flag before materializing syntax nodes * Assert that the definition part of an event doesn't have accessors * Keep pre-existing event attribute target behavior * Improve code * Add WorkItem attribute * Strengthen asserts Co-authored-by: Rikki Gibson <[email protected]> --------- Co-authored-by: Rikki Gibson <[email protected]> * Use proj dependent checksum instead now that it contains SG info * disable realtime monitoring * Update debugger contracts to fix hot reload issue * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250219.1 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.608004 -> To Version 10.0.611901 * Return stronger-ly typed results from completion * Forgot to change the file header when copying the file contents * One level deep * Add test * Add an option to ignore aliases when moving using directives outside a namespace * Add test * Add work items * Update runtime async spec (#76872) * Update runtime async spec Updates from the runtime side: dotnet/runtime#110420. * Update method signature * Add note about blocking * Fill in more examples * Remove outdated paragraph * A bit of reorganization for clarity * Collapse a number of samples to expandable section * Fix heading * Remove out of support branches from PublishData.json * Switch TextDocumentState.GetTextVersionAsync to ValueTask to reduce allocations. (#77213) The task creation from calling this method accounts for 36 MB (0.2%) of allocations in the CSharpEditingTests.Completion speedometer test. Speedometer results with this change show marked improvement in these allocations. * Fix * Update PublicAPI files for 17.13 ship (#77266) (#77295) (cherry picked from commit f8f04dd) * Expand doc comment * Update azure-pipelines-official.yml * Don't use hardcoded line endings (#77296) Was causing these files to change every build on linux. * Allow Semantic Search query function to take symbol parameter (#77272) Fix * Update to release nuget packages (#77261) (#77298) * Update to release nuget packages In order to publish 17.13 nuget packages, we need to update a few dependencies. I updated the minimum number of packages required from our public packages, then fixed up any restore issues. For each updated package, I then went and confirmed that the dll was actually present in 17.13.0, so we should be safe from dependency loading issues. This change does not need to be inserted into VS. * Skip tests for microsoft/vs-solutionpersistence#95 * Skip failing tests, see #77293. (cherry picked from commit 3350036) * Generate Documentation Auto-Insert (#76665) * wip * wip * wip * wip * wip * wip * wip * remove some commented out code * still cleaning up * revert file * clean up + option * wip * dispose of disable of intellicode line completions * comments * comments * add quota exceeded checks * fix tests * revert file * remove unused optional params * last few fixes * feedback * fix tests * small fix * fix * feedback * feedback * removed per comments * remove unused usings * feedback * feedback * move stuff around with new EA structure in roslyn * wip * feedback * feedbacl * fix * feedback * feedback * feedback * revert * remove null check * clean up * fix * fix tests * last fixes * telemetry + fixes * last comments * Move check to 'make field readonly' * Add test * Update src/Tools/SemanticSearch/ReferenceAssemblies/Apis/Microsoft.CodeAnalysis.CSharp.txt * Partial events and constructors: more tests (#77280) * Add Copilot UI prototype to Semantic Search window (#77222) * disable the WPF ComboBox Search behavior in Rename popup * Change override completion to select text after updating the buffer. (#76983) * Change override completion to select text after updating the buffer. This (along with #76969) will allow a user to commit an override completion and easily replace the default text (and still use the expression body fixers). This required the CompletionChange class to now allow the data to represent a selection instead of just a cursor position. * Explictly jump off the main thread before serializing data to call Razor * Reduce allocations during completion in FilterToVisibleAndBrowsableSymbols (#77315) * Reduce allocations during completion in FilterToVisibleAndBrowsableSymbols This method was visible in allocations in one of the completion speedometer profiles I was looking at and has a couple of easily removable allocations. Can combine two WhereAsArray calls Can switch to using WhereAsArray(ImmutableAray) instead of the IEnumerable overload Can use the pooling support in MetadataUnifyingSymbolHashSet Can avoid EditorBrowsableInfo allocation when input symbols is empty Can avoid the closure allocation that was present in RemoveOverriddenSymbolsWithinSet Even with all these changes, the net benefit didn't prove large from the speedometer run, but I've done the work, it doesn't complicate things, and every bit helps. * Fix typos (#77316) * Refresh diagnostics when fading options change * Mix options into diagnostic data checksum * Apply suggestions from code review * Support 'view call hierarchy' on primary consstructor * Fix * Arrays * cleanup tests * Add test * Docs * NRT * Primary constructor * Add nuget to documentation (#77330) * Implement TypeScript ExternalAccess wrappers for AsynchronousOperationListener (#77318) * Update configs for 17.14 P2 snap (#77329) * wip * update configs for 17.14p2 snap * Update eng/config/PublishData.json Co-authored-by: Joey Robichaud <[email protected]> --------- Co-authored-by: Joey Robichaud <[email protected]> * Simplify * Disallow initializers on partial constructor definitions (#77275) * Partial events and constructors: public API (#77202) * Partial events and constructors: public API * Simplify code * Add more `sealed` * Add more asserts * Null annotate MetadataHelpers (#77309) * Null annotate MetadataHelpers Issue #76394 indicites there is a null reference happening inside `MetadataHelpers.GetInfoForImmediateNamespaceMembers`. The symptoms indicate it's non-deterministic (deterministic ordering or race condition). Decided to null annotate the type and a few surrounding calls to see if I could get lucky and spot the source of the NRE. Was not able to do this but decided to submit the annotations. * more * more * pr feedback * pr feedback * Allow expression body refactorings on non empty selections (#76969) Currently, all expression/body refactorings only work with an empty selection. I've started looking into #29495 which requests overload completion to set the selection upon commit. Upon commit (and setting the selection), it's desirable to be able to do the expression/body refactoring. This change should allow that. * PERF: Reduce the number of nodes walked during import completion commit. (#77305) ImportAdderService.AddImportsAsync was previously doing a full tree walk (and realizing all the nodes in the tree) during override completion commit. Noticed while looking at the csharp completion speedometer profile for the completion commit of override methods. Previously, this accounted for about 28% of CPU and 23% of allocations during CommitManager.TryCommit. With these changes applied, this enumeration is basically free. See PR for details on the performance numbers * Handle ModuleCancellationTokenExpression in AbstractFlow visitor (#77310) * Fix issue in watch window completion (#77204) * Fix issue in watch window completion The issue here is when the watch window contains for (int i = 0; i < 10; i++) [Console.WriteLine(i);] where [] denotes the current statement. In that case, the adjusted context should be at the start of Console.WriteLine. Additionally, a semicolon should not be placed in the generated projection buffer before debuggerMappedSpan, as that would place debuggerMappedSpan outside the for loop. Fixes #42718 * Don't realize the SourceText in SyntaxTree.OverlapsHiddenPosition if not needed (#77334) This shows up in override completion commit in the speedometer profiles as 12.5% of allocations and 11.6% of CPU under CommitManager.TryCommit. We can avoid needing the SourceText in this method if the line mappings in the tree are empty. With this change, OverlapsHiddenRegion no longer appears in either allocations or cpu usage during override completion commit. I've also verified that these costs associated with creating this sourcetext weren't incurred elsewhere (at least under CommitManager.TryCommit) See PR for more detailed numbers * Expose some functionality from SourceGeneratedDocumentUri to Razor * Update src/Tools/ExternalAccess/Razor/RazorUri.cs * add resource to automate github action merge PRs * Update resourceManagement.yml * Preserve encoding during DocumentState updates Fixes dotnet/sdk#46780 * Fix issue where we were crashing trying to get inheritance margin information for SG docs * Docs * Generate-Documentation Check if file is excluded (#77353) * check for excluded files * remove unused usings * fix indentation * PR Validation - allow insertion to run on commit that isn't latest (#77369) * allow insertion to run on commit that isn't latest * remove comments * Allow PR Val pipeline runs to opt-out of the latest commit validation * Update CI excludes paths to include some yml files * Update setup-pr-validation.ps1 * Apply suggestions from code review --------- Co-authored-by: Joey Robichaud <[email protected]> * Fix yml typos * Update PublishData.json after VS Snap (#77383) * Fix formatting in tests * Collection expr * Collection expr * Make selaed * Make selaed * Primary constructors * Fix out of bounds in text extent computation * Fix out of bounds in text extent computation * Fix issue where goto-def wouldn't go to the right place after invoking twice * [main] Update dependencies from dotnet/source-build-reference-packages (#77335) * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250224.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.611901 -> To Version 10.0.612402 * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250225.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.611901 -> To Version 10.0.612502 * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250227.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.611901 -> To Version 10.0.612702 * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250228.4 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.611901 -> To Version 10.0.612804 * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250303.3 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.611901 -> To Version 10.0.615303 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Partial events and constructors: IDE (#77337) * Partial events and constructors: IDE * Fix event keyword recommender * Fix symbol suggestions * Fixup failing tests * Fixup SymbolKey * Improve code * Use `Equal` instead of `Same` in SymbolKey tests * Extend a test * Use collection expressions Co-authored-by: Cyrus Najmabadi <[email protected]> --------- Co-authored-by: Cyrus Najmabadi <[email protected]> * Partial events and constructors: compact error codes * Detect more null check patterns * Emit call to ThrowIfNull * Add string checks * Add string checks * Add work items * Improve performance of SyntaxReplacer.ReplaceNode (#77314) Improve performance of SyntaxReplacer.ReplaceNode This should allow better performance when replacing nodes. I've noticed overrides completion commit profiles as indicating a large amount of time/memory spent in their ReplaceNode calls. (about 18% of CPU time and 20% of memory allocations spent within CommitManager.TryCommit are attributable to ReplaceNode calls) See PR for more details on the performance benefit the speedometer achieves with this change. * Fix issue where we were changing semantics when converting to a collection expr * Work item * Update test * Update formatting tests * File scoped namespace * Fix projects * Configure main-vs-deps (#77408) * Configure main-vs-deps * Run every 3 hours * Don't use an implicit-object if the lang version doesn't support it * Add workitem * Fix bot name (#77436) * Auto merge PRs instead of just applying a label to them (#77438) * Small cleanup in the packages and editor factories (#77439) * Small cleanup in the packages and editor factories This area is going to get some larger changes in Dev18. As I'm waiting for that branch to become available, do a small cleanup in the area in anticipation of those changes. These changes are mostly cosmetic. The only change that is a bit more than that is the (Un)RegisterObjectBrowserLibraryManager changes to not be sync. This was done to remove a couple JTF.Runs in the code, and was done after verifying with JeffRo that our package disposal will indeed always be done on the main thread. * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250305.3 (#77452) Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.615303 -> To Version 10.0.615503 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Remove workaround of WPF bug This shouldn't be needed anymore, and prevents some design time build breaks if NuGet restore wasn't able to get this package. Fixes #74923 * Fix design time build error in Roslyn.VisualStudio.Setup The MSBuild task invocation here is a copy/paste from earlier in the file, but the item group renamed so it didn't conflict. The rename was in complete, so the use in the Properties property was still using the old name; this had the effect that we wouldn't set the TFM property, which would cause problems for multi-targeting projects. * [Rename flyout] Fixed cropping issue on small screens * Addressed feedback * Remove BoundConversion.OriginalUserDefinedConversionsOpt property (#77465) * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250306.1 (#77470) Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.615503 -> To Version 10.0.615601 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Update dev18 target branch (#77443) * Remove unnecessary imports * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250310.1 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.615601 -> To Version 10.0.616001 * Mark partial events and ctors as complete (#77525) --------- Co-authored-by: Jan Jones <[email protected]> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: David Barbet <[email protected]> Co-authored-by: Ankita Khera <[email protected]> Co-authored-by: David Wengier <[email protected]> Co-authored-by: Rikki Gibson <[email protected]> Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: Rikki Gibson <[email protected]> Co-authored-by: akhera99 <[email protected]> Co-authored-by: Joey Robichaud <[email protected]> Co-authored-by: Todd Grunke <[email protected]> Co-authored-by: Youssef1313 <[email protected]> Co-authored-by: Joey Robichaud <[email protected]> Co-authored-by: Tomáš Matoušek <[email protected]> Co-authored-by: Amadeusz Wieczorek <[email protected]> Co-authored-by: Charles Stoner <[email protected]> Co-authored-by: Jared Parsons <[email protected]> Co-authored-by: Sam Harwell <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Jason Malinowski <[email protected]> Co-authored-by: Etienne Baudoux <[email protected]> Co-authored-by: AlekseyTs <[email protected]> Co-authored-by: Jan Jones <[email protected]>

jjonescz added Area-Compilers Feature - Partial Events and Constructors labels Jan 22, 2025

ghost added the untriaged Issues and PRs which have not yet been triaged by a lead label Jan 22, 2025

Parse partial events and constructors

ba8ca9d

jjonescz force-pushed the PartialEventsCtors-01-Parsing branch from 91ccc2a to ba8ca9d Compare January 22, 2025 16:09

jjonescz changed the base branch from main to features/PartialEventsCtors January 22, 2025 16:10

This comment was marked as resolved.

Sign in to view