Skip to content

[SwiftLexicalLookup][GSoC] Add initial name lookup functionality #2719

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 10 commits into from
Jul 23, 2024
141 changes: 141 additions & 0 deletions Sources/SwiftLexicalLookup/LookupName.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import SwiftSyntax

@_spi(Experimental) public enum LookupName {
/// Identifier associated with the name.
/// Could be an identifier of a variable, function or closure parameter and more
case identifier(String, SyntaxProtocol)
/// Declaration associated with the name.
/// Could be class, struct, actor, protocol, function and more
case declaration(String, DeclSyntaxProtocol)
Copy link
Member

Choose a reason for hiding this comment

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

All of the declarations with names conform to the NamedDeclSyntax protocol. Please replace the (String, DeclSyntaxProtocl) with NamedDeclSyntax rather than duplicating the name.

Copy link
Contributor Author

@MAJKFL MAJKFL Jul 9, 2024

Choose a reason for hiding this comment

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

I changed it now to use NamedDeclSyntax. I also introduced IdentifiableSyntax protocol to introduce similar sort of logic to the rest of the relevant syntax nodes.


/// Syntax associated with this name.
@_spi(Experimental) public var syntax: SyntaxProtocol {
switch self {
case .identifier(_, let syntax):
syntax
case .declaration(_, let syntax):
syntax
}
}

/// Introduced name.
@_spi(Experimental) public var name: String {
switch self {
case .identifier(let name, _):
name
case .declaration(let name, _):
name
}
}

/// Checks if this name was introduced before the syntax used for lookup.
func isBefore(_ lookedUpSyntax: SyntaxProtocol) -> Bool {
syntax.position < lookedUpSyntax.position
}

/// Checks if this name refers to the looked up phrase.
func refersTo(_ lookedUpName: String) -> Bool {
name == lookedUpName
}

/// Extracts names introduced by the given `from` structure.
static func getNames(from syntax: SyntaxProtocol) -> [LookupName] {
switch Syntax(syntax).as(SyntaxEnum.self) {
case .variableDecl(let variableDecl):
variableDecl.bindings.flatMap { binding in
getNames(from: binding.pattern)
}
case .tuplePattern(let tuplePattern):
tuplePattern.elements.flatMap { tupleElement in
getNames(from: tupleElement.pattern)
}
case .valueBindingPattern(let valueBindingPattern):
getNames(from: valueBindingPattern.pattern)
case .expressionPattern(let expressionPattern):
getNames(from: expressionPattern.expression)
case .sequenceExpr(let sequenceExpr):
sequenceExpr.elements.flatMap { expression in
getNames(from: expression)
}
case .patternExpr(let patternExpr):
getNames(from: patternExpr.pattern)
case .optionalBindingCondition(let optionalBinding):
getNames(from: optionalBinding.pattern)
case .identifierPattern(let identifierPattern):
handle(identifierPattern: identifierPattern)
case .closureShorthandParameter(let closureShorthandParameter):
handle(closureShorthandParameter: closureShorthandParameter)
case .closureParameter(let closureParameter):
handle(closureParameter: closureParameter)
case .functionDecl(let functionDecl):
handle(functionDecl: functionDecl)
case .classDecl(let classDecl):
handle(classDecl: classDecl)
case .structDecl(let structDecl):
handle(structDecl: structDecl)
case .actorDecl(let actorDecl):
handle(actorDecl: actorDecl)
case .protocolDecl(let protocolDecl):
handle(protocolDecl: protocolDecl)
default:
[]
}
}

/// Extracts name introduced by `identifierPattern`.
private static func handle(identifierPattern: IdentifierPatternSyntax) -> [LookupName] {
[.identifier(identifierPattern.identifier.text, identifierPattern)]
}

/// Extracts name introduced by `closureParameter`.
private static func handle(closureParameter: ClosureParameterSyntax) -> [LookupName] {
[.identifier(closureParameter.secondName?.text ?? closureParameter.firstName.text, closureParameter)]
}

/// Extracts name introduced by `closureShorthandParameter`.
private static func handle(closureShorthandParameter: ClosureShorthandParameterSyntax) -> [LookupName] {
Copy link
Member

Choose a reason for hiding this comment

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

I know you haven't covered the whole language yet, but closure captures are another thing to consider in the design.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out, I forgot about this case. Apart from closure captures, I'll try to also include guard and file scope with this PR. With those changes, it should be pretty straight forward to introduce those and then we could move on to more complex scopes with the next PR.

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 added closure capture, if case support, guard and file scope. I couldn't find a case where ClosureCaptureSyntax expression wouldn't be a DeclReferenceExprSyntax hence the force unwrap. Do you think we can make this assumption?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we discussed on Monday, the desired behavior for the file scope can be adjusted by passing the right config to the lookup method. By introducing LookupConfig protocol we could also specify more configuration options that could add more specific behavior to name lookup in the future (something like: setting some default shadowing rules, filtering specific compiler result flags etc.). Also there's currently one quirk in the guard scope implementation, even though it responds to all name lookup queries correctly, it assigns the declarations to enclosing scope. If you think we shouldn't merge it like this, I can put it fixed in the next PR instead. This one is already much larger than I anticipated.

let name = closureShorthandParameter.name.text
if name != "_" {
return [.identifier(name, closureShorthandParameter)]
} else {
return []
}
}

/// Extracts name introduced by `functionDecl`.
private static func handle(functionDecl: FunctionDeclSyntax) -> [LookupName] {
[.declaration(functionDecl.name.text, functionDecl)]
}

/// Extracts name introduced by `classDecl`.
private static func handle(classDecl: ClassDeclSyntax) -> [LookupName] {
[.declaration(classDecl.name.text, classDecl)]
}

/// Extracts name introduced by `structDecl`.
private static func handle(structDecl: StructDeclSyntax) -> [LookupName] {
[.declaration(structDecl.name.text, structDecl)]
}

/// Extracts name introduced by `actorDecl`.
private static func handle(actorDecl: ActorDeclSyntax) -> [LookupName] {
[.declaration(actorDecl.name.text, actorDecl)]
}

/// Extracts name introduced by `protocolDecl`.
private static func handle(protocolDecl: ProtocolDeclSyntax) -> [LookupName] {
[.declaration(protocolDecl.name.text, protocolDecl)]
}
}
150 changes: 150 additions & 0 deletions Sources/SwiftLexicalLookup/ScopeImplementations.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import SwiftSyntax

extension SyntaxProtocol {
/// Parent scope of this syntax node, or scope introduced by this syntax node.
var scope: ScopeSyntax? {
switch Syntax(self).as(SyntaxEnum.self) {
case .sourceFile(let sourceFile):
sourceFile
case .codeBlock(let codeBlock):
codeBlock
case .forStmt(let forStmt):
forStmt
case .closureExpr(let closureExpr):
closureExpr
case .whileStmt(let whileStmt):
whileStmt
case .ifExpr(let ifExpr):
ifExpr
case .memberBlock(let memberBlock):
memberBlock
default:
self.parent?.scope
}
}
}

extension SourceFileSyntax: ScopeSyntax {
var parentScope: ScopeSyntax? {
nil
}

var introducedNames: [LookupName] {
[]
}

func lookup(for name: String, at syntax: SyntaxProtocol) -> [LookupName] {
[]
}
}

extension CodeBlockSyntax: ScopeSyntax {
var introducedNames: [LookupName] {
statements.flatMap { codeBlockItem in
LookupName.getNames(from: codeBlockItem.item)
}
}

func lookup(for name: String, at syntax: SyntaxProtocol) -> [LookupName] {
defaultLookupImplementation(for: name, at: syntax, positionSensitive: true)
}
}

extension ForStmtSyntax: ScopeSyntax {
var introducedNames: [LookupName] {
LookupName.getNames(from: pattern)
}

func lookup(for name: String, at syntax: SyntaxProtocol) -> [LookupName] {
defaultLookupImplementation(for: name, at: syntax)
}
}

extension ClosureExprSyntax: ScopeSyntax {
var introducedNames: [LookupName] {
signature?.parameterClause?.children(viewMode: .sourceAccurate).flatMap { parameter in
Copy link
Member

Choose a reason for hiding this comment

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

We want closure captures here, too

if let parameterList = parameter.as(ClosureParameterListSyntax.self) {
parameterList.children(viewMode: .sourceAccurate).flatMap { parameter in
LookupName.getNames(from: parameter)
}
} else {
LookupName.getNames(from: parameter)
}
} ?? []
}

func lookup(for name: String, at syntax: SyntaxProtocol) -> [LookupName] {
defaultLookupImplementation(for: name, at: syntax)
}
}

extension WhileStmtSyntax: ScopeSyntax {
var introducedNames: [LookupName] {
conditions.flatMap { element in
LookupName.getNames(from: element.condition)
}
}

func lookup(for name: String, at syntax: SyntaxProtocol) -> [LookupName] {
defaultLookupImplementation(for: name, at: syntax)
}
}

extension IfExprSyntax: ScopeSyntax {
var parentScope: ScopeSyntax? {
getParent(for: self.parent, previousIfElse: self.elseKeyword == nil)
}

/// Finds the parent scope, omitting parent `if` statements if part of their `else if` clause.
private func getParent(for syntax: Syntax?, previousIfElse: Bool) -> ScopeSyntax? {
guard let syntax else { return nil }

if let lookedUpScope = syntax.scope, lookedUpScope.id != self.id {
if let currentIfExpr = lookedUpScope.as(IfExprSyntax.self), previousIfElse {
return getParent(for: syntax.parent, previousIfElse: currentIfExpr.elseKeyword == nil)
} else {
return lookedUpScope
}
} else {
return getParent(for: syntax.parent, previousIfElse: previousIfElse)
}
}

var introducedNames: [LookupName] {
conditions.flatMap { element in
LookupName.getNames(from: element.condition)
}
}

func lookup(for name: String, at syntax: SyntaxProtocol) -> [LookupName] {
if let elseBody, elseBody.position <= syntax.position, elseBody.endPosition >= syntax.position {
parentScope?.lookup(for: name, at: syntax) ?? []
} else {
defaultLookupImplementation(for: name, at: syntax, positionSensitive: true)
}
}
}

extension MemberBlockSyntax: ScopeSyntax {
var introducedNames: [LookupName] {
members.flatMap { member in
LookupName.getNames(from: member.decl)
}
}

func lookup(for name: String, at syntax: SyntaxProtocol) -> [LookupName] {
defaultLookupImplementation(for: name, at: syntax)
}
}
76 changes: 76 additions & 0 deletions Sources/SwiftLexicalLookup/ScopeSyntax.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import SwiftSyntax

extension SyntaxProtocol {
/// Returns all names that `for` refers to at this syntax node.
///
/// - Returns: An array of names referred to by `for` at this syntax node,
/// ordered by visibility. The order is from the innermost to the outermost
/// scope, and within each scope, names are ordered by their introduction
/// in the source code.
///
/// Example usage:
/// ```swift
/// class C {
/// var a = 42
///
/// func a(a: Int) {
/// a // <--- lookup here
///
/// let a = 0
/// }
///
/// func a() {
/// // ...
/// }
/// }
/// ```
/// When calling this function on the declaration reference `a` within its name,
/// the function returns the parameter first, then the identifier of the variable
/// declaration, followed by the first function name, and then the second function name,
/// in this exact order. The constant declaration within the function body is omitted
/// due to the ordering rules that prioritize visibility within the function body.
@_spi(Experimental) public func lookup(for name: String) -> [LookupName] {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should proactively unify the queries for "give me all of the entities visible at this point" (which is used for code completion, typo correction, etc.) and "give me all of the entities matching at this point" (which is used for resolving names). That could be as simple as making the name parameter optional and, when it's non-nil, filtering out any names that don't match.

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 made the name parameter optional and added support for looking up all names accessible at a given node. I made one test case that exercises this type of lookup. I'm only curious about the desired ordering.

class foo {
  let 1️⃣a = 0
  let 2️⃣b = 0

  3️⃣func foo() {
    let 4️⃣a = 0
    let 5️⃣c = 0
  
    6️⃣x
  }
}

Like in this snippet, when running the lookup for all names at position 6️⃣, should we return the result as [(4️⃣, 5️⃣), (1️⃣, 2️⃣, 3️⃣)] or [(5️⃣, 4️⃣), (3️⃣, 2️⃣, 1️⃣)] (i.e. in the order of introduction or accessibility from the node used for lookup)? I think the first option would make more sense as we wouldn't have to account for cases like an additional declaration in a member block under the scope we perform lookup in and it's also the way name lookup works now, but I'm also curious of your take on this one.

Copy link
Member

Choose a reason for hiding this comment

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

Additionally and unrelatedly, I think we're going to need more structure in the result than a flat array [LookupName]. The current structure does not allow a client to distinguish between "there are two things that match f in the same scope" and "there are two things that match f but the first one shadows the second one", which is important for clients. I suggest returning an array of "interesting" scopes, where an interesting scope contains the names the found in that scope, and perhaps other directions like "look into the instance members of ". For now, we could just have an array of scopes, where each scope has the LookupNames found in it, so we can handle shadowing.

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 introduced a new enum LookupResult that, as for now, has one case fromScope that holds ScopeSyntax and [LookupName] associated with that scope. With the lookup function result type now being [LookupResult], I think this could be a nice structure to work with. We could easily add new cases for the result that could encapsulate more information and e.g. act as compiler flags or provide suggestions for shadowing. What do you think about it?

scope?.lookup(for: name, at: self) ?? []
}
}

