Skip to content

[SwiftLexicalLookup] Composite names #2905

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

Merged
merged 7 commits into from
Dec 6, 2024
35 changes: 34 additions & 1 deletion Sources/SwiftLexicalLookup/LookupName.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ import SwiftSyntax
?? subscriptDecl.endPositionBeforeTrailingTrivia
case .variableDecl(let variableDecl):
return variableDecl.bindings.first?.accessorBlock?.positionAfterSkippingLeadingTrivia
?? variableDecl.endPosition
?? variableDecl.bindings.positionAfterSkippingLeadingTrivia
case .accessorDecl(let accessorDecl):
return accessorDecl.accessorSpecifier.positionAfterSkippingLeadingTrivia
case .deinitializerDecl(let deinitializerDecl):
Expand Down Expand Up @@ -139,6 +139,19 @@ import SwiftSyntax
case implicit(ImplicitDecl)
/// Dollar identifier introduced by a closure without parameters.
case dollarIdentifier(ClosureExprSyntax, strRepresentation: String)
/// Represents equivalent identifiers grouped together.
/// - Important: The array should be non-empty.
///
/// ### Example:
/// ```swift
/// switch X {
/// case .a(let x), .b(let x):
/// print(x) // <-- lookup here
/// }
/// ```
/// For lookup at the given position, the result
/// contains only one (composite) name
case compositeName([LookupName])

/// Syntax associated with this name.
@_spi(Experimental) public var syntax: SyntaxProtocol {
Expand All @@ -151,6 +164,8 @@ import SwiftSyntax
return implicitName.syntax
case .dollarIdentifier(let closureExpr, _):
return closureExpr
case .compositeName(let names):
return names.first!.syntax
}
}

Expand All @@ -165,6 +180,8 @@ import SwiftSyntax
return kind.identifier
case .dollarIdentifier(_, strRepresentation: _):
return nil
case .compositeName(let names):
return names.first!.identifier
}
}

Expand All @@ -185,6 +202,8 @@ import SwiftSyntax
return implicitName.position
case .dollarIdentifier(let closureExpr, _):
return closureExpr.positionAfterSkippingLeadingTrivia
case .compositeName(let names):
return names.first!.position
}
}

Expand Down Expand Up @@ -319,6 +338,20 @@ import SwiftSyntax
return "implicit: \(strName)"
case .dollarIdentifier(_, strRepresentation: let str):
return "dollarIdentifier: \(str)"
case .compositeName(let names):
var result = "Composite name: [ "

for (index, name) in names.enumerated() {
result += name.debugDescription

if index < names.count - 1 {
result += ", "
} else {
result += " ]"
}
}

return result
}
}
}
69 changes: 63 additions & 6 deletions Sources/SwiftLexicalLookup/Scopes/ScopeImplementations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ import SwiftSyntax

/// Capture, parameter and body names introduced in this scope.
@_spi(Experimental) public var defaultIntroducedNames: [LookupName] {
captureNames + parameterNames + introducedNamesInBody
parameterNames + captureNames + introducedNamesInBody
}

@_spi(Experimental) public var scopeDebugName: String {
Expand Down Expand Up @@ -202,7 +202,7 @@ import SwiftSyntax
} else {
signatureResults = LookupResult.getResultArray(
for: self,
withNames: (captureNames + parameterNames).filter { name in
withNames: (parameterNames + captureNames).filter { name in
checkIdentifier(identifier, refersTo: name, at: lookUpPosition)
}
)
Expand Down Expand Up @@ -574,13 +574,44 @@ import SwiftSyntax
@_spi(Experimental) extension SwitchCaseSyntax: SequentialScopeSyntax {
/// Names introduced within `case` items.
var namesFromLabel: [LookupName] {
label.as(SwitchCaseLabelSyntax.self)?.caseItems.flatMap { child in
guard let switchCaseItemList = label.as(SwitchCaseLabelSyntax.self)?.caseItems else { return [] }

let extractedNames = switchCaseItemList.flatMap { child in
if let exprPattern = child.pattern.as(ExpressionPatternSyntax.self) {
return LookupName.getNames(from: exprPattern.expression)
} else {
return LookupName.getNames(from: child.pattern)
}
} ?? []
}

if switchCaseItemList.count <= 1 {
return extractedNames
}

var orderedKeys: [Identifier] = []
var partitioned: [Identifier: [LookupName]] = [:]

for extractedName in extractedNames {
guard let identifier = extractedName.identifier else { continue }

if !partitioned.keys.contains(identifier) {
orderedKeys.append(identifier)
}

if partitioned[identifier] == nil {
partitioned[identifier] = [extractedName]
} else {
partitioned[identifier]?.append(extractedName)
}
}

return
orderedKeys
.compactMap { key in
guard let names = partitioned[key] else { return nil }

return .compositeName(names)
}
}

/// Names introduced within `case` items
Expand All @@ -607,7 +638,7 @@ import SwiftSyntax
checkIdentifier(identifier, refersTo: name, at: lookUpPosition)
}

if label.range.contains(lookUpPosition) {
if label.range.contains(lookUpPosition) && !isInWhereClause(lookUpPosition: lookUpPosition) {
return config.finishInSequentialScope ? [] : lookupInParent(identifier, at: lookUpPosition, with: config)
} else if config.finishInSequentialScope {
return sequentialLookup(
Expand All @@ -629,6 +660,20 @@ import SwiftSyntax
+ lookupInParent(identifier, at: lookUpPosition, with: config)
}
}

/// Returns `true` if `lookUpPosition` is inside a `where`
/// clause associated with one of the case items of this scope.
private func isInWhereClause(lookUpPosition: AbsolutePosition) -> Bool {
guard let switchCaseItemList = label.as(SwitchCaseLabelSyntax.self)?.caseItems else { return false }

for item in switchCaseItemList {
if item.whereClause?.range.contains(lookUpPosition) ?? false {
return true
}
}

return false
}
}

@_spi(Experimental) extension ProtocolDeclSyntax: ScopeSyntax, LookInMembersScopeSyntax {
Expand Down Expand Up @@ -903,7 +948,9 @@ extension SubscriptDeclSyntax: WithGenericParametersScopeSyntax, CanInterleaveRe
at lookUpPosition: AbsolutePosition,
with config: LookupConfig
) -> [LookupResult] {
if bindings.first?.accessorBlock?.range.contains(lookUpPosition) ?? false {
if (bindings.first?.accessorBlock?.range.contains(lookUpPosition) ?? false)
|| shouldIntroduceSelfIfLazy(lookUpPosition: lookUpPosition)
{
return defaultLookupImplementation(
in: (isMember ? [.implicit(.self(self))] : LookupName.getNames(from: self)),
identifier,
Expand All @@ -929,6 +976,16 @@ extension SubscriptDeclSyntax: WithGenericParametersScopeSyntax, CanInterleaveRe

return resultsToInterleave + lookupInParent(identifier, at: lookUpPosition, with: config)
}

/// Returns `true`, if `lookUpPosition` is in initializer of
/// this variable declaration and the declaration is lazy.
private func shouldIntroduceSelfIfLazy(lookUpPosition: AbsolutePosition) -> Bool {
guard bindings.first?.initializer?.range.contains(lookUpPosition) ?? false else { return false }

return modifiers.contains {
$0.name.tokenKind == .keyword(.lazy)
}
}
}

@_spi(Experimental) extension DeinitializerDeclSyntax: ScopeSyntax {
Expand Down
10 changes: 7 additions & 3 deletions Tests/SwiftLexicalLookupTest/Assertions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,18 @@ func assertLexicalNameLookup(
ResultExpectation.assertResult(marker: marker, result: result, expectedValues: expectedValues)

return result.flatMap { lookUpResult in
lookUpResult.names.map { lookupName in
lookupName.syntax
lookUpResult.names.flatMap { lookupName in
if case .compositeName(let names) = lookupName {
return names.map(\.syntax)
} else {
return [lookupName.syntax]
}
}
}
},
expected: references.mapValues { expectations in
expectations.flatMap { expectation in
expectation.expectedNames.map { expectedName in
expectation.expectedNames.flatMap { expectedName in
expectedName.marker
}
}
Expand Down
28 changes: 22 additions & 6 deletions Tests/SwiftLexicalLookupTest/ExpectedName.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ import XCTest

/// Used to define lookup name assertion.
protocol ExpectedName {
var marker: String { get }
var marker: [String] { get }
}

extension String: ExpectedName {
var marker: String {
self
var marker: [String] {
[self]
}
}

Expand Down Expand Up @@ -63,15 +63,22 @@ enum NameExpectation: ExpectedName {
case declaration(String)
case implicit(ImplicitNameExpectation)
case dollarIdentifier(String, String)
case compositeName([NameExpectation])

var marker: String {
var marker: [String] {
switch self {
case .identifier(let marker),
.declaration(let marker),
.dollarIdentifier(let marker, _):
return marker
return [marker]
case .implicit(let implicitName):
return implicitName.marker
return [implicitName.marker]
case .compositeName(let expectedNames):
return
expectedNames
.flatMap { expectedName in
expectedName.marker
}
}
}

Expand All @@ -86,6 +93,15 @@ enum NameExpectation: ExpectedName {
actualStr == expectedStr,
"For marker \(marker), actual identifier \(actualStr) doesn't match expected \(expectedStr)"
)
case (.compositeName(let actualNames), .compositeName(let expectedNames)):
XCTAssert(
actualNames.count == expectedNames.count,
"For marker \(marker), actual composite name count \(actualNames.count) doesn't match expected \(expectedNames.count)"
)

for (actualName, expectedName) in zip(actualNames, expectedNames) {
expectedName.assertExpectation(marker: marker, for: actualName)
}
default:
XCTFail("For marker \(marker), actual name kind \(name) doesn't match expected \(self)")
}
Expand Down
42 changes: 38 additions & 4 deletions Tests/SwiftLexicalLookupTest/NameLookupTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -791,9 +791,11 @@ final class testNameLookup: XCTestCase {
assertLexicalNameLookup(
source: """
switch {
case .x(let 1️⃣a, let 2️⃣b), .y(.c(let 3️⃣c), .z):
print(4️⃣a, 5️⃣b, 6️⃣c)
case .z(let 7️⃣a), .smth(let 8️⃣a)
case .x(let 1️⃣a, let 2️⃣b):
print(4️⃣a, 5️⃣b)
case .y(.c(let 3️⃣c), .z):
print(6️⃣c)
case .z(let 7️⃣a)
print(9️⃣a)
default:
print(0️⃣a)
Expand All @@ -803,13 +805,45 @@ final class testNameLookup: XCTestCase {
"4️⃣": [.fromScope(SwitchCaseSyntax.self, expectedNames: ["1️⃣"])],
"5️⃣": [.fromScope(SwitchCaseSyntax.self, expectedNames: ["2️⃣"])],
"6️⃣": [.fromScope(SwitchCaseSyntax.self, expectedNames: ["3️⃣"])],
"9️⃣": [.fromScope(SwitchCaseSyntax.self, expectedNames: ["7️⃣", "8️⃣"])],
"9️⃣": [.fromScope(SwitchCaseSyntax.self, expectedNames: ["7️⃣"])],
"0️⃣": [],
],
expectedResultTypes: .all(IdentifierPatternSyntax.self)
)
}

func testCompositeNames() {
assertLexicalNameLookup(
source: """
switch X {
case .x(let 1️⃣a, let 2️⃣b), .y(let 3️⃣a, let 4️⃣b):
print(5️⃣x)
case .z(let 7️⃣a), .smth(let 8️⃣a):
print(9️⃣x)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn’t this be print(a)? x is never defined in this test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I purposefully called it x to further indicate that we are not performing name matching in this case. We use useNilAsTheParameter: true for that. This test is supposed to check if we properly partition names equivalent names (like in the first case here).

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I missed that and thought that we were looking up x, not that we were collecting all names available at that location. The test makes sense now 😄

}
""",
references: [
"5️⃣": [
.fromScope(
SwitchCaseSyntax.self,
expectedNames: [
NameExpectation.compositeName([.identifier("1️⃣"), .identifier("3️⃣")]),
NameExpectation.compositeName([.identifier("2️⃣"), .identifier("4️⃣")]),
]
)
],
"9️⃣": [
.fromScope(
SwitchCaseSyntax.self,
expectedNames: [NameExpectation.compositeName([.identifier("7️⃣"), .identifier("8️⃣")])]
)
],
],
expectedResultTypes: .all(IdentifierPatternSyntax.self),
useNilAsTheParameter: true
)
}

func testSimpleGenericParameterScope() {
assertLexicalNameLookup(
source: """
Expand Down