Redundant type casting in generated `visit` methods of `SyntaxRewriter`

[Matejkob]

Description

In the SyntaxRewriter implementation, I noticed that many visit methods are written in the following format:

open func visit(_ node: AttributeListSyntax) -> AttributeListSyntax {
    return Syntax(visitChildren(node)).cast(AttributeListSyntax.self)
}

Since the node argument is already of type AttributeListSyntax, and the visitChildren(node) method returns the same type, the type casting using .cast(AttributeListSyntax.self) appears to be redundant and unnecessary.

Suggested Improvement:

Consider refactoring the generated visit methods to remove this redundant type casting and Syntax wrapping. The improved version would look like:

open func visit(_ node: AttributeListSyntax) -> AttributeListSyntax {
    return visitChildren(node)
}

This simplification would make the codebase cleaner, more readable, and reduce potential points of failure.

[ahoppen]

Tracked in Apple’s issue tracker as rdar://114400377

[natikgadzhi]

So it looks like:

• The potential change should be in CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxRewriterFile.swift
• visit() is not guaranteed to return same type, it has a bunch of overloads. visitChildren(), on the other hand, always returns the same type as the node that's passed into it.

How wrong am I? 🙃

[Matejkob]

The potential change should be in CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxRewriterFile.swift

That's correct. Everything that's located in the generated folder in the main package is generated by CodeGeneration. with SyntaxRewriter coming out of SyntaxRewriterFile.swift.

visit() is not guaranteed to return the same type, it has a bunch of overloads. visitChildren(), on the other hand, always returns the same type as the node that's passed into it.

And you're right about the visit() functions as well; they have various overloads that can return different node types, unlike visitChildren() which is consistent in its return type.

---

If you find the time, it'd be worthwhile to sift through the CodeGeneration codebase to identify more spots where nodes are being cast to the same type using cast, as, or is. With the changes from issue #2092, we'll be able to notify users if they attempt to cast leaf nodes to other nodes—or even to the same node type. The tricky part is when someone upcasts a node to a base node like Syntax(node) and then downcasts it back to the original node type using Syntax(node).cast(node.Type).