Replaces the MultiLineTrailingCommas phase 1 rule with Pretty Printer tokens.
#120
Conversation
| @@ -243,8 +233,12 @@ extension LintPipeline { | |||
| return .visitChildren | |||
| } | |||
|
|
|||
| func visit(_ node: SwitchStmtSyntax) -> SyntaxVisitorContinueKind { | |||
| func visit(_ node: SwitchCaseListSyntax) -> SyntaxVisitorContinueKind { | |||
There was a problem hiding this comment.
Oh, crap, I never regenerated this, did I 😞
At some point we should add a test that verifies that this file is up-to-date.
There was a problem hiding this comment.
Maybe a pre-commit hook? We might want to do the same for running the formatter on the formatter soon too.
There was a problem hiding this comment.
We could make a test that invokes generate-pipeline and then verifies that the output is the same as the file in the source tree. Then once we're on CI, failure to regenerate would catch it.
A pre-commit hook that just auto-generates it could be smoother since it would prevent failure, but I'm a little wary of executing that much logic in a pre-commit hook (I'm not sure if there's ever a time where we wouldn't want that thing to run automatically).
| @@ -47,6 +47,10 @@ private final class TokenStreamCreator: SyntaxVisitor { | |||
| /// stream. | |||
| private var pendingMultilineStringSegmentPrefixLengths = [TokenSyntax: Int]() | |||
|
|
|||
| /// Lists tokens that shouldn't be appended to the token stream as `syntax` tokens. They will be | |||
| /// printed conditionally using a different type of token. | |||
| private var ignoredTokens = [TokenSyntax]() | |||
There was a problem hiding this comment.
In practice this is probably minor, but since TokenSyntaxes are Hashable, you can use a Set<> instead of an array here to make the lookup closer to constant-time.
dylansturg
force-pushed
the
trailing_comma_markers
branch
from
8c121bf to
42a4f23
Compare
| @@ -243,8 +233,12 @@ extension LintPipeline { | |||
| return .visitChildren | |||
| } | |||
|
|
|||
| func visit(_ node: SwitchStmtSyntax) -> SyntaxVisitorContinueKind { | |||
| func visit(_ node: SwitchCaseListSyntax) -> SyntaxVisitorContinueKind { | |||
There was a problem hiding this comment.
We could make a test that invokes generate-pipeline and then verifies that the output is the same as the file in the source tree. Then once we're on CI, failure to regenerate would catch it.
A pre-commit hook that just auto-generates it could be smoother since it would prevent failure, but I'm a little wary of executing that much logic in a pre-commit hook (I'm not sure if there's ever a time where we wouldn't want that thing to run automatically).
The phase 1 rule for adding trailing commas was incapable of being 100% correct, because a phase 1 cannot accurately determine whether an array or dictionary literal requires multiple lines. The only part of the formatter that has the necessary information is the PrettyPrinter.
There is a new pair of tokens:
commaDelimitedRegionStartandcommaDelimitedRegionEnd. A comma is inserted at the end token if and only if the end token is on a different line than the start token. The PrettyPrinter uses a stack of line numbers for the start token to implement this.