rectorphp / swiss-knife

Swiss knife in pocket of every upgrade architect!
https://getrector.com
MIT License
72 stars 8 forks source link

Support root-namespace parent classes #19

Closed staabm closed 1 week ago

staabm commented 3 months ago

before this PR, base-classes without a namespace were not properly detected

(they were skipped, as they were mistakenly detected as a php-src native class)

staabm commented 3 months ago

ping @TomasVotruba :)

TomasVotruba commented 3 months ago

I'm not sure what is being changed here :laughing:

The reflection should be avoided to keep this analysis static.

staabm commented 3 months ago

a naiv "php src native classes detection" was used before. I don't know why it was there, but with this PR the tool learnt to differentiate between userland classes which are not part in any namespace and real builtin classes.

at least thats what I guess the code beeing changed tried todo.

I am doing this change because before this PR the finalize tool put final on way too many classes because it assumed all userland classes would be in a namespace

staabm commented 3 months ago

The reflection should be avoided to keep this analysis static.

I just dropped the "native class detection" code which did not serve a purpose. I guess it was a premture performance optimization

TomasVotruba commented 4 weeks ago

I'm checking this after while, as I'm still not sure what is the fix for.

Please send a failing test fixture for the invalid final completion. Then I can see what is being fixed clearly and merge this :+1:

staabm commented 1 week ago

a file with content

<?php

class X {}

class Y extends X {}

class Z extends ArrayObject {}

before this PR is fixed into

<?php

final class X {}

final class Y extends X {}

final class Z extends ArrayObject {}

but I expected class X not to get final. Its hard to unit-test because all tests atm require a namespace.

(I am still looking into why the fix atm does not work)

staabm commented 1 week ago

I don't know why the PR looks like it does (too much time went by until I created it) and therefore I opened a issue instead so its not forgotten.

will close here