Skip to content
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.

Sign up for GitHub

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

Add almost testable comments to .scm files #1535

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Add almost testable comments to .scm files #1535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions queries/javascript.core.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
;; import javascript.function.scm

;; `name` scope without `export`
(
(_
name: (_) @name
) @_.domain
(#not-parent-type? @_.domain export_statement)

;; We have special cases for these defined elsewhere
(#not-type?
@_.domain
variable_declarator
Expand All @@ -14,20 +17,33 @@
field_definition
)
)

;; `name` scope with `export`
(export_statement
(_
name: (_) @name
) @dummy

;; We have a special case for this one. Note we don't need to list the other
;; special cases from above because they can't be exported
(#not-type? @dummy variable_declarator)
) @_.domain

;; Special cases for `(let | const | var) foo = ...;` because the full statement
;; is actually a grandparent of the `name` node, so we want the domain to include
;; this full grandparent statement.
(
[
;;!! (const | let) foo = ...;
Copy link
Member Author

@pokey pokey Jun 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the (const | let) and ... make this not strictly Javascript, so doesn't fully implement the idea from #1524 (comment). On the fence about whether we want to go all out and make these testable or just use it as a convenient notation

I wonder if in cases where it's not fully valid syntax we leave off the !! so that future auto testing impl doesn't pick these up 🤔. Or maybe we leave it so that we're forced to clean these up if we go down that road

We could also leave off the !! and ! altogether, but in some cases the clarity is helpful; see below

Or we could think about having a different marker eg ;;!? or ;;!~ that indicates that we're using a limited set of regex-style expansions and then actually support these 🤷‍♂️. We wouldn't support the ... tho; would need to replace that with something valid but that's not much of a loss

;;! --------------^^^-------
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that I merged into one line for compactness

(lexical_declaration
(variable_declarator
name: (_) @name
)
)

;;!! var foo = ...;
;;! ----^^^-------
;; Note that we can't merge this with the variable declaration above because
;; of https://github.com/tree-sitter/tree-sitter/issues/1442#issuecomment-1584628651
(variable_declaration
Expand All @@ -37,24 +53,38 @@
)
] @_.domain
(#not-parent-type? @_.domain export_statement)

;; Handle multiple variable declarators in one statement, eg
;;!! (let | const | var) aaa = ..., ccc = ...;
;;! --------------------^^^--------^^^-------
(#allow-multiple! @name)
)

(
(export_statement
(_
;;!! export [default] (let | const | var) foo = ...;
;;! -------------------------------------^^^-------
(variable_declarator
name: (_) @name
)
)
) @_.domain

;; Handle multiple variable declarators in one statement, eg
;;!! var foo = ..., bar = ...;
;;! ----^^^--------^^^-------
(#allow-multiple! @name)
)

;;!! foo += ...;
;;! ^^^--------
(augmented_assignment_expression
left: (_) @name
) @_.domain

;;!! foo = ...;
;;! ^^^-------
(assignment_expression
left: (_) @name
) @_.domain
Expand All @@ -64,6 +94,9 @@
(formal_parameters)
] @name.iteration

;; Treat interior of all bodies as iteration scopes for `name`, eg
;;!! function foo() { }
;;! ***
(
(_
body: (_
Expand Down
52 changes: 26 additions & 26 deletions queries/javascript.function.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,54 +1,54 @@
;; Anonymous functions
[
;; function() {}
;;!! function() {}
(function
!name
)

;; function *() {}
;;!! function *() {}
(generator_function
!name
)

;; () => {}
;;!! () => {}
(arrow_function)
] @anonymousFunction

;; If we export an anonymous function as default, it semantically feels like a
;; named function.
(export_statement
[
;; export default function() {}
;;!! export default function() {}
(function
!name
)

;; export default function *() {}
;;!! export default function *() {}
(generator_function
!name
)

;; export default () => {}
;;!! export default () => {}
(arrow_function)
]
) @namedFunction

;; Named functions without export
(
[
;; function foo() {}
;;!! function foo() {}
(function_declaration
name: (_) @functionName
)

;; function *foo() {}
;;!! function *foo() {}
(generator_function_declaration
name: (_) @functionName
)

;; (let | const) foo = () => {}
;; (let | const) foo = function() {}
;; (let | const) foo = function *() {}
;;!! (let | const) foo = () => {}
;;!! (let | const) foo = function() {}
;;!! (let | const) foo = function *() {}
(lexical_declaration
(variable_declarator
name: (_) @functionName
Expand All @@ -64,9 +64,9 @@
)
)

;; var foo = () => {}
;; var foo = function() {}
;; var foo = function *() {}
;;!! var foo = () => {}
;;!! var foo = function() {}
;;!! var foo = function *() {}
;; Note that we can't merge this with the variable declaration above because
;; of https://github.com/tree-sitter/tree-sitter/issues/1442#issuecomment-1584628651
(variable_declaration
Expand All @@ -90,19 +90,19 @@
;; Exported named functions
(export_statement
[
;; export [default] function foo() {}
;;!! export [default] function foo() {}
(function_declaration
name: (_) @functionName
)

;; export [default] function *foo() {}
;;!! export [default] function *foo() {}
(generator_function_declaration
name: (_) @functionName
)

;; export [default] (let | const | var) foo = () => {}
;; export [default] (let | const | var) foo = function() {}
;; export [default] (let | const | var) foo = function *() {}
;;!! export [default] (let | const | var) foo = () => {}
;;!! export [default] (let | const | var) foo = function() {}
;;!! export [default] (let | const | var) foo = function *() {}
(_
(variable_declarator
name: (_) @functionName
Expand All @@ -125,25 +125,25 @@
;; We also don't handle function declarations that only exist in Javascript;
;; see javascript.scm.
[
;; (function foo() {})
;;!! (function foo() {})
(function
name: (_) @functionName
)

;; (function *foo() {})
;;!! (function *foo() {})
(generator_function
name: (_) @functionName
)

;; foo() {}
;; (in class bodies)
;;!! class Foo { foo() {} }
;;! ^^^^^^^^
(method_definition
name: (_) @functionName
)

;; foo = () => {};
;; foo = function() {};
;; foo = function *() {};
;;!! foo = () => {};
;;!! foo = function() {};
;;!! foo = function *() {};
(assignment_expression
left: (_) @functionName
right: [
Expand Down
28 changes: 27 additions & 1 deletion queries/javascript.jsx.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,56 +1,82 @@
;;!! <foo>bar</foo>
;;! ^^^^^^^^^^^^^^
;;! ###
;;! ***
(
(jsx_element) @xmlElement @_.interior @_.iteration
(#child-range! @_.interior 0 -1 true true)
(#child-range! @_.iteration 0 -1 true true)
)

;;!! <foo>bar</foo>
;;! ***
(
(jsx_element) @xmlStartTag.iteration @xmlEndTag.iteration @xmlBothTags.iteration
(#child-range! @xmlStartTag.iteration 0 -1 true true)
(#child-range! @xmlEndTag.iteration 0 -1 true true)
(#child-range! @xmlBothTags.iteration 0 -1 true true)
)

;;!! <foo>bar</foo>
;;! ^^^^^---------
(jsx_element
(jsx_opening_element) @xmlStartTag @xmlBothTags
(#allow-multiple! @xmlBothTags)
) @_.domain

;;!! <foo>bar</foo>
;;! --------^^^^^^
(jsx_element
(jsx_closing_element) @xmlEndTag @xmlBothTags
(#allow-multiple! @xmlBothTags)
) @_.domain

;;!! <foo/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's no annotation below, it's implied that the whole thing is the content range and domain

(jsx_self_closing_element) @xmlElement

;; JSX fragments, eg <>foo</>
;; ======== JSX fragments, eg <>foo</> ==========

;;!! <>foo</>
;;! ^^^^^^^^
;;! ###
;;! ***
(
(jsx_fragment) @xmlElement @_.interior @_.iteration
(#child-range! @_.interior 1 -3 true true)
(#child-range! @_.iteration 1 -3 true true)
)

;;!! <>foo</>
;;! ***
(
(jsx_fragment) @xmlStartTag.iteration @xmlEndTag.iteration @xmlBothTags.iteration
(#child-range! @xmlStartTag.iteration 1 -3 true true)
(#child-range! @xmlEndTag.iteration 1 -3 true true)
(#child-range! @xmlBothTags.iteration 1 -3 true true)
)

;;!! <>foo</>
;;! ^^------
(
(jsx_fragment) @xmlStartTag @xmlBothTags @_.domain
(#child-range! @xmlStartTag 0 1)
(#child-range! @xmlBothTags 0 1)
(#allow-multiple! @xmlBothTags)
)

;;!! <>foo</>
;;! -----^^^
(
(jsx_fragment) @xmlEndTag @xmlBothTags @_.domain
(#child-range! @xmlEndTag -3)
(#child-range! @xmlBothTags -3)
(#allow-multiple! @xmlBothTags)
)

;; Sets `name` to be empty range inside the fragment tag:
;;!! <>foo</>
;;! {} {}
;;! -- ---
Comment on lines +77 to +79
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the example that motivated me to use the !! and !, because in this case it might not be clear what's code and what's annotations

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that I'm including multiple scopes on one line here, as otherwise it gets pretty long

(
(jsx_fragment
"<" @_.domain.start
Expand Down
15 changes: 10 additions & 5 deletions queries/javascript.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@
;; Define this here because the `field_definition` node type doesn't exist
;; in typescript.
(
;; foo = () => {};
;; foo = function() {};
;; foo = function *() {};
;; (inside class bodies)
;;!! class Foo {
;;!! foo = () => {};
;;! ^^^^^^^^^^^^^^^
;;!! foo = function() {};
;;! ^^^^^^^^^^^^^^^^^^^^
;;!! foo = function *() {};
;;! ^^^^^^^^^^^^^^^^^^^^^^
;;!! }
(field_definition
property: (_) @functionName
value: [
Expand All @@ -28,7 +32,8 @@
)

(
;; foo = ...;
;;!! foo = ...;
;;! ^^^-------
(field_definition
property: (_) @name
) @name.domain.start
Expand Down
Loading