protocol ScopeSyntax: SyntaxProtocol {
/// Parent of this scope, or `nil` if it is the root.
var parentScope: ScopeSyntax? { get }
/// Names found in this scope. Ordered from first to last introduced.
var introducedNames: [LookupName] { get }
/// Finds all declarations `name` refers to. `at` specifies the node lookup was triggered with.
func lookup(for name: String, at syntax: SyntaxProtocol) -> [LookupName]
}

extension ScopeSyntax {
var parentScope: ScopeSyntax? {
self.parent?.scope
}

/// Returns all names introduced in this scope that `name` refers to and then
/// passes lookup to the parent. Optionally, if `positionSensitive` is set to `true`,
/// the method filters names that were introduced in this scope after `syntax`.
func defaultLookupImplementation(
for name: String,
at syntax: SyntaxProtocol,
positionSensitive: Bool = false
) -> [LookupName] {
introducedNames
.filter { introducedName in
(!positionSensitive || introducedName.isBefore(syntax)) && introducedName.refersTo(name)
Copy link
Member

Choose a reason for hiding this comment

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

This "is before" check isn't going to work when we start modeling the precise location where a variable in a pattern is introduced.

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 changed it now to isAccessible which is used in every filtering condition and accounts for the precise location of introduction set by the parent scope. Do you think this could work in the long run?

} + (parentScope?.lookup(for: name, at: syntax) ?? [])
}
}
8 changes: 5 additions & 3 deletions Sources/SwiftLexicalLookup/SimpleLookupQueries.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ extension SyntaxProtocol {

// MARK: - lookupCatchNode

/// Given syntax node location, finds where an error could be caught. If `traverseCatchClause` is set to `true` lookup will skip the next do statement.
/// Given syntax node location, finds where an error could be caught.
/// If `traverseCatchClause` is set to `true` lookup will skip the next do statement.
private func lookupCatchNodeHelper(traversedCatchClause: Bool) -> Syntax? {
guard let parent else { return nil }

Expand Down Expand Up @@ -108,7 +109,8 @@ extension SyntaxProtocol {
collectNodesOfTypeUpToFunctionBoundary(type, stopWithFirstMatch: true).first
}

/// Collect syntax nodes matching the collection type up until encountering one of the specified syntax nodes. The nodes in the array are inside out, with the innermost node being the first.
/// Collect syntax nodes matching the collection type up until encountering one of the specified syntax nodes.
/// The nodes in the array are inside out, with the innermost node being the first.
fileprivate func collectNodesOfTypeUpToFunctionBoundary<T: SyntaxProtocol>(
_ type: T.Type,
stopWithFirstMatch: Bool = false
Expand All @@ -128,7 +130,7 @@ extension SyntaxProtocol {
)
}

/// Callect syntax nodes matching the collection type up until encountering one of the specified syntax nodes.
/// Collect syntax nodes matching the collection type up until encountering one of the specified syntax nodes.
private func collectNodes<T: SyntaxProtocol>(
ofType type: T.Type,
upTo stopAt: [SyntaxProtocol.Type],
Expand Down
Loading