public enum DeclarationKind must have an InitAccessor member

[Youssef1313]

https://docs.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.editing.declarationkind?view=roslyn-dotnet

The current enum members include GetAccessor, SetAccessor, but not InitAccessor.

[sharwell]

Why not just use SetAccessor?

[Youssef1313]

@sharwell Did you mean that SyntaxGenerator.GetDeclarationKind(SyntaxNode) method should return "DeclarationKind.SetAccessor" for both set and init declarations?
I think they need to be treated differently as they're 3 different accessors.

[Youssef1313]

Also, SyntaxGenerator currently have GetSetAccessorStatements, and GetGetAccessorStatements. A new method GetInitAccessorStatements should be considered.

[sharwell]

Isn't an init accessor just a set accessor with use-site restrictions?

[Youssef1313]

@sharwell Yes, it's even compiled to set_PropName, but I think that's from a semantics.
The enumeration is involved in usage with SyntaxGenerator, so I think if they are separated, that will give consumers more flexibility to do what they want.

[CyrusNajmabadi]

Isn't an init accessor just a set accessor with use-site restrictions?

Yes. And my understanding is that that's how the compiler is exposing this info.

[Youssef1313]

I'm okay if you agree that both should fall into DeclarationKind.SetAccessor. I'll open a PR

[sharwell]

I could end up going either way. I can see advantages and disadvantages to both approaches.

[Youssef1313]

Does this method need to account for init as well?

roslyn/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs

Lines 2535 to 2539 in 06c4312

	public override IReadOnlyList<SyntaxNode> GetSetAccessorStatements(SyntaxNode declaration)
	{
	var accessor = GetAccessor(declaration, SyntaxKind.SetAccessorDeclaration);
	return accessor?.Body?.Statements ?? s_EmptyList;
	}

I think it currently doesn't. I can put that in #48137 as well

[CyrusNajmabadi]

i think it's challenign to answer some of these without having clear cases of where consumers use this so we can judge what their expectations would be .

[Youssef1313]

@CyrusNajmabadi Is this discussable in a design meeting? or the issue needs to get some comments from consumers to decide?

[CyrusNajmabadi]

We would likely need comments from consumers here.

[Youssef1313]

DeclarationKind enum now contains "RecordClass" member, which is kind of syntactic sugar of a "class" with extra synthesized members.

Given that, I think it makes sense to do that with "init" vs "set" since it's a similar case.

@sharwell @CyrusNajmabadi What are your thoughts?

[Youssef1313]

Also note that GetDeclarationKind will currently return None for init accessors. So if InitAccessor is going to be rejected as a member of the enum, then GetDeclarationKind should be adjusted to return SetAccessor instead of None:

roslyn/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs

Lines 2047 to 2057 in 90ff6db

	case SyntaxKind.GetAccessorDeclaration:
	return DeclarationKind.GetAccessor;
	case SyntaxKind.SetAccessorDeclaration:
	return DeclarationKind.SetAccessor;
	case SyntaxKind.AddAccessorDeclaration:
	return DeclarationKind.AddAccessor;
	case SyntaxKind.RemoveAccessorDeclaration:
	return DeclarationKind.RemoveAccessor;
	}
	return DeclarationKind.None;

[sharwell]

DeclarationKind enum now contains "RecordClass" member

This seems unfortunate. Do we know when it was added?

Edit: It looks like it shipped in 16.9. I'm not sure we need to make the rest of DeclarationKind more complex just because RecordClass wasn't implemented as a duplicate of Class.

[Youssef1313]

Hmmm, from the discussion in #10094, it seems that you're indeed correct. "RecordClass" shouldn't have been added at all. But removing it is a breaking change :/

Now the question is:

• Should we take a breaking change to keep things consistent?
• Should GetDeclarationKind return DeclarationKind.None or DeclarationKind.SetAccessor for init declarations?

[sharwell]

It might not be to late to mark RecordClass Obsolete with the message "This value is not used".

[Youssef1313]

@sharwell Should I open a PR or wait until other team members agree?

[sharwell]

You could open a draft PR to frame the discussion