Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 14, 2025

Plan for Improving InsertTokensBefore/After and ReplaceToken Documentation and Error Messages

Based on the issue analysis:

  • Update XML documentation for InsertTokensBefore, InsertTokensAfter, and ReplaceToken(tokenInList, IEnumerable<SyntaxToken>) methods to clearly state:

    • The tokenInList parameter must be a direct element of a SyntaxTokenList
    • The tokens will be inserted/replaced within that list
    • An exception will be thrown if the token is not part of a SyntaxTokenList
  • Keep the original MissingListItem error message for nodes

  • Add new MissingTokenListItem error message specifically for tokens

  • Update TokenListEditor.VisitToken in both C# and VB to use the new token-specific error message

  • Update .xlf localization files for the new error message

  • Verify tests still pass and error messages are improved

All changes complete! The original MissingListItem error message has been preserved for nodes, and a new MissingTokenListItem error message has been added specifically for tokens. The error message for tokens is now: "The specified token must be a direct element of a SyntaxTokenList (such as a modifier in a list of modifiers). The token cannot be an arbitrary token from the syntax tree."

Original prompt

This section details on the original issue you should resolve

<issue_title>InsertTokensBefore has non-obvious restriction</issue_title>
<issue_description>I wrote a fixer like this, to mark a class as static:

    private async Task<Document> MarkClassAsStatic(Document document, ClassDeclarationSyntax classDeclaration, CancellationToken c)
    {
        SyntaxToken classDeclarationToken = classDeclaration.Keyword;
        SyntaxToken staticKeywordToken = SyntaxFactory.Token(SyntaxKind.StaticKeyword);

        SyntaxNode root = await document.GetSyntaxRootAsync();
        SyntaxNode newRoot = root.InsertTokensBefore(classDeclarationToken, new[] { staticKeywordToken });

        Document newDocument = document.WithSyntaxRoot(newRoot);
        return newDocument;
    }

The call to InsertTokensBefore fails with the error "System.InvalidOperationException: The item specified is not the element of a list."

It turns out that the first argument to InsertTokensBefore must be an element of a list, and the "class" keyword is not an element of a list. In fact that's why the parameter name is tokenInList. Intellisense shows that, but when I saw the Intellisense for this method I had no idea what the name meant.

And it turns out there's a direct way to do what I want, using the AddModifiers method:

        ClassDeclarationSyntax newClassDeclaration = classDeclaration.AddModifiers(new SyntaxToken[] { staticKeywordToken });

        SyntaxNode root = await document.GetSyntaxRootAsync();
        SyntaxNode newRoot = root.ReplaceNode(classDeclaration, newClassDeclaration);

Still, InsertTokensBefore was so seductive, it was really not obvious to me that it wouldn't work, and the error message didn't lead me towards the right answer.
</issue_description>

<agent_instructions>Copilot, can you update the docs for InsertTokensBefore, InsertTokensAfter and ReplaceToken to clearly state that the tokenInList parameter must be a direct element of a SyntaxTokenList, and the newTokens will be added before/after tokenInList in that list. If this is called on a token not in a SyntaxTokenList then it will throw an exception.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@ A more verbose error message on the exception would help. Something to the effect of "you can't pass an arbitrary token as the first argument. The token has to be an element of a SyntaxTokenList." @CyrusNajmabadi @rtnlmeme-DestroyerOfDeath

You should throw a better error and put a better docstring.

PRs welcome :-)</comment_new>
<comment_new>@CyrusNajmabadi

I still don't get it :( I have UsingDirectiveSyntax and I would like to add global keyword token to it. Can I use InsertTokensBefore?

No, you cannot. InsertTokensBefore is only for updating a SyntaxTokenList (like the modifiers-list on a MethodDeclarationSyntax).

That's not how UsingDirectiveSyntax works. It doesn't have a SYntaxTokenList in it. It either has a direct GlobalKeyword token or not. If you want to add a global keyword, just take the UisngDirective and call .Update on it, passing in an appropraite new token for that space.</comment_new>

Fixes #3082


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…nd ReplaceToken methods

Co-authored-by: CyrusNajmabadi <[email protected]>
@Copilot Copilot AI changed the title [WIP] Fix InsertTokensBefore non-obvious restriction issue Improve documentation and error message for InsertTokensBefore/After and ReplaceToken methods Oct 14, 2025
@Copilot Copilot AI requested a review from CyrusNajmabadi October 14, 2025 09:24
Copilot finished work on behalf of CyrusNajmabadi October 14, 2025 09:24
@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review October 14, 2025 10:36
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner October 14, 2025 10:36
</data>
<data name="MissingListItem" xml:space="preserve">
<value>The item specified is not the element of a list.</value>
<value>The specified token must be a direct element of a SyntaxTokenList (such as a modifier in a list of modifiers). The token cannot be an arbitrary token from the syntax tree.</value>
Copy link
Member

Choose a reason for hiding this comment

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

@copilot This is not ok. THis error is also used when a node is not found that needs to be in a list, not just a token. We'll need two errors. Keep the original one, and add a new MissingTokenListItem. Then update SyntaxReplacer.VisitToken to throw the new message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've kept the original MissingListItem error and added a new MissingTokenListItem error. Updated both C# and VB TokenListEditor.VisitToken methods to use the new error message. Commit: 3b60862 (will be updated in next push)

@Copilot Copilot AI requested a review from CyrusNajmabadi October 15, 2025 11:06
Copilot finished work on behalf of CyrusNajmabadi October 15, 2025 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InsertTokensBefore has non-obvious restriction

3 participants