feat(compiler): Parse query parameter metadata from comments #2850
Conversation
| return strings.Join(lines, "\n"), comments, s.Err() | ||
| } | ||
|
|
||
| func CleanedComments(rawSQL string, cs CommentSyntax) ([]string, error) { |
There was a problem hiding this comment.
We've now implemented comment parsing/cleaning in three places. I was hoping to consolidate but walked it back in favor of a separate code path.
It's some unfortunate technical debt though, since at some point someone is going to complain that we don't support multi-line slash-star comment syntax and we'll have to refactor all of this...
There was a problem hiding this comment.
So a comment like
/*
name: Foo :one
@param bar? text
*/
won't work? If that's the case, we'll want to have a fast follow to enable that.
There was a problem hiding this comment.
Looking over the implementation of CleanedComments, I don't think it's be too hard to add support for multiline comments. But let's address that in a future PR.
There was a problem hiding this comment.
Yeah the multi-line comment syntax has never worked but there wasn't really a reason for anyone to ask before this change.
Agree we can implement later
| OldFunc func(string) int | ||
| } | ||
|
|
||
| type CommentSyntax struct { |
There was a problem hiding this comment.
Handling comments seems like a source code thing, so I popped this in the source package. The new CleanedComments func I wrote depends on comment syntax, and we already have a func StripComments (which doesn't depend on comment syntax but I think that's just an oversight/bug).
| return strings.Join(lines, "\n"), comments, s.Err() | ||
| } | ||
|
|
||
| func CleanedComments(rawSQL string, cs CommentSyntax) ([]string, error) { |
There was a problem hiding this comment.
So a comment like
/*
name: Foo :one
@param bar? text
*/
won't work? If that's the case, we'll want to have a fast follow to enable that.
| return strings.Join(lines, "\n"), comments, s.Err() | ||
| } | ||
|
|
||
| func CleanedComments(rawSQL string, cs CommentSyntax) ([]string, error) { |
There was a problem hiding this comment.
Looking over the implementation of CleanedComments, I don't think it's be too hard to add support for multiline comments. But let's address that in a future PR.
…v#2850) * feat(compiler): Parse query parameter metadata from comments * update comment stripping, fix a test * parse `@param` name correctly * add a test for metadata param parsing * use a word scanner for flag and param comment line parsing * forgot that I partially cleaned comments already * slightly more useful metadata test output * drop unnecessary metadata test cases * walk back changes, implement comment cleaning in source package * cleanup * simplify diff * ensure at least one test case for parsing @whatever without a leading space
Groundwork for #2800 (explicit type annotations in query comments metadata).
There are lots of tests that failed while I was refactoring this, so I have a fairly high degree of confidence that this refactor doesn't break anything. The changed endtoend test output is due to parsing bugs I fixed.
I chose to add a new
Metadatastruct type to house the growing set of stuff we associate with a query, but I don't have to so let me know if an alternative approach is preferred.