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.

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

Use SwiftSyntax's new SwiftParser #4216

Merged
merged 4 commits into from
Oct 1, 2022
Merged
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
4 changes: 4 additions & 0 deletions .swiftlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,7 @@ custom_rules:
message: Rule Test Function mustn't end with `rule`
regex: func\s*test\w+(r|R)ule\(\)
severity: error

unused_import:
always_keep_imports:
- SwiftSyntaxBuilder # we can't detect uses of string interpolation of swift syntax nodes
6 changes: 3 additions & 3 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ swift_library(
visibility = ["//visibility:public"],
deps = [
"@com_github_jpsim_sourcekitten//:SourceKittenFramework",
"@com_github_keith_swift_syntax//:SwiftSyntax",
"@com_github_keith_swift_syntax//:SwiftSyntaxBuilder",
"@com_github_keith_swift_syntax//:SwiftSyntaxParser",
"@com_github_apple_swift_syntax//:SwiftSyntax",
"@com_github_apple_swift_syntax//:SwiftSyntaxBuilder",
"@com_github_apple_swift_syntax//:SwiftParser",
"@sourcekitten_com_github_jpsim_yams//:Yams",
] + select({
"@platforms//os:linux": ["@com_github_krzyzanowskim_cryptoswift//:CryptoSwift"],
Expand Down
16 changes: 12 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@

#### Enhancements

* SwiftSyntax libraries have been updated from the previous 5.6 release and now
use the new parser written in Swift.
Swift 5.7+ features should now be parsed more accurately.
We've also measured an improvement in lint times of up to 15%.
This should also fix some deployment issues where the exact version of the
internal SwiftSyntax parser needed to be available.
If you notice any unexpected changes to lint results, please file an issue on
the SwiftLint issue tracker. We can look into it and if it's a SwiftSyntax
parser regression we can re-file it upstream.
[JP Simard](https://github.com/jpims)
[#4031](https://github.com/realm/SwiftLint/issues/4031)

* Add new `excludes_trivial_init` configuration for `missing_docs` rule
to exclude initializers without any parameters.
[Marcelo Fabri](https://github.com/marcelofabri)
Expand Down Expand Up @@ -78,10 +90,6 @@
[SimplyDanny](https://github.com/SimplyDanny)
[#4202](https://github.com/realm/SwiftLint/issues/4202)

* SwiftSyntax libraries were updated to their 5.7 releases, improving how newer
Swift language features are handled.
[JP Simard](https://github.com/jpims)

#### Bug Fixes

* Respect `validates_start_with_lowercase` option when linting function names.
Expand Down
4 changes: 2 additions & 2 deletions Package.resolved
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
"repositoryURL": "https://github.com/apple/swift-syntax.git",
"state": {
"branch": null,
"revision": "04d4497be6b88e524a71778d828790e9589ae1c4",
"version": "0.50700.0"
"revision": "093e5ee151d206454e2c1950d81333c4d4a4472e",
"version": null
}
},
{
Expand Down
19 changes: 4 additions & 15 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,18 @@ import PackageDescription

#if os(macOS)
private let addCryptoSwift = false
private let staticSwiftSyntax = true
#else
private let addCryptoSwift = true
private let staticSwiftSyntax = false
#endif

let frameworkDependencies: [Target.Dependency] = [
.product(name: "SourceKittenFramework", package: "SourceKitten"),
.product(name: "SwiftSyntax", package: "SwiftSyntax"),
.product(name: "SwiftSyntaxBuilder", package: "SwiftSyntax"),
.product(name: "SwiftSyntaxParser", package: "SwiftSyntax"),
.product(name: "SwiftParser", package: "SwiftSyntax"),
"Yams",
]
+ (addCryptoSwift ? ["CryptoSwift"] : [])
+ (staticSwiftSyntax ? ["lib_InternalSwiftSyntaxParser"] : [])

let package = Package(
name: "SwiftLint",
Expand All @@ -28,7 +25,7 @@ let package = Package(
],
dependencies: [
.package(name: "swift-argument-parser", url: "https://github.com/apple/swift-argument-parser.git", .upToNextMinor(from: "1.1.3")),
.package(name: "SwiftSyntax", url: "https://github.com/apple/swift-syntax.git", .exact("0.50700.0")),
.package(name: "SwiftSyntax", url: "https://github.com/apple/swift-syntax.git", .revision("093e5ee151d206454e2c1950d81333c4d4a4472e")),
.package(url: "https://github.com/jpsim/SourceKitten.git", from: "0.33.0"),
.package(url: "https://github.com/jpsim/Yams.git", from: "5.0.1"),
.package(url: "https://github.com/scottrhoyt/SwiftyTextTable.git", from: "0.9.0"),
Expand All @@ -46,11 +43,7 @@ let package = Package(
),
.target(
name: "SwiftLintFramework",
dependencies: frameworkDependencies,
// Pass `-dead_strip_dylibs` to ignore the dynamic version of `lib_InternalSwiftSyntaxParser`
// that ships with SwiftSyntax because we want the static version from
// `StaticInternalSwiftSyntaxParser`.
linkerSettings: staticSwiftSyntax ? [.unsafeFlags(["-Xlinker", "-dead_strip_dylibs"])] : []
dependencies: frameworkDependencies
),
.testTarget(
name: "SwiftLintFrameworkTests",
Expand All @@ -61,9 +54,5 @@ let package = Package(
"Resources",
]
),
] + (staticSwiftSyntax ? [.binaryTarget(
name: "lib_InternalSwiftSyntaxParser",
url: "https://github.com/keith/StaticInternalSwiftSyntaxParser/releases/download/5.7/lib_InternalSwiftSyntaxParser.xcframework.zip",
checksum: "99803975d10b2664fc37cc223a39b4e37fe3c79d3d6a2c44432007206d49db15"
)] : [])
]
)
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import SwiftSyntax

extension SourceFileSyntax {
func windowsOfThreeTokens() -> [(TokenSyntax, TokenSyntax, TokenSyntax)] {
Array(tokens)
Array(tokens(viewMode: .sourceAccurate))
.windows(ofCount: 3)
.map { tokens in
let previous = tokens[tokens.startIndex]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import Foundation
import SourceKittenFramework
import SwiftParser
import SwiftSyntax
#if canImport(SwiftSyntaxParser)
import SwiftSyntaxParser
#endif

private let warnSyntaxParserFailureOnceImpl: Void = {
queuedPrintError("Could not parse the syntax tree for at least one file. Results may be invalid.")
Expand Down Expand Up @@ -38,7 +36,7 @@ private var structureDictionaryCache = Cache({ file in

private var syntaxTreeCache = Cache({ file -> SourceFileSyntax? in
do {
return try SyntaxParser.parse(source: file.contents)
return try Parser.parse(source: file.contents)
} catch {
warnSyntaxParserFailureOnce()
return nil
Expand Down
2 changes: 1 addition & 1 deletion Source/SwiftLintFramework/Helpers/CommandVisitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ final class CommandVisitor: SyntaxVisitor {

init(locationConverter: SourceLocationConverter) {
self.locationConverter = locationConverter
super.init()
super.init(viewMode: .sourceAccurate)
}

override func visitPost(_ node: TokenSyntax) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public struct BlockBasedKVORule: SwiftSyntaxRule, ConfigurationProviderRule {
)

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
Visitor()
Visitor(viewMode: .sourceAccurate)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ private extension DiscouragedObjectLiteralRule {

init(configuration: ObjectLiteralConfiguration) {
self.configuration = configuration
super.init(viewMode: .sourceAccurate)
}

override func visitPost(_ node: ObjectLiteralExprSyntax) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public struct DiscouragedOptionalBooleanRule: OptInRule, ConfigurationProviderRu
)

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
Visitor()
Visitor(viewMode: .sourceAccurate)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public struct FallthroughRule: SwiftSyntaxRule, ConfigurationProviderRule, OptIn
)

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
Visitor()
Visitor(viewMode: .sourceAccurate)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public struct ForceCastRule: ConfigurationProviderRule, SwiftSyntaxRule {
)

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
ForceCastRuleVisitor()
ForceCastRuleVisitor(viewMode: .sourceAccurate)
}
}

Expand All @@ -29,4 +29,10 @@ private final class ForceCastRuleVisitor: SyntaxVisitor, ViolationsSyntaxVisitor
violationPositions.append(node.asTok.positionAfterSkippingLeadingTrivia)
}
}

override func visitPost(_ node: UnresolvedAsExprSyntax) {
if node.questionOrExclamationMark?.tokenKind == .exclamationMark {
violationPositions.append(node.asTok.positionAfterSkippingLeadingTrivia)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public struct ForceTryRule: ConfigurationProviderRule, SwiftSyntaxRule {
)

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
Visitor()
Visitor(viewMode: .sourceAccurate)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public struct ForceUnwrappingRule: OptInRule, SwiftSyntaxRule, ConfigurationProv
)

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
ForceUnwrappingVisitor()
ForceUnwrappingVisitor(viewMode: .sourceAccurate)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ private extension GenericTypeNameRule {

init(configuration: NameConfiguration) {
self.configuration = configuration
super.init(viewMode: .sourceAccurate)
}

override func visitPost(_ node: GenericParameterSyntax) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public struct IfLetShadowingRule: OptInRule, SwiftSyntaxCorrectableRule, Configu
public init() {}

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
Visitor()
Visitor(viewMode: .sourceAccurate)
}

public func makeRewriter(file: SwiftLintFile) -> ViolationsSyntaxRewriter? {
Expand Down Expand Up @@ -135,7 +135,7 @@ private class Rewriter: SyntaxRewriter, ViolationsSyntaxRewriter {
private extension OptionalBindingConditionSyntax {
var isShadowingOptionalBinding: Bool {
if let id = pattern.as(IdentifierPatternSyntax.self),
let value = initializer.value.as(IdentifierExprSyntax.self),
let value = initializer?.value.as(IdentifierExprSyntax.self),
id.identifier.text == value.identifier.text {
return true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public struct RedundantNilCoalescingRule: OptInRule, SwiftSyntaxCorrectableRule,
)

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
Visitor()
Visitor(viewMode: .sourceAccurate)
}

public func makeRewriter(file: SwiftLintFile) -> ViolationsSyntaxRewriter? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public struct ReturnValueFromVoidFunctionRule: ConfigurationProviderRule, OptInR
)

public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor? {
ReturnValueFromVoidFunctionVisitor()
ReturnValueFromVoidFunctionVisitor(viewMode: .sourceAccurate)
}
}

Expand All @@ -26,7 +26,7 @@ private final class ReturnValueFromVoidFunctionVisitor: SyntaxVisitor, Violation
override func visitPost(_ node: ReturnStmtSyntax) {
if node.expression != nil,
let functionNode = Syntax(node).enclosingFunction(),
functionNode.returnsVoid {
functionNode.returnsVoid {
violationPositions.append(node.positionAfterSkippingLeadingTrivia)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,41 @@ internal struct ReturnValueFromVoidFunctionRuleExamples {
var foo: Int {
return 0
}
""")
"""),
Example(#"""
final class SearchMessagesDataSource: ValueCellDataSource {
internal enum Section: Int {
case emptyState
case messageThreads
}
internal func load(messageThreads: [MessageThread]) {
self.set(
values: messageThreads,
cellClass: MessageThreadCell.self,
inSection: Section.messageThreads.rawValue
)
}
internal func emptyState(isVisible: Bool) {
self.set(
cellIdentifiers: isVisible ? ["SearchMessagesEmptyState"] : [],
inSection: Section.emptyState.rawValue
)
}
internal override func configureCell(tableCell cell: UITableViewCell, withValue value: Any) {
switch (cell, value) {
case let (cell as MessageThreadCell, value as MessageThread):
cell.configureWith(value: value)
case (is StaticTableViewCell, is Void):
return
default:
assertionFailure("Unrecognized combo: \(cell), \(value).")
}
}
}
"""#, excludeFromDocumentation: true)
]

static let triggeringExamples = [
Expand Down
19 changes: 9 additions & 10 deletions Source/SwiftLintFramework/Rules/Idiomatic/SyntacticSugarRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public struct SyntacticSugarRule: CorrectableRule, ConfigurationProviderRule, So
)

public func validate(file: SwiftLintFile) -> [StyleViolation] {
let visitor = SyntacticSugarRuleVisitor()
let visitor = SyntacticSugarRuleVisitor(viewMode: .sourceAccurate)
return visitor.walk(file: file) { visitor in
flattenViolations(visitor.violations)
}.map { violation in
Expand All @@ -34,7 +34,7 @@ public struct SyntacticSugarRule: CorrectableRule, ConfigurationProviderRule, So
}

public func correct(file: SwiftLintFile) -> [Correction] {
let visitor = SyntacticSugarRuleVisitor()
let visitor = SyntacticSugarRuleVisitor(viewMode: .sourceAccurate)
return visitor.walk(file: file) { visitor in
var context = CorrectingContext(rule: self, file: file, contents: file.contents)
context.correctViolations(visitor.violations)
Expand Down Expand Up @@ -137,13 +137,6 @@ private final class SyntacticSugarRuleVisitor: SyntaxVisitor {
}
}

override func visitPost(_ node: AsExprSyntax) {
// json["recommendations"] as? ↓Array<[String: Any]>
if let violation = violation(in: node.typeName) {
violations.append(violation)
}
}

override func visitPost(_ node: TypeInitializerClauseSyntax) {
// typealias Document = ↓Dictionary<String, AnyBSON?>
if let violation = violation(in: node.value) {
Expand Down Expand Up @@ -172,7 +165,7 @@ private final class SyntacticSugarRuleVisitor: SyntaxVisitor {
// let x = ↓Array<String>.array(of: object)
// Skip checks for 'self' or \T Dictionary<Key, Value>.self
if let parent = node.parent?.as(MemberAccessExprSyntax.self),
let lastToken = Array(parent.tokens).last?.tokenKind,
let lastToken = Array(parent.tokens(viewMode: .sourceAccurate)).last?.tokenKind,
[.selfKeyword, .identifier("Type"), .identifier("none"), .identifier("Index")].contains(lastToken) {
return
}
Expand All @@ -194,6 +187,12 @@ private final class SyntacticSugarRuleVisitor: SyntaxVisitor {
.map { violations.append($0) }
}

override func visitPost(_ node: TypeExprSyntax) {
if let violation = violation(in: node.type) {
violations.append(violation)
}
}

private func violation(in typeSyntax: TypeSyntax?) -> SyntacticSugarRuleViolation? {
if let optionalType = typeSyntax?.as(OptionalTypeSyntax.self) {
return violation(in: optionalType.wrappedType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ internal enum SyntacticSugarRuleExamples {
Example("""
@_specialize(where S == ↓Array<Character>)
public init<S: Sequence>(_ elements: S)
"""),

Example("""
let dict: [String: Any] = [:]
_ = dict["key"] as? ↓Optional<String?> ?? Optional<String?>.none
""")
]

Expand Down
Loading