given this example, i guess my surprise is that we are passing in hte class-statement, and not hte class-block for this feature.

@CyrusNajmabadi, yep, it passes the class statement.

i guess my fix would be at that level :) There is also an ISymbolDeclarationService that helps ensure that we work with the right node (if this is a feature that uses symbols).

@CyrusNajmabadi The fix is mainly for the public SyntaxGenerator.WithModifiers API

i guess my point is this: i think it's hte feature working improperly. i would update the feature. if we update the generator, we need to update it to support all statements for blocks, and it was intentional that we were not conflating these at that level.

@CyrusNajmabadi For the purpose of WithModifiers, supporting other statements won't have effect. For SyntaxGenerator in general, I'm not sure what other parts are affected by but supporting blocks only and not statements?

i'm not sure i get your question. it feels very strange to just special case class statements here (what about structs/interfaces/modules)?

@CyrusNajmabadi In this specific code path. They have no effect. The declaration kind is passed to GetModifierList:

And it doesn't perform any checks against interfaces, structures, etc.

i don't get it. this is directly making a change purely for classes. i dont' see why we wouldn't need the same for structs/interfaces/modules here.

And, like i said before, i thinik the bug is that hte caller is passing ina class statement, it should pass in a classblock.

i thinik the bug is that hte caller is passing ina class statement, it should pass in a classblock.

I believe API consumers won't expect this at all.

i don't get it. this is directly making a change purely for classes. i dont' see why we wouldn't need the same for structs/interfaces/modules here.

Because it literally doesn't have any effect. If you look at the following method:

Any type declaration other than a class will be treated the same. I could handle all other statements, but it doesn't have any functional/behavioral change.

i thinik the bug is that hte caller is passing ina class statement, it should pass in a classblock.

I believe API consumers won't expect this at all.

i don't get it. this is directly making a change purely for classes. i dont' see why we wouldn't need the same for structs/interfaces/modules here.

Because it literally doesn't have any effect. If you look at the following method:

Any type declaration other than a class will be treated the same. I could handle all other statements, but it doesn't have any functional/behavioral change.

To put it another way, there is no way to write a unit test for interface/structure that can fail now and be fixed by handling statements in this code path. WithModifiers already works correctly for the other declarations.

i don't really get it.

I believe API consumers won't expect this at all.

I don't have an issue with htat. It was an explicit decision that we do not try to blur the lines between the statements and blocks. The block corresponds to the decl, and the api here intends to operate on the decls.

this is my approach: #50003

@CyrusNajmabadi Well, Part of the disconnect is whether WithModifiers should work with statements, or only blocks. It currently works with statements very well, except for the class case (more specifically sealing the class). Anything other than sealing a class works correctly using either a statement or a block. This is one of the reasons I think WithModifiers should get modified to work with class statement, even if the original design didn't intend that.

Otherwise, a clear doc comment should make this clear. (and in this case, you can close the PR and #53627)

i would then update GetModifierList to put the special logic there. specifically, it should not pass in a declaration kind in isolation. it can pass in both. or just pass in hte node, and compute the declaration kind there, but use it with the node to figure out what to do.

@CyrusNajmabadi The suggested approach is a lot easier to follow and to reason about. Only problem I found is that most callers don't have a relevant SyntaxNode to pass. So I passed Nothing for these cases.