From 041b8ee4e71de42e686cd787d7fe28e7036209eb Mon Sep 17 00:00:00 2001
From: JohnnyMorganz <johnnymorganz@outlook.com>
Date: Tue, 4 Jun 2024 22:53:01 +0200
Subject: [PATCH] Fix edge case in 'findBindingAtPosition' when looking up
 global binding at start of file (#1254)

The 'findBindingAtPosition' AstQuery function can be used to lookup a
local or global binding.

Inside of this function is a check to "Ignore this binding if we're
inside its definition. e.g. local abc = abc -- Will take the definition
of abc from outer scope".

However, this check is incorrect when we are looking up a global binding
at the start of a file.

Consider a complete file with the contents:
```lua
local x = stri|ng.char(1)
```

and we pass the location of the marker `|` as the position to the find
binding position. We will pick up the global binding of the definition
`string` coming from a builtin source (either defined via C++ code or a
definitions file and loaded into the global scope).

The global binding `string` will have a zero position: `0,0,0,0`.
However, the `findBindingLocalStatement` check works by looking up the
AstAncestry at the binding's defined begin position *in the current
source module*. This will then incorrectly return the local statement
for `local x`, as that is at the start of the source code. Then in turn,
we assume we are in the `local abc = abc` case, and end up skipping over
the correct binding.

We fix this by checking if the binding is at the global position. If so,
we early exit because it is impossible for a global binding to be
defined in a local statement.
---
 Analysis/src/AstQuery.cpp |  6 ++++++
 tests/AstQuery.test.cpp   | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/Analysis/src/AstQuery.cpp b/Analysis/src/AstQuery.cpp
index cebb226a..928e5dfb 100644
--- a/Analysis/src/AstQuery.cpp
+++ b/Analysis/src/AstQuery.cpp
@@ -12,6 +12,7 @@
 #include <algorithm>
 
 LUAU_FASTFLAG(DebugLuauDeferredConstraintResolution);
+LUAU_FASTFLAGVARIABLE(LuauFixBindingForGlobalPos, false);
 
 namespace Luau
 {
@@ -332,6 +333,11 @@ std::optional<TypeId> findExpectedTypeAtPosition(const Module& module, const Sou
 
 static std::optional<AstStatLocal*> findBindingLocalStatement(const SourceModule& source, const Binding& binding)
 {
+    // Bindings coming from global sources (e.g., definition files) have a zero position.
+    // They cannot be defined from a local statement
+    if (FFlag::LuauFixBindingForGlobalPos && binding.location == Location{{0, 0}, {0, 0}})
+        return std::nullopt;
+
     std::vector<AstNode*> nodes = findAstAncestryOfPosition(source, binding.location.begin);
     auto iter = std::find_if(nodes.rbegin(), nodes.rend(), [](AstNode* node) {
         return node->is<AstStatLocal>();
diff --git a/tests/AstQuery.test.cpp b/tests/AstQuery.test.cpp
index 769637a5..c53fe731 100644
--- a/tests/AstQuery.test.cpp
+++ b/tests/AstQuery.test.cpp
@@ -6,6 +6,8 @@
 #include "doctest.h"
 #include "Fixture.h"
 
+LUAU_FASTFLAG(LuauFixBindingForGlobalPos);
+
 using namespace Luau;
 
 struct DocumentationSymbolFixture : BuiltinsFixture
@@ -331,4 +333,16 @@ TEST_CASE_FIXTURE(Fixture, "find_expr_ancestry")
     CHECK(ancestry.back()->is<AstExprFunction>());
 }
 
+TEST_CASE_FIXTURE(BuiltinsFixture, "find_binding_at_position_global_start_of_file")
+{
+    ScopedFastFlag sff{FFlag::LuauFixBindingForGlobalPos, true};
+    check("local x = string.char(1)");
+    const Position pos(0, 12);
+
+    std::optional<Binding> binding = findBindingAtPosition(*getMainModule(), *getMainSourceModule(), pos);
+
+    REQUIRE(binding);
+    CHECK_EQ(binding->location, Location{Position{0, 0}, Position{0, 0}});
+}
+
 TEST_SUITE_END();