Skip to content

Commit 10fccda

Browse files
aahanaggarwalfacebook-github-bot
authored andcommitted
Update symbol Id parsing to allow filters and use that to search for references
Summary: # Doc https://docs.google.com/document/d/1zlJlGSmDhQxuJ87K1bH59wPJk2IyY5WMQFTEgKA4Ak0/edit?tab=t.0#heading=h.qn82j2uc6gbw # Stack - Currently for directory branches like xlformers-branches when finding references, the references for all branches are returned. This is because the **symbol id is the same across branches**. - In order to fix this in a more general way (that can apply to other directory based branches) we are implementing nhawkes design of adding filters to symbol ID's which can then be used to decide which references to return. - With this set, glass will now only issue a new query, where the go-to-definition for the symbol's match. This means that the go-to-defintion for the symbol you searched for must point to the same file to that the references searched for point to. - When codehub issues a request, we append this filter since the go-to-defintion information is available at load time. # This diff - Add a thrift `union` type which describes the filters we will allow on SymbolId. - Update parsing logic for symbolId to return a list of SymbolFilters. - Update `findReferenceRanges` in glass to extract the definition file from the symbol filters if it exists. If this exists issue a new query. - Add a new query which adds constraints onto the EntityReferences predicate such that the filename of the reference's xref must match the filename of the original entities xref. (Both the go-to-definition's must match). Reviewed By: pepeiborra Differential Revision: D78165656 fbshipit-source-id: ea1a3697b4e887dc060211224ac58f4fe4eed8d9
1 parent 0ac5dcd commit 10fccda

6 files changed

Lines changed: 111 additions & 20 deletions

File tree

glean/glass/Glean/Glass/Handler/Symbols.hs

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,22 @@ import qualified Glean.Glass.Handler.Utils as Utils
109109
import Glean.Glass.SymbolKind (findSymbolKind)
110110
import Glean.Glass.Env (Env' (tracer, sourceControl, useSnapshotsForSymbolsList))
111111

112+
extractDefFileFilter :: [SymbolFilter] -> Maybe Text
113+
extractDefFileFilter [] = Nothing
114+
extractDefFileFilter (SymbolFilter_definition_file filePath : _) = Just filePath
115+
extractDefFileFilter (_ : rest) = extractDefFileFilter rest
116+
117+
112118
-- | Symbol-based find-references.
113119
findReferenceRanges
114120
:: Glass.Env -> SymbolId -> RequestOptions -> IO [LocationRange]
115121
findReferenceRanges env@Glass.Env{..} sym opts@RequestOptions{..} =
116122
withSymbol "findReferenceRanges" env opts sym
117-
$ \gleanDBs _dbInfo (repo, lang, toks) ->
118-
fetchSymbolReferenceRanges env repo lang toks limit
119-
GleanBackend{..}
123+
$ \gleanDBs _dbInfo (repo, lang, toks, symbolFilters) -> do
124+
let filename = extractDefFileFilter symbolFilters
125+
(ranges, maybeError) <-
126+
fetchSymbolReferenceRanges env repo lang toks limit filename GleanBackend{..}
127+
return (ranges, maybeError)
120128
where
121129
limit = fmap fromIntegral requestOptions_limit
122130

@@ -125,8 +133,11 @@ findReferenceRanges env@Glass.Env{..} sym opts@RequestOptions{..} =
125133
symbolLocation :: Glass.Env -> SymbolId -> RequestOptions -> IO SymbolLocation
126134
symbolLocation env@Glass.Env{..} sym opts =
127135
withSymbol "symbolLocation" env opts sym $
128-
\gleanDBs dbInfo (scmRepo, lang, toks) ->
136+
\gleanDBs dbInfo (scmRepo, lang, toks, filters) ->
129137
backendRunHaxl GleanBackend{..} env $ do
138+
case filters of
139+
[] -> return ()
140+
_ -> throwM (ServerException "symbolLocation: filters not supported")
130141
r <- Search.searchEntityLocation lang toks
131142
(entity, err) <- case r of
132143
None t -> throwM (ServerException t)
@@ -150,8 +161,11 @@ describeSymbol
150161
-> IO SymbolDescription
151162
describeSymbol env@Glass.Env{..} symId opts =
152163
withSymbol "describeSymbol" env opts symId $
153-
\gleanDBs dbInfo (scmRepo, lang, toks) ->
164+
\gleanDBs dbInfo (scmRepo, lang, toks, filters) ->
154165
backendRunHaxl GleanBackend{..} env $ do
166+
case filters of
167+
[] -> return ()
168+
_ -> throwM (ServerException "describeSymbol: filters not supported")
155169
r <- Search.searchEntityLocation lang toks
156170
(first :| rest, err) <- case r of
157171
None t -> throwM (ServerException t)
@@ -178,8 +192,11 @@ resolveSymbols env@Glass.Env{..} (ResolveSymbolsRequest symIds) opts = do
178192
results <- forM symIds $ \symId -> do
179193
-- Try to resolve the symbol, returning any error encountered
180194
(resolutions, err) <- withSymbol "resolveSymbols" env opts symId $
181-
\gleanDBs dbInfo (scmRepo, lang, toks) ->
195+
\gleanDBs dbInfo (scmRepo, lang, toks, filters) ->
182196
backendRunHaxl GleanBackend{..} env $ do
197+
case filters of
198+
[] -> return ()
199+
_ -> throwM (ServerException "resolveSymbols: filters not supported")
183200
r <- Search.searchEntityLocation lang toks
184201
(entities, err) <- case r of
185202
None t ->
@@ -608,9 +625,10 @@ fetchSymbolReferenceRanges
608625
-> Language
609626
-> [Text]
610627
-> Maybe Int
628+
-> Maybe Text
611629
-> GleanBackend b
612630
-> IO ([LocationRange], Maybe ErrorLogger)
613-
fetchSymbolReferenceRanges env scsrepo lang toks limit b =
631+
fetchSymbolReferenceRanges env scsrepo lang toks limit filename b =
614632
backendRunHaxl b env $ do
615633
er <- symbolToAngleEntities lang toks
616634
let logDBs x = logError (gleanDBs b) <> x
@@ -621,7 +639,11 @@ fetchSymbolReferenceRanges env scsrepo lang toks limit b =
621639
withRepo entityRepo $ do
622640
let convert (targetFile, rspan) =
623641
rangeSpanToLocationRange scsrepo targetFile rspan
624-
uses <- searchWithLimit limit $ Query.findReferenceRangeSpan query
642+
uses <- searchWithLimit limit $ case (filename, lang) of
643+
-- Definition file filter only supported for Python
644+
(Just filename, Language_Python) ->
645+
Query.findReferenceRangeSpanByDef query filename
646+
_ -> Query.findReferenceRangeSpan query
625647
mapM convert uses
626648
return (nubOrd $ concat ranges, fmap (logDBs . logError) searchErr)
627649

@@ -687,9 +709,12 @@ searchRelated
687709
searchRelated env@Glass.Env{..} sym opts@RequestOptions{..}
688710
SearchRelatedRequest{..} =
689711
withSymbol "searchRelated" env opts sym $
690-
\gleanDBs_ dbInfo (repo, lang, toks) ->
712+
\gleanDBs_ dbInfo (repo, lang, toks, filters) ->
691713
let gleanDBs = filterDBs lang gleanDBs_ in
692714
backendRunHaxl GleanBackend{..} env $ do
715+
case filters of
716+
[] -> return ()
717+
_ -> throwM (ServerException "searchRelated: filters not supported")
693718
entity <- searchFirstEntity lang toks
694719
withRepo (entityRepo entity) $ do
695720
(symPairs, desc, merr) <- case searchRelatedRequest_relatedBy of
@@ -922,8 +947,11 @@ searchRelatedNeighborhood
922947
-> IO RelatedNeighborhoodResult
923948
searchRelatedNeighborhood env@Glass.Env{..} sym opts@RequestOptions{..} req =
924949
withSymbol "searchRelatedNeighborhood" env opts sym $
925-
\gleanDBs dbInfo (repo, lang, toks) ->
950+
\gleanDBs dbInfo (repo, lang, toks, filters) ->
926951
backendRunHaxl GleanBackend{..} env $ do
952+
case filters of
953+
[] -> return ()
954+
_ -> throwM (ServerException "searchRelatedNeighborhood: filters not supported")
927955
baseEntity <- searchFirstEntity lang toks
928956
let lang = entityLanguage (fst4 (decl baseEntity))
929957
(,Nothing) <$> searchNeighborhood limit req sym repo

glean/glass/Glean/Glass/Handler/Utils.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,14 +206,14 @@ withSymbol
206206
-> SymbolId
207207
-> ( NonEmpty (GleanDBName, Glean.Repo)
208208
-> GleanDBInfo
209-
-> (RepoName, Language, [Text])
209+
-> (RepoName, Language, [Text], [SymbolFilter])
210210
-> IO (res, Maybe ErrorLogger))
211211
-> IO res
212212
withSymbol method env@Glass.Env{..} opts sym fn =
213213
fmap fst $ withRequest method env sym opts $ \dbInfo ->
214214
case symbolTokens sym of
215215
Left err -> throwM $ ServerException err
216-
Right req@(repo, lang, _toks) -> do
216+
Right req@(repo, lang, _toks, _filters) -> do
217217
dbs <- getGleanRepos tracer sourceControl repoMapping dbInfo repo
218218
(Just lang) (requestOptions_revision opts)
219219
(dbChooser repo opts) gleanDB

glean/glass/Glean/Glass/Query.hs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ module Glean.Glass.Query
2121

2222
-- * Finding references to declarations
2323
, findReferenceRangeSpan
24+
, findReferenceRangeSpanByDef
2425

2526
-- * Finding references for call hierarchy
2627
, findReferenceEntities
@@ -151,6 +152,43 @@ findReferenceRangeSpan ent =
151152
)
152153
]
153154

155+
-- | Entity-based find-references returning native range or bytespan filtered by
156+
-- the referenced symbols being defined in the given file.
157+
findReferenceRangeSpanByDef
158+
:: Angle Code.Entity
159+
-> Text
160+
-> Angle (Src.File, Code.RangeSpan)
161+
findReferenceRangeSpanByDef ent filename =
162+
vars $ \(reffile :: Angle Src.File)
163+
(rangespan :: Angle Code.RangeSpan)
164+
(deffile :: Angle Src.File)
165+
(xref :: Angle Code.XRefLocation)
166+
(targetLoc :: Angle Code.Location) ->
167+
Angle.tuple (reffile, rangespan) `where_` [
168+
wild .= predicate @Code.EntityReferences (
169+
rec $
170+
field @"target" ent $
171+
field @"file" (asPredicate reffile) $
172+
field @"range" rangespan
173+
end
174+
)
175+
, deffile .= predicate @Src.File (string filename)
176+
, wild .= predicate @Code.FileEntityXRefLocations (
177+
rec $
178+
field @"file" (asPredicate reffile) $
179+
field @"xref" xref $
180+
field @"entity" ent
181+
end
182+
)
183+
, xref .= rec (
184+
field @"target" targetLoc $
185+
field @"source" rangespan
186+
end)
187+
, targetLoc .= rec (
188+
field @"file" (asPredicate deffile)
189+
end)
190+
]
191+
154192
generatedEntityToIdlEntity
155193
:: Angle Code.Entity
156194
-> Angle (Code.Entity, Src.File)

glean/glass/Glean/Glass/SymbolId.hs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ module Glean.Glass.SymbolId
4343
,nativeSymbol) where
4444

4545
import Control.Monad.Catch ( throwM, try )
46-
import Data.Maybe ( fromMaybe )
46+
import Data.Maybe ( fromMaybe, mapMaybe )
4747
import Data.Tuple ( swap )
4848
import Util.Text ( textShow )
4949
import Data.Text ( Text )
@@ -59,7 +59,8 @@ import Glean.Glass.Types as Glass
5959
Language(..),
6060
SymbolId(SymbolId),
6161
RepoName(..),
62-
Name, )
62+
Name,
63+
SymbolFilter(..) )
6364

6465
import Glean.Angle ( alt, Angle )
6566
import qualified Glean.Haxl.Repos as Glean
@@ -109,11 +110,12 @@ import Glean.Schema.CodeCsharp.Types as CSharp ( Entity(Entity_decl) )
109110
-- If we can't encode the entity, it is still useful to return the path and
110111
-- file, as this is enough to navigate with.
111112
--
112-
-- Symbol IDs have 3 parts:
113+
-- Symbol IDs have 3 fixed parts and 1 optional part:
113114
--
114115
-- - scm repo (corpus in biggrep term), e.g. "fbsource"
115116
-- - language short code (e.g py or php or cpp)
116117
-- - entity encoding
118+
-- - [optional] filters as defined in the thrift schema
117119
--
118120
-- We uri encode the pieces and separate with / so they look like nice urls
119121
--
@@ -165,16 +167,32 @@ toSymbolQualifiedContainer entity = do
165167
Right (_, qualifiedName_container) -> Just qualifiedName_container
166168
Left _ -> Nothing
167169

170+
parseFilter :: Text -> Maybe SymbolFilter
171+
parseFilter f = case Text.splitOn "=" f of
172+
[k, v] -> case k of
173+
"definition_file" -> Just (SymbolFilter_definition_file v)
174+
_ -> Nothing
175+
_ -> Nothing
176+
177+
parseFilters :: Text -> [SymbolFilter]
178+
parseFilters = mapMaybe parseFilter . Text.split (=='&')
179+
168180
-- | Tokenize a symbol (inverse of the intercalate "/")
169181
-- Leaves the path/qname/syms for further search.
170-
symbolTokens :: SymbolId -> Either Text (RepoName, Language, [Text])
182+
symbolTokens
183+
:: SymbolId
184+
-> Either Text (RepoName, Language, [Text], [SymbolFilter])
171185
symbolTokens (SymbolId symid)
172186
| (repo: code: pieces) <- tokens
173187
, Just lang <- fromShortCode code
174-
= Right (RepoName repo, lang, map URI.decodeText pieces)
188+
= Right (RepoName repo, lang, map URI.decodeText pieces, filters)
175189
| otherwise = Left $ "Invalid symbol: " <> symid
176190
where
177-
tokens = Text.split (=='/') symid
191+
(ident, filters) = case Text.split (=='?') symid of
192+
[identOnly] -> (identOnly, [])
193+
[identPart, filtersPart] -> (identPart, parseFilters filtersPart)
194+
_ -> (symid, [])
195+
tokens = Text.split (=='/') ident
178196

179197
-- | SymbolID-encoded language, used for db name lookups
180198
shortCodeTable :: [(Language,Text)]

glean/glass/if/glass.thrift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,12 @@ struct DocumentSymbolsRequest {
187187

188188
// response types
189189

190+
// Filter for symbol based search results
191+
union SymbolFilter {
192+
// Filters the symbol such that it is defined in the given file
193+
1: string definition_file;
194+
}
195+
190196
// Human-readable opaque, stable, globally unique symbol identifier
191197
typedef string SymbolId (hs.newtype)
192198

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ import qualified Data.ByteString.Lazy as B
4040
import qualified Data.Map.Strict as Map
4141
import qualified Data.Yaml as Yaml
4242
import Util.List ( uniq )
43-
import Data.Tuple.Extra
4443
import Data.Either.Extra
4544

4645
import qualified Thrift.Protocol
@@ -226,8 +225,10 @@ validateCxxSymbols glassEnv req def = do
226225
refs = map Glass.referenceRangeSymbolX_sym
227226
(Glass.documentSymbolListXResult_references res)
228227
syms = uniq (defns ++ refs)
229-
toks = map thd3 (filter ((== Language_Cpp) . snd3) -- only cxx ids thx
228+
toks = map getTokens (filter ((== Language_Cpp) . getLanguage) -- only cxx ids thx
230229
(rights (map Glass.symbolTokens syms)))
230+
getLanguage (_, lang, _, _) = lang
231+
getTokens (_, _, tokens, _) = tokens
231232
return (map Cxx.validateSymbolId toks)
232233

233234
decodeObjectAsThriftJson

0 commit comments

Comments
 (0)