Function names prefixed with an underscore are treated as comments

[rhysforyou]

Like the title says, a call to a function whose name begins with an underscore character, such as in the example above, is incorrectly treated as a comment.

Here's what I get when logging the scope at line 3, column 42:

[keathley]

Variables with underscores in elixir are a way to mark them as unused. Hence we highlight them to show that. In your example there isn't any semantic reason to use an underscore in front of the function name. Its marked as a private function so there's no risk in other consumers of your api re-using your function. Honestly, I'm not sure that this is a bug.

[rhysforyou]

Yeah I'm fairly new to Elixir so I'm still working out naming conventions, for what it's worth this code is copied pretty much verbatim from Programing Elixir 1.3. But this is still a syntactically legal name for a function, so I'd say highlighting it as an ignored pattern match is a bug.

[ricmarinovic]

I submitted #79 to handle cases like this. Would you mind taking a look at it, @keathley ?

[keathley]

@rkma I'm not sure how your PR resolves this. It marks everything starting with an underscore as an other variable which wouldn't syntax highlight in the same way that other functions are highlighted either.

In order to solve this we need to not identify unused variables outside of pattern matching. That means we'll need to identify unused variables only inside of function arguments, case, cond, and =. Thats what I can come up with off the top of my head. There might be more. The end result is that functions starting with underscores will be matched in the same way as any other variable. It also means that unused variables that are later used will be matched as variables. But using a variable marked as "unused" results in a compiler warning so it shouldn't be a huge deal.

Does anyone want to try their hand at this?

[ymtszw]

@keathley I was on the same boat in that we sometimes use functions with leading _ to denote generated-and-used-behind-the-scenes functions. So I have tried to implement that behavior but found it very cumbersome to cover perfectly.

Instead, I have found a band-aid solution which MIGHT suit for certain use cases.

Current pattern for unused variable:

\\b_(\\w*)

Make it like this (could use some refinements):

\\b_(\\w*)\\b(?!\\()

with this negative-lookahead, it won't match for _-prefixed strings when they are trailed by (, which obviously denotes a beginning of function calling. e.g.

# match for this '_unused' binding
_unused = some_function(:arg1)

# not match for this function name
_underscored_function_call(:arg1)

There are an obvious case which cannot be covered by this pattern: 0-arity function calls.

# matching; cannot distinguish whether it is a variable or a function
_underscored_no_arity_function_call

And I haven't come up with good solution to distinguish 0-arity function calls and variables/bindings (it is too hard for me to implement within regex pattern matching).

Thus I call it band-aid solution. I heard that in future versions of Elixir, ()-less function call will be warned. When this comes into effect, people may start to change ()-less function call to explicit style, and with it, the above negative-lookahead for trailing ( might become sufficient, if not perfect, to distinguish unused variables and _-prefixed function calls.

Of course I haven't tested against all of the existing coding styles and use cases. Would be glad if i can hear your thoughts. ❤️

[keathley]

@ymtszw Now that functions without parens are being warned we might be able to solve this. One use case that you'll have to watch out for though is when you have a function that comes after a pipe like: |> _underscored_one_arity_function_call since the parens are still optional at that point.

In general I guess I'm not sure what the utility of prefixing a function with _ is. If its supposed to be a private function then you could just use defp or move the functionality to a different module.

Let me know what you think. If this is still an issue and you want to have a go at a PR then that would be great. Otherwise I'd like to close this issue. :smile: :+1:

[ymtszw]

Well I am not so enthusiastic about this issue since at the end of the day, _-prefixed functions are, if any, rare occasions in Elixir codes. And yes, one can simply use normally-named private functions or just put them in "behind the scene" modules.

Although with Elixir 1.4+, function calls with explicit parens became popular in my sight, so my proposed "band-aid" solution is now actually okay to have, I assume. I'll post PR later after checking it works as I described.

---

Concerning utility of _-prefixed functions - I agree that it does not have significant one. An example use case I can come up with is Phoenix.Router, where various functions (including ones injected via __using__/1 or __before_compile__/1 macros) are doubly surrounded by _.

      @doc """
      Callback invoked by Plug on every request.
      """
      def call(conn, _opts) do
        conn
        |> prepare()
        |> __match_route__(conn.method, Enum.map(conn.path_info, &URI.decode/1), conn.host)
        |> Phoenix.Router.__call__()
      end

We MIGHT consider these double-_-surrounded functions/macros as an idiom for denoting "behind the scene interfaces that need to avoid name collision"? But I don't assume it's widely accepted, nor used frequently in many library development. Heck, such naming should even be discouraged to language users since Elixir might introduce other predefined _-surrounded macros as compiler feature someday.

[ymtszw]

I had thought changing this

  {
    'match': '\\b_([\\w]+[?!]?)'
    'name': 'unused.comment.elixir'
  }

to this

  {
    'match': '\\b_([\\w]+[?!]?)\\b(?!\\()'
    'name': 'unused.comment.elixir'
  }

could exclude functions calls with negative lookahead. Though it was partly okay, it turned out that it cannot nicely cover "underscored AND ?- or !-suffixed" function.

Without word boundary character,

  {
    'match': '\\b_([\\w]+[?!]?)(?!\\()'
    'name': 'unused.comment.elixir'
  }

it stops matching at rightmost character which satisfies "not trailed by (" condition.

O my, this is frustrating! Perhaps positively matching for "proper function call syntax" may result in better result? But that's somewhat different matter and possibly break some existing styling/theme. Since currently, function calls are "unscoped" in language-elixir (only default source.elixir scope attached).

I'm not pursuing this no further; you may close this for now. 😞 Unless someone can come up with more precise regular expression to cover this pattern.

[keathley]

I totally understand how frustrating some of this stuff can be :smile:. Thanks for having a go at it. I'll go ahead and close this for now. We can revisit it later if we need to.