Skip to content

Pretty results of dump() for Syntax #94

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

Jump to bottom
Merged
merged 6 commits into from
Mar 5, 2019
Merged

Pretty results of dump() for Syntax #94

merged 6 commits into from
Mar 5, 2019

Conversation

Kuniwak
Copy link
Contributor

@Kuniwak Kuniwak commented Feb 27, 2019 •
edited
Loading

The result of dump() with Syntax is not useful. For example,
the dumped result of Tests/SwiftSyntaxTest/Inputs/closure.swift is the following:

▿ // A closure without a signature. The test will ensure it stays the same after
// applying a rewriting pass.
let x: () -> Void = {}
  ▿ data: SwiftSyntax.SyntaxData
    - parent: nil
    ▿ absoluteRaw: SwiftSyntax.AbsoluteRawSyntax
      - raw: // A closure without a signature. The test will ensure it stays the same after
// applying a rewriting pass.
let x: () -> Void = {} #0
        ▿ super: Swift.ManagedBuffer<SwiftSyntax.RawSyntaxBase, Swift.UInt64>
          ▿ header: SwiftSyntax.RawSyntaxBase
	  ...

This patch improves this result to the following:

▿ SwiftSyntax.SourceFileSyntax
  ▿ statements: SwiftSyntax.CodeBlockItemListSyntax
    ▿ SwiftSyntax.CodeBlockItemSyntax
      ▿ item: SwiftSyntax.VariableDeclSyntax
        - attributes: nil
        - modifiers: nil
        ▿ letOrVarKeyword: SwiftSyntax.TokenSyntax
          - text: "let"
          ▿ leadingTrivia: SwiftSyntax.Trivia
            ▿ pieces: 4 elements
              ▿ TriviaPiece
                - lineComment: "// A closure without a signature. The test will ensure it stays the same after"
                ...

@Kuniwak
Copy link
Contributor Author

Kuniwak commented Feb 27, 2019

@swift-ci Please test

harlanhaskins
harlanhaskins requested changes Feb 27, 2019
View reviewed changes
Copy link
Contributor

@harlanhaskins harlanhaskins left a comment

Choose a reason for hiding this comment

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

Thank you!!! I love this change, and I think you've taken the best approach with it.

Could you add a few tests to test the full output of a Syntax node against the expected string? The test to make sure all children are custom reflectable is great!

harlanhaskins
harlanhaskins reviewed Feb 27, 2019
View reviewed changes
@Kuniwak
Copy link
Contributor Author

Kuniwak commented Feb 28, 2019

I'm working for necessary tests.

@Kuniwak Kuniwak changed the title Pretty results of dump() for Syntax WIP: Pretty results of dump() for Syntax Feb 28, 2019
harlanhaskins
harlanhaskins approved these changes Mar 1, 2019
View reviewed changes
Copy link
Contributor

@harlanhaskins harlanhaskins left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for adding those tests!

@harlanhaskins
Copy link
Contributor

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Mar 1, 2019

Build failed
Swift Test OS X Platform
Git Sha - 9d1393f

@Kuniwak
Copy link
Contributor Author

Kuniwak commented Mar 1, 2019 •
edited
Loading

I added the tests for full output of a Syntax node against the expected string at 1ff1a22.

Then, I found type names without their module names. But it was not intended so I also fixed it at 5fc0f00.

@Kuniwak Kuniwak changed the title WIP: Pretty results of dump() for Syntax Pretty results of dump() for Syntax Mar 1, 2019
@rintaro
Copy link
Member

rintaro commented Mar 5, 2019

@Kuniwak Thank you for your patience. Could you resolve the conflict?

@Kuniwak Kuniwak changed the title Pretty results of dump() for Syntax WIP: Pretty results of dump() for Syntax Mar 5, 2019
@Kuniwak
Copy link
Contributor Author

Kuniwak commented Mar 5, 2019

I'm reading the diff for resolving the conflict.

Kuniwak and others added 6 commits March 5, 2019 20:29
@Kuniwak
Pretty results of dump() for Syntax
38b042b 
The result of dump() with Syntax is not useful. For example,
the dumped result of Tests/SwiftSyntaxTest/Inputs/closure.swift is the following:

```
▿ // A closure without a signature. The test will ensure it stays the same after
// applying a rewriting pass.
let x: () -> Void = {}
  ▿ data: SwiftSyntax.SyntaxData
    - parent: nil
    ▿ absoluteRaw: SwiftSyntax.AbsoluteRawSyntax
      - raw: // A closure without a signature. The test will ensure it stays the same after
// applying a rewriting pass.
let x: () -> Void = {} #0
        ▿ super: Swift.ManagedBuffer<SwiftSyntax.RawSyntaxBase, Swift.UInt64>
          ▿ header: SwiftSyntax.RawSyntaxBase
	  ...
```

This patch improve this result to the following:

```
▿ SourceFileSyntax
  ▿ statements: CodeBlockItemListSyntax
    ▿ CodeBlockItemSyntax
      ▿ item: VariableDeclSyntax
        - attributes: nil
        - modifiers: nil
        ▿ letOrVarKeyword: TokenSyntax
          - text: "let"
          ▿ leadingTrivia: SwiftSyntax.Trivia
            ▿ pieces: 4 elements
              ▿ TriviaPiece
                - lineComment: "// A closure without a signature. The test will ensure it stays the same after"
                ...
```
@noppefoxwolf @Kuniwak
Use UnlabeledChildren
95b9761
@Kuniwak
Fix a typo
4607a52
@Kuniwak
Use 2 indent width
6de29d5
@Kuniwak
Add tests for dumped results of Syntax
b3bf615
@Kuniwak
Print a type name with the namespace
64c43d2
@Kuniwak Kuniwak changed the title WIP: Pretty results of dump() for Syntax Pretty results of dump() for Syntax Mar 5, 2019
@Kuniwak
Copy link
Contributor Author

Kuniwak commented Mar 5, 2019

@rintaro I resolved the conflict.

@rintaro
Copy link
Member

rintaro commented Mar 5, 2019

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Mar 5, 2019

Build failed
Swift Test OS X Platform
Git Sha - 5fc0f00

@harlanhaskins
Copy link
Contributor

Thanks again, @Kuniwak!

@harlanhaskins harlanhaskins merged commit 7be7157 into swiftlang:master Mar 5, 2019
@Kuniwak Kuniwak deleted the pretty-dump branch March 6, 2019 02:45
adevress pushed a commit to adevress/swift-syntax that referenced this pull request Jan 14, 2024
@allevato
Merge pull request swiftlang#94 from allevato/fix-freestanding-inits
94482c3 
Put freestanding `init` decls in a test into a type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rintaro rintaro rintaro approved these changes

@harlanhaskins harlanhaskins harlanhaskins approved these changes

@akyrtzi akyrtzi Awaiting requested review from akyrtzi

@nkcsgexi nkcsgexi Awaiting requested review from nkcsgexi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Kuniwak @harlanhaskins @swift-ci @rintaro @noppefoxwolf