simplify symbol id parsing

Summary:
This completes and simplifies the decoding logic, by using derived predicate introduced in D59333044 to decode all symbol ids with three components (file, typename, field).

The derived predicate DeclarationMember replaces the fieldDecl and functionName queries and also contains the enum value case which was missing.

Next step is to disambiguate the symbol id (see comment in Fbthrift.hs)

Reviewed By: simonmar

Differential Revision: D59331450

fbshipit-source-id: ac4364290fe15993ca3651f2ace9ece95db6cb94

glean/glass/Glean/Glass/Search/Thrift.hs

@@ -59,27 +59,22 @@ instance Search Thrift.Entity where
   symbolSearch toks = fmap Thrift.Entity_decl <$> symbolSearch toks
 
 instance Search (ResultLocation Thrift.Declaration) where
-  symbolSearch = symbolSearchGen searchQNameWithLoc searchFunctionNameWithLoc
-    searchThriftFileWithLoc searchFieldDeclWithLoc
+  symbolSearch = symbolSearchGen searchQNameWithLoc searchThriftFileWithLoc
+    searchMemberDeclWithLoc
 
 instance Search Thrift.Declaration where
   symbolSearch =
-    symbolSearchGen searchQName searchFunctionName searchThriftFile
-      searchFieldDecl
+    symbolSearchGen searchQName searchThriftFile searchMemberDecl
 
 -- Resolve symbol id tokens into an entity, with or without location.
--- Thrift symbol ids are ambiguous and it may take more than one query
--- to resolve them.
 symbolSearchGen ::
   (Typeable t, Show t, Glean.Typed.Binary.Type t, QueryRepos u)
   => (Text -> Text -> Angle t)
-  -> (Text -> Text -> Text -> Angle t)
   -> (Text -> Angle t)
   -> (Text -> Text -> Text -> Angle t)
   -> [Text]
   -> ReposHaxl u v (SearchResult t)
-symbolSearchGen
-  searchQName searchFunctionName searchThriftFile searchFieldDecl toks =
+symbolSearchGen searchQName searchThriftFile searchMemberDecl toks =
   case toks of
   [] -> return $ None "Thrift.symbolSearch: empty query"
   _ -> case (init toks, last toks) of
@@ -90,61 +85,14 @@ symbolSearchGen
         None{} -> case (init pieces, last pieces) of
           (morePieces, serviceName) -> do
             let path = joinFragments morePieces
-            moreResult <- searchSymbolId toks $ searchFunctionName
-                path serviceName name
+            moreResult <-
+              searchSymbolId toks $ searchMemberDecl path serviceName name
             case moreResult of
-              None{} -> do
-                moreResult <- searchSymbolId toks $ searchFieldDecl
-                  path serviceName name
-                case moreResult of
-                  None{} ->
-                    searchSymbolId toks $ searchThriftFile (joinFragments toks)
-                  r -> return r
+              None{} ->
+                searchSymbolId toks $ searchThriftFile (joinFragments toks)
               r -> return r
         r -> return r
 
-functionNameDecl
-  :: Text -> Text -> Text -> Angle Src.File -> Angle Thrift.File
-  -> Angle Thrift.QualName -> Angle Thrift.XRefTarget -> [AngleStatement]
-functionNameDecl path servicename name file thriftFile qname decl =
-    [ file .= predicate @Src.File (string path),
-    thriftFile .= predicate @Thrift.File file,
-    qname .= predicate @Thrift.QualName (
-      rec $
-        field @"file" (asPredicate thriftFile) $
-        field @"name" (string servicename)
-      end
-    ),
-    wild .= predicate @Thrift.FunctionDeclarationName (
-      rec $
-        field @"qname" (asPredicate qname) $
-        field @"name" (string name) $
-        field @"decl" decl
-      end)
-  ]
-
-searchFunctionName
-  :: Text -> Text -> Text -> Angle Thrift.Declaration
-searchFunctionName path servicename name =
-  vars $ \(file :: Angle Src.File) (qname :: Angle Thrift.QualName)
-     (thriftFile :: Angle Thrift.File) (decl :: Angle Thrift.Declaration) ->
-  decl `where_`
-    functionNameDecl path servicename name file thriftFile qname decl
-
-searchFunctionNameWithLoc
-  :: Text -> Text -> Text -> Angle (ResultLocation Thrift.Declaration)
-searchFunctionNameWithLoc path servicename name =
-  vars $ \(file :: Angle Src.File)
-     (rangespan :: Angle Code.RangeSpan) (lname :: Angle Text)
-     (qname :: Angle Thrift.QualName)
-     (thriftFile :: Angle Thrift.File) (decl :: Angle Thrift.Declaration) ->
-
-  tuple (decl, file, rangespan, lname) `where_`
-  (functionNameDecl path servicename name file thriftFile qname decl
-  ++ [
-    entityLocation (alt @"fbthrift" (alt @"decl" decl)) file rangespan lname
-  ])
-
 thriftFileDecl
   :: Text -> Angle Src.File -> Angle Thrift.File -> Angle Thrift.Declaration
   -> [AngleStatement]
