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.

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

Add own wrappers for SyntaxTokens and Syntax Map #2955

Merged
merged 4 commits into from
Nov 12, 2019
Merged

Add own wrappers for SyntaxTokens and Syntax Map #2955

merged 4 commits into from
Nov 12, 2019

Conversation

PaulTaykalo
Copy link
Collaborator

@PaulTaykalo PaulTaykalo commented Nov 10, 2019 •
edited
Loading

What

This PR adds SwiftLintSyntaxToken and SwiftLintSyntaxMap wrappers over the existing SyntaxToken and SyntaxMap structures.

SwiftLintSyntaxToken

  • Now has precomputed kind: SyntaxKind property

SwiftLintSyntaxMap

  • Now have precached tokens: [SwiftLintSyntaxToken]

Why

  • Wrappers are used mainly for storing additional information about the structures
  • Precomputed values with types allowing to write simpler code since no unneeded conversation between String and enums are needed

@SwiftLintBot
Copy link

SwiftLintBot commented Nov 10, 2019 •
edited
Loading

1 Warning
⚠️ This PR may need tests.
12 Messages
📖 Linting Aerial with this PR took 1.27s vs 1.27s on master (0% slower)
📖 Linting Alamofire with this PR took 2.26s vs 2.23s on master (1% slower)
📖 Linting Firefox with this PR took 9.03s vs 8.95s on master (0% slower)
📖 Linting Kickstarter with this PR took 14.39s vs 14.27s on master (0% slower)
📖 Linting Moya with this PR took 1.14s vs 1.18s on master (3% faster)
📖 Linting Nimble with this PR took 1.39s vs 1.43s on master (2% faster)
📖 Linting Quick with this PR took 0.54s vs 0.54s on master (0% slower)
📖 Linting Realm with this PR took 2.39s vs 2.37s on master (0% slower)
📖 Linting SourceKitten with this PR took 1.01s vs 1.0s on master (1% slower)
📖 Linting Sourcery with this PR took 6.78s vs 6.8s on master (0% faster)
📖 Linting Swift with this PR took 11.57s vs 11.54s on master (0% slower)
📖 Linting WordPress with this PR took 14.64s vs 14.67s on master (0% faster)

Generated by 🚫 Danger

@PaulTaykalo PaulTaykalo force-pushed the improvement/own-wrappers-over-syntax-tokens branch 2 times, most recently from dd32040 to ebb8b5f Compare November 10, 2019 20:45
@PaulTaykalo PaulTaykalo force-pushed the improvement/own-wrappers-over-syntax-tokens branch 2 times, most recently from 7dbe68f to d8c07d5 Compare November 10, 2019 21:40
@PaulTaykalo PaulTaykalo force-pushed the improvement/own-wrappers-over-syntax-tokens branch from d8c07d5 to 61a6a27 Compare November 10, 2019 21:54
@jpsim
Copy link
Collaborator

jpsim commented Nov 11, 2019

Were you hoping that this would improve performance or is this for some other reason?

@PaulTaykalo
Copy link
Collaborator Author

I hoped that it would improve performance. But there's nothing apart from readability :)

@PaulTaykalo
Copy link
Collaborator Author

PaulTaykalo commented Nov 11, 2019 •
edited
Loading

I, personally don't like the long names SwiftLintSyntaxToken etc.
We could probably use SyntaxToken in the Swiftlint itself, but this can lead to confusion about which one is where.

Still, since all of them were renamed, it can be a viable solution :)

Switlint.SyntaxToken <-- SourceKitten.SyntaxToken

@PaulTaykalo PaulTaykalo requested a review from jpsim November 11, 2019 22:55
@PaulTaykalo PaulTaykalo merged commit 3f0db8f into master Nov 12, 2019
@PaulTaykalo PaulTaykalo deleted the improvement/own-wrappers-over-syntax-tokens branch November 12, 2019 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpsim jpsim jpsim approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@PaulTaykalo @SwiftLintBot @jpsim