Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve documentation comments of some commonly used types #2922

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Jeehut
Copy link
Contributor

@Jeehut Jeehut commented Dec 18, 2024

This resolves #2906 for an initial set of types. To keep the PR relatively small, I have focused on improving docs of those types that I have personally needed & used for my first macro (which I haven't released yet, as it's not finished).

@ahoppen I didn't quite understand if your "I would" in this comment literally meant that it's something you would do after my PR is merged, of if it's something you suggested me to do.

In any case, I found it very confusing that all the types I searched for were inside a generated folder and the file names were suffixed by alphabetic characters like QRS in SyntaxNodesQRS.swift. I have a feeling that I added the documentation comments to the wrong place. But I couldn't find any mentions of StringSegmentSyntax, for example inside the CodeGeneration folder. In that particular case, I did find the text of the comment "A literal segment inside a string segment." inside the ExprNodes.swift file, but then again where to add the documentation comments for the initializers, which are very important when discovering APIs via code completion in Xcode? 🤔

I think I need some guidance here to adjust this PR properly. Thank you in advance for your patience!

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks for adding the documentation @Jeehut 🙏🏽 You added the documentation to generated files. Could you add the documentation in the corresponding places in CodeGeneration and run code generation to make sure the committed files match what would be generated?

Sources/SwiftSyntax/generated/SyntaxBaseNodes.swift Outdated Show resolved Hide resolved
Sources/SwiftSyntax/generated/SyntaxBaseNodes.swift Outdated Show resolved Hide resolved
Sources/SwiftSyntax/generated/SyntaxCollections.swift Outdated Show resolved Hide resolved
Sources/SwiftSyntax/generated/SyntaxCollections.swift Outdated Show resolved Hide resolved
Sources/SwiftSyntax/generated/SyntaxCollections.swift Outdated Show resolved Hide resolved
Sources/SwiftSyntax/generated/SyntaxCollections.swift Outdated Show resolved Hide resolved
Sources/SwiftSyntax/generated/SyntaxCollections.swift Outdated Show resolved Hide resolved
@Jeehut
Copy link
Contributor Author

Jeehut commented Dec 19, 2024

@ahoppen I've really done my best to accommodate all your comments and address them all. But please note that, like I've mentioned in my initial comment on this PR, I'm having a hard time finding the right places in the CodeGeneration package to place the documentation comments. I have now done my best to find the right places and add the documentation code to the type, which I could find some of, but it was a very time-consuming task.

But there were a few reuses of some comments in multiples places (such as StringSegmentSyntax once for the type and once for the enum case), although they should probably be more detailed in the type and shorter for the case.

But most importantly, pretty much ALL of my provided documentation comments for any initializers I couldn't find a place to add them in the CodeGeneration package. It doesn't seem to be trivial to add documentation comments, which is a bummer. If you checkout my branch and run swift run generate-swift-syntax from the CodeGeneration folder, you'll see in the git diff all the initializers that are missing their documentation (and few other things).

If you can guide me how to add documentation comments for those, e.g. by giving an example for one of those, I might be able to get more comments into the CodeGeneration package. For now, this is all I could do.

@ahoppen
Copy link
Member

ahoppen commented Jan 2, 2025

Thanks for your effort @Jeehut.

The initializers are generated here.

DeclSyntax(
"""
public init?(_ node: __shared some SyntaxProtocol) {
guard node.raw.kind == .\(node.enumCaseCallName) else { return nil }
self._syntaxNode = node._syntaxNode
}
"""
)
let initSignature = InitSignature(node)
try! InitializerDeclSyntax(
"""
\(initSignature.generateInitializerDocComment())\
\(initSignature.generateInitializerDeclHeader())
"""
) {

I didn’t check all the comments you wrote on initializers but I think we should be able to re-use some of the existing infrastructure to generate them:

  • Comments for the parameters (eg. https://github.com/swiftlang/swift-syntax/pull/2922/files#diff-c5a0310c344fd64bfa2e1a25205f2faa3ee2a6f2a77686ada19dd4bfa8e40842R671): If you add those comments to the the corresponding Child types in SyntaxSupport, they should automatically show up
  • Documentation for unexpected*. I wonder if we could automatically generate them based on the information that we have.
  • The documentation for the conversion initializer should be possible to add to all init?(_ node: __shared some SyntaxProtocol) initializers
  • The remaining hand-written initializer comments might make sense to put on the type themselves. Alternatively, we could add a field to Node and then pass that through InitSignature to add it the generated memberwise initializer. But if there’s a way to generate equivalent information using the fields we already have, I would prefer that because it’s less maintenance burden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StringLiteralExprSyntax adds whitespace in front of closingQuote
2 participants