Skip to content
This repository has been archived by the owner on Oct 6, 2021. It is now read-only.

add unused_identifier and special_identifier #20

Merged
merged 3 commits into from
May 16, 2021
Merged

Conversation

ananthakumaran
Copy link
Owner

@ananthakumaran ananthakumaran commented May 16, 2021

This introduces a couple of new issues. Earlier we used to alias
identifier as remote_identifier and function_identifier. I removed
remote_identifier as it was not actually used for any highlighting
(dot_call itself provides enough ways to highlight if
required). function_identifier on the other hand is tricky to remove,
as we use it for def* highlight case.

One way is to introduce a child node (function_identifier (special_identifier))
which keeps the info. Do we need this though?

fixes #19

@akash-akya
Copy link
Collaborator

There are some corner cases with unused_identifier

def func(_foo?, _bar!, <<_baz::binary>>) do
  :ok
end
(program [0, 0] - [3, 0]
  (call [0, 0] - [2, 3]
    function: (function_identifier [0, 0] - [0, 3])
    (call [0, 4] - [0, 40]
      function: (function_identifier [0, 4] - [0, 8])
      (arguments [0, 8] - [0, 40]
        (identifier [0, 9] - [0, 14])
        (identifier [0, 16] - [0, 21])
        (binary [0, 23] - [0, 39]
          (binary_op [0, 25] - [0, 37]
            left: (identifier [0, 25] - [0, 29])
            right: (identifier [0, 31] - [0, 37])))))
    (do_block [0, 41] - [2, 3]
      (atom [1, 2] - [1, 5]
        (atom_literal [1, 2] - [1, 5])))))

I think we have to check for unused_identifier at all the places wherever we might return identifier

@akash-akya
Copy link
Collaborator

akash-akya commented May 16, 2021

We can remove (#match? @variable.parameter "^[^_]") query clauses from highlight.scm I think.

This introduces a couple of new issues .... function_identifier on the other hand is tricky to remove,
as we use it for def* highlight case.

btw, which issues are you referring to? 🤔
Do we need to remove function_identifier?

@ananthakumaran
Copy link
Owner Author

__using__(x)
_another(y)
(program [0, 0] - [3, 0]
  (call [0, 0] - [0, 12]
    function: (function_identifier [0, 0] - [0, 9])
    (arguments [0, 9] - [0, 12]
      (identifier [0, 10] - [0, 11])))
  (call [1, 0] - [1, 11]
    function: (function_identifier [1, 0] - [1, 8])
    (arguments [1, 8] - [1, 11]
      (identifier [1, 9] - [1, 10]))))
<program>
  <call>
    <function_identifier type="function">__using__</function_identifier>

    <arguments>(
      <identifier>x</identifier>
)</arguments>
</call>

  <call>
    <function_identifier type="function">_another</function_identifier>

    <arguments>(
      <identifier>y</identifier>
)</arguments>
</call>
</program>

in the above example, the identifier is represented function_identifier and we lose the info whether it's special or unused

@ananthakumaran
Copy link
Owner Author

We can remove (#match? @variable.parameter "^[^_]") query clauses from highlight.scm I think.

removed

@akash-akya
Copy link
Collaborator

in the above example, the identifier is represented function_identifier and we lose the info whether it's special or unused

Ok, it's about keeping the AST information. From query perspective we can still use #match? like before right?

(call function: (function_identifier) @special_function
      (#match? @special_function "^__.+__$"))

But if we are thinking of keeping AST consistent then we can use child-node as well like you said.

@ananthakumaran
Copy link
Owner Author

Ok, it's about keeping the AST information. From query perspective we can still use #match? like before right?

yes, #match? would still work.

But if we are thinking of keeping AST consistent then we can use child-node as well like you said.

child-node would expose the info, but not that consistent with AST nodes. Anyhow this is probably something we can solve later.

@akash-akya
Copy link
Collaborator

akash-akya commented May 16, 2021

Thinking again about child-node, If we add child-node for function_identifier then highlighting might be tricky. Since we don't want to highlight _foo in def _foo(...) as comment right? We might come back to using negation query

@ananthakumaran ananthakumaran merged commit cb7c3cb into master May 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Special-case _unused and __MODULE__
2 participants