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

Fix all PHPStan Level 4 problems #110

Closed
wants to merge 5 commits into from

Conversation

szepeviktor
Copy link
Contributor

@szepeviktor szepeviktor commented Jan 17, 2021

 ------ ---------------------------------------------------------------------------------------------
  Line   Mover.php
 ------ ---------------------------------------------------------------------------------------------
  73     Access to an undefined property CoenJacobs\Mozart\Composer\Autoload\Autoloader::$namespace.
 ------ ---------------------------------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------------------
  Line   Replace/NamespaceReplacer.php
 ------ ---------------------------------------------------------------------------------------------------
  22     Call to an undefined method CoenJacobs\Mozart\Composer\Autoload\Autoloader::getSearchNamespace().
 ------ ---------------------------------------------------------------------------------------------------

These interface-things are the 2 remaining ones.
Please use @phpstan.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jan 17, 2021

💡 The fun begins on Level 5.

@coenjacobs
Copy link
Owner

Thank you @szepeviktor, I have merged this with the latest master branch as I have today introduced Psalm (via #112) in our CI chain. This is merely a personal preference, as I think PHPStan and Psalm basically come down to the same principle.

As it turns out, Psalm found an issue with one of your changes and I don't know why the tests are failing. Will have to investigate a little bit. Might have to do with the fact that Psalm already changed some parts of the code that you also did and the changes might not add up any longer. Will have a look soon, but if you can have a look and update based on my merge commit, please let me know.

@coenjacobs coenjacobs added this to the 0.7.0 milestone Jan 17, 2021
@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jan 17, 2021

Okay. I leave it up to you as Psalm has an unsustainable origin and I dug deep into PHPStan.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jan 17, 2021

Actually I've spent hundreds of hours advocating PHPStan...

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jan 17, 2021

Regarding

src/Composer/Autoload/NamespaceAutoloader.php#L29
$this->paths with declared type 'array<array-key, string>' cannot be assigned type 'non-empty-array<array-key, array<array-key, mixed>|string>'

autoload can be many different thing https://getcomposer.org/doc/04-schema.md#autoload not just strings.

@coenjacobs
Copy link
Owner

coenjacobs commented Jan 17, 2021

@szepeviktor Care to elaborate on this statement?

Psalm has an unsustainable origin

I'm not saying that Psalm is perfect, it's just a tool to run static analysis. If there are any real issues with Psalm, that I'm obviously not aware of, I'm open to changing. But I need some more motivation on that, other than just your word. So some links on the topic, or an educated opinion are very much appreciated.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jan 17, 2021

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.

2 participants