-
-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
migrate Go keys and values to treesitter #1839
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
package p | ||
|
||
func returnZero() { | ||
return | ||
} | ||
|
||
func returnOne() string { | ||
return "lorem" | ||
} | ||
|
||
func returnOneWithCommentBefore() string { | ||
return /* comment */ "lorem" | ||
} | ||
|
||
func returnOneWithCommentAfter() string { | ||
return "lorem" /* comment */ | ||
} | ||
|
||
|
||
func returnTwo() (string, string) { | ||
return "lorem", "ipsum" | ||
} | ||
|
||
func returnTwoWithCommentInside() (string, string) { | ||
return "lorem" /* comment */, "ipsum" | ||
} | ||
|
||
func returnTwoWithCommentBefore() (string, string) { | ||
return /* comment */ "lorem", "ipsum" | ||
} | ||
|
||
func returnTwoWithCommentAfter() (string, string) { | ||
return "lorem", "ipsum" /* comment */ | ||
} | ||
|
||
|
||
func returnThree() (string, string, string) { | ||
return "lorem", "ipsum", "blah" | ||
} | ||
|
||
func returnThreeWithCommentInside() (string, string, string) { | ||
return "lorem" /* comment */, "ipsum", "blah" | ||
} | ||
|
||
func returnThreeWithCommentInside2() (string, string, string) { | ||
return "lorem", "ipsum" /* comment */, "blah" | ||
} | ||
|
||
func returnThreeWithCommentBefore() (string, string, string) { | ||
return /* comment */ "lorem", "ipsum", "blah" | ||
} | ||
|
||
func returnThreeWithCommentAfter() (string, string, string) { | ||
return "lorem", "ipsum", "blah" /* comment */ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,7 +60,9 @@ export function checkCaptureStartEnd( | |
showError( | ||
messages, | ||
"TreeSitterQuery.checkCaptures.mixRegularStartEnd", | ||
`Please do not mix regular captures and start/end captures: ${captures}`, | ||
`Please do not mix regular captures and start/end captures: ${captures.map( | ||
({ name, range }) => name + "@" + range.toString(), | ||
)}`, | ||
); | ||
shownError = true; | ||
} | ||
|
@@ -71,7 +73,7 @@ export function checkCaptureStartEnd( | |
messages, | ||
"TreeSitterQuery.checkCaptures.duplicate", | ||
`A capture with the same name may only appear once in a single pattern: ${captures.map( | ||
({ name }) => name, | ||
({ name, range }) => name + "@" + range.toString(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as above |
||
)}`, | ||
); | ||
shownError = true; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,22 @@ class HasMultipleChildrenOfType extends QueryPredicateOperator<HasMultipleChildr | |
} | ||
} | ||
|
||
/** | ||
* A predicate operator that returns true if the node has more than 1 child not of | ||
* type {@link type} (inclusive). For example, `(has-multiple-children-not-of-type? | ||
* @foo bar)` will accept the match if the `@foo` capture has 2 or more children | ||
* not of type `bar`. | ||
*/ | ||
class HasMultipleChildrenNotOfType extends QueryPredicateOperator<HasMultipleChildrenNotOfType> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow this really feels like a slippery slope 😅. Getting to the point where I wonder if we want to create our own mini-language There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dsl ftw! :D |
||
name = "has-multiple-children-not-of-type?" as const; | ||
schema = z.tuple([q.node, q.string]); | ||
|
||
run({ node }: MutableQueryCapture, type: string) { | ||
const count = node.children.filter((n) => n.type !== type).length; | ||
return count > 1; | ||
} | ||
} | ||
|
||
class ChildRange extends QueryPredicateOperator<ChildRange> { | ||
name = "child-range!" as const; | ||
schema = z.union([ | ||
|
@@ -187,4 +203,5 @@ export const queryPredicateOperators = [ | |
new AllowMultiple(), | ||
new InsertionDelimiter(), | ||
new HasMultipleChildrenOfType(), | ||
new HasMultipleChildrenNotOfType(), | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -222,3 +222,87 @@ | |
. | ||
) | ||
) @anonymousFunction @namedFunction | ||
|
||
;; keys in maps | ||
(literal_value | ||
"{" @collectionKey.iteration.start.endOf | ||
(keyed_element | ||
(_) @collectionKey @collectionKey.trailing.start.endOf | ||
":" | ||
(_) @collectionKey.trailing.end.startOf | ||
) @collectionKey.domain | ||
"}" @collectionKey.iteration.end.startOf | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have mixed feelings about defining a scope’s pattern and its iteration scope’s pattern in one go. I think it's often clearer to define them separately. You could arguably define That being said, it's more of a rule of thumb than a hard rule; if you feel defining There is a slight performance implication as well fwiw, because this pattern matches once per |
||
|
||
;; values in maps | ||
(literal_value | ||
"{" @value.iteration.start.endOf | ||
(keyed_element | ||
(_) @value.leading.start.endOf | ||
":" | ||
(_) @value @value.leading.end.startOf | ||
) @value.domain | ||
"}" @value.iteration.end.startOf | ||
) | ||
|
||
;; values in return statements | ||
|
||
;; one value within a return statement | ||
(return_statement | ||
(expression_list | ||
. | ||
(_) | ||
. | ||
) @value | ||
Comment on lines
+252
to
+256
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what happens with comments here? |
||
(#insertion-delimiter! @value ", ") | ||
) @value.domain @value.iteration | ||
|
||
;; multiple values within a return statement | ||
|
||
;; NB: gofmt puts the comma after block comments in lists of things | ||
;; | ||
;; Like this: | ||
;; return "lorem" /* comment */, "ipsum" | ||
;; Not like this: | ||
;; return "lorem", /* comment */ "ipsum" | ||
;; | ||
;; It's really hard to deal with commas both before and after, | ||
;; and they're rare anyway, so assume gofmt for now. | ||
;; Non-gofmt commas will mess up removal ranges. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I generally lean slightly towards being conservative wrt removing comments, but this behaviour might be nice if it's almost always what you want. I'll defer to you on this one |
||
|
||
;; the first value | ||
|
||
;; BUG: in this code: | ||
;; return "lorem" /* comment */ , "ipsum" | ||
;; the comment is included in the removal range of "lorem". | ||
Comment on lines
+275
to
+277
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this a bug? I might argue the comment is part of |
||
;; This is too hard to fix now, because it would require | ||
;; disjoint removal ranges. And it is rare anyway. | ||
|
||
;; the first of many return values... | ||
(return_statement | ||
(expression_list | ||
. | ||
(_) @value @value.trailing.start.endOf | ||
(#insertion-delimiter! @value ", ") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interesting to put predicate right after the capture itself |
||
. | ||
(comment)* @value.trailing.omit | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what does this |
||
. | ||
"," | ||
. | ||
(_) @value.trailing.end.startOf | ||
) @_exprlist @value.leading.start.startOf | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me |
||
(#not-type? @value comment) | ||
(#has-multiple-children-not-of-type? @_exprlist comment) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's a bit annoying we need to use negative specifier here rather than specifying the type itself, but I'm assuming the individual expressions have no wrapper nodes of uniform type, so I'm not sure what else we can do 🤷♂️ |
||
) @value.iteration | ||
|
||
;; ...and the rest of the values | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be nice to have a comment explaining why you need to handle first / rest separately |
||
(return_statement | ||
(expression_list | ||
"," @value.leading.start.startOf | ||
. | ||
(_) @value @value.leading.start.startOf | ||
(#insertion-delimiter! @value ", ") | ||
) @_exprlist | ||
(#not-type? @value comment) | ||
(#has-multiple-children-not-of-type? @_exprlist comment) | ||
) @value.iteration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the conversion to string not happen automatically?