Move functions.php into Framework

[fooman]

Description

functions.php provides the custom Magento function __() to trigger translations. For some unknown reason this file lives under app/code. As such it is only distributed via the full magento distribution package. This in turn makes it a lot harder for developers to work with Magento in a modular fashion.

Ideally I want to be able to run .
composer require magento/framework .
run my unit tests

Due to __() only being available in the complete distribution I need to require all of Magento for 1 single file. A difference of 100+ modules.

I currently do not see any rationale for why this file is not part of any module. This PR proposes to move it into the Framework module. It also simplifies the bootstrapping processing as this file can be autoloaded. Happy to move it into any other module if desired.

Fixed Issues (if relevant)

1. N/A

Manual testing scenarios

1. install Magento
2. install any language pack, for example composer require splendidinternet/mage2-locale-de-de
3. enable locale and ensure translations still work

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

Port for: #16800

[magento-cicd2]

All committers have signed the CLA.

[magento-engcom-team]

Hi @fooman. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[orlangur]

I need to require all of Magento for 1 single file. A difference of 100+ modules.

This is not critical due to Composer cache.

@fooman please clarify steps: how can we run Magento module without Magento application itself?

[fooman]

My module should only require on modules that it needs to work.

So for example

    "require": {
        "php": "~5.6.0|7.0.2|7.0.4|~7.0.6|~7.1.0",
        "magento/framework": "^100.0 | ^101.0.0",
        "magento/module-backend": "^100.1.0",
        "magento/module-sales": "^100.1.0 | ^101.0.0"
    },

should be sufficient for my module to declare all it's dependencies. Due to the homeless functions.php I would need to use magento/magento2-base which does away with any modularity.

Another argument for the above change is that technically nearly all magento/module-* currently have an undeclared dependency on functions.php.

This is for running unit tests which should not require the complete Magento application.

[fooman]

This is not critical due to Composer cache.

Aside from relying on a cache as a solution it makes a huge difference for PhpStorm if it needs to index 100 modules or only 1 though.

[orlangur]

PhpStorm if it needs to index 100 modules or only 1 though.

Still quite fast, isn't it? Is it possible to have magento-base as external library instead of indexing it in vendor folder maybe?

Do you have any idea how to get rid of __ function at all (not like a ready-to-go plan but maybe some ideas)?

[fooman]

Do you have any idea how to get rid of __ function at all (not like a ready-to-go plan but maybe some ideas)?

I think the __() function as a convenience function for translations is fine. Replacing them would not really be an option until Magento 3. It's used about 3000 times in the code.

[orlangur]

Replacing is not a big problem, at least we can deprecate it and not use in new code.

New translation mechanism should provide some killer features like context-dependent translation or something.

This is for running unit tests which should not require the complete Magento application.

Ok, that's another case, but to have a proper unit tests bootstrap you should require base anyway.

Can we have this function as a separate Composer component? Probably bootstrap for unit tests or at least additional logic on top of Phrase should be refactored the same way.

__ is not used within framework, it is purely application-specific syntax sugar thus it is not logical to put it in framework. On the other hand we don't have a stinky module like Mage_Core nowadays, probably Magento_Translation would be a good place but I would like to see it as separate microcomponent if possible.

[orlangur]

@fooman ok, looks like I've figured out a solution I would really like.

1. Please put this function into Magento/Framework/Phrase/__.php (to highlight we do not encourage global functions creation)
2. Require it in old app/functions.php marking it as deprecated and in registration.php of framework with conditional if (!function_exists('__') { require Magento/Framework/Phrase/__.php}) (there is no need in require_once)
3. Apply to 2.2-develop first as it is completetly backward-compatible

[orlangur]

Then /app/functions.php can be removed in 2.4-develop (you can mention this together with deprecation comment). The common rule is to have something deprecated for a complete minor release (from 2.X.0 to 2.X.MAX) and only after this removed in 2.X+1.0.

[fooman]

@orlangur good idea on the naming. I have updated this pull request (squashed commits) with your suggestions as well as created #16800 for 2.2

Installations via Composer (ie vendor/magento/framework)
They should be fine with the autoload Phrase/__.php

Installations via git clone (ie lib/internal/Magento/Framework)
The NonComposerComponentRegistration will kick in and find the new Phrase\registration.php (this would not be loaded via a Composer install so we have the benefit of avoiding it when we don't have to)

Other uses utilising the old app/etc/functions.php and expecting __() to be available as a result.
The conditional requires depending on installation method should take care.

[fooman]

Closing this pull request in favour of forward porting the final implementation in #16800