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

Bump static analysis #365

Merged
merged 6 commits into from
Nov 1, 2021

Conversation

VincentLanglet
Copy link
Member

@VincentLanglet VincentLanglet commented Oct 10, 2021

Subject

closes #236

Changelog

### Deprecated
- Passing null to `DoctrineORMAdapter::getNormalizedIdentifier()`
- Passing null to `DoctrineORMAdapter::getUrlSafeIdentifier()`
- Passing null to `DoctrinePHPCRAdapter::getNormalizedIdentifier()`
- Passing null to `DoctrinePHPCRAdapter::getUrlSafeIdentifier()`
- `BaseDocumentManager::__get()` method
- `BasePHPCRManager::__get()` method
- `BaseEntityManager::__get()` method
- `ManagerInterface::getTableName()` method
- `ManagerInterface::getConnection()` method
- `BaseManager::getTableName()` method
- `BasePHPCRManager::getTableName()` method
- `BasePHPCRManager::getConnection()` method
- `BaseEntityManager::getConnection()` method
- `BaseDocumentManager::getConnection()` method

@VincentLanglet
Copy link
Member Author

Does someone (@sonata-project/contributors ) is familiar with
https://github.com/sonata-project/sonata-doctrine-extensions/blob/1.x/src/Mapper/DoctrineCollector.php#L119-L130
and
https://github.com/sonata-project/sonata-doctrine-extensions/blob/1.x/src/Mapper/ORM/DoctrineORMMapper.php#L68-L83

One method is doing

$this->associations[$class][$type][] =

the other is doing

$this->associations[$class][$type] =

Is there a bug or does it means that DoctrineORMMapper::addAssociation can only be called once and should pass array<array<string, mixed>> maybe ?

@VincentLanglet
Copy link
Member Author

Looking at the usage, the DoctrineCollector seems right
https://github.com/sonata-project/SonataPageBundle/blob/5bd4b5cde9833b25ce15e323a9fc5a7b69d18d3d/src/DependencyInjection/SonataPageExtension.php#L543-L561

So I updated the phpdoc of the DoctrineORMMapper to follow the same strategy.

But I found no usage of DoctrineORMMapper, should we deprecate this class ?

@VincentLanglet
Copy link
Member Author

Please review @sonata-project/contributors

@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

composer.json Outdated
@@ -70,5 +74,13 @@
"psr-4": {
"Sonata\\Doctrine\\Tests\\": "tests/"
}
},
"scripts": {
Copy link
Member

Choose a reason for hiding this comment

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

This should be adapted

@VincentLanglet VincentLanglet requested review from jordisala1991 and a team November 1, 2021 14:24
src/Adapter/ORM/DoctrineORMAdapter.php Outdated Show resolved Hide resolved
src/Document/BasePHPCRManager.php Outdated Show resolved Hide resolved
src/Entity/BaseEntityManager.php Outdated Show resolved Hide resolved
@@ -2,13 +2,12 @@ includes:
- phpstan-baseline.neon

parameters:
level: 5

level: max
Copy link
Member

Choose a reason for hiding this comment

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

we should stop using max, on phpstan 1.0 max is level 9, currently is level 8

Copy link
Member Author

Choose a reason for hiding this comment

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

Not a big deal since it require the bump from ^0.12 to ^1.0

Copy link
Member

Choose a reason for hiding this comment

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

Fine, it is either changing it now or later, most likely we are not going to pass level 9 that easy

@phansys phansys requested a review from a team November 1, 2021 16:39
@VincentLanglet VincentLanglet merged commit 0587eea into sonata-project:1.x Nov 1, 2021
@VincentLanglet VincentLanglet deleted the staticAnalysis branch November 1, 2021 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove ORM related function to agnostic interfaces
5 participants