Skip to content

List the token kinds in child documentation in SyntaxNodes #2168

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Sep 18, 2023
Merged

List the token kinds in child documentation in SyntaxNodes #2168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e379aee
List the token kinds in child documentation
natikgadzhi Sep 8, 2023
3f5dbd6
Always backtick token choices in docc comments
natikgadzhi Sep 8, 2023
4fd2ade
Improve token choices doc comment
natikgadzhi Sep 8, 2023
11582df
Improved wording in token documentation for node children
natikgadzhi Sep 12, 2023
8862f9a
Refactor docCommentTrivia() to Trivia extension
natikgadzhi Sep 12, 2023
b788135
Refactor joining trivia pieces into Trivia(joining:)
natikgadzhi Sep 12, 2023
b4fbbd9
Refactor child token choices comment generator
natikgadzhi Sep 15, 2023
4b76a66
Use Trivia(joining:) in SyntaxCollections
natikgadzhi Sep 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions CodeGeneration/Sources/SyntaxSupport/Child.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,25 @@ public class Child {
/// This is used to e.g. describe the child if all of its tokens are missing in the source file.
public let nameForDiagnostics: String?

/// A doc comment describing the child.
public let documentation: SwiftSyntax.Trivia
/// A summary of a doc comment describing the child. Full docc comment for the
/// child is available in ``Child/documentation``, and includes detailed list
/// of possible choices for the child if it's a token kind.
public let documentationSummary: SwiftSyntax.Trivia

/// A docc comment describing the child, including the trivia provided when
/// initializing the ``Child``, and the list of possible token choices inferred automatically.
public var documentation: SwiftSyntax.Trivia {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The part that I don't quite like about this is that we're essentially templating the doc comment for Child in here. But for Node, it's in templates/SyntaxNodeFile.swift. So this feels like we have a way of storing templates — in templates, but this sort of dilutes that distinction. "Don't put logic in your views", as we say in the web apps world.

Copy link
Member

Choose a reason for hiding this comment

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

We already have quite a bit of logic in the templates (I’m not even sure if templates really is the right term here but it’s what we have right now)… My take on this has always been that it’s not too bad because we control the entire pipeline (no other clients of the model and we even control the inputs) and thus we can get away with a slightly more sloppy separation of concerns.

We could make this part of GrammarGenerator and then stitch the documentationSummary and the Tokens section together in SyntaxNodesFile.swift and SyntaxTraitsFile.swift.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think perhaps this is a little bit bigger than one occurrence. The pattern is as follows:

  • A file in templates/, let's say SyntaxNodesFile.swift has a result builder that renders the code. Suddenly, you need to generate something that is not easily available as an object property or a function on a Node or a Child.
  • You can't comfortably put that code in the result builder, so you shove that code somwehere.
  • So far, I've seen extensions of LayoutNode (in the other PR of mine) and now I'm doing a function on Child.

That feels slightly unorthodox, but not too bad:

  • templates should be as easy to read as possible. Sure, we're not SwuftUI here, but the closer we can get in terms of readability, the better.
  • Node and Child and other definitions should ideally contain the information you need to generate the code, but not the generation code itself — it's like the view layer and data layer to me.
  • Seems like Utils / extensions solve that problem partially, but since we're extending Child / Node, it feels like we're injecting view code in the model.

I'm uncertain if my opinion is valid — I might not have enough exposure and experience. But so far, my idea is something like:

  • Add templates/helpers/*.Swift
  • Make enums or structs that you can't instantiate with static functions that are clean. They take the object in, and render syntax out. They can only depend on their arguments, without side effects.

@ahoppen want me to start a discussion / post on forums on this?

Copy link
Member

Choose a reason for hiding this comment

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

I agree that we aren’t super strict on the distinction between the pure data layer and the view layer in CodeGeneration and you probably notice this a lot more than I do because I haven’t been doing much view-based / UI development in a few years, where you think a lot more about that separation.

But then also, the distinction is a little bit muddy IMO. Just taking the documentation as an example. I think we both agree that the documentation should be a property of a node/child but should that documentation already contain the generated Token section? I think you can argue in either direction.

That being said, if you have ideas how to make that distinction clearer, I’m all in favor for them. One thing that’s important to me, though, is that we don’t introduce too many new abstraction layers that make the code less readable. Since CodeGeneration is still fairly reasonable in size and I don’t expect it to grow too much faster in the future, I would prefer an unorthodox decision here and there to an overengineered system of abstraction layers.

if case .token(let choices, _, _) = kind {
let tokenChoicesTrivia = SwiftSyntax.Trivia.docCommentTrivia(
from: GrammarGenerator.childTokenChoices(for: choices)
)

return SwiftSyntax.Trivia(joining: [documentationSummary, tokenChoicesTrivia])
}

// If this child is not a token kind, return documentation summary without the choices list.
return documentationSummary
}

/// The first line of the child's documentation
public let documentationAbstract: String
Expand Down Expand Up @@ -220,7 +237,7 @@ public class Child {
self.deprecatedName = deprecatedName
self.kind = kind
self.nameForDiagnostics = nameForDiagnostics
self.documentation = docCommentTrivia(from: documentation)
self.documentationSummary = SwiftSyntax.Trivia.docCommentTrivia(from: documentation)
self.documentationAbstract = String(documentation?.split(whereSeparator: \.isNewline).first ?? "")
self.isOptional = isOptional
}
Expand Down
30 changes: 28 additions & 2 deletions CodeGeneration/Sources/SyntaxSupport/GrammarGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,19 @@ import SwiftSyntax

/// Generates grammar doc comments for syntax nodes.
struct GrammarGenerator {

/// Returns grammar for a ``TokenChoice``.
///
/// - parameters:
/// - tokenChoice: ``TokenChoice`` to describe
private func grammar(for tokenChoice: TokenChoice) -> String {
switch tokenChoice {
case .keyword(let keyword):
return "`'\(keyword.spec.name)'`"
return "`\(keyword.spec.name)`"
case .token(let token):
let tokenSpec = token.spec
if let tokenText = tokenSpec.text {
return "`'\(tokenText)'`"
return "`\(tokenText)`"
} else {
return "`<\(tokenSpec.varOrCaseName)>`"
}
Expand Down Expand Up @@ -60,4 +65,25 @@ struct GrammarGenerator {
.map { " - `\($0.varOrCaseName)`: \(generator.grammar(for: $0))" }
.joined(separator: "\n")
}

/// Generates a markdown string describing possible choices for the given child
/// token.
static func childTokenChoices(for choices: [TokenChoice]) -> String {
let grammar = GrammarGenerator()

if choices.count == 1 {
return """
### Tokens

For syntax trees generated by the parser, this is guaranteed to be \(grammar.grammar(for: choices.first!)).
"""
} else {
return """
### Tokens

For syntax trees generated by the parser, this is guaranteed to be one of the following kinds:
\(choices.map { " - \(grammar.grammar(for: $0))" }.joined(separator: "\n"))
"""
}
}
}
10 changes: 5 additions & 5 deletions CodeGeneration/Sources/SyntaxSupport/Node.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public class Node {
self.base = base
self.isExperimental = isExperimental
self.nameForDiagnostics = nameForDiagnostics
self.documentation = docCommentTrivia(from: documentation)
self.documentation = SwiftSyntax.Trivia.docCommentTrivia(from: documentation)
self.parserFunction = parserFunction

let childrenWithUnexpected: [Child]
Expand Down Expand Up @@ -211,7 +211,7 @@ public class Node {
}
.joined(separator: "\n")

return docCommentTrivia(
return .docCommentTrivia(
from: """
### Contained in

Expand All @@ -237,7 +237,7 @@ public class Node {
self.base = base
self.isExperimental = isExperimental
self.nameForDiagnostics = nameForDiagnostics
self.documentation = docCommentTrivia(from: documentation)
self.documentation = SwiftSyntax.Trivia.docCommentTrivia(from: documentation)
self.parserFunction = parserFunction

assert(!elementChoices.isEmpty)
Expand Down Expand Up @@ -299,7 +299,7 @@ public struct LayoutNode {
return []
}

return docCommentTrivia(
return .docCommentTrivia(
from: """
### Children

Expand Down Expand Up @@ -352,7 +352,7 @@ public struct CollectionNode {
grammar = "(\(elementChoices.map { "``\($0.syntaxType)``" }.joined(separator: " | "))) `*`"
}

return docCommentTrivia(
return .docCommentTrivia(
from: """
### Children

Expand Down
2 changes: 1 addition & 1 deletion CodeGeneration/Sources/SyntaxSupport/Traits.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class Trait {
init(traitName: String, documentation: String? = nil, children: [Child]) {
self.traitName = traitName
self.protocolName = .identifier("\(traitName)Syntax")
self.documentation = docCommentTrivia(from: documentation)
self.documentation = SwiftSyntax.Trivia.docCommentTrivia(from: documentation)
self.children = children
}
}
Expand Down
45 changes: 32 additions & 13 deletions CodeGeneration/Sources/SyntaxSupport/Utils.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,40 @@ public func lowercaseFirstWord(name: String) -> String {
return name.prefix(wordIndex).lowercased() + name[name.index(name.startIndex, offsetBy: wordIndex)..<name.endIndex]
}

/// Give a (possibly multi-line) string, prepends `///` to each line and creates
/// a `.docLineComment` trivia piece for each line.
public func docCommentTrivia(from string: String?) -> SwiftSyntax.Trivia {
guard let string else {
return []
}
let lines = string.split(separator: "\n", omittingEmptySubsequences: false)
let pieces = lines.enumerated().map { (index, line) in
var line = line
if index != lines.count - 1 {
line = "\(line)\n"
// Helpers to create trivia pieces
extension SwiftSyntax.Trivia {
/// Make a new trivia from a (possibly multi-line) string, prepending `///`
/// to each line and creating a `.docLineComment` trivia piece for each line.
public static func docCommentTrivia(from string: String?) -> SwiftSyntax.Trivia {
guard let string else {
return []
}

let lines = string.split(separator: "\n", omittingEmptySubsequences: false)
let pieces = lines.enumerated().map { (index, line) in
var line = line
if index != lines.count - 1 {
line = "\(line)\n"
}
return SwiftSyntax.TriviaPiece.docLineComment("/// \(line)")
}
return SwiftSyntax.TriviaPiece.docLineComment("/// \(line)")
return SwiftSyntax.Trivia(pieces: pieces)
}

/// Make a new trivia by joining together ``SwiftSyntax/TriviaPiece``s from `joining`,
/// and gluing them together with pieces from `separator`.
public init(
joining items: [SwiftSyntax.Trivia],
separator: SwiftSyntax.Trivia = SwiftSyntax.Trivia(pieces: [TriviaPiece.newlines(1), TriviaPiece.docLineComment("///"), TriviaPiece.newlines(1)])
) {

self.init(
pieces:
items
.filter { !$0.isEmpty }
.joined(separator: separator)
)
}
return SwiftSyntax.Trivia(pieces: pieces)
}

public extension Collection {
Expand Down
2 changes: 1 addition & 1 deletion CodeGeneration/Sources/generate-swift-syntax/LayoutNode+Extensions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ extension LayoutNode {
If the node is empty, there is no token to attach the trivia to and the parameter is ignored.
""".removingEmptyLines

return docCommentTrivia(from: formattedParams)
return SwiftSyntax.Trivia.docCommentTrivia(from: formattedParams)
}

/// Create a builder-based convenience initializer, if needed.
Expand Down
11 changes: 2 additions & 9 deletions ...eneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxCollectionsFile.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,11 @@ import Utils

let syntaxCollectionsFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
for node in SYNTAX_NODES.compactMap(\.collectionNode) {
let documentationSections = [
let documentation = SwiftSyntax.Trivia(joining: [
node.documentation,
node.grammar,
node.containedIn,
]

let documentation =
documentationSections
.filter { !$0.isEmpty }
.map { [$0] }
.joined(separator: [Trivia.newline, Trivia.docLineComment("///"), Trivia.newline])
.reduce(Trivia(), +)
])

try! StructDeclSyntax(
"""
Expand Down
14 changes: 1 addition & 13 deletions CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxNodesFile.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,12 @@ import Utils
func syntaxNode(nodesStartingWith: [Character]) -> SourceFileSyntax {
SourceFileSyntax(leadingTrivia: copyrightHeader) {
for node in SYNTAX_NODES.compactMap(\.layoutNode) where nodesStartingWith.contains(node.kind.syntaxType.description.first!) {
let documentationSections = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While this is clean, there's a way to make this even nicer — we can rename Node.documentation to Node.documenationSummary and make a computed property documentation that works like Child.documentation and internally joins them together.

I'll push that change in a bit to this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. In particular, it would be nice to be consistent with how we stitch together the comment sections.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we agree on this, then perhaps I should include that cleanup in this PR.

I'm hesitant to bring new abstractions, or even move things into Grammar yet — I want to understand it's place better. But what we can do, is we can absolutely make Node and Child consistent with each other. I'm clear on how to do that, and have everything to prepare a commit.

node.documentation,
node.grammar,
node.containedIn,
]
let documentation =
documentationSections
.filter { !$0.isEmpty }
.map { [$0] }
.joined(separator: [Trivia.newline, Trivia.docLineComment("///"), Trivia.newline])
.reduce(Trivia(), +)

// We are actually handling this node now
try! StructDeclSyntax(
"""
// MARK: - \(node.kind.syntaxType)

\(documentation)
\(SwiftSyntax.Trivia(joining: [node.documentation, node.grammar, node.containedIn]))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ahoppen I think you told me there's this code in three spots, but I only caught two: here, and in Child.swift. Where's the third one?

Copy link
Member

Choose a reason for hiding this comment

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

There’s another one in SyntaxCollectionsFile.swift:21

\(node.node.apiAttributes())\
public struct \(node.kind.syntaxType): \(node.baseType.syntaxBaseName)Protocol, SyntaxHashable, \(node.base.leafProtocolType)
"""
Expand Down
44 changes: 44 additions & 0 deletions Sources/SwiftSyntax/generated/SyntaxTraits.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,17 @@


public protocol BracedSyntax: SyntaxProtocol {
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `{`.
var leftBrace: TokenSyntax {
get
set
}

/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `}`.
var rightBrace: TokenSyntax {
get
set
Expand Down Expand Up @@ -121,6 +127,11 @@ public protocol EffectSpecifiersSyntax: SyntaxProtocol {
set
}

/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be one of the following kinds:
/// - `async`
/// - `reasync`
var asyncSpecifier: TokenSyntax? {
get
set
Expand All @@ -131,6 +142,11 @@ public protocol EffectSpecifiersSyntax: SyntaxProtocol {
set
}

/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be one of the following kinds:
/// - `throws`
/// - `rethrows`
var throwsSpecifier: TokenSyntax? {
get
set
Expand Down Expand Up @@ -173,11 +189,17 @@ public extension SyntaxProtocol {


public protocol FreestandingMacroExpansionSyntax: SyntaxProtocol {
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `#`.
var pound: TokenSyntax {
get
set
}

/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `<identifier>`.
var macroName: TokenSyntax {
get
set
Expand All @@ -188,6 +210,9 @@ public protocol FreestandingMacroExpansionSyntax: SyntaxProtocol {
set
}

/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `(`.
var leftParen: TokenSyntax? {
get
set
Expand All @@ -198,6 +223,9 @@ public protocol FreestandingMacroExpansionSyntax: SyntaxProtocol {
set
}

/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `)`.
var rightParen: TokenSyntax? {
get
set
Expand Down Expand Up @@ -245,6 +273,9 @@ public extension SyntaxProtocol {


public protocol NamedDeclSyntax: SyntaxProtocol {
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `<identifier>`.
var name: TokenSyntax {
get
set
Expand Down Expand Up @@ -285,6 +316,10 @@ public extension SyntaxProtocol {
/// See the types conforming to this protocol for examples of where missing nodes can occur.
public protocol MissingNodeSyntax: SyntaxProtocol {
/// A placeholder, i.e. `<#placeholder#>`, that can be inserted into the source code to represent the missing node.
///
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `<identifier>`.
var placeholder: TokenSyntax {
get
set
Expand Down Expand Up @@ -322,11 +357,17 @@ public extension SyntaxProtocol {


public protocol ParenthesizedSyntax: SyntaxProtocol {
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `(`.
var leftParen: TokenSyntax {
get
set
}

/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `)`.
var rightParen: TokenSyntax {
get
set
Expand Down Expand Up @@ -558,6 +599,9 @@ public extension SyntaxProtocol {


public protocol WithTrailingCommaSyntax: SyntaxProtocol {
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `,`.
var trailingComma: TokenSyntax? {
get
set
Expand Down
Loading