DeclareStrictTypesFixer split

[keradus]

So far, we have a (risky) fixer that adds/relocate declare(strict_types=1).

I think we should split it.

1. fixer to add declare statement if missing (risky)
2. fixer to relocate declaration (non-risky) - here, with option what should be the position (same line with opening tag or next one).

We will be more generic and allow to have better user experience, like allowing to move declaration to same place for all files but not enforcing adding it to every single file. Or allow to place the declaration to place defined by Extended Coding Style Guide proposal.

Also, a ref: https://github.com/php-fig/fig-standards/blob/master/proposed/extended-coding-style-guide.md#3-declare-statements-namespace-and-use-declarations

Each declare statement (e.g. declare(ticks=);) MUST be on its own line.
When the opening <?php tag is on the first line of the file, it MUST be on its own line with no other statements

[keradus]

cc @Seldaek @SpacePossum discussion/help needed

[SpacePossum]

Hmmm....
We have been discussing the placement of both the header, strict type declaration and namespace location in various PR's and issues. PSR does help a bit, but also seems incomplete on new features.

Maybe we could take a diff route:

• create a new fixer, something like start_of_file-fixer (terrible name, but for the sake of discussion)
• remove the relocate part of the strict type fixer
• remove the relocate part of the header fixer and the "spacing above" part
• remove the blankline_after_open_tag fixer
• remove the newline_after_open_tag fixer

The new fixer can be configured to order the first parts of a PHP file;

• header
• declare strict (note: must always be the first statement)
• namespace

Together with:

• empty line after open tag
• empty line after/before header
• empty line after/before namespace

This will result in no more conflicts and should give everyone a way to configure what they want.

(for example #1993 (comment) would be fixed as well)

[keradus]

I like the idea. But it looks like a lot of work.
So, one way or another, DeclareStrictTypesFixer needs to be split first.

[sstok]

Can we at least for now place the declare(strict_types=1); on it's own line?
That's also what is proposed in the psr document.

I'm facing a conflict between StyleCI and php-cs-fixer, because php-cs-fixer wants to keep it on the opening tag line, but StyleCI insists on placing it on it's own line.

@GrahamCampbell is StyleCI using a custom fixer for this? because it should not give a different result.

[SpacePossum]

ping @GrahamCampbell is there a conflict between the current fixers or does StyleCI have a custom fixer which is in conflict?

[Arkemlar]

Is there any progress so far? I'd really like to make my files look like:
<?php declare(strict_types=1);
and so fixer wont move declare statement anywhere.

[kubawerlos]

@Arkemlar do you mean like DeclareAfterOpeningTagFixer?

[keradus]

@Arkemlar , no, as ppl were not finding this important enough. Feel free to raise the PR proposal.

@kubawerlos , if you have anything ready, I encourage to open a PR

[kubawerlos]

@keradus it only does half of point 2, so not really ready :)

[heiglandreas]

When making any changes to the way declare is handled, it would be awesome when the rule would not be applied to Interfaces as a declare doesn't make any sense in an interface.

See note at https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.strict

Note:

Strict typing applies to function calls made from within the file with strict typing enabled, not to the functions declared within that file. If a file without strict typing enabled makes a call to a function that was defined in a file with strict typing, the caller's preference (coercive typing) will be respected, and the value will be coerced.

[julienfalque]

@heiglandreas Thanks for the suggestion! I opened #6688 as it can be implemented separately.

[Arkemlar]

If someone needs just only feature of having declare(strict_types=1) on same line as php opening tag:
<?php declare(strict_types=1)
Then look at this one #4252
And at the bottom you will find a link to https://github.com/kubawerlos/php-cs-fixer-custom-fixers that has DeclareAfterOpeningTagFixer rule beside other useful rules. It solves my need just fine.

Upd: just noticed @kubawerlos 's answer about that right after my previous message up there.😄 Awesome job with that fastidious rules for such a clean-code-paranoids like me!

[kubawerlos]

@heiglandreas can you help me a bit? I've seen that note you have copied from https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.strict:

Note:

Strict typing applies to function calls made from within the file with strict typing enabled, not to the functions declared within that file. If a file without strict typing enabled makes a call to a function that was defined in a file with strict typing, the caller's preference (coercive typing) will be respected, and the value will be coerced.

and got surprised because I thought it is not how strict types are working.

So I've tried to do it myself at https://github.com/werlos/php_strict_types_test and cannot make it work the way it is - at least with my understanding - described in the note. What am I doing wrong?

[heiglandreas]

Thanks for bringing that up.

And I have no good explanation for that issue that you are scetching out. Perhaps that is something for the bugtracker of the php-src.

The reason why I brought up that citation though was because the declare_strict is only relevant within files that actually call other functions. As an interface does - by definition - only define function-signatures and never calls other functions. A declare(stict_types) is therefore always unnecessary in an interface-file.

[Wirone]

Just for the record: in #7053 we added ability to place namespace declaration in the same line as <?php or to enforce it in consequent lines. Probably we need similar flexibility for declare() 🙂.

[github-actions]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

[github-actions]

The fact this was automatically closed doesn't mean that the idea got rejected - it simply didn't get any priority for way too long to keep it open. If you're still interested in this, please let as know, we can consider re-opening it.