Enforce leading backslash even in non-yet-namespaced classes #9680
Conversation
mvorisek
force-pushed
the
fqcn
branch
3 times, most recently
from
bde43c7
to
ebe0838
Compare
mvorisek
changed the title
|
@alecpl I have rebased this PR, can it be merged to prevent further merge conflicts? |
|
I think I would prefer a |
mvorisek
force-pushed
the
fqcn
branch
2 times, most recently
from
08b3865
to
329427a
Compare
regex: (?<=\n)(use )(\w+);
|
@alecpl thank you for the idea using import to minimize conflicts! So I reworked this PR: After #9481 the dummy imports will be replaced with meaningful imports. Code can stay as it is, so conflicts across release branches are minimal. Leading |
mvorisek
changed the title
| use enigma_userid as enigma_userid; | ||
| use rcmail as rcmail; | ||
| use rcube as rcube; | ||
| use rcube_user as rcube_user; |
There was a problem hiding this comment.
What's up with these. Why not just a use without as? It is temporary, I hope?
There was a problem hiding this comment.
We need use because of https://github.com/roundcube/roundcubemail/blob/5dcb791647/.php-cs-fixer.dist.php#L83, the config lines will be removed once the files and properly namespaced and the use changes to namespaced classes.
We need the use X as X; to suppress php warning - https://3v4l.org/dS4TX
| @@ -327,7 +343,7 @@ public function create_message($headers, $body, $isHtml = false, $attachments = | |||
| } | |||
|
|
|||
| // create PEAR::Mail_mime instance | |||
| $MAIL_MIME = new Mail_mime("\r\n"); | |||
| $MAIL_MIME = new \Mail_mime("\r\n"); | |||
There was a problem hiding this comment.
For cases like this, when we use an external class many times, I'd like to see a use statement too.
There was a problem hiding this comment.
The convection in php community is to use backslash /wo import if the classes are not namespaced.
It makes sense, as for ex. catch (\Exception $e) as much more readable than catch (Exception $e) - in the later case, you might think it catches only project's exceptions.
Is this ok to keep?
If you worry stictly about backport conflicts, I can make a followup PR into v1.6 branch.
|
I'm not happy with this change. Where does it lead to, is there an end goal? Are we going to make a big BC break, call it Roundcube 2.0, where e.g. |
|
@alecpl good questions. We should namespace the classes sooner or later. As doing so will be imply BC break, I created #9481 which namespace the classes, but at the same time provide BCible layer for loading the classes as non-namespaced. So these PRs which might look like a mess are actually very precisely crafted to allow smooth transition. As #9481 might be hard to review/discuss at once, I created this preparatory PR which focuses solely on enabling
In #9481 I have demonstrated it working incl. the BCible autoloading layer. Nothing is broken, everything works. If you support this, I would be happy if this PR can be merged as is. This PR is safe and does not imply any BC breaks. It is pure CS update. |
|
@alecpl does this work for you? |
extracted from #9481
let's land this in a separate PR and minimize future diffs/conflicts