@@ -170,53 +118,58 @@ searchThriftFile path = vars $ \(file :: Angle Src.File)
   (thriftFile :: Angle Thrift.File) (decl :: Angle Thrift.Declaration) ->
     decl `where_` thriftFileDecl path file thriftFile decl
 
-thriftFieldDecl
+thriftMemberDecl
   :: Text -> Text -> Text -> Angle Src.File -> Angle Thrift.File
-  -> Angle Thrift.QualName -> Angle Thrift.FieldDecl -> Angle Thrift.XRefTarget
+  -> Angle Thrift.QualName -> Angle Thrift.Identifier -> Angle Thrift.XRefTarget
   -> [AngleStatement]
-thriftFieldDecl path typename name file thriftFile qname fieldDecl decl =
-    [ file .= predicate @Src.File (string path),
+thriftMemberDecl
+  path typename name file thriftFile qname thriftIdentifier decl =
+ [
+    file .= predicate @Src.File (string path),
     thriftFile .= predicate @Thrift.File file,
+    thriftIdentifier .= predicate @Thrift.Identifier (string name),
     qname .= predicate @Thrift.QualName (
       rec $
         field @"file" (asPredicate thriftFile) $
         field @"name" (string typename)
       end
     ),
-    fieldDecl .= predicate @Thrift.FieldDecl (
+    wild .= predicate @Thrift.DeclarationMember (
       rec $
         field @"qname" (asPredicate qname) $
-        field @"name" (string name)
-      end),
-    decl .= sig (alt @"field"
-            (asPredicate fieldDecl) :: Angle Thrift.Declaration)
+        field @"member" (asPredicate thriftIdentifier) $
+        field @"decl" decl
+      end)
   ]
 
-searchFieldDecl
+-- Member lookup. This will find fields (struct, exception, union), enum values,
+-- and functions. They are uniquely indentified by a triple
+-- (path, typename, name)
+searchMemberDecl
   :: Text -> Text -> Text -> Angle Thrift.Declaration
-searchFieldDecl path typename name =
+searchMemberDecl path typename name =
   vars $ \(file :: Angle Src.File) (qname :: Angle Thrift.QualName)
-     (thriftFile :: Angle Thrift.File) (fieldDecl' :: Angle Thrift.FieldDecl)
+     (thriftFile :: Angle Thrift.File) (identifier :: Angle Thrift.Identifier)
      (decl :: Angle Thrift.Declaration) ->
   decl `where_`
-    thriftFieldDecl path typename name file thriftFile qname fieldDecl' decl
+    thriftMemberDecl path typename name file thriftFile qname identifier decl
 
-searchFieldDeclWithLoc
+-- Same as searchMemberDecl but with location
+searchMemberDeclWithLoc
   :: Text -> Text -> Text -> Angle (ResultLocation Thrift.Declaration)
-searchFieldDeclWithLoc path typename name =
+searchMemberDeclWithLoc path typename name =
   vars $ \(file :: Angle Src.File)
      (rangespan :: Angle Code.RangeSpan) (lname :: Angle Text)
      (qname :: Angle Thrift.QualName)
-     (thriftFile :: Angle Thrift.File) (fieldDecl' :: Angle Thrift.FieldDecl)
+     (thriftFile :: Angle Thrift.File) (identifier :: Angle Thrift.Identifier)
      (decl :: Angle Thrift.Declaration) ->
 
   tuple (decl, file, rangespan, lname) `where_`
-  (thriftFieldDecl path typename name file thriftFile qname fieldDecl' decl
+  (thriftMemberDecl path typename name file thriftFile qname identifier decl
   ++ [
     entityLocation (alt @"fbthrift" (alt @"decl" decl)) file rangespan lname
   ])
 
-
 qNameDecl
   :: Text -> Text -> Angle Thrift.File -> Angle Thrift.QualName
   -> Angle Src.File -> Angle Thrift.XRefTarget -> [AngleStatement]
@@ -235,9 +188,16 @@ qNameDecl path name thriftFile qname file decl =
       field @"decl" decl
     end)]
 
--- A basic entity lookup:  this will find named decls, exceptions, constants and
--- service names, which are indexd by QualName
--- The separate thrift.File fact is slightly annoying.
+-- Entity lookup: this will find type declarations (union, struct,
+-- typedef), exceptions, constants and service names, which are uniquely
+-- identified by a qualified name (path, name)
+searchQName :: Text -> Text -> Angle Thrift.Declaration
+searchQName path name =
+  vars $ \(file :: Angle Src.File) (qname :: Angle Thrift.QualName)
+     (thriftFile :: Angle Thrift.File) (decl :: Angle Thrift.Declaration) ->
+  decl `where_` qNameDecl path name thriftFile qname file decl
+
+-- Same as searchQName but also get location
 searchQNameWithLoc :: Text -> Text -> Angle (ResultLocation Thrift.Declaration)
 searchQNameWithLoc path name =
   vars $ \(file :: Angle Src.File)
@@ -249,9 +209,3 @@ searchQNameWithLoc path name =
     (qNameDecl path name thriftFile qname file decl ++ [
     entityLocation (alt @"fbthrift" (alt @"decl" decl)) file rangespan lname
   ])
-
-searchQName :: Text -> Text -> Angle Thrift.Declaration
-searchQName path name =
-  vars $ \(file :: Angle Src.File) (qname :: Angle Thrift.QualName)
-     (thriftFile :: Angle Thrift.File) (decl :: Angle Thrift.Declaration) ->
-  decl `where_` qNameDecl path name thriftFile qname file decl

glean/glass/Glean/Glass/SymbolId/Fbthrift.hs

@@ -8,6 +8,20 @@
 
 {-# OPTIONS_GHC -Wno-orphans #-}
 
+-- Thrift symbol ids can be of three forms
+-- repo/thrift/FILE
+-- repo/thrift/FILE/NAME
+--    NAME can refer to a union, struct, exception, service, constant, typedef
+--    there's no possible ambiguity as these names are guaranteed to be unique
+--    in a file
+-- repo/thrift/FILE/NAME/MEMBER
+--    MEMBER can be a struct/union/exception field, an enum value,
+--    or a service function
+--
+-- The symbol id is ambiguous, repo/thrift/a.thrift/b.thrift/c can refer to
+-- field c in struct b.thrift, or struct c in file b.thrift.
+-- TODO add more structure to disambiguate
+--
 module Glean.Glass.SymbolId.Fbthrift
   ({- instances -})
   where

glean/glass/test/regression/tests/thrift/describe_enum.out

@@ -0,0 +1,51 @@
+[
+    "@generated",
+    {
+        "comments": [],
+        "contains_relation": {
+            "firstChild": "nondeterministic",
+            "firstParent": "nondeterministic",
+            "hasMoreChildren": false,
+            "hasMoreParents": false
+        },
+        "extends_relation": {
+            "firstChild": "nondeterministic",
+            "firstParent": "nondeterministic",
+            "hasMoreChildren": false,
+            "hasMoreParents": false
+        },
+        "kind": 12,
+        "language": 9,
+        "location": {
+            "filepath": "lib.thrift",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 163,
+                "lineEnd": 194
+            },
+            "repository": "test"
+        },
+        "modifiers": [],
+        "name": {
+            "container": "lib",
+            "localName": "SymbolKind"
+        },
+        "pretty_comments": [],
+        "repo_hash": "testhash",
+        "signature": "enum lib::SymbolKind",
+        "sym": "test/thrift/lib.thrift/SymbolKind",
+        "sym_location": {
+            "filepath": "lib.thrift",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 163,
+                "lineEnd": 194
+            },
+            "repository": "test"
+        },
+        "sym_other_locations": [],
+        "type_xrefs": []
+    }
+]

glean/glass/test/regression/tests/thrift/describe_enum.query

@@ -0,0 +1,2 @@
+action: describeSymbol
+args: "test/thrift/lib.thrift/SymbolKind"

glean/glass/test/regression/tests/thrift/describe_enum_value.out

@@ -0,0 +1,51 @@
+[
+    "@generated",
+    {
+        "comments": [],
+        "contains_relation": {
+            "firstChild": "nondeterministic",
+            "firstParent": "nondeterministic",
+            "hasMoreChildren": false,
+            "hasMoreParents": false
+        },
+        "extends_relation": {
+            "firstChild": "nondeterministic",
+            "firstParent": "nondeterministic",
+            "hasMoreChildren": false,
+            "hasMoreParents": false
+        },
+        "kind": 24,
+        "language": 9,
+        "location": {
+            "filepath": "lib.thrift",
+            "range": {
+                "columnBegin": 3,
+                "columnEnd": 12,
+                "lineBegin": 165,
+                "lineEnd": 165
+            },
+            "repository": "test"
+        },
+        "modifiers": [],
+        "name": {
+            "container": "SymbolKind",
+            "localName": "Type"
+        },
+        "pretty_comments": [],
+        "repo_hash": "testhash",
+        "signature": "enum value lib::SymbolKind::Type",
+        "sym": "test/thrift/lib.thrift/SymbolKind/Type",
+        "sym_location": {
+            "filepath": "lib.thrift",
+            "range": {
+                "columnBegin": 3,
+                "columnEnd": 12,
+                "lineBegin": 165,
+                "lineEnd": 165
+            },
+            "repository": "test"
+        },
+        "sym_other_locations": [],
+        "type_xrefs": []
+    }
+]

glean/glass/test/regression/tests/thrift/describe_enum_value.query

@@ -0,0 +1,2 @@
+action: describeSymbol
+args: "test/thrift/lib.thrift/SymbolKind/Type"

glean/glass/test/regression/tests/thrift/describe_exception.out

@@ -0,0 +1,51 @@
+[
+    "@generated",
+    {
+        "comments": [],
+        "contains_relation": {
+            "firstChild": "nondeterministic",
+            "firstParent": "nondeterministic",
+            "hasMoreChildren": false,
+            "hasMoreParents": false
+        },
+        "extends_relation": {
+            "firstChild": "nondeterministic",
+            "firstParent": "nondeterministic",
+            "hasMoreChildren": false,
+            "hasMoreParents": false
+        },
+        "kind": 25,
+        "language": 9,
+        "location": {
+            "filepath": "lib.thrift",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 139,
+                "lineEnd": 141
+            },
+            "repository": "test"
+        },
+        "modifiers": [],
+        "name": {
+            "container": "lib",
+            "localName": "ServerException"
+        },
+        "pretty_comments": [],
+        "repo_hash": "testhash",
+        "signature": "struct lib::ServerException",
+        "sym": "test/thrift/lib.thrift/ServerException",
+        "sym_location": {
+            "filepath": "lib.thrift",
+            "range": {
+                "columnBegin": 1,
+                "columnEnd": 2,
+                "lineBegin": 139,
+                "lineEnd": 141
+            },
+            "repository": "test"
+        },
+        "sym_other_locations": [],
+        "type_xrefs": []
+    }
+]

glean/glass/test/regression/tests/thrift/describe_exception.query

@@ -0,0 +1,2 @@
+action: describeSymbol
+args: "test/thrift/lib.thrift/ServerException"