Skip to content
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

[3.8] Fix fatal error on Stub generator script #17925

Closed
wants to merge 1 commit into from
Closed

[3.8] Fix fatal error on Stub generator script #17925

wants to merge 1 commit into from

Conversation

joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

This small PR fixes fatal error while running Stub generator script introduced by PR #16291

Testing Instructions

Follow the instructions at #16291 to run the script. Before patch, you will get fatal error. After path, the error is gone, stubs.php is generated properly.

Additional comment

There is still an issue with the generated subs.php file. The class name is in lower case

class jregistry extends \Joomla\Registry\Registry {}

instead of

class JRegistry extends \Joomla\Registry\Registry {}

as expected. It causes by some changes in JLoader in this PR #17834 .

@Bakual
Copy link
Contributor

Bakual commented Sep 29, 2017

The fatal error is due to a bug in PHP 7. Update to PHP 7.1 and the stubGenerator will work.

I'm not sure why you think the lowercasing of the class name is an issue. In PHP class names are case insensitive, so this shouldn't matter at all.

@joomdonation
Copy link
Contributor Author

The fatal error is due to a bug in PHP 7. Update to PHP 7.1 and the stubGenerator will work.

Ah, thanks. I still run on PHP 7.0.4 on my local computer.

I'm not sure why you think the lowercasing of the class name is an issue. In PHP class names are case insensitove, so this shouldn't matter at all.

In my case, the IDE still complains Class reference is not case-sensitive, so I think it is better to have class names in correct case (no complains from IDE at all)

Since the main issue fatal error only happens with PHP 7.0.x, I am closing this PR.

@Bakual
Copy link
Contributor

Bakual commented Sep 29, 2017

When the alias is registered, it is cast to lowercase: https://github.com/joomla/joomla-cms/blob/staging/libraries/loader.php#L381

That's why the alias are lowercased in the stubs file as well. I don't think we can change that for the stubs file.

@joomdonation
Copy link
Contributor Author

Yes, I see that, it causes by the change in PR #17834 as I mentioned in this PR description. There is a way to fix it (require changes to JLoader), but since it is not a big thing (and it is not safe to change JLoader at this time), so we will leave it as how it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants