Skip to content

Describe scoping rules for extension declaration type parameters and receiver parameter #9229

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

MadsTorgersen
Copy link
Contributor

Based on LDM feedback, these are revised scoping rules for extension declarations.

@MadsTorgersen MadsTorgersen requested a review from a team as a code owner March 17, 2025 20:09
@MadsTorgersen MadsTorgersen marked this pull request as draft March 17, 2025 20:12
@MadsTorgersen MadsTorgersen marked this pull request as ready for review March 17, 2025 20:23
@@ -50,6 +50,38 @@ receiver_parameter // add
Extension declarations shall only be declared in non-generic, non-nested static classes.
It is an error for a type to be named `extension`.

### Scoping rules

The type parameters and receiver parameter of an extension declaration are in scope within the body of the extension declaration. It is an error to refer to the receiver parameter from within a static member. It is an error for members to declare type parameters or parameters with the same name as a type parameter or receiver parameter of the extension declaration.
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an error to refer to the receiver parameter from within a static member.

It is an error to refer to the receiver parameter from within a static member.

Consider relaxing this restriction for references inside nameof (nameof arguments). We do that for primary constructor parameters as well.

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (commit 9)

@@ -50,6 +50,38 @@ receiver_parameter // add
Extension declarations shall only be declared in non-generic, non-nested static classes.
It is an error for a type to be named `extension`.

### Scoping rules

The type parameters and receiver parameter of an extension declaration are in scope within the body of the extension declaration. It is an error to refer to the receiver parameter from within a static member. It is an error for members to declare type parameters or parameters with the same name as a type parameter or receiver parameter of the extension declaration.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an error for members to declare type parameters or parameters with the same name as a type parameter or receiver parameter of the extension declaration.

I think it is worth mentioning that the same goes for local variables and local function declared directly by members (not inside lamndas or local functions)

