Skip to content

Extensions: allow cref references to extension members #78735

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

Merged
merged 12 commits into from
Jun 12, 2025

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented May 28, 2025

Relates to test plan #76130
Review of public API: #78738

@jcouv jcouv self-assigned this May 28, 2025
@jcouv jcouv added Area-Compilers Feature - Extension Everything The extension everything feature labels May 28, 2025
@jcouv jcouv force-pushed the extensions-cref branch from 34b59c7 to 34ef1fb Compare May 28, 2025 21:34
@dotnet-policy-service dotnet-policy-service bot added the Needs API Review Needs to be reviewed by the API review council label May 28, 2025
@jcouv jcouv force-pushed the extensions-cref branch from 34ef1fb to 15fdb0b Compare May 28, 2025 23:01
@@ -181,7 +181,7 @@ public override object VisitNamedType(NamedTypeSymbol symbol, StringBuilder buil
builder.Append('.');
}

builder.Append(symbol.IsExtension ? symbol.ExtensionName : symbol.Name);
builder.Append(symbol.IsExtension ? escape(symbol.ExtensionName) : symbol.Name);
Copy link
Member Author

@jcouv jcouv May 29, 2025

Choose a reason for hiding this comment

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

📝 I debated whether to escape here, or push that responsibility to the caller of DocumentationCommentIDVisitor. What do you think? #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we replace </> with {/} for type argument list below, it probably makes sense to avoid using them here as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did some additional thinking about this issue. Now, I think we need to consider possible usage scenarios for the ids that we produce here. What users actually might want to do with them. On one hand, the ids are included into an XML document and reserved XML characters must be replaced with XML entities inside the document. On the other hand, if I am not mistaken, when an XML document is read by an XML reader, the reader replaces the XML entities back to the corresponding characters. Is there a scenario when one tries to match an id gotten from an XML reader with an id that we produce here and exposing through GetDocumentationCommentId API? Or vise versa? If so, doing this transformation here is not the right thing to do. The replacement for </> with {/} for type argument lists is different, it doesn't involve XML entities, an XML reader wouldn't reverse it.

I am leaning towards doing one of the following:

  • Moving this "escaping" to the step that serializes an XML document. Basically, during serialization all ids should be checked for presence of </> and they should be replaced with corresponding XML entities.
  • Or we can by pass the problem by simply avoiding using reserved XML characters in unspeakable extension names that we generate.

Copy link
Member Author

Choose a reason for hiding this comment

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

One additional data point: in error scenarios, we already escape CREF strings in ToBadCrefString (that's done outside of GetDocumentationCommentId.
I'll move the escaping outside for now. We can discuss the other option (using a different unspeakable name) in WG.

@jcouv jcouv marked this pull request as ready for review May 29, 2025 17:30
@jcouv jcouv requested review from a team as code owners May 29, 2025 17:30
<Field Name="DotToken" Type="SyntaxToken">
<Kind Name="DotToken"/>
</Field>
<Field Name="Member" Type="MemberCrefSyntax"/>
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

"MemberCrefSyntax"

This feels too flexible. For example, this cannot be ExtensionMemberCrefSyntax. Could we restrict allowed nodes by kind? If not, perhaps for the syntax model it is fine as is. Is there a corresponding proposal for the grammar change?
#Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

The member cref syntax currently covered name members, indexers, operators and conversion operators. We'll need all those as extension members at some point.
I think the only questionable scenario then is nested extensions.

If we really care to block this in the syntax model, I think we'd need to create an additional level in the hierarchy:
MemberCrefSyntax <- NonExtensionCrefSyntax <- (existing types for named members, indexers, etc)
Then ExtensionMemberCrefSyntax would only hold a NonExtensionCrefSyntax as its member.

I think such model seems a bit overkill and the flexibility doesn't disturb me. We can just restrict the scenario in the parser...
This is scheduled for discussion in API review tomorrow.

Is there a corresponding proposal for the grammar change?

Not yet. I didn't find a spec baseline for the CREF grammar... The Annex D section is as close as it gets.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the following question was not answered:

Could we restrict allowed nodes by kind?

I was specifically asking about an ability to restrict nodes by kind in this description. For example, like we do this for tokens:

    <Field Name="ExtensionKeyword" Type="SyntaxToken">
      <Kind Name="ExtensionKeyword"/>
    </Field>

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the clarification. Adding <Kind Name="NameMemberCrefSyntax"/> to the Member doesn't do anything even after regenerating compiler code. WriteGreenFactory only validates Kind on fields of type SyntaxToken.
I'll block the nested scenario by adding an error in parser.

}

SyntaxToken dotToken = EatToken(SyntaxKind.DotToken);
MemberCrefSyntax member = ParseMemberCref();
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

ParseMemberCref()

It looks like this is going to allow an indirect recursion, which is not expected. #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

There was no response to this comment. To clarify, I think we need to restrict the parsing and not accept nested extensions even if our syntax nodes technically allow creating such a hierarchy.


TypeArgumentListSyntax? extensionTypeArguments = syntax.TypeArgumentList;
int extensionArity = extensionTypeArguments?.Arguments.Count ?? 0;
sortedSymbols = computeSortedAndFilteredCrefExtensionMembers(namedContainer, simpleName.Identifier.ValueText, extensionArity, arity, extensionTypeArguments, diagnostics, syntax);
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

ValueText

Do we have a test with escaped identifier at this position? #Closed

if (sortedSymbols.IsDefaultOrEmpty)
{
ambiguityWinner = null;
return [];
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

return [];

Consider leaving a follow up comment to handle operators. #Closed

refCustomModifiers: [],
explicitInterfaceImplementations: []);

ImmutableArray<ParameterSymbol> extensionParameterSymbols = syntax.Parameters is { } extensionParameterListSyntax ? BindCrefParameters(extensionParameterListSyntax, diagnostics) : default;
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

syntax.Parameters is { } extensionParameterListSyntax

Can this be false? #Closed


var constructedNested = (NamedTypeSymbol)ConstructWithCrefTypeParameters(extensionArity, extensionTypeArguments, nested);

var candidateExtensionSignature = new SignatureOnlyMethodSymbol(
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

var candidateExtensionSignature = new SignatureOnlyMethodSymbol(

Consider adding a comment that we are using signature method symbols to match extension blocks #Closed


ImmutableArray<ParameterSymbol> extensionParameterSymbols = syntax.Parameters is { } extensionParameterListSyntax ? BindCrefParameters(extensionParameterListSyntax, diagnostics) : default;

var providedExtensionSignature = new SignatureOnlyMethodSymbol(
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

providedExtensionSignature

It looks like we can initialize outside of the loop #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

We need an arity which is specific to an iteration of the loop

Copy link
Contributor

Choose a reason for hiding this comment

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

We need an arity which is specific to an iteration of the loop

Don't we make sure nested.Arity == extensionArity when we reach this point, and extensionArity doesn't change in the loop?

refCustomModifiers: [],
explicitInterfaceImplementations: []);

ImmutableArray<ParameterSymbol> extensionParameterSymbols = syntax.Parameters is { } extensionParameterListSyntax ? BindCrefParameters(extensionParameterListSyntax, diagnostics) : default;
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

BindCrefParameters(extensionParameterListSyntax, diagnostics)

It looks like we should attempt to bind parameters even when the enclosing loop never reaches this line. #Closed

@@ -357,6 +357,8 @@ internal static bool HasParameterList(CrefSyntax crefSyntax)
return ((OperatorMemberCrefSyntax)crefSyntax).Parameters != null;
case SyntaxKind.ConversionOperatorMemberCref:
return ((ConversionOperatorMemberCrefSyntax)crefSyntax).Parameters != null;
case SyntaxKind.ExtensionMemberCref:
return HasParameterList(((ExtensionMemberCrefSyntax)crefSyntax).Member);
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

return HasParameterList(((ExtensionMemberCrefSyntax)crefSyntax).Member);

What about ExtensionMemberCrefSyntax.Parameters? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Parameters on extension are intentionally not considered here.
The value from HasParameterList affects the result kind from GetCrefSymbolInfo below, to report "ambiguous" vs. "overload resolution failure". In E.extension(int).M... we only allow the last parameter list to be omitted, so the presence/absence of the parameter list on extension is not relevant to was result kind we report.

N(SyntaxKind.CloseParenToken);
}
N(SyntaxKind.DotToken);
N(SyntaxKind.ExtensionMemberCref);
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

N(SyntaxKind.ExtensionMemberCref);

The nested ExtensionMemberCref feels wrong #Closed

@@ -1915,7 +1915,10 @@ void I.M() { }
comp.VerifyEmitDiagnostics(
// (10,16): error CS0541: 'Extensions.extension(object).M()': explicit interface declaration can only be declared in a class, record, struct or interface
// void I.M() { }
Diagnostic(ErrorCode.ERR_ExplicitInterfaceImplementationInNonClassOrStruct, "M").WithArguments("Extensions.extension(object).M()").WithLocation(10, 16));
Diagnostic(ErrorCode.ERR_ExplicitInterfaceImplementationInNonClassOrStruct, "M").WithArguments("Extensions.extension(object).M()").WithLocation(10, 16),
// (10,16): error CS9282: Extension declarations can include only methods or properties
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

// (10,16): error CS9282: Extension declarations can include only methods or properties

Technically this error reported for a method. #Closed

public void Cref_44()
{
var src = """
/// <see cref="extension(int).M"/>
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

extension(int).M

Do we have a test for a success scenario with syntax like that? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's possible. If extension(int) is interpreted as a reference to an extension block, then it's an error (shown). If it's interpreted as a named member cref, it would refer to a method and the .M would therefore fail to bind.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a test for a success scenario with syntax like that?

I don't think that's possible.

Even when it is used on or inside the static class containing the target extension block? Could you please add tests like that nonetheless

/// <see cref="E.extension(int).M2"/>
static class E
{
extension(int i)
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

extension(int i)

Consider adding a test where ambiguous candidates are in different blocks. #Closed

@AlekseyTs
Copy link
Contributor

AlekseyTs commented May 30, 2025

Done with review pass (commit 1) #Closed

@@ -181,7 +181,7 @@ public override object VisitNamedType(NamedTypeSymbol symbol, StringBuilder buil
builder.Append('.');
}

builder.Append(symbol.IsExtension ? symbol.ExtensionName : symbol.Name);
builder.Append(symbol.IsExtension ? escape(symbol.ExtensionName) : symbol.Name);
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 2, 2025

Choose a reason for hiding this comment

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

escape(symbol.ExtensionName)

It looks like now one can refer to an extension type by name across assembly boundaries. I.e. doc comments for one assembly can refer to an extension type defined in a different assembly by using its unspeakable name. BTW, are we testing this scenario? Should a reference like this be considered a subject for a binary breaking change when a source change in the referenced assembly causes a change in the unspeakable name? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Should a reference like this be considered a subject for a binary breaking change when a source change in the referenced assembly causes a change in the unspeakable name?

I have a follow-up to discuss with WG: "Are extension metadata names problematic for versioning docs?".
Yes, I think the current scheme means that shuffling extensions around breaks CREF/XML doc consumers.

internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
{
#pragma warning disable CS8524 // The switch expression does not handle some values of its input type (it is not exhaustive) involving an unnamed enum value.
return code switch
Copy link
Contributor

Choose a reason for hiding this comment

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

ERR_PartialMisplaced

Adding a duplicate pattern?


SyntaxToken dotToken = EatToken(SyntaxKind.DotToken);
MemberCrefSyntax member = ParseMemberCref();
if (member is ExtensionMemberCrefSyntax)
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 4, 2025

Choose a reason for hiding this comment

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

if (member is ExtensionMemberCrefSyntax)

My preference would be to pass a flag to ParseMemberCref instructing to ignore the extension case, but I can live with this approach as well. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm leaning to keep current approach (parse and report an error)

var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments);
comp.VerifyEmitDiagnostics(
// (10,24): warning CS1574: XML comment has cref attribute 'M(string)' that could not be resolved
// /// <see cref="M(string)"/>
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 4, 2025

Choose a reason for hiding this comment

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

///

Should this work instead? Let's capture a design question. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Confirmed with Mads over email thread

// /// <see cref="E2.extension()"/>
Diagnostic(ErrorCode.WRN_BadXMLRef, "E2.extension()").WithArguments("extension()").WithLocation(9, 16),
// (10,16): warning CS1574: XML comment has cref attribute 'extension(int)' that could not be resolved
// /// <see cref="E2.extension(int)"/>
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 4, 2025

Choose a reason for hiding this comment

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

// ///

Let's capture a design question whether this should work #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Resolved in LDM today

[Fact]
public void Cref_47()
{
// TODO2 error in Ioperation
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 4, 2025

Choose a reason for hiding this comment

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

// TODO2 error in Ioperation

Is the comment still relevant? #Closed

{
extension(int)
{
/// <see cref="extension(int).M2"/>
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 4, 2025

Choose a reason for hiding this comment

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

///

It looks like we are missing a test covering usage of cref like this outside the extension block, but inside class E. For example, on another extension block, on a method declaration, etc. #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

Also on class E itself.

@AlekseyTs
Copy link
Contributor

Done with review pass (commit 5)

@jcouv jcouv marked this pull request as draft June 4, 2025 21:11
@jcouv jcouv marked this pull request as ready for review June 10, 2025 04:07
@jcouv jcouv requested a review from AlekseyTs June 10, 2025 04:29
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 10)

@jcouv jcouv requested a review from jjonescz June 10, 2025 18:08
{
var src = """
/// <see cref="E.extension(int).M(string)"/>
/// <see cref="E.M(int, string)"/>
Copy link
Member

@jjonescz jjonescz Jun 11, 2025

Choose a reason for hiding this comment

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

Consider testing E.M(string) as well #Resolved

private ImmutableArray<Symbol> BindExtensionMemberCref(ExtensionMemberCrefSyntax syntax, NamespaceOrTypeSymbol? containerOpt, out Symbol? ambiguityWinner, BindingDiagnosticBag diagnostics)
{
// Tracked by https://github.com/dotnet/roslyn/issues/76130 : handle extension operators
CheckFeatureAvailability(syntax, MessageID.IDS_FeatureExtensions, diagnostics);
Copy link
Member

@jjonescz jjonescz Jun 11, 2025

Choose a reason for hiding this comment

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

Is there a test for this langversion check? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

See Cref_54

Debug.Assert(CurrentToken.ContextualKind == SyntaxKind.ExtensionKeyword);

SyntaxToken identifierToken = EatToken();
TypeArgumentListSyntax? typeArguments = (CurrentToken.Kind == SyntaxKind.LessThanToken) ? ParseTypeArguments(typeArgumentsMustBeIdentifiers: true) : null;
Copy link
Member

@jjonescz jjonescz Jun 11, 2025

Choose a reason for hiding this comment

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

Do we have a test for errors produced by typeArgumentsMustBeIdentifiers: true? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

See Cref_43

@jcouv jcouv enabled auto-merge (squash) June 11, 2025 21:03
@jcouv jcouv disabled auto-merge June 12, 2025 16:38
@jcouv jcouv enabled auto-merge (squash) June 12, 2025 18:09
@jcouv jcouv merged commit 345419e into dotnet:main Jun 12, 2025
27 of 28 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jun 12, 2025
333fred added a commit to 333fred/roslyn that referenced this pull request Jun 16, 2025
* upstream/main: (353 commits)
  PR Feedback
  Update enum values
  Expose `IsIterator` as a public API (dotnet#78813)
  move up
  Docs
  :Extract type
  Only add annotations to potential references
  Add test
  Simplify
  Specify single reducer
  Code simplification
  [main] Source code updates from dotnet/dotnet (dotnet#78805)
  Obsolete api
  Remove more unneeded references
  Expose a couple of things to Razor
  Remove now unused Progression references and related hacks
  Fix scoped variance checks involving ref struct interface implementation (dotnet#78883)
  Extensions: allow cref references to extension members (dotnet#78735)
  Collect stats
  Revert
  ...
333fred added a commit that referenced this pull request Jun 17, 2025
* Simpli

* Simplify

* Simplify

* docs

* docs

* Clear

* Update src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs

* Remove method

* Fix ILBuilder visualization (#78764)

Turns out that we do have visualization of in-progress IL building, but it's been broken since 2020, when we renamed Roslyn.Test.Utilities to Microsoft.CodeAnalysis.Test.Utilities. It was then further broken by the change a few months ago to pass IL visualization formats along. This gets the debugger display working again.

* Remove EditorFeaturesWpf TestComposition (#78769)

After the EditorFeatures.WPF => EditorFeatures refactoring, this TestComposition became equivalent to the EditorFeatures one, so just removing the WPF one.

* Fix renames across different extensions

* Add docs

* Add docs

* Update src/VisualStudio/Core/Def/PackageRegistration.pkgdef

* Explicitly reset

* Reset state

* Add docs

* Simplify

* Revert

* Simplify

* Simplify

* Docs

* Update src/VisualStudio/Core/Def/Commands.vsct

Co-authored-by: David Barbet <[email protected]>

* Update src/VisualStudio/Core/Impl/SolutionExplorer/SymbolTree/SymbolItemContextMenuController.cs

Co-authored-by: David Barbet <[email protected]>

* Simplify

* Simplify

* multi notify

* only if it changed

* Don't move backwards

* Docs

* Update src/VisualStudio/Core/Impl/SolutionExplorer/SymbolTree/SymbolTreeChildCollection.cs

* Ensure sln load uses project absolute paths

* lint

* Rename field to make it clearer what it's used for

It's only for dynamic files, so make that clear.

* Don't refresh dynamic files under a lock

When we were getting the notification of a dynamic file being changed,
we were taking the project and workspace locks before fetching the
actual content. This is really expensive now that the fetching itself
might not be instant.

Fundamentally this change is just switching the work to acquire the
locks to get the setup info, then releasing the locks and calling the
provider to get the info. I'm putting this around a batching queue so
we can deduplicate lots of events at once, and also to ensure ordering
so if we were to try refreshing the document more than once we don't
end up in a case where we have two threads trying to put different
versions of the document into the workspace and getting the ordering
wrong.

Fixes #78734

* Fix cast before we unsubscribe

This was otherwise throwing an exception every shutdown.

* Adjust implementation to respect diagnostic flag and update tests

* Remove workarounds for bugs that have been fixed

Closes #77995

* Remove a workaround we had for an old version of Copilot

This bug was fixed a long time ago.

* Switch return to continue

* Fixed multi-variable declaration support in RemoveUnnecessarySuppressions.

* Removed whitespace.

Co-authored-by: DoctorKrolic <[email protected]>

* Simplified TestRemoveDiagnosticSuppression_Attribute_MultiVariableDeclaration with inline data.

* Expanded remove unnecessary suppression tests, removed unnecessary loop and fixed partial method/property support in AbstractRemoveUnnecessaryInlineSuppressionsDiagnosticAnalyzer.

* Directly create virtual project when dotnet run-api is missing for now

* Avoid dereferencing null CheckConstraintsArgs.CurrentCompilation (#78729)

Fixes #78430.

* Updates to unblock dartlab pipeline (#78793)

* update timeout

* wip

* Add main to main-vs-deps flow (#78798)

* Create branch-merge2.jsonc

* Update main-merge.yml

* Rename branch-merge2.jsonc to branch-merge-2.jsonc

* Update PublishData.json

* Update and rename branch-merge.jsonc to main-to-main-vs-deps-branch-merge.jsonc

* feedbacl

* make clear main is for validation

* EnC: Simplify diagnostic reporting (#78708)

* Refactoring of extension methods in source packages (#78620)

* Change namespace of OneOrMany and TemporaryArray to MS.CA.Collections

* Cleanup Enumerable and ImmutableArray extensions

* Update imports/usings

* Fix VB remove-imports not showing up

* add the schedule back to main-merge (#78807)

* Fix LSP references for using alias

* Clean up HasDuplicates extension method (#78808)

* Extensions: Do not consider extension block methods as entry points, consider their implementation methods instead (#78667)

* Decouple EditorFeatures.Test.Utilities from VS services

Microsoft.CodeAnalysis.EditorFeatures.Test.Utilities depended on
Microsoft.VisualStudio.Editor which brought along various VS layer
services. Further, we had some stub implementations of those VS services
there. Digging into this this coupling wasn't really necessary:

1. There were EditorFeatures tests using the editor adapters stub,
   but those tests pass just fine without it, so that was easy to
   move to the VS specific tests.
2. StubVsServiceExporters didn't appear to be used either, so unsure
   why those were in this layer at all. Those were also moved.
3. We had implementations of some settings services, which it's easier
   to just delete and remove the options persister from the composition
   in the first place. This also better ensures that we don't have
   code that might try creating registry keys from ILocalRegistry
   which we absolutely don't want.

* Extensions: mark skeleton type with 'specialname' (#78690)

* Fix await completion in an async iterator

* Extensions: fix lowering of implicit conversion on receiver in pattern-based scenarios (#78685)

* Add test

* Find metadata reference for alias

* Add a fallback path when launching the BuildHost

* refactor

* Added notes to update GetBuildHostPath if packaging changes

* Update comments

* Add heuristic for the loaded from nuget package scenario

* Fix corruption of sliding text window when trying to peek backwards.  (#78774)

Co-authored-by: Fred Silberberg <[email protected]>

* Track changed text instead of clearing immediate window.

* EnC: Partial solution updates (#78744)

* Restore some parts of the progression api some legacy components (like codemap) use.

* Remove unused internal APIs

* Remove unused internal APIs

* Remove unused internal APIs

* Remove unused internal APIs

* Make static

* Simplify

* Avoid ignored directive errors in disabled regions (#78158)

* Extensions: use specific tracking issues for different areas (#78834)

* Extensions: relax inferrability rule (#78758)

* Remove blank line output from the node writer proportional to the number of non-abstract nodes (#78844)

* Revert "Refactoring of extension methods in source packages (#78620)" (#78850)

This reverts commit 917401d, broke typescript

* Switch off of mutlidictionary (#78851)

This is from ICSharpCompiler, and is therefore causing unnecessary dll loads.

* Switch behavior of "Go to definition" and "Go to implementation" for partial members

* Fix LSP tests

* Fix peek test

* Switch to existing helpers for multi-dictionary use

* call TryGetValue to get dictionary entry (#78863)

* Remove more progression code

* Remove more progression code

* Remove more progression code

* Remove more progression code

* Remove more progression code

* Remove more progression code

* Remove more progression code

* Remove more progression code

* Add VSTypeScriptAsynchronousTaggerProvider2 for TypeScript that avoids TemporaryArray (#78866)

* Remove more progression code

* Remove more progression code

* Remove more progression code

* Remove more progression code

* Fix build break (#78870)

* Add more ETW events to trace assembly loading (#78840)

* Add more ETW events to trace assembly loading

* Update src/Compilers/Core/Portable/CodeAnalysisEventSource.Common.cs

Co-authored-by: Joey Robichaud <[email protected]>

* Put correct alc reference

* Use binary literals

---------

Co-authored-by: Joey Robichaud <[email protected]>

* Introduce EA layer for TS to integrate with classification

* Simplify

* Add CommandLineResource API (#78679)

* Revert "Revert "Refactoring of extension methods in source packages (#78620)" (#78850)"

This reverts commit e083be9.

* Fix

* Revert "Fix"

This reverts commit 574935d.

* Revert "Revert "Revert "Refactoring of extension methods in source packages (#78620)" (#78850)""

This reverts commit 27ae586.

* Lint

* Remove all progression code

* Revert

* remove code

* Fix

* Reduce allocations under ContextMutableIntervalTree (#78885)

There are some closure allocations unnecessarilly happening during ContextMutableIntervalTree construction. Simply changed a couple methods to static and added a parameter for the extra bit of data they need to get rid of those allocations.

This is a very small improvement, only about 0.1% (6 MB) of allocations during typing in the razor speedometer test.

* Reduce allocations in the ImageElementConverter and ImageIdConverter Read methods (#78881)

* Reduce allocations in the ImageElementConverter and ImageIdConverter Read methods

These methods show up in the typing scenario in the razor speedometer test as about 0.9% (63 MB) of allocations.

1) Changed ImageIdConverter to be more like ImageElementConverter and not create a JsonDocument object to query
2) Changed several Utf8JsonReader.GetText calls to instead use Utf8JsonReader.CopyString
3) Changed JsonElement.GetString and new Guid(...) to instead use Utf8JsonReader.GetGuid()

Note that if this PR is merged, I'll also try to make a change to vslanguageserverclient to also do the same as that code has the same issues.

* Fix issue serializing null options in VB

* Yield in task continuation to prevent stack overflow

* Move to .NET 10 Preview 5

* Remove RemoteControl workaround

* Lint response

* Lint response

* Lint response

* Lint response

* Lint response

* Fix issue with not resetting valueLength (#78890)

* Fix issue with not resetting valueLength

#78881 was just merged with a small bug, in that valueLength wasn't reset before it was used a second time. If the typeName is > 64 chars, then this would have thrown a different exception than it should have down a couple lines.

* remove unused method

* Lint response

* Lint response

* fix compiler side

* Simplify workspace hookup in syntactic tagger

* Move

* Switch to ITextBuffer2

* REvert

* REvert

* Update src/EditorFeatures/Core/InlineDiagnostics/AbstractDiagnosticsTaggerProvider.cs

* Update src/EditorFeatures/Core/StringIndentation/StringIndentationTaggerProvider.cs

* Update src/EditorFeatures/Core/Tagging/AsynchronousViewportTaggerProvider.cs

* REvert

* remove more workspace registration code

* Move to .NET 10 Preview 5 (#78906)

* Move to .NET 10 Preview 5

* Lint response

* Lint response

* Lint response

* Lint response

* Lint response

* remove unused method

* Lint response

* Lint response

* fix compiler side

* more

---------

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

* Fix

* Inline Hints - do not allow double click for collection expression type hint (#78900)

* do not allow double click for collection expression hint

* Update src/EditorFeatures/Test2/InlineHints/CSharpInlineTypeHintsTests.vb

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

* comment

---------

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

* Add tests

* Revert "Update code to use null propagation (#78733)"

This reverts commit d785549, reversing
changes made to a8808be.

* Revert "Move to .NET 10 Preview 5 (#78906)"

This reverts commit a7fa681.

* fix warning

* Revert "Update to using unbound nameof(X<>) expressions (#78731)"

This reverts commit 6a09280, reversing
changes made to a7fa681.

* Revert "Update to using simple untyped lambdas (#78732)"

This reverts commit a8808be, reversing
changes made to 6a09280.

* Fix OOP crash issue with copilot change analysis

* Re-enable IDE0051 (#78919)

This re-enables the unused member analyzer and removes all of the
methods it flagged as unused.

* Extensions: address some follow-up comments (#78847)

* Pass missing Hot Reload result properties (#78929)

* Fix deadlock if an MSBuild task is writing to stdout

When we switched over to communicating over a named pipe rather than
stdin/stdout, we were still redirecting stdin and stdout. This had
the side effect that if a build task was directly writing to standard
out, the build would eventually deadlock since we weren't reading from
the other side.

I thought about simply not redirecting stdin/stdout, but I could imagine
other problems might come up if we were to have multiple build hosts
trying to share stdin/stdout. So now we'll log stdout the same way
we log stderr, and explicitly close stdin so readers won't deadlock
waiting for input.

Fixes #78766

* Revert

* Collect stats

* Extensions: allow cref references to extension members (#78735)

* Fix scoped variance checks involving ref struct interface implementation (#78883)

* Remove now unused Progression references and related hacks

* Expose a couple of things to Razor

* Remove more unneeded references

* Obsolete api

* [main] Source code updates from dotnet/dotnet (#78805)

* [VMR] Codeflow a528f84-a528f84

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 270315
No dependency updates to commit

* [VMR] Codeflow f5faea9-f5faea9

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 270450
No dependency updates to commit

* Update dependencies from https://github.com/dotnet/dotnet build 270603
No dependency updates to commit

* Update dependencies from https://github.com/dotnet/dotnet build 270662
No dependency updates to commit

* [VMR] Codeflow 86826d3-86826d3

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 271018
No dependency updates to commit

* [VMR] Codeflow 32a2620-32a2620

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 271181
No dependency updates to commit

* [VMR] Codeflow 25357a9-25357a9

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 271343
No dependency updates to commit

* Update dependencies from https://github.com/dotnet/dotnet build 271417
No dependency updates to commit

* Revert incorrect Roslyn.sln changes

---------

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

* Code simplification

* Specify single reducer

* Simplify

* Add test

* Only add annotations to potential references

* :Extract type

* Docs

* move up

* Expose `IsIterator` as a public API (#78813)

* Expose `IsIterator` as a public API

* Fix workspace generated method symbol implementation

* Implement new property in yet another non-compiler-owned method symbol implementation

* SemanticSearch.ReferenceAssemblies

* Verify public code path in C#

* Add negative tests

* Add C# local function tests

* Test VB lambda iterators

* Revert accessibility change

* Disable warnings

* Use explicit type

* Simplify lambda tests

* Test interface property

* Add additional VB case

* Update enum values

* PR Feedback

---------

Co-authored-by: Cyrus Najmabadi <[email protected]>
Co-authored-by: Cyrus Najmabadi <[email protected]>
Co-authored-by: Todd Grunke <[email protected]>
Co-authored-by: David Barbet <[email protected]>
Co-authored-by: David Barbet <[email protected]>
Co-authored-by: Jason Malinowski <[email protected]>
Co-authored-by: John Douglas Leitch <[email protected]>
Co-authored-by: DoctorKrolic <[email protected]>
Co-authored-by: Rikki Gibson <[email protected]>
Co-authored-by: AlekseyTs <[email protected]>
Co-authored-by: Ankita Khera <[email protected]>
Co-authored-by: Tomáš Matoušek <[email protected]>
Co-authored-by: Julien Couvreur <[email protected]>
Co-authored-by: Joey Robichaud <[email protected]>
Co-authored-by: Jan Jones <[email protected]>
Co-authored-by: DoctorKrolic <[email protected]>
Co-authored-by: Chris Sienkiewicz <[email protected]>
Co-authored-by: Joey Robichaud <[email protected]>
Co-authored-by: tmat <[email protected]>
Co-authored-by: Jared Parsons <[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: Viktor Hofer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Feature - Extension Everything The extension everything feature Needs API Review Needs to be reviewed by the API review council
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants