Merge pull request #2789 from MAJKFL/guard-sequential-suggeested-changes

[SwiftLexicalLookup][GSoC] New lookup method signature and `Identifier` initializer changes

Author: DougGregor

Sources/SwiftLexicalLookup/IntroducingToSequentialParentScopeSyntax.swift

@@ -19,8 +19,8 @@ protocol IntroducingToSequentialParentScopeSyntax: ScopeSyntax {
   /// Returns results matching lookup that should be
   /// interleaved with results of the sequential parent.
   func lookupFromSequentialParent(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult]
 }

Sources/SwiftLexicalLookup/LookupName.swift

@@ -110,10 +110,6 @@ import SwiftSyntax
   case declaration(NamedDeclSyntax)
   /// Name introduced implicitly by certain syntax nodes.
   case implicit(ImplicitDecl)
-  /// Explicit `self` keyword.
-  case `self`(IdentifiableSyntax, accessibleAfter: AbsolutePosition?)
-  /// Explicit `Self` keyword.
-  case `Self`(IdentifiableSyntax, accessibleAfter: AbsolutePosition?)
 
   /// Syntax associated with this name.
   @_spi(Experimental) public var syntax: SyntaxProtocol {
@@ -124,8 +120,6 @@ import SwiftSyntax
       return syntax
     case .implicit(let implicitName):
       return implicitName.syntax
-    case .self(let syntax, _), .Self(let syntax, _):
-      return syntax
     }
   }
 
@@ -138,36 +132,24 @@ import SwiftSyntax
       return Identifier(syntax.name)
     case .implicit(let kind):
       return kind.identifier
-    case .self:
-      return Identifier("self")
-    case .Self:
-      return Identifier("Self")
     }
   }
 
   /// Point, after which the name is available in scope.
   /// If set to `nil`, the name is available at any point in scope.
   var accessibleAfter: AbsolutePosition? {
     switch self {
-    case .identifier(_, let absolutePosition),
-      .self(_, let absolutePosition),
-      .Self(_, let absolutePosition):
+    case .identifier(_, let absolutePosition):
       return absolutePosition
     default:
       return nil
     }
   }
 
   /// Checks if this name was introduced before the syntax used for lookup.
-  func isAccessible(at origin: AbsolutePosition) -> Bool {
+  func isAccessible(at lookUpPosition: AbsolutePosition) -> Bool {
     guard let accessibleAfter else { return true }
-    return accessibleAfter <= origin
-  }
-
-  /// Checks if this name refers to the looked up phrase.
-  func refersTo(_ lookedUpIdentifier: Identifier) -> Bool {
-    guard let identifier else { return false }
-    return identifier == lookedUpIdentifier
+    return accessibleAfter <= lookUpPosition
   }
 
   /// Extracts names introduced by the given `syntax` structure.
@@ -229,16 +211,10 @@ import SwiftSyntax
     accessibleAfter: AbsolutePosition? = nil
   ) -> [LookupName] {
     switch identifiable.identifier.tokenKind {
-    case .keyword(.self):
-      return [.self(identifiable, accessibleAfter: accessibleAfter)]
-    case .keyword(.Self):
-      return [.Self(identifiable, accessibleAfter: accessibleAfter)]
+    case .wildcard:
+      return []
     default:
-      if identifiable.identifier.tokenKind != .wildcard {
-        return [.identifier(identifiable, accessibleAfter: accessibleAfter)]
-      } else {
-        return []
-      }
+      return [.identifier(identifiable, accessibleAfter: accessibleAfter)]
     }
   }
 

Sources/SwiftLexicalLookup/LookupResult.swift

@@ -36,4 +36,14 @@ import SwiftSyntax
       return names
     }
   }
+
+  /// Returns result specific for the particular `scope` kind with provided `names`.
+  static func getResult(for scope: ScopeSyntax, withNames names: [LookupName]) -> LookupResult {
+    switch Syntax(scope).as(SyntaxEnum.self) {
+    case .sourceFile(let sourceFileSyntax):
+      return .fromFileScope(sourceFileSyntax, withNames: names)
+    default:
+      return .fromScope(scope, withNames: names)
+    }
+  }
 }

Sources/SwiftLexicalLookup/ScopeImplementations.swift

@@ -97,15 +97,15 @@ import SwiftSyntax
   /// - for `memberBlock` - a, b, c, d, e, f
   /// - for `codeBlock` - a
   @_spi(Experimental) public func lookup(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult] {
     switch config.fileScopeHandling {
     case .memberBlock:
       let names = introducedNames(using: .memberBlock)
         .filter { lookupName in
-          checkName(identifier, refersTo: lookupName, at: origin)
+          checkIdentifier(identifier, refersTo: lookupName, at: lookUpPosition)
         }
 
       return names.isEmpty ? [] : [.fromFileScope(self, withNames: names)]
@@ -119,24 +119,22 @@ import SwiftSyntax
 
         if encounteredNonDeclaration {
           sequentialItems.append(codeBlockItem)
+        } else if item.is(DeclSyntax.self) {
+          let foundNames = LookupName.getNames(from: item)
+          members.append(
+            contentsOf: foundNames.filter { checkIdentifier(identifier, refersTo: $0, at: lookUpPosition) }
+          )
         } else {
-          if item.is(DeclSyntax.self) {
-            let foundNames = LookupName.getNames(from: item)
-
-            members.append(contentsOf: foundNames.filter { checkName(identifier, refersTo: $0, at: origin) })
-          } else {
-            encounteredNonDeclaration = true
-            sequentialItems.append(codeBlockItem)
-          }
+          encounteredNonDeclaration = true
+          sequentialItems.append(codeBlockItem)
         }
       }
 
       let sequentialNames = sequentialLookup(
         in: sequentialItems,
-        for: identifier,
-        at: origin,
-        with: config,
-        createResultsForThisScopeWith: { .fromFileScope(self, withNames: $0) }
+        identifier,
+        at: lookUpPosition,
+        with: config
       )
 
       return (members.isEmpty ? [] : [.fromFileScope(self, withNames: members)]) + sequentialNames
@@ -154,16 +152,15 @@ import SwiftSyntax
   }
 
   @_spi(Experimental) public func lookup(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult] {
     sequentialLookup(
       in: statements,
-      for: identifier,
-      at: origin,
-      with: config,
-      createResultsForThisScopeWith: { .fromScope(self, withNames: $0) }
+      identifier,
+      at: lookUpPosition,
+      with: config
     )
   }
 }
@@ -274,14 +271,14 @@ import SwiftSyntax
   /// }
   /// ```
   @_spi(Experimental) public func lookup(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult] {
-    if let elseBody, elseBody.position <= origin, elseBody.endPosition >= origin {
-      return lookupInParent(for: identifier, at: origin, with: config)
+    if let elseBody, elseBody.range.contains(lookUpPosition) {
+      return lookupInParent(identifier, at: lookUpPosition, with: config)
     } else {
-      return defaultLookupImplementation(for: identifier, at: origin, with: config)
+      return defaultLookupImplementation(identifier, at: lookUpPosition, with: config)
     }
   }
 }
@@ -319,15 +316,14 @@ import SwiftSyntax
   /// // a is visible here
   /// ```
   func lookupFromSequentialParent(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult] {
-    guard body.position > origin || body.endPosition < origin
-    else { return [] }
+    guard !body.range.contains(lookUpPosition) else { return [] }
 
     let names = namesIntroducedToSequentialParent.filter { introducedName in
-      checkName(identifier, refersTo: introducedName, at: origin)
+      checkIdentifier(identifier, refersTo: introducedName, at: lookUpPosition)
     }
 
     return names.isEmpty ? [] : [.fromScope(self, withNames: names)]

Sources/SwiftLexicalLookup/ScopeSyntax.swift

@@ -44,10 +44,10 @@ extension SyntaxProtocol {
   /// 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 identifier: Identifier?,
+    _ identifier: Identifier?,
     with config: LookupConfig = LookupConfig()
   ) -> [LookupResult] {
-    scope?.lookup(for: identifier, at: self.position, with: config) ?? []
+    scope?.lookup(identifier, at: self.position, with: config) ?? []
   }
 }
 
@@ -58,10 +58,9 @@ extension SyntaxProtocol {
   var introducedNames: [LookupName] { get }
   /// Finds all declarations `identifier` refers to. `syntax` specifies the node lookup was triggered with.
   /// If `identifier` set to `nil`, returns all available names at the given node.
-  /// `state` represents lookup state passed between lookup methods.
   func lookup(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult]
 }
@@ -74,47 +73,50 @@ extension SyntaxProtocol {
   /// Returns `LookupResult` of all names introduced in this scope that `identifier`
   /// refers to and is accessible at given syntax node then passes lookup to the parent.
   /// If `identifier` set to `nil`, returns all available names at the given node.
-  /// `state` represents lookup state passed between lookup methods.
   @_spi(Experimental) public func lookup(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult] {
-    defaultLookupImplementation(for: identifier, at: origin, with: config)
+    defaultLookupImplementation(identifier, at: lookUpPosition, with: config)
   }
 
   /// Returns `LookupResult` of all names introduced in this scope that `identifier`
   /// refers to and is accessible at given syntax node then passes lookup to the parent.
   /// If `identifier` set to `nil`, returns all available names at the given node.
   func defaultLookupImplementation(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult] {
     let filteredNames =
       introducedNames
       .filter { introducedName in
-        checkName(identifier, refersTo: introducedName, at: origin)
+        checkIdentifier(identifier, refersTo: introducedName, at: lookUpPosition)
       }
 
     if filteredNames.isEmpty {
-      return lookupInParent(for: identifier, at: origin, with: config)
+      return lookupInParent(identifier, at: lookUpPosition, with: config)
     } else {
       return [.fromScope(self, withNames: filteredNames)]
-        + lookupInParent(for: identifier, at: origin, with: config)
+        + lookupInParent(identifier, at: lookUpPosition, with: config)
     }
   }
 
   /// Looks up in parent scope.
   func lookupInParent(
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
     with config: LookupConfig
   ) -> [LookupResult] {
-    parentScope?.lookup(for: identifier, at: origin, with: config) ?? []
+    parentScope?.lookup(identifier, at: lookUpPosition, with: config) ?? []
   }
 
-  func checkName(_ name: Identifier?, refersTo introducedName: LookupName, at origin: AbsolutePosition) -> Bool {
-    introducedName.isAccessible(at: origin) && (name == nil || introducedName.refersTo(name!))
+  func checkIdentifier(
+    _ identifier: Identifier?,
+    refersTo introducedName: LookupName,
+    at lookUpPosition: AbsolutePosition
+  ) -> Bool {
+    introducedName.isAccessible(at: lookUpPosition) && (identifier == nil || introducedName.identifier == identifier!)
   }
 }

Sources/SwiftLexicalLookup/SequentialScopeSyntax.swift

@@ -40,12 +40,19 @@ extension SequentialScopeSyntax {
   /// code block scope in this exact order.
   func sequentialLookup(
     in codeBlockItems: some Collection<CodeBlockItemSyntax>,
-    for identifier: Identifier?,
-    at origin: AbsolutePosition,
-    with config: LookupConfig,
-    createResultsForThisScopeWith getResults: ([LookupName]) -> (LookupResult)
+    _ identifier: Identifier?,
+    at lookUpPosition: AbsolutePosition,
+    with config: LookupConfig
   ) -> [LookupResult] {
+    // Sequential scope needs to ensure all type declarations are
+    // available in the whole scope (first loop) and
+    // then that results from IntroducingToSequentialParentScopeSyntax
+    // are properly interleaved with the results produced by this scope.
     var results: [LookupResult] = []
+    // We need to use currentChunk because we
+    // can't add the names directly to results
+    // as we need to partition them based on results
+    // obtained from IntroducingToSequentialParentScopeSyntax
     var currentChunk: [LookupName] = []
     var itemsWithoutNamedDecl: [CodeBlockItemSyntax] = []
 
@@ -55,23 +62,23 @@ extension SequentialScopeSyntax {
           from: codeBlockItem.item,
           accessibleAfter: codeBlockItem.endPosition
         ).filter { introducedName in
-          checkName(identifier, refersTo: introducedName, at: origin)
+          checkIdentifier(identifier, refersTo: introducedName, at: lookUpPosition)
         }
       } else {
         itemsWithoutNamedDecl.append(codeBlockItem)
       }
     }
 
     for codeBlockItem in itemsWithoutNamedDecl {
-      guard codeBlockItem.position < origin else { break }
+      guard codeBlockItem.position <= lookUpPosition else { break }
 
       if let introducingToParentScope = Syntax(codeBlockItem.item).asProtocol(SyntaxProtocol.self)
         as? IntroducingToSequentialParentScopeSyntax
       {
         // Get results from encountered scope.
         let introducedResults = introducingToParentScope.lookupFromSequentialParent(
-          for: identifier,
-          at: origin,
+          identifier,
+          at: lookUpPosition,
           with: config
         )
 
@@ -80,7 +87,7 @@ extension SequentialScopeSyntax {
 
         // If there are some names collected, create a new result for this scope.
         if !currentChunk.isEmpty {
-          results.append(getResults(currentChunk))
+          results.append(LookupResult.getResult(for: self, withNames: currentChunk))
           currentChunk = []
         }
 
@@ -91,16 +98,16 @@ extension SequentialScopeSyntax {
           from: codeBlockItem.item,
           accessibleAfter: codeBlockItem.endPosition
         ).filter { introducedName in
-          checkName(identifier, refersTo: introducedName, at: origin)
+          checkIdentifier(identifier, refersTo: introducedName, at: lookUpPosition)
         }
       }
     }
 
     // If there are some names collected, create a new result for this scope.
     if !currentChunk.isEmpty {
-      results.append(getResults(currentChunk))
+      results.append(LookupResult.getResult(for: self, withNames: currentChunk))
     }
 
-    return results.reversed() + lookupInParent(for: identifier, at: origin, with: config)
+    return results.reversed() + lookupInParent(identifier, at: lookUpPosition, with: config)
   }
 }