AlekseyTs added a commit to AlekseyTs/roslyn that referenced this pull request Mar 20, 2025
AlekseyTs added a commit to dotnet/roslyn that referenced this pull request Mar 21, 2025
jjonescz added a commit to dotnet/roslyn that referenced this pull request Apr 1, 2025
* Change vs deps flow (#77651)

* Add tests

* Add version check

* Fix type argument removal

* Add tests

* Add tests

* Add tests

* Add tests

* Add tests

* Add tests

* Add tests

* Docs

* move to local functions

* Remove unused using

* Docs

* Add tests

* Add tests

* Add tests

* Loc strings

* Loc strings

* Fix generation of attribute with array constant

* Make ISemanticSearchCopilotUIProvider import lazy to avoid loading VS.EA.Copilot (#77516)

* Fix watch window completion window upon manual completion invocation (#77656)

* Fix watch window completion window upon manual completion invocation

Addresses https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2287966

This recently regressed due to this recent PR: #77204

The issue here is that I changed the csharp debugger context's projection buffer creation code from always having a one char string as the second sourceSpan to sometimes having that string be empty. This allowed the completion context to include items for which a semicolon would close that scope. However, by having a zero length span, a seam was created for a tracking span at priorTrackingSpan to be introduced into the set of tracking positions that can contribute to completion. Editor completion doesn't handle this well, so we're best off always sending a non-zero length source Span, so we do so and just use a space as the value.

* Extensions: flesh out behavior for some pattern-based binding scenarios (#77608)

* Generate Documentation - Bug Fixes (#77641)

* fix some bugs

* Feedback

* More receiverType check out of EnumerateAllExtensionMembersInSingleBinder

* Address feedback

* Remove TODO

* Fix formatting

* Update maintenance-packages versions conditioned to sourcebuild only

* Fix reflection

* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250318.1 (#77672)

Microsoft.SourceBuild.Intermediate.source-build-reference-packages
 From Version 10.0.616701 -> To Version 10.0.616801

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Fix typo (#77678)

disalloed => disallowed.

* Extensions: Adjust accessibility and file-type checks for extension members (#77657)

* Avoid adding duplicate suppressors to the host analyzer arrays

* Remove unnecessary code

* Add docs

* Simplify

* Add docs

* Strong types

* move removed api

* Fix SkipApplyOptimizationData parameter (#77677)

* Avoid alloc

* Add repro for IDE issue

* Don't check constraint in explicit type argument scenario for now

* Support textDocument/semanticTokens/full

* Revert "Don't check constraint in explicit type argument scenario for now"

This reverts commit e3af843.

* Use type parameter definitions and avoid creating new type maps in checkConstraintsIncludingExtension

* Add more tests

* Upgrade from VSTelemetryApi 17.14.2 to 17.14.8

* Cleanup and make semantic token processing and testing code more consistent

* Fix test

* Add CI validation of Semantic Search API lists (#77535)

* raw strings

* Fix bug where exact path match would throw for additional files (#77583)

From /pull/77517/files/45c0e103f76f36bed6004f836d3dcfeae4bfae0d#r1992506030

* Extensions: update semantic model APIs (#77619)

* Update to latest in nuget.org

* Update CSharpCopilotCodeAnalysisService.cs (#77691)

* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250319.2 (#77694)

Microsoft.SourceBuild.Intermediate.source-build-reference-packages
 From Version 10.0.616801 -> To Version 10.0.616902

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Disable integration tests on PRs to main-vs-deps and above

* Make the ci build happy

* Update aps

* Simplify

* Update compiler

* Update src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs

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

* Update src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb

* Update src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb

* Update src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb

* Remove old vs4mac code

* Files

* Fix copilot layering

* Fix tests

* Fix tests

* Fix

* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250320.2

Microsoft.SourceBuild.Intermediate.source-build-reference-packages
 From Version 10.0.616902 -> To Version 10.0.617002

* Do not consider `ref` argument to `in`/`ref readonly` parameter as being written to (#77682)

Fixes #77528.
Related to test plan #68056.

* Update dotnet format instructions (#77699)

* Add -solution option to build.ps1 (#77698)

* Update infra

* Implement scoping and shadowing rules for extension parameter and type parameters (#77704)

See dotnet/csharplang#9229 (commit 9)

* Fix updating committed solution with insignificant changes (#77648)

* Extensions: only look for new extensions in new LangVer (#77690)

* Improve raw string completion

* Fix

* Fix speeling

* Extensions: definite assignment and region analysis (#77675)

* Make things more strongly typed in compilation tracker

* Show lightbulb on member name as well as on throw node

* Rework analyzer assembly loading (#77004)

This change restructures our analyzer assembly loading to allow for composable customizations to meet the demands of customers like VBCSCompiler, VS IDE, Razor and VS Code. Specifically, these layers want to customize two aspects of assembly loading:

1. The location where an assembly is loaded from. For example shadow copying does not change the content of what is loaded but it does change where it is loaded from 
2. The `Assembly` loaded for a given path / name. For example Razor wants to control exactly which `Assembly` from which `AssemblyLoadContext` is loaded when running inside the VS OOP process.

The existing customization approach was to derive from `AnalyzerAssemblyLoader`, override the right `virtual` and customize the behavior. That had limits, particularly because different parts of the VS IDE want to override these behaviors in different ways. That forced us to centralize logic that we didn't want to centralize (baking Razor into workspaces) and was challenging to integrate with our MEF composition. 

The new approach is to have a `sealed` type that allows customization through interfaces. 

This PR only establishes the new architecture and leaves the IDE unchanged. I originally attempted to do both at one time and the PR was hard to follow, the perf implications were challenging to track down and verify. Instead going to approach this by getting the basics in place then start migrating parts of the IDE over one at a time.

* Remove all the existing workaround checks

All of these have been fixed long ago (or we suspect they've long been
fixed.)

Closes #35123

* Ensure we also package Microsoft.Extensions.DependencyInjection.Abstractions

* Add new skips for Codelens

For reasons I'm unable to diagnose the codelens ServiceHub service is
failing, possibly because it's missing a reference. I'm unable to sort
out why, but it's not critical for integration tests so just ignore
the failure.

* Remove exclusion of main-vs-deps for running integration tests

* Cleanup for code review

* Improve the lightbulb detection

* Fix handling of partial definition deletes (#77735)

* Extensions: account for new extensions in function type scenarios (#77755)

* Do not allow System.CommandLine to swallow all exceptions

* Set the default namespace for the project and proper folders for documents.

Resovles dotnet/vscode-csharp#8091

* Fix move to namespace offering for primary constructor (#77751)

* add comment

* Update src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs

Co-authored-by: Jason Malinowski <[email protected]>

* Update tests to verify Folders change.

* Clean up branches that we're excluding

* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250324.2

Microsoft.SourceBuild.Intermediate.source-build-reference-packages
 From Version 10.0.617002 -> To Version 10.0.617402

* Update infra

* Fix issue with disappearing newlines after multiline documentation comment (#77521)

* Fix issue with disappearing newlines after multiline documentation comments

* Fix formatting issues

* Convert LineBreaksAfterLeading to local function

* Update function syntax to match conventions

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

* Fix compilation errors

---------

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

* Disallow complex forms of IndexerName attributes in extensions (#77781)

* Implement name an signature conflict check across extension containers with the same receiver type (#77747)

Implements the following rule from the spec:
"Within a given enclosing static class, the set of extension member declarations with the same receiver type (modulo identity conversion and type parameter name substitution) are treated as a single declaration space similar to the members within a class or struct declaration, and are subject to the same rules about uniqueness."

* Address PROTOTYPE comment in ExtensionMethodBodyRewriter (#77782)

* Simplify tests

* Simplify tests

* Simplify tests

* Simplify tests

* Remove unnecessary nullable directives

* Remove unnecessary nullable directives

* Remove unnecessary nullable directives

* Remove unnecessary nullable directives

* Remove unnecessary nullable directives

* Remove unnecessary nullable directives

* Remove unnecessary nullable directives

* Remove unnecessary nullable directives

* Update behavior to only report when throw in member

* Fix tests

* Remove unnecessary nullable directives

* fixed test

* Generate Documentation - Add remarks if available (#77783)

* add remarks to documentation if available

* add named parameter for nulls

* Reduce likelihood of needing a dual insertion in VS Code (#77715)

* Create new folder structure for Razor EA.

/Shared : will eventually be the area for all source items shared between
the current EA and the new EA.Razor.Features
/Features : will have an EA that is restricted to depending only on the
features layer. This will help provide a way to ship a more sane EA
structure for VS Code as well as reduce the likelyhood and work required
for a dual insertion
/EditorFeatures : contains the current EA. The project name is kept the
same for now but the folder is moved. This denotes that the binary does
depend on the EditorFeatures layer in Roslyn

* Update to new behavior - always show on throw

* nit cleanup

* Remove whitespaces

* Remove extra blank line

* Remove allThrows array

* Simplify logic

* Simplify

* Comment

* Workaround issue related to crossgen and specific .NET 9 runtimes

* Show fleshed out example

* Show fleshed out example

* CommentS

* Fixes tests

Left to do: => case

* Fixes => case

* use patterns

* use patterns

* formatting

* Complete fix

* Set PackageProjectUrl (#77824)

Avoids the SDK setting this to dotnet/dotnet automatically, which is generally not useful for customers.

* [main] Update dependencies from dotnet/source-build-reference-packages (#77829)

This pull request updates the following dependencies

[marker]: <> (Begin:4e88e544-5668-4a45-0f48-08dc0c57b4c0)
## From https://github.com/dotnet/source-build-reference-packages
- **Subscription**: 4e88e544-5668-4a45-0f48-08dc0c57b4c0
- **Build**:
[20250325.1](https://dev.azure.com/dnceng/internal/_build/results?buildId=2671647)
- **Date Produced**: March 25, 2025 5:28:05 PM UTC
- **Commit**:
[d2fc98192bb9780acbe2ad3df284da19203cc26d](dotnet/source-build-reference-packages@d2fc981)
- **Branch**: refs/heads/main

[DependencyUpdate]: <> (Begin)

- **Updates**:
-
**Microsoft.SourceBuild.Intermediate.source-build-reference-packages**:
[from 10.0.617402 to 10.0.617501][1]

[1]:
dotnet/source-build-reference-packages@4a8b582...d2fc981

[DependencyUpdate]: <> (End)


[marker]: <> (End:4e88e544-5668-4a45-0f48-08dc0c57b4c0)

* Extensions: check inferrability (#77815)

* Extensions: Review/adjust call sites of `MethodSymbol.IsExtensionMethod` API (#77820)

* Extensions: compact error codes (#77832)

* Review call sites of extension methods related APIs (#77838)

- BoundCollectionElementInitializer.InvokedAsExtensionMethod
- BoundCall.InvokedAsExtensionMethod
- BoundDelegateCreationExpression.IsExtensionMethod
- MethodGroup.IsExtensionMethodGroup
- MethodGroupResolution.IsExtensionMethodGroup

* Update Ubuntu and MacOS images (#77833)

* Apply PR feedback

* Block use of extension properties in unsupported scenarios (#77844)

* Extensions: remove PROTOTYPE comments (#77836)

* Update src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionDiagnosticAnalyzer.cs

Co-authored-by: Cyrus Najmabadi <[email protected]>

* Log trace messages as "debug" so it formats right

Otherwise this gets logged by the LSP client as a generic message,
which isn't prefixed with a severity like everything else.

* Support ILogger.BeginScope() for LSP logging

This surrounds the processing of each LSP request with a call to
ILogger.BeginScope() with the name of the method that's being invoked.
This is prefixed to the message we send back to the LSP client, so
it's much easier to understand the source of various pieces of logging.

ILspLogger.LogStartContext/LogEndContext are deprecated here, as
setting scopes plus an explicit start/end as appropriate makes things
work much better when we're handling requests in parallel.

* Completely delete the obsolete methods

* Resolve merge conflicts

* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250326.1

Microsoft.SourceBuild.Intermediate.source-build-reference-packages
 From Version 10.0.617501 -> To Version 10.0.617601

* Skip failing tests on macOS

* Parse ignored directives (#77696)

Speclet:
https://github.com/dotnet/csharplang/blob/main/proposals/ignored-directives.md
Public API: Resolves #77697.

* Null-conditional assignment: Add tests from test plan review (#77717)

* Implement support for Semantic Search agent and tool (#77784)

* Another attempt to fix the SDK insertion issue related to runtime/apphost downloads (#77845)

Use the EnableRuntimePackDownload/EnableAppHostPackDownload flags to indicate whether the restore should download those packages./

* Upgrade to VSSDK.BuildTools 17.14.1043-preview2

The original reason to upgrade to this is carries along updates
to the Fast Up to Date Check support, which means Roslyn can delete
our own copy of the targets.

The upgrade caused one break: there's newer support in the VS SDK now
where they look for ForceIncludeInVSIX metadata on PackageReferences
as a way to bypass the suppression logic that exists to ensure certain
references don't end up in the final VSIX. We had our own custom logic
where you'd set just IncludeInVSIX="true" as metadata, which would
produce items under teh covers with ForceIncludeInVSIX=true. There's
arguably a bug in the targets that they should be respecting our
underlying metadata rather than looking back to the PackageReference
metadata, but it still seems reasonable for us just to adopt the same
metadata name as the VS SDK. At this point it's not clear to me if
we even need our custom logic anymore or if it's now obsolete.

# Conflicts:
#	eng/Versions.props
#	src/VisualStudio/Setup.Dependencies/Roslyn.VisualStudio.Setup.Dependencies.csproj

# Conflicts:
#	src/VisualStudio/Setup.Dependencies/Roslyn.VisualStudio.Setup.Dependencies.csproj

* Generate Documentation - Add loading state (#77718)

* wip

* loading state works

* clean up

* feedback/comments

* fix formatting

* fix formatting

* feedback

* change enqueueaction

* last feedback

* feedback

* commented out line of code

* Use --opt-cross-module only for assemblies within the servicing bubble

* Fix up comment

* Remove condition

* Fix up urls in docs

* Add rules docs

* Regenerate documentation

* Silence warning about release note header

* Update default .NET Core metadata references

* Allow the new Razor project to access protocol types

* Remove richnav pipeline and package (#77902)

The service no longer exists.

* Update the link to the file Portable PDB v1.0: Format Specification (#77911)

* Feedback from analyzer loading change (#77780)

* Feedback from analyzer loading change

Few changes here:

1. Jason had suggested I add an assert into the shadow copy loader, I
   did but apparently forgot to save.
2. Rename the file to match type name

Follow up to #77004

* PR feedback

* Apply suggestions from code review

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

---------

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

* Use NoWarn for RS2007

* Skip tests

* Razor EA: load from ServiceHubCore (#77720)

* Razor EA: load from ServiceHubCore

* Update src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs

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

* Trim trailing slashes

* Update src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs

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

* Fix trim call

* PR feedback

---------

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

---------

Co-authored-by: Cyrus Najmabadi <[email protected]>
Co-authored-by: dotnet-policy-service[bot] <123482357+dotnet-policy-service[bot]@users.noreply.github.com>
Co-authored-by: David Barbet <[email protected]>
Co-authored-by: Tomáš Matoušek <[email protected]>
Co-authored-by: Todd Grunke <[email protected]>
Co-authored-by: AlekseyTs <[email protected]>
Co-authored-by: Ankita Khera <[email protected]>
Co-authored-by: Julien Couvreur <[email protected]>
Co-authored-by: Joey Robichaud <[email protected]>
Co-authored-by: Carlos Sánchez López <[email protected]>
Co-authored-by: David Wengier <[email protected]>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Bill Wagner <[email protected]>
Co-authored-by: Sam Harwell <[email protected]>
Co-authored-by: PaddiM8 <[email protected]>
Co-authored-by: Arun Chander <[email protected]>
Co-authored-by: Andrew Hall <[email protected]>
Co-authored-by: Julien Couvreur <[email protected]>
Co-authored-by: Cyrus Najmabadi <[email protected]>
Co-authored-by: Maryam Ariyan <[email protected]>
Co-authored-by: Jared Parsons <[email protected]>
Co-authored-by: Jason Malinowski <[email protected]>
Co-authored-by: David Barbet <[email protected]>
Co-authored-by: Jason Malinowski <[email protected]>
Co-authored-by: Joey Robichaud <[email protected]>
Co-authored-by: Dimitris Gounaris <[email protected]>
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Rikki Gibson <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
Co-authored-by: Dong A. <[email protected]>
Co-authored-by: Chris Sienkiewicz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants