Skip to content

Commit

Permalink
Provide a 'symbolLocation' thrift method
Browse files Browse the repository at this point in the history
Summary:
The most common reason people are using Glass in ad hoc ways is to find symbol locations (i.e. a mixture of search or our symbol id builders + describe().

describe() is still more than we need for this, and I keep forgetting we have 'resolveSymbolRange()'.

So make it clearer by calling it `symbolLocaton` and then I'll go through and fix a ton of the clients to use this much cheaper method. Then expose it to the main clients properly and refactor clients of describe()

Reviewed By: simonmar

Differential Revision: D59952037

fbshipit-source-id: ef7b4678995056a37d2bebda666418075c2d6381
  • Loading branch information
donsbot authored and facebook-github-bot committed Jul 20, 2024
1 parent 3b32a29 commit 3770aaa
Show file tree
Hide file tree
Showing 48 changed files with 76 additions and 76 deletions.
10 changes: 6 additions & 4 deletions glean/glass/Glean/Glass/Handler.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ module Glean.Glass.Handler
, findReferenceRanges

-- * working with symbol ids
, resolveSymbolRange
, symbolLocation
, describeSymbol

-- * searching by string
, searchSymbol

-- ** by relationship
, searchRelated

-- ** a large report of all information we know about a symbol
, searchRelatedNeighborhood

) where
Expand Down Expand Up @@ -214,13 +216,13 @@ findReferenceRanges [email protected]{..} sym opts@RequestOptions{..} =


-- | Resolve a symbol identifier to its range-based location in the latest db
resolveSymbolRange
symbolLocation
:: Glass.Env
-> SymbolId
-> RequestOptions
-> IO LocationRange
resolveSymbolRange env@Glass.Env{..} sym opts = do
withSymbol "resolveSymbolRange" env opts sym
symbolLocation env@Glass.Env{..} sym opts = do
withSymbol "symbolLocation" env opts sym
$ \gleanDBs _dbInfo (repo, lang, toks) ->
findSymbolLocationRange env GleanBackend{..} repo lang toks

Expand Down
4 changes: 1 addition & 3 deletions glean/glass/Glean/Glass/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,9 @@ glassHandler env0 cmd =
FindReferences r opts -> Handler.findReferences env r opts
FindReferenceRanges r opts -> Handler.findReferenceRanges env r opts

-- Resolving symbol information
ResolveSymbolRange r opts -> Handler.resolveSymbolRange env r opts

-- Symbol info
DescribeSymbol r opts -> Handler.describeSymbol env r opts
SymbolLocation r opts -> Handler.symbolLocation env r opts

-- Search for symbols
-- by string
Expand Down
4 changes: 2 additions & 2 deletions glean/glass/Glean/Glass/Tracing.hs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ glassTraceEvent (GlassTraceWithId tid (TraceCommand cmd)) = case cmd of
( "FindReferences", tid, json $ toEncoding r)
Glass.FindReferenceRanges r opts ->
("FindReferenceRanges", tid, json $ toEncoding r)
Glass.ResolveSymbolRange r opts ->
("ResolveSymbolRange", tid, json $ toEncoding r)
Glass.SymbolLocation r opts ->
("SymbolLocation", tid, json $ toEncoding r)
Glass.DescribeSymbol r opts ->
("DescribeSymbol", tid, json $ toEncoding r)
Glass.SearchSymbol r opts ->
Expand Down
8 changes: 4 additions & 4 deletions glean/glass/if/glass.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -751,14 +751,14 @@ service GlassService extends fb303.FacebookService {
2: RequestOptions options,
) throws (1: ServerException e, 2: GlassException g);

// Resolve a symbol id to its definition location range
LocationRange resolveSymbolRange(
// Return details about a symbol, such as its location or type signature
SymbolDescription describeSymbol(
1: SymbolId symbol,
2: RequestOptions options,
) throws (1: ServerException e, 2: GlassException g);

// Return details about a symbol, such as its location or type signature
SymbolDescription describeSymbol(
// Return just the symbol's location as efficiently as possible
LocationRange symbolLocation(
1: SymbolId symbol,
2: RequestOptions options,
) throws (1: ServerException e, 2: GlassException g);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ evalQuery glassEnv qFile Query{..} oFile = case action of
(Glass.findReferences glassEnv)
"findReferenceRanges" -> withSymbolId oFile args
(Glass.findReferenceRanges glassEnv)
"resolveSymbolRange" -> withSymbolId oFile args
(Glass.resolveSymbolRange glassEnv)
"symbolLocation" -> withSymbolId oFile args
(Glass.symbolLocation glassEnv)
"describeSymbol" -> withSymbolId oFile args
(Glass.describeSymbol glassEnv)
"searchSymbol" -> withObjectArgs qFile oFile args
Expand Down
16 changes: 8 additions & 8 deletions glean/glass/test/regression/lib/Glean/Glass/Regression/Tests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module Glean.Glass.Regression.Tests (
testDescribeSymbolHasAnnotations,
testDescribeSymbolHasVisibility,
testSearchRelated,
testResolveSymbol,
testSymbolLocation,
) where

import Data.Default
Expand Down Expand Up @@ -54,15 +54,15 @@ testDocumentSymbolListX path get =
(not (null (documentSymbolListXResult_references res)) &&
not (null (documentSymbolListXResult_definitions res)))

testResolveSymbol :: SymbolId -> Path -> Getter -> Test
testResolveSymbol sym@(SymbolId name) path get =
testSymbolLocation :: SymbolId -> Path -> Getter -> Test
testSymbolLocation sym@(SymbolId name) path get =
TestLabel (Text.unpack name) $ TestCase $ do
(backend, _repo) <- get
withTestEnv backend $ \env -> do
LocationRange{..} <- resolveSymbolRange env sym def
assertEqual "resolveSymbol Path matches" locationRange_filepath path
LocationRange{..} <- symbolLocation env sym def
assertEqual "symbolLocation Path matches" locationRange_filepath path

-- | Test that both describeSymbol and resolveSymbol for a SymbolId
-- | Test that both describeSymbol and symbolLocation for a SymbolId
-- find the symbol in a given Path.
testDescribeSymbolMatchesPath
:: SymbolId -> Path -> Getter -> Test
Expand All @@ -74,8 +74,8 @@ testDescribeSymbolMatchesPath sym@(SymbolId name) path get =
assertEqual "describeSymbol Path matches"
(symbolPath_filepath symbolDescription_location)
path
LocationRange{..} <- resolveSymbolRange env sym def
assertEqual "resolveSymbol Path matches" locationRange_filepath path
LocationRange{..} <- symbolLocation env sym def
assertEqual "symbolLocation Path matches" locationRange_filepath path

-- | Test that both describeSymbol has expected comment
testDescribeSymbolComments
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/buck/t/buck_project/PATH/./foo"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/buck/d/buck_project/my_defs.bzl/some_library"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/buck/f/buck_project/toto.c"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/buck/t/buck_project/PATH/./foo"
2 changes: 1 addition & 1 deletion glean/glass/test/regression/tests/cpp/resolveSymbol.query
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//h"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//h"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//maybe/Nothing"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//maybe/Nothing/.c/maybe::Nothing::_secret"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//maybe/Nothing/.ctor//.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//maybe/nothing/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//maybe/Nothing/_secret"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//maybe/Nothing/_secret/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/S/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/bar/T/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/bar/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/f/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/foo"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//h/i/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/fooFn/fooI/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/S/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/bar/T/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/bar/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//GlobalClass"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//Global"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/f/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/.decl"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//foo/foo"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//baz"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//baz/U"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//baz/U/InU"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//baz/U/InU/UA"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/cpp//baz/U/InUClass/UCA"
2 changes: 1 addition & 1 deletion glean/glass/test/regression/tests/flow/resolveSymbol.query
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/js/imports/C"


Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/js/imports/C"


2 changes: 1 addition & 1 deletion glean/glass/test/regression/tests/hack/resolveSymbol.query
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/php/Position"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/php/Position"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/java/lsif/com/glean/app/App%23getMessage%28%29"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/py/all.py/all"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/py/all.py/all"
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
action: resolveSymbolRange
action: symbolLocation
args: "test/thrift/lib.thrift/Attribute/aBool"
14 changes: 7 additions & 7 deletions glean/glass/tools/Glean/Glass/Test/DemoClient.hs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ data Command
= List RepoName Path
| Describe SymbolId
| FindRefs SymbolId
| Resolve SymbolId
| FindLocation SymbolId
| Search RepoName Text

options :: ParserInfo Options
Expand Down Expand Up @@ -79,7 +79,7 @@ options = info (helper <*> parser) (fullDesc <>
]
)
) <>
command "resolve" (info resolveCommand
command "location" (info locationCommand
(progDesc $ unlines
["Find the definition location of a symbol"
]
Expand All @@ -101,7 +101,7 @@ options = info (helper <*> parser) (fullDesc <>
listCommand = cmd readListRepoPath (metavar "REPO/PATH")
describeCommand = cmd (Describe <$> readSymbol) symbolHelp
findRefsCommand = cmd (FindRefs <$> readSymbol) symbolHelp
resolveCommand = cmd (Resolve <$> readSymbol) symbolHelp
locationCommand = cmd (FindLocation <$> readSymbol) symbolHelp
searchCommand = cmd readSearch searchHelp

readService :: ReadM Service
Expand Down Expand Up @@ -163,7 +163,7 @@ main = Glean.withOptions options $ \Options{..} ->
List repo path -> runListSymbols repo path
Describe sym -> runDescribe sym
FindRefs sym -> runFindRefs sym
Resolve sym -> runResolve sym
FindLocation sym -> runLocation sym
Search repo str -> runSearch repo str
mapM_ Text.putStrLn res

Expand Down Expand Up @@ -198,9 +198,9 @@ runSearch repoName strName = do
def -- kinds
def -- default search options

runResolve :: Protocol p => SymbolId -> GlassM p [Text]
runResolve sym = do
range <- resolveSymbolRange sym def
runLocation :: Protocol p => SymbolId -> GlassM p [Text]
runLocation sym = do
range <- symbolLocation sym def
return [pprLocationRange range]

runFindRefs :: Protocol p => SymbolId -> GlassM p [Text]
Expand Down
12 changes: 6 additions & 6 deletions glean/glass/tools/Glean/Glass/Test/Symbol.hs
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ main =

case cfgCmd of
FindLocation -> do
let resolveLocation :: SymbolId -> IO LocationRange
resolveLocation r =
Handle.resolveSymbolRange env r (def :: RequestOptions)
forM_ syms $ Text.putStrLn <=< testResolveSymbol resolveLocation
let symbolLocation :: SymbolId -> IO LocationRange
symbolLocation r =
Handle.symbolLocation env r (def :: RequestOptions)
forM_ syms $ Text.putStrLn <=< testSymbolLocation symbolLocation

FindReferences -> do
let findReferences :: SymbolId -> IO [LocationRange]
Expand Down Expand Up @@ -121,11 +121,11 @@ testFindReferences handler symbol@(SymbolId name) = do

return $ title : body

testResolveSymbol
testSymbolLocation
:: (SymbolId -> IO LocationRange)
-> SymbolId
-> IO Text
testResolveSymbol handler symbol@(SymbolId name) = do
testSymbolLocation handler symbol@(SymbolId name) = do
handle (\(ServerException e) -> return ("FAIL " <> e)) $ do
res <- runTest handler symbol
return $ Text.concat
Expand Down

0 comments on commit 3770aaa

Please sign in to comment.