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.

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

Multiple underscores are allowed in names for methods #4054

Closed
djavorszky opened this issue Mar 14, 2022 · 1 comment · Fixed by #4061
Closed

Multiple underscores are allowed in names for methods #4054

djavorszky opened this issue Mar 14, 2022 · 1 comment · Fixed by #4061
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@djavorszky
Copy link
Contributor

According to the Naming Rules in the tutorial:

After an underscore for private or special methods (behaviors, constructors, and functions), any method or variable, including parameters and fields, must start with a lowercase letter. In all cases underscores in a row or at the end of a name are not allowed, but otherwise, any combination of letters and numbers is legal.

I understand this as: multiple consecutive underscores are not allowed (and some other rules). However, it seems to be allowed for parameters (arguments), as the following compiles (and runs) happily:

actor Main
  new create(___: Env) =>
    ___.out.print("Hello world")

  new prime(___': Env) =>
    ___'.out.print("umm....")
    ___'.out.print("Hello prime")

  fun do_someting(___a: Env) =>
    ___a.out.print("Do something")

  be happy(___a_: Env) =>
    ___a_.out.print("Be happy")

Link to playground: https://playground.ponylang.io/?gist=327bddd28fbe66b24d58c681a93d60f2

There are two issues:

  1. only underscores are accepted
  2. If we're doing an underscore-only prime (___'), the syntax highlighting breaks on the playground (though I guess this isn't an issue per se, as it only occurs because issue1 exists)

Cheers!

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Mar 14, 2022
@SeanTAllen
Copy link
Member

SeanTAllen commented Mar 15, 2022

check_id_param isn't being called anymore. The was removed with case functions in #2542. The removal of check_params in sugar.c shouldn't have happened. Other check methods perhaps shouldn't have been removed as well.

Tests should be added for anything that was incorrectly removed so they can be removed again in the future.

@SeanTAllen SeanTAllen added help wanted Extra attention is needed bug Something isn't working good first issue Good for newcomers and removed discuss during sync Should be discussed during an upcoming sync labels Mar 15, 2022
@SeanTAllen SeanTAllen changed the title Multiple underscores are allowed as new/fun/be arguments Multiple underscores are allowed in names for methods Mar 15, 2022
SeanTAllen added a commit that referenced this issue Mar 19, 2022
In 2018, when we
[removed case functions](#2542) from
Pony, we also removed the checking to make sure that parameters names were
valid.

We've added checking of parameter names to make sure they match the naming
rules for parameters in Pony.

Fixes #4054
SeanTAllen added a commit that referenced this issue Mar 19, 2022
In 2018, when we
[removed case functions](#2542) from
Pony, we also removed the checking to make sure that parameters names were
valid.

We've added checking of parameter names to make sure they match the naming
rules for parameters in Pony.

Fixes #4054
@SeanTAllen SeanTAllen removed the help wanted Extra attention is needed label Mar 19, 2022
@SeanTAllen SeanTAllen self-assigned this Mar 19, 2022
SeanTAllen added a commit that referenced this issue Mar 19, 2022
In 2018, when we
[removed case functions](#2542) from
Pony, we also removed the checking to make sure that parameters names were
valid.

We've added checking of parameter names to make sure they match the naming
rules for parameters in Pony.

Fixes #4054
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants