Do not reuse BlockSyntax if there are attributes#62153
Do not reuse BlockSyntax if there are attributes#62153AlekseyTs merged 2 commits intodotnet:mainfrom
Conversation
| // Check again for incremental re-use, since ParseBlock is called from a bunch of places | ||
| // other than ParseStatementCore() | ||
| if (this.IsIncrementalAndFactoryContextMatches && this.CurrentNodeKind == SyntaxKind.Block) | ||
| if (this.IsIncrementalAndFactoryContextMatches && this.CurrentNodeKind == SyntaxKind.Block && attributes.Count == 0) |
There was a problem hiding this comment.
- attributes are allowed on all statements. are tehre other incremental reuse locations where this causes a problem?
- what was the actual issue here that caused a problem?
There was a problem hiding this comment.
attributes are allowed on all statements. are tehre other incremental reuse locations where this causes a problem?
Sure but, apparently, we decided to not implement proper reuse of statements with attributes, see canReuseStatement helper in ParseStatementCore.
what was the actual issue here that caused a problem?
We were reusing a node and dropping the attributes from the new text.
There was a problem hiding this comment.
ah, i see. this might be worth a comment. I also misread what the logic was. I thought it wsa checking the block for attributes, not that attributes had been passed in. Maybe something like "Also, if our caller produced any attributes, we don't want to reuse an existing block syntax directly as we don't want to lose those attributes).
Sure but, apparently, we decided to not implement proper reuse of statements with attributes, see canReuseStatement helper in ParseStatementCore.
That's perfect. That validation is what i was concerned about. e.g. that we weren't missing some other case.
Fixes #62126.