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

Object instantiation #109

Merged
merged 1 commit into from
Sep 28, 2022
Merged

Object instantiation #109

merged 1 commit into from
Sep 28, 2022

Conversation

dingo-d
Copy link
Member

@dingo-d dingo-d commented Aug 20, 2022

This PR depends on #108. Once that is merged, this PR should be rebased to the master branch.

The PR adds rules about the object instantiation and is the continuation of the additions of 'modern' PHP code in the WordPress PHP Coding Standards handbook based on the make post by Juliette Reinders Folmer.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Just one small remark.

wordpress-coding-standards/php.md Outdated Show resolved Hide resolved
@dingo-d dingo-d marked this pull request as ready for review August 26, 2022 08:24
@dingo-d dingo-d force-pushed the object-instantiation branch from 8f1f77e to ea47837 Compare September 26, 2022 07:54
@dingo-d dingo-d requested review from GaryJones and jrfnl September 26, 2022 07:55
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

LGTM. ✅

Might it be an idea to have at least one of the object instantiations for "Correct" pass one of more variables ?
(obviously doesn't apply to "Incorrect")

@dingo-d
Copy link
Member Author

dingo-d commented Sep 26, 2022

@jrfnl like this?

// Correct.
$foo = new Foo();
$anonymous_class = new class( $dependency ) { ... };
$instance = new static();

// Incorrect.
$foo = & new Foo;
$foo = new Foo ();

@jrfnl
Copy link
Member

jrfnl commented Sep 26, 2022

@dingo-d Actually, after posting, I realized that for the whitespace issue in "Incorrect" a param could also be used.

As for the code change, yes, something like that, though maybe keep the param name more "generalized" ? i.e. $param, $bar, $input or something.

@dingo-d
Copy link
Member Author

dingo-d commented Sep 27, 2022

Last sanity check

// Correct.
$foo = new Foo();
$anonymous_class = new class( $parameter ) { ... };
$instance = new static();

// Incorrect.
$foo = & new Foo; // Assign by reference is not allowed for object instantiation.
$foo = new Foo ();
$anonymous_class = new class ($input) { ... };

The parameter in the incorrect example is bothering me a bit because the sniffs for object instantiation are not checking for whitespace inside the parenthesis, so those are two different sniffs. Or did you mean, adding a parameter, but with correct spacing inside the parenthesis (but it's still incorrect because the space before the opening parenthesis)?

@jrfnl
Copy link
Member

jrfnl commented Sep 27, 2022

Or did you mean, adding a parameter, but with correct spacing inside the parenthesis (but it's still incorrect because the space before the opening parenthesis)?

Exactly ;-)

@dingo-d dingo-d force-pushed the object-instantiation branch from 6ef6029 to 02bf1bf Compare September 27, 2022 12:25
wordpress-coding-standards/php.md Show resolved Hide resolved
$instance = new static();

// Incorrect.
$foo = & new Foo; // Assign by reference is not allowed for object instantiation.
Copy link
Member

Choose a reason for hiding this comment

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

Assign by reference is not allowed for object instantiation.

Is that a WPCS decision or a PHP technical thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a parse error in PHP: https://3v4l.org/b8vrf

Copy link
Member

Choose a reason for hiding this comment

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

OK, so why are we highlighting ways to break PHP in our incorrect examples? There are infinite ways to create parse errors. I'd rather see the incorrect example focus on the WPCS aspect (i.e. use of paratheses, spacing around / inside / after parentheses, etc.).

Copy link
Member Author

Choose a reason for hiding this comment

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

It was mentioned in the original make post: https://make.wordpress.org/core/2020/03/20/updating-the-coding-standards-for-modern-php/

So I added it. I can remove it (makes sense not to have parse errors in the code examples).

I'll just remove the & in this example 👍🏼

Co-authored-by: Juliette <[email protected]>
Co-authored-by: Gary Jones <[email protected]>
@dingo-d dingo-d force-pushed the object-instantiation branch from 8634d24 to aa247a9 Compare September 28, 2022 09:29
Copy link
Member

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

3 participants