Extend symbolLocation with the repo revision used

Summary:
Glass.symbolLocation should also provide a repo_hash. This diff adds the hash.

```
fbdbg> =await GleanGlass::genSymbolLocation($glass, "www/php/GleanGlass", null)
GleanGlass\SymbolLocation Object
(
    [location] => GleanGlass\LocationRange Object
        (
            [repository] => "www"
            [filepath] => "flib/intern/glean/GleanGlass.php"
            [range] => GleanGlass\Range Object
                (
                    [lineBegin] => 21
                    [columnBegin] => 1
                    [lineEnd] => 438
                    [columnEnd] => 2
                )

        )

    [revision] => "f25c89ae41f2a367d59fb7d7bc14949b0f08e3d9"
)
```

Reviewed By: phlalx

Differential Revision: D60025655

fbshipit-source-id: 1fa87718be82926ab8ce4c1d53999bd107f1c26e

glean/glass/Glean/Glass/Handler.hs

@@ -214,23 +214,32 @@ findReferenceRanges [email protected]{..} sym opts@RequestOptions{..} =
   where
     limit = fmap fromIntegral requestOptions_limit
 
-
 -- | Resolve a symbol identifier to its range-based location in the latest db
-symbolLocation
-  :: Glass.Env
-  -> SymbolId
-  -> RequestOptions
-  -> IO LocationRange
-symbolLocation env@Glass.Env{..} sym opts = do
-  withSymbol "symbolLocation" env opts sym
-    $ \gleanDBs _dbInfo (repo, lang, toks) ->
-      findSymbolLocationRange env GleanBackend{..} repo lang toks
+-- This is about 10x cheaper than a full describeSymbol() call
+symbolLocation :: Glass.Env -> SymbolId -> RequestOptions -> IO SymbolLocation
+symbolLocation env@Glass.Env{..} sym opts =
+  withSymbol "symbolLocation" env opts sym $
+    \gleanDBs dbInfo (scmRepo, lang, toks) ->
+      backendRunHaxl GleanBackend{..} env $ do
+        r <- Search.searchEntityLocation lang toks
+        (entity, err) <- case r of
+          None t -> throwM (ServerException t)
+          One e -> return (e, Nothing)
+          Many { initial = e, message = t } ->
+            return (e, Just (GlassExceptionReason_entitySearchFail t))
+        let SearchEntity { entityRepo, decl = (_,file,rangespan,_) } = entity
+        let scmRevs = scmRevisions dbInfo
+        (, fmap logError err) <$> withRepo entityRepo (do
+          location <- rangeSpanToLocationRange scmRepo file rangespan
+          repo_hash <- getRepoHashForLocation location scmRevs <$> Glean.haxlRepo
+          return $ SymbolLocation {
+            symbolLocation_location = location,
+            symbolLocation_revision = repo_hash
+          })
 
 -- | Describe characteristics of a symbol
 describeSymbol
-  :: Glass.Env
-  -> SymbolId
-  -> RequestOptions
+  :: Glass.Env -> SymbolId -> RequestOptions
   -> IO SymbolDescription
 describeSymbol env@Glass.Env{..} symId opts =
   withSymbol "describeSymbol" env opts symId $
@@ -642,18 +651,6 @@ toFileReference :: RepoName -> Path -> FileReference
 toFileReference repo path =
   FileReference repo (toGleanPath $ SymbolRepoPath repo path)
 
--- | Symbol search: try to resolve the line/col range of an entity
-findSymbolLocationRange
-  :: Glean.Backend b
-  => Glass.Env
-  -> GleanBackend b
-  -> RepoName
-  -> Language
-  -> [Text]
-  -> IO (LocationRange, Maybe ErrorLogger)
-findSymbolLocationRange env b repo lang toks = backendRunHaxl b env $
-  withEntity rangeSpanToLocationRange repo lang toks
-
 -- | Symbol search for references
 fetchSymbolReferences
   :: Glean.Backend b
@@ -732,23 +729,6 @@ symbolToAngleEntities lang toks = do
     Left err -> Left (logError (GlassExceptionReason_entityNotSupported err))
     Right results -> Right (results, searchErr)
 
-
--- | Run an action on the result of looking up a symbol id
-withEntity
-  :: (RepoName -> Src.File -> Code.RangeSpan -> RepoHaxl u w a)
-  -> RepoName
-  -> Language
-  -> [Text]
-  -> Glean.ReposHaxl u w (a, Maybe ErrorLogger)
-withEntity f scsrepo lang toks = do
-  r <- Search.searchEntityLocation lang toks
-  (SearchEntity{entityRepo, decl=(_,file,rangespan,_)}, err) <- case r of
-    None t -> throwM (ServerException t)
-    One e -> return (e, Nothing)
-    Many { initial = e, message = t } ->
-      return (e, Just (GlassExceptionReason_entitySearchFail t))
-  (, fmap logError err) <$> withRepo entityRepo (f scsrepo file rangespan)
-
 fetchSymbolsAndAttributesGlean
   :: Glean.Backend b
   => Glass.Env

glean/glass/Glean/Glass/Logging.hs

@@ -169,6 +169,10 @@ instance LogResult LocationRange where
     , Logger.setRepo $ unRepoName locationRange_repository
     ]
 
+instance LogResult SymbolLocation where
+  logResult SymbolLocation{..} = logResult symbolLocation_location
+      <> Logger.setRevisionUsed (coerce symbolLocation_revision)
+
 instance LogResult SymbolDescription where
   logResult SymbolDescription{..} =
     logResult symbolDescription_location <>

glean/glass/Glean/Glass/Repos.hs

@@ -530,7 +530,6 @@ findLanguages RepoMapping{..} repoName@(RepoName repo) pLang =
       langs = filter (Text.isPrefixOf pLang) allLangs
   in map (\lang -> SymbolId $ repo <> "/" <> lang <> "/") langs
 
-
 -- TODO (T122759515): Get repo revision from db properties
 getRepoHash :: Glean.Repo -> Revision
 getRepoHash repo = Revision (Text.take 40 (Glean.repo_hash repo))

glean/glass/if/glass.thrift

@@ -84,6 +84,14 @@ struct LocationRange {
   3: Range range (hs.strict);
 }
 
+// A Location associated with a specific revision
+struct SymbolLocation {
+  // Repository, filepath and line / column range
+  1: LocationRange location;
+  // the revision for which the location is defined
+  2: Revision revision;
+}
+
 // Generic request options, supported by most calls
 struct RequestOptions {
   // repo-global preferred revision identifier
@@ -758,7 +766,7 @@ service GlassService extends fb303.FacebookService {
   ) throws (1: ServerException e, 2: GlassException g);
 
   // Return just the symbol's location as efficiently as possible
-  LocationRange symbolLocation(
+  SymbolLocation symbolLocation(
     1: SymbolId symbol,
     2: RequestOptions options,
   ) throws (1: ServerException e, 2: GlassException g);

glean/glass/test/regression/lib/Glean/Glass/Regression/Snapshot.hs

@@ -333,6 +333,9 @@ instance (DeterministicResponse a, Ord a) => DeterministicResponse [a] where
 instance DeterministicResponse Range where det = id
 instance DeterministicResponse Location where det = id
 instance DeterministicResponse LocationRange where det = id
+instance DeterministicResponse SymbolLocation where
+  det (SymbolLocation loc _rev) = SymbolLocation (det loc) (Revision "testhash")
+
 instance DeterministicResponse SearchRelatedResult where
   det (SearchRelatedResult xs ys) = -- to edit the desc hash
     SearchRelatedResult (det xs) (det ys)

glean/glass/test/regression/lib/Glean/Glass/Regression/Tests.hs

@@ -59,7 +59,8 @@ testSymbolLocation sym@(SymbolId name) path get =
   TestLabel (Text.unpack name) $ TestCase $ do
     (backend, _repo) <- get
     withTestEnv backend $ \env -> do
-      LocationRange{..} <- symbolLocation env sym def
+      SymbolLocation{..} <- symbolLocation env sym def
+      let LocationRange{..} = symbolLocation_location
       assertEqual "symbolLocation Path matches" locationRange_filepath path
 
 -- | Test that both describeSymbol and symbolLocation for a SymbolId
@@ -74,7 +75,8 @@ testDescribeSymbolMatchesPath sym@(SymbolId name) path get =
       assertEqual "describeSymbol Path matches"
         (symbolPath_filepath symbolDescription_location)
         path
-      LocationRange{..} <- symbolLocation env sym def
+      SymbolLocation{..} <- symbolLocation env sym def
+      let LocationRange{..} = symbolLocation_location
       assertEqual "symbolLocation Path matches" locationRange_filepath path
 
 -- | Test that both describeSymbol has expected comment

glean/glass/test/regression/tests/buck/resolveSymbolRange_target.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "buck_project/TEST_TARGETS",
-        "range": {
-            "columnBegin": 1,
-            "columnEnd": 2,
-            "lineBegin": 12,
-            "lineEnd": 18
+        "location": {
+            "filepath": "buck_project/TEST_TARGETS",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 12,
+                "lineEnd": 18
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]

glean/glass/test/regression/tests/buck/resolveSymbol_def.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "buck_project/my_defs.bzl",
-        "range": {
-            "columnBegin": 1,
-            "columnEnd": 6,
-            "lineBegin": 9,
-            "lineEnd": 12
+        "location": {
+            "filepath": "buck_project/my_defs.bzl",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 6,
+                "lineBegin": 9,
+                "lineEnd": 12
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]

glean/glass/test/regression/tests/buck/resolveSymbol_file.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "buck_project/toto.c",
-        "range": {
-            "columnBegin": 0,
-            "columnEnd": 0,
-            "lineBegin": 0,
-            "lineEnd": 0
+        "location": {
+            "filepath": "buck_project/toto.c",
+            "range": {
+                "columnBegin": 0,
+                "columnEnd": 0,
+                "lineBegin": 0,
+                "lineEnd": 0
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]

glean/glass/test/regression/tests/buck/resolveSymbol_target.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "buck_project/TEST_TARGETS",
-        "range": {
-            "columnBegin": 1,
-            "columnEnd": 2,
-            "lineBegin": 12,
-            "lineEnd": 18
+        "location": {
+            "filepath": "buck_project/TEST_TARGETS",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 12,
+                "lineEnd": 18
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]

glean/glass/test/regression/tests/cpp/resolveSymbol.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "test.cpp",
-        "range": {
-            "columnBegin": 1,
-            "columnEnd": 2,
-            "lineBegin": 59,
-            "lineEnd": 67
+        "location": {
+            "filepath": "test.cpp",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 59,
+                "lineEnd": 67
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]

glean/glass/test/regression/tests/cpp/resolveSymbolRange.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "test.cpp",
-        "range": {
-            "columnBegin": 1,
-            "columnEnd": 2,
-            "lineBegin": 59,
-            "lineEnd": 67
+        "location": {
+            "filepath": "test.cpp",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 59,
+                "lineEnd": 67
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]

glean/glass/test/regression/tests/cpp/resolveSymbolRange_Nothing.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "test.cpp",
-        "range": {
-            "columnBegin": 1,
-            "columnEnd": 2,
-            "lineBegin": 74,
-            "lineEnd": 78
+        "location": {
+            "filepath": "test.cpp",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 74,
+                "lineEnd": 78
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]

glean/glass/test/regression/tests/cpp/resolveSymbolRange_Nothing_ctor.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "test.cpp",
-        "range": {
-            "columnBegin": 3,
-            "columnEnd": 41,
-            "lineBegin": 77,
-            "lineEnd": 77
+        "location": {
+            "filepath": "test.cpp",
+            "range": {
+                "columnBegin": 3,
+                "columnEnd": 41,
+                "lineBegin": 77,
+                "lineEnd": 77
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]

glean/glass/test/regression/tests/cpp/resolveSymbolRange_Nothing_ctor_decl.out

@@ -1,13 +1,16 @@
 [
     "@generated",
     {
-        "filepath": "test.cpp",
-        "range": {
-            "columnBegin": 30,
-            "columnEnd": 37,
-            "lineBegin": 77,
-            "lineEnd": 77
+        "location": {
+            "filepath": "test.cpp",
+            "range": {
+                "columnBegin": 30,
+                "columnEnd": 37,
+                "lineBegin": 77,
+                "lineEnd": 77
+            },
+            "repository": "test"
         },
-        "repository": "test"
+        "revision": "testhash"
     }
 ]