Skip to content

Support for comments modifiers #385

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
80 changes: 80 additions & 0 deletions Sources/SwiftSyntaxBuilder/BuildableNodes.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public struct ${type.buildable()}: ${base_type.buildable()}, ${type.expressible_
% for child in children:
let ${child.name()}: ${child.type().buildable()}
% end
public var itemLeadingTriviaPieces: [TriviaPiece] = []

/// Creates a `${type.buildable()}` using the provided parameters.
/// - Parameters:
Expand Down Expand Up @@ -106,7 +107,32 @@ public struct ${type.buildable()}: ${base_type.buildable()}, ${type.expressible_

/// Conformance to `${base_type.buildable()}`.
public func build${base_type.base_name()}(format: Format, leadingTrivia: Trivia? = nil) -> ${base_type.syntax()} {
% if type.buildable() != 'SourceFile':
let result = build${type.base_name()}(format: format, leadingTrivia: leadingTrivia)
% end
% if type.buildable() == 'SourceFile':
let indentedLines = itemLeadingTriviaPieces
.map { Trivia(pieces: [$0, .newlines(1)]) }
.reduce(.zero, +)

let combinedTrivia = [format._makeIndent(),
leadingTrivia,
indentedLines]
.compactMap { $0 }
.reduce(.zero, +)
% if type.buildable() == 'SourceFile':
let result = buildSourceFile(format: format, leadingTrivia: combinedTrivia)
% end
% else:

if !itemLeadingTriviaPieces.isEmpty {
let indentedLeadingTrivia = itemLeadingTriviaPieces
.compactMap { Trivia(pieces:[ $0, .newlines(1)]) }
.map { $0 + format._makeIndent() }
.reduce(.zero, +)
return ${base_type.syntax()}(result).withLeadingTrivia(indentedLeadingTrivia)
}
% end
Comment on lines +110 to +135
Copy link
Member

Choose a reason for hiding this comment

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

I don’t think the special handling for SourceFile is sufficient here. The same issue will arise for any syntax node that doesn’t have a token before its first child node (I’m happy to be proven wrong by a test case). For example we should see the same issue if you try to create a TypealiasDecl like the following where both the private attribute and the TypealiasDecl itself have comments attached to it.

/// This comment is added to the TypealiasDecl
/// This comment is added to the `private` keyword
private typealias MyInt = Int

Is there any issue with applying the solution you have implemented for SourceFile to all syntax nodes (I haven’t checked).


Also, I think there’s a few end missing and the ones that do exist have the wrong indentation level sometimes. Could you make sure that every if-statement has a matching end on the same indentation level?

return ${base_type.syntax()}(result)
}

Expand All @@ -130,5 +156,59 @@ public struct ${type.buildable()}: ${base_type.buildable()}, ${type.expressible_
return self
}
% end

public func lineComment(_ text: String) -> Self {

let textWithSlashesPieces = prefixLines(of: text, with: "//")
.map { TriviaPiece.lineComment("\($0)") }
return addCommentPiece(commentPieces: textWithSlashesPieces)
}

/// A documentation block comment, starting with '/**' and ending with '*/'.
/// - Parameter text: inserted between /* and */.
/// - Returns: a `${type.buildable()}` with the added line comment.
public func blockComment(_ text: String) -> Self {
let piece = TriviaPiece.blockComment("/* \(text) */")
return addCommentPiece(commentPieces: [piece])
}

/// A documentation line comment, starting with '/**' and ending with '*/'.
/// - Parameter text: comment to be inserted after ///
/// - Returns: a `${type.buildable()}` with the added documentation line comment.
public func docLineComment(_ text: String) -> Self {
let textWithSlashesPieces = prefixLines(of: text, with: "//")
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be

Suggested change
let textWithSlashesPieces = prefixLines(of: text, with: "//")
let textWithSlashesPieces = prefixLines(of: text, with: "///")

Could you also add a test case for it?

.map { TriviaPiece.docLineComment("\($0)") }
return addCommentPiece(commentPieces: textWithSlashesPieces)
}


/// A documentation block comment, starting with '/**' and ending with '*/'.
/// - Parameter text: comment to be inserted between /** and '*/.
/// - Returns: a `${type.buildable()}` with the added documentation block comment.
public func docBlockComment(_ text: String) -> Self {
let piece = TriviaPiece.docBlockComment("/** \(text) */")
return addCommentPiece(commentPieces: [piece])
}

private func addCommentPiece(commentPieces: [TriviaPiece]) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

Per the Swift Naming guidelines this should be named addingCommentPiece because it returns a modified version of self instead of modifying self in-place.

var newSelf = self
newSelf.itemLeadingTriviaPieces.append(contentsOf: commentPieces)
return newSelf
}

/// Extracts Lines from a given text, and add a prefix to it
/// - Parameters:
/// - text: Text which its lines will be prefixed
/// - prefix: prefix for each line
/// - Returns: an array of new string, which has prefixed lines
private func prefixLines(of text: String, with prefix: String) -> [String] {

let prefixedLines = text
.split(whereSeparator: \.isNewline)
.map {"\(prefix) \($0)"}

return prefixedLines
}
Comment on lines +199 to +211
Copy link
Member

Choose a reason for hiding this comment

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

A little bit of a nitpick but I think this is a general helper method and not something that needs to access instance members of *Buildable. Thus, I think it would make sense to make it a free function instead of an instance function.


}
% end
Loading