NotificationsYou must be signed in to change notification settings
Fork1.8k
Star9k

C#: Replace initializer splitting with an ObjectInitMethod.#20922

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Draft

aschackmull wants to merge8 commits intogithub:main

base:main

Choose a base branch

fromaschackmull:csharp/object-initializer

Draft

C#: Replace initializer splitting with an ObjectInitMethod.#20922

aschackmull wants to merge8 commits intogithub:mainfromaschackmull:csharp/object-initializer

+2,540 −1,638

Conversation

Copy link

Contributor

aschackmull commentedNov 26, 2025

No description provided.

aschackmull added2 commits

November 25, 2025 15:51

C#: Add object initializer test.

7c18195

C#: Add extractor support for object initializer methods.

4d9e078

github-actionsbot added the C# label

Nov 26, 2025

aschackmull force-pushed thecsharp/object-initializer branch 2 times, most recently from49eed3a to0ef7beeCompare

November 27, 2025 13:21

michaelnebel reviewed

Nov 27, 2025

View reviewed changes

Copy link

Contributor

michaelnebel left a comment

Choose a reason for hiding this comment

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

Uh, very neat!
Just some quick initial comments. It appears that there are some tests failing.

csharp/extractor/Semmle.Extraction.CSharp/Entities/ObjectInitMethod.cs Outdated

		{
		internalsealedclassObjectInitMethod:CachedEntity,IMethodEntity
		{
		TypeContainingType{get;}

Copy link

Contributor

Choose a reason for hiding this comment

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

private?

csharp/extractor/Semmle.Extraction.CSharp/Entities/ObjectInitMethod.cs Outdated

		this.ContainingType=containingType;
		}

		publicstringName=>"<object initializer>";

Copy link

Contributor

Choose a reason for hiding this comment

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

Can be made a readonly field.

Copy link

Contributor

michaelnebel commentedNov 27, 2025

Do you also intend to model some synthetic assignments in the new synthetic method?

Copy link

ContributorAuthor

aschackmull commentedNov 28, 2025

Do you also intend to model some synthetic assignments in the new synthetic method?

The assignments are already extracted as top-level expressions in the enclosing class. We could move them into this new method - that's what Java does, but Java also allow one to write a chunk of code that goes directly into the object initializer, so the situation there is slightly different. However, I opted not to do that since it would be a much more invasive change and require us to change any current QL code that relies on identifying field initializers as they would have moved and would look different. Though, if we were building from scratch then I would have put the assignments inside the method, but as it stands I don't think it's worth it to make the change.