Skip to content

Display a more meaningful error message in case of misspelt module name #12843

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

Merged
merged 5 commits into from
Feb 15, 2018

Conversation

JanisE
Copy link
Contributor

@JanisE JanisE commented Dec 21, 2017

Description

Displays a more meaningful error message in case of misspelt module name.

[LogicException] Component 'VendorA_ModuleB' of type '' is not correctly registered.
instead of
[Magento\Framework\Exception\FileSystemException] The file "/composer.json" doesn't exist

Manual testing scenarios

  1. Create a new module and misspell its name in "registration.php" (e.g., 'Vendora_Popup' while the directory is named 'Vendora/Popups').
  2. Run bin/magento module:disable Magento_Bundle (or bin/magento module:enable Magento_Bundle, if it is disabled); yes, it does not have to be the same incorrectly configured module, any will do.
  3. You'll get the mentioned error.

[LogicException] Component 'VendorA_ModuleB' of type '' is not correctly registered.
instead of
[Magento\Framework\Exception\FileSystemException] The file "/composer.json" doesn't exist
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Dec 21, 2017

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ JanisE
❌ Jānis Elmeris


Jānis Elmeris seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

@@ -45,6 +45,10 @@ public function getDir($moduleName, $type = '')
{
$path = $this->componentRegistrar->getPath(ComponentRegistrar::MODULE, $moduleName);

if (! isset($path)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove space before isset function, it's not needed there

@@ -45,6 +45,10 @@ public function getDir($moduleName, $type = '')
{
$path = $this->componentRegistrar->getPath(ComponentRegistrar::MODULE, $moduleName);

if (! isset($path)) {
throw new \LogicException("Component '$moduleName' of type '$type' is not correctly registered.");
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is not backward compatible, you're adding new exception type that could be thrown. I think it would be better to use \InvalidArgumentException in order to keep backward compatibility there.

@ihor-sviziev ihor-sviziev self-requested a review December 28, 2017 14:28
@magento-engcom-team magento-engcom-team added this to the February 2018 milestone Feb 15, 2018
@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, thank you for the review.
MAGETWO-87908 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@JanisE thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

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.

4 participants