Skip to content

declare must follow the rule for assignment operator#992

Closed
PowerKiKi wants to merge 1 commit intophp-fig:masterfrom
PowerKiKi:patch-3
Closed

declare must follow the rule for assignment operator#992
PowerKiKi wants to merge 1 commit intophp-fig:masterfrom
PowerKiKi:patch-3

Conversation

@PowerKiKi
Copy link
Copy Markdown
Contributor

declare statements must follow the rule for assignment operator which dictates to have exactly 1 space around the operator.

There was no reason to make it an exception. It made the rule harder to explain, and harder to remember. By keeping a single rule for all cases everything is made simpler.

`declare` statements must follow the rule for assignment operator which dictates to have exactly 1 space around the operator.

There was no reason to make it an exception. It made the rule harder to explain, and harder to remember. By keeping a single rule for all cases everything is made simpler.
Copy link
Copy Markdown
Member

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

Since we're going this way, shouldn't we go "follow the common rule" all the way?

~~~

Declare statements MUST contain no spaces and MUST be exactly `declare(strict_types=1)`
Declare statements MUST be exactly `declare(strict_types = 1)`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't we make the semi-colon mandatory, so it follows the common rule about that too? At that point, this row can be changed into something like

declade follows the same rules as the rest of the code: MUST have one space around the = operator and it MUST terminate with a semicolon

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe the mention of semicolon is only for cases like <?php declare(strict_types = 1) ?>. As shown in the example starting with "When wishing to declare...". Because otherwise it would be a parse error AFAIK. I don't think the case of semicolon before ?> end tag is mentioned anywhere else. Maybe it should be removed from here and addressed separately ?

@Jean85
Copy link
Copy Markdown
Member

Jean85 commented Dec 21, 2017

This also relates to #984

@nicwortel
Copy link
Copy Markdown

Does this mean that PSR-12 is going to ignore the ~90% votes for not using spaces?

@michaelcullum
Copy link
Copy Markdown
Member

We already agreed not to do this and it's in review now so it cannot be modified without a push back to draft.

@rentalhost
Copy link
Copy Markdown

rentalhost commented Apr 17, 2018

While I understand that ~90% voters prefer no spaces for declare() as mentioned by @nicwortel, I think that it should be re-reviewed and better argumented to voters in according with @PowerKiKi when he mentions that PSR suggests a space between operators in assignments.

Actually, the Review of Extended Coding Style Guide in 6. Operators declares that "all binary and ternary (but not unary) operators MUST be preceded and followed by at least one space. This includes all arithmetic, comparison, assignment, bitwise, logical (excluding ! which is unary), string concatenation, type operators, trait operators (insteadof and as), and the single pipe operator (e.g. ExceptionType1 | ExceptionType2 $e). Other operators are left undefined".

It was mentioned on mailing by @PowerKiKi but not was replied.

--

I have sent to mailing: https://groups.google.com/forum/#!topic/php-fig/ZJTLn-aLSq4

@alcaeus
Copy link
Copy Markdown

alcaeus commented May 18, 2018

With PSR-12 being moved back to DRAFT status (see https://groups.google.com/forum/#!topic/php-fig/1TBEvCIHFv0), can this be reopened and reconsidered, or should this be brought up in the mailing list first?

@PowerKiKi
Copy link
Copy Markdown
Contributor Author

I cannot re-open my PR, but I'll gladly submit a new one with the same patch if the PSR guys agree to reconsider the situation.

@Jean85
Copy link
Copy Markdown
Member

Jean85 commented May 18, 2018

Hi @PowerKiKi, we recently discussed this, and come to the conclusion that the common usage is without spaces, since it's been like this even in the orignal RFC that introduced the strict_type directive; I'm also not sure that this should be considered an assignment operator...

@rentalhost
Copy link
Copy Markdown

@Jean85 so I guess that the "common usage" is counterintuitive here, once that it is definitively an assignment.

The RFC code is not a code style reference, it just should exemplify how a new feature should works. In the same page it put brackets on same line from class definition, so it even respect PSR.

I really don't see any reason to avoid spaces in declare(X = 1) but put spaces in const X = 1, for instance.

@rob006
Copy link
Copy Markdown

rob006 commented May 18, 2018

@rentalhost strict_types it is neither a variable nor a constant, you can't use the result of this "assignment" and you can't "assign" anything to it. Compiler directive is a different thing than code, and it makes perfect sense to use different formatting rules for it. The same situation as with echo - you're not using rules for functions formatting (echo('something')) because it is not a function and different rules help to notice the difference.

@rentalhost
Copy link
Copy Markdown

@rob006 Actually echo doesn't requires () as function does. The () here works like an expression group, as in (1 + 2). So in that case, the correct usage should be echo ('something') or even better, just echo 'something', that is compatible with other language constructor like return 5 (that must be writed as return (5) and not as return(5) once that it is not a function). That comes from PSR-2 1.8 ("Control structure keywords MUST have one space after them; method and function calls MUST NOT") -- and I hope that control structure here could includes too the echo (although that it is a language constructor). But I think that in that point we are according.

About strict_types not be neither a variable nor a constant, I should disagree with you. It acts like a "write-once" constants, but without the read possibility after initial assignment. But the question here is not "what it does", but "what it seems like", and it seems like a constant assignment, where the constant name could be ticks, encoding or strict_types (it just doesn't respects PSR constant uppercase, but PHP itself doesn't care about that).

I just do not understand "why" everything should be spaced except that one. I don't think that "the common usage" should be a valid argument here, I think that it deserves a logic definition that everyone could understand the reason without come to here to understand "the why".

@rob006
Copy link
Copy Markdown

rob006 commented May 18, 2018

It acts like a "write-once" constants, but without the read possibility after initial assignment.

Sorry, but this is ridiculous. Then we can say that variable is a constant, but with possibility to overwrite after initialization. And true is also a variable, but read-only...

But the question here is not "what it does", but "what it seems like"

I could not disagree more. This is only about "what it is". And your arguments like "it acts like constant" are very good reason to use different rules for declare() to distinguish the compiler directives from constant assignments.

@rentalhost
Copy link
Copy Markdown

Sorry, but this is ridiculous. Then we can say that variable is a constant, but with possibility to overwrite after initialization. And true is also a variable, but read-only...

No, it should sounds like: assignments could/must occur in constants, variables or declarations.

  • Constants: assignment must occur once, and turn read-only after;
  • Variables: assignment could occur anytime and are keeped as RW;
  • Declarations: assignment must occur once (similar to contants), and could not be recovered anymore;

I could not disagree more. This is only about "what it is".

Okay, it could be about "what it is", and it is an assignment (I mean the internal part like strict_types = 1), once that you assign a value to an internal compiler property via userland code.

And your arguments like "it acts like constant" are very good reason to use different rules for declare() to distinguish the compiler directives from constant assignments.

It doesn't make any sense. declare() could acts as a language constructor or a flow control constructor (when it is declare(...) { ... }). Then if we use like the second case, it should be declare (...) { ... } (like for (...) { ... }), but if first case should be declare(...), but ops... It seems a function! We must distinguish it to not confuses the code developer to make sure that declare<space?>() is not a function.

@Jean85
Copy link
Copy Markdown
Member

Jean85 commented May 18, 2018

@rentalhost you're mixing up a lot of names...

That comes from PSR-2 1.8 ("Control structure keywords MUST have one space after them; method and function calls MUST NOT") -- and I hope that control structure here could includes too the echo (although that it is a language constructor). But I think that in that point we are according.

Nope, those are language constructs, and we are going to cover them explicitly in an other addition to the PSR. Currently we have #1021 pending, but we will probably change that a little.

No, it should sounds like: assignments could/must occur in constants, variables or declarations.

That's not what the PSRs says, and that's not what happens with a compiler directive like strict_types. It's not an assignment and, from the little that I understand of programming languages and PHP internals, even the handling of those parts is hugely different: hence, the difference in styling.

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.

8 participants