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

Bake migration snapshot should not allow reserved words #693

Open
1 of 3 tasks
dereuromark opened this issue Feb 21, 2024 · 2 comments
Open
1 of 3 tasks

Bake migration snapshot should not allow reserved words #693

dereuromark opened this issue Feb 21, 2024 · 2 comments
Labels

Comments

@dereuromark
Copy link
Member

dereuromark commented Feb 21, 2024

This is a (multiple allowed):

  • bug
  • enhancement
  • feature-discussion (RFC)

bin/cake bake migration_snapshot New

class New extends AbstractMigration
{
    ...
}

When trying to generate it, it throws this at the end

[ParseError] syntax error, unexpected token "new", expecting identifier in /shared/httpd/app/config/Migrations/20240221034352_New.php on line 6

I guess it is mainly about this list? https://www.php.net/manual/en/reserved.php

If someone can help to gather the list of all possible words, I can whip up a PR addressing the issue.

My approach would be to ask() the user about a possible prefix in this case to avoid the collision (prefix defaulting to "Migration"), or allow aborting.
If continue, it would then use e.g. "MigrationNew" and that would be fine.

@dereuromark dereuromark added this to the 4.x (CakePHP 5) milestone Feb 21, 2024
@ravage84
Copy link
Member

Is this really a problem we should fix or prevent from happening?

Naming things is hard and if a developer really thinks naming anything just New is going to be a good choice, I'd question his decision making. Even if he/she is a novice.

Apart from that, I wouldn't try to gather all possible reserved keywords beforehand.
The list provided by https://www.php.net/manual/en/reserved.php is already a good start.

Anything beyond that can be added any time.

Pre- or Suffixing is possible way. Personally, I would prefer suffixing, as the name is more important than the added filler word.

Alternatively, it could just check the list and request the name choice with a link to https://www.php.net/manual/en/reserved.php.

@ravage84
Copy link
Member

Wouldn't a all time Migration suffix be an optuon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants