Implement LINQ LeftJoin and RightJoin

[roji]

Closes #110292

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[Copilot]

Copilot reviewed 7 out of 14 changed files in this pull request and generated 2 comments.

Files not reviewed (7)

• src/libraries/System.Linq.Queryable/tests/System.Linq.Queryable.Tests.csproj: Language not supported
• src/libraries/System.Linq/src/System.Linq.csproj: Language not supported
• src/libraries/System.Linq/tests/System.Linq.Tests.csproj: Language not supported
• src/libraries/System.Linq/ref/System.Linq.cs: Evaluated as low risk
• src/libraries/System.Linq.Queryable/ref/System.Linq.Queryable.cs: Evaluated as low risk
• src/libraries/System.Linq/tests/JoinTests.cs: Evaluated as low risk
• src/libraries/System.Linq.Queryable/tests/JoinTests.cs: Evaluated as low risk

Comments suppressed due to low confidence (3)

src/libraries/System.Linq/src/System/Linq/RightJoin.cs:43

• The IsEmptyArray method is not defined in the provided code. Ensure that this method is defined and works as expected.

if (IsEmptyArray(inner))

src/libraries/System.Linq.Queryable/src/System/Linq/Queryable.cs:245

• The DynamicDependency attribute should be corrected to use the format: DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, typeof(Enumerable)).

[DynamicDependency("LeftJoin`4", typeof(Enumerable))]

src/libraries/System.Linq.Queryable/src/System/Linq/Queryable.cs:260

• The nullable type for q is not necessary. Use a simple cast instead: IQueryable q = source as IQueryable;

IQueryable<TSource>? q = source as IQueryable<TSource>;

[eiriktsarpalis]

Looks great, thanks!

[roji]

Hey @eiriktsarpalis, thanks for the quick reviewing.

Re the styling nits, I basically tried to follow the existing code (Join implementation and tests) for maximum consistency, although there are indeed many opportunities for syntax modernization (namespace declaration, using declaration, the new collection initializers...). I'm happy to make the code more modern for Left/RightJoin in this PR only as you suggest (at which point things would be inconsistent), or I can submit a separate PR later to do some syntax cleanup for System.Linq as a whole - whatever you prefer.

[eiriktsarpalis]

Re the styling nits

That's fine, they're all optional. Assuming we do add enforcement in the future I'm guessing they would be updated in bulk anyway.

[stephentoub]

We should add XML doc comments to all of the new public APIs.

[roji]

Ah, I was following the existing code on this (e.g. Join has no xmldocs) - are we already in a place where the API docs are generated from the xmldocs?

In any case, I'll take care of this.

[stephentoub]

are we already in a place where the API docs are generated from the xmldocs?

Yes and no.

Some libraries have the src as the source of truth for docs. Unfortunately, System.Linq isn't yet one of them. However, even for such libraries, we add XML comments for all new surface area, and those comments then seed the docs for those methods, even if they're not then the source of truth moving forward.

[roji]

#111177