Introduce the beginning of the scope graph API #497
Conversation
src/Semantic/Api/ScopeGraph.hs
Outdated
| import Serializing.Format (Format) | ||
| import Source.Loc as Loc | ||
|
|
||
| parseScopeGraph :: ( Has Distribute sig m |
There was a problem hiding this comment.
FYI, Distribute is going away soon, as it doesn’t do what it says on the tin (#40), so I wouldn’t introduce it here. If you need to use mapConcurrently, do it in IO if possible.
There was a problem hiding this comment.
Left this alone for now as I found it tricky to mix and match task, effects, and things just in IO. This matches existing handling in existing api endpoints, and we've scope threaded to see green thread behavior on this a couple of times.
src/Semantic/Api/ScopeGraph.hs
Outdated
| import Source.Loc as Loc | ||
|
|
||
| parseScopeGraph :: ( Has Distribute sig m | ||
| , Has (Error SomeException) sig m |
There was a problem hiding this comment.
Error SomeException is kind of an antipattern, as it is often much too general. Unless we’re planning to throw this interface away, can we stub out a ScopeGraphParseError type or something and use it instead?
There was a problem hiding this comment.
This attempting to protect from any error that might occur in parsing and generating a graph for a single file. One file failing shouldn't bring down the entire response, instead we return the rest of the results and an error placeholder for the failed one.
proto/semantic.proto
Outdated
| @@ -174,3 +178,30 @@ message Span { | |||
| Position start = 1; | |||
| Position end = 2; | |||
| } | |||
|
|
|||
| message ScopeGraphFile { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
👍 let me rename these all to StackGraph*
proto/semantic.proto
Outdated
| int32 id = 1; | ||
| string name = 2; | ||
| string line = 3; | ||
| string syntax = 4; |
There was a problem hiding this comment.
What does syntax mean in this context? Does this describe the node type (push, pop, etc?) If so I imagine we’ll do some parsing when converting to the native data type.
There was a problem hiding this comment.
I'm going to rename this to kind (which is what we call this in the other proto responses) - It's essential the name of the syntax term (Function, Call, etc).
proto/semantic.proto
Outdated
| } | ||
|
|
||
| message ScopeGraphPath { | ||
| string starting_symbol_stack = 1; |
There was a problem hiding this comment.
These will be encoded values too, right? Colon-separated or something?
There was a problem hiding this comment.
Yes, these are currently stacks encoded and using ascii delimiters.
There was a problem hiding this comment.
Would it make more sense to have this be a repeated string instead? Are we using different ascii characters as different delimiters, or as sigils at the beginning of a symbol name to indicate its type? If the former, this is worth a comment describing the format more precisely since it becomes part of the API contract. If the latter that would line up well with repeated string, and then only aleph would need to know specifically how we turn the list of symbols on the stack into a scalar string in the db. And the per-language stack graph generators would have full flexibility to choose sigils and whatnot as needed for the language.
proto/semantic.proto
Outdated
| } | ||
|
|
||
| message ScopeGraphNode { | ||
| int32 id = 1; |
There was a problem hiding this comment.
FYI Haskell native Int is 64-bits wide, so we could have slightly more efficient conversion (no fromIntegral upcasting) if we went with int64.
src/Semantic/Api/ScopeGraph.hs
Outdated
| pure $ defMessage & P.files .~ toList terms | ||
| where | ||
| go :: (Has (Error SomeException) sig m, Has Parse sig m) => Blob -> m ScopeGraphFile | ||
| go blob = catching $ graphToFile <$> graphForBlob blob |
There was a problem hiding this comment.
Really elegant definition 👍
Implements the external API plumbing to support producing incremental scope graphs for blobs.