The road to version 9.0.0

[oliverklee]

Hi @JakeQZ @sabberworm,

I'd like to propose a battle plan to make version 9.0.0 as stable, reliable and maintainable as possible. What do you think?

These are the steps (in order) I'd like (us) to tackle:

1. switch to a current coding standard (PER2) to make the code more readable and consistent with other projects
2. create a class diagram (in Mermaid embedded in Markdown) to help us understand the system
3. make the code coverage publicly visible, add a link or badge from the README to the code coverage reports, and have a tool report the effects of PRs on the code coverage numbers
4. comb through the existing tests, make them more fine-grained, clean them up and divide them into unit tests (which test only one class at a time) and functional tests (which test the whole system)
5. add proper dedicated getters and setters to OutputFormat and drop/deprecate the dynamic property accesses and magic methods
6. add more basic unit tests so we have at least the basic functionality covered
7. announce on social media that we are looking for more contributors (and possibly co-maintainers, if the collaboration is constantly good)
8. go through the open PRs one by one, from old to new, decide if we want to have it at all, and re-create it with good test coverage (and then merge it)
9. go through the open tickets and categorize or close them
10. fix the bugs reported in the tickets
11. hard-require the mbstring PHP extension to reduce complexity (and also because it's very hard to automatically test the paths that are intended to work without mbstring)
12. drop the switch for the lenient mode and instead use the behavior that matches what browsers do
13. use PHP 7.2 language-level code features (using Rector)
14. add native type declarations (using Rector)
15. do some code automated cleanup (using Rector)
16. raise the PHPStan levels one by one and fix all warnings
17. set up mutation testing using Infection, go through the findings, and add test for the most important missed things
18. add a PHPStan extension for cognitive complexity and refactor the code to be easier to grok
19. rename internal things to ditch the Hungarian notation
20. make the types less ambiguous and more strict
21. find and fix the worst performance bottlenecks
22. every now and then test the latest development version with Emogrifier
23. release version 9.0.0

[JakeQZ]

8. go through the open PRs one by one, from old to new, decide if we want to have it at all, and re-create it with good test coverage (and then merge it)

I've noticed a few waiting for review, so was planning to attempt to do so, though they are mostly not straightforward changes.

12. drop the switch for the lenient mode and instead use the behavior that matches what browsers do

I think lenient/strict mode is a misnomer. Lenient mode is what browsers do: silently drop invalid constructs and move on to the next (though info is made available in the browser developer tools). Strict mode effectively means throw an exception upon encountering invalid syntax: so it is more like a debugging aid (to help find why some elements are not being parsed). On that basis, I think the switch should be retained, but renamed 'debug' to make its intent clear.

Other items all seem worthy at first glance.

PS. Re renaming, I much prefer unit test data providers to be named provideXyz rather than xyzDataProvider. It's more succinct and a commonly-used convention across other projects. It makes it easier to spot which methods are the data providers, though maybe they should all be placed first - IDK. (I suppose if your cultural language has the verb at the end of the sentence, the latter may seem more natural :)

(Can't seem to reply more than once, so have to edit.)

[JakeQZ]

I think lenient/strict mode is a misnomer.

Both PHP and JavaScript have the concept of strict mode parsing of source files, which is where I think the name originates. I would never enable the JavaScript one in a production environment, since there's no guarantee that some or other browser version you haven't tested against will reject your code due their incorrect implementation and leave your site functionless (which has happened - thank you Microsoft), but while developing I enable it to catch potential problems. PHP is a bit different, as there is only one PHP intepreter (though multiple versions thereof). But in all cases, these are tools to catch problems during development, and of little or no value in a production environment - which is why browsers allow all sorts of invalid HTML and CSS.

[oliverklee]

I like your thoughts on data provider renaming and have just added #458 for this.

[oliverklee]

I've noticed a few waiting for review, so was planning to attempt to do so, though they are mostly not straightforward changes.

Thanks for driving this forward! I really appreciate that.

[sabberworm]

2. create a class diagram (in Mermaid embedded in Markdown) to help us understand the system

We already have the doxygen-generated diagram at https://myintervals.github.io/PHP-CSS-Parser/annotated.html, though it hasn’t been updated in a while. Should we keep generating some form of documentation?

11. hard-require the mbstring PHP extension to reduce complexity (and also because it's very hard to automatically test the paths that are intended to work without mbstring)

I think we can just forget about parsing in multibyte charsets. For single-byte charsets, if we encounter a @charset declaration (or if the charset in the settings is anything other than UTF-8), we just convert to UTF-8 using iconv and re-start parsing.
This way, we can use the regular string functions everywhere and no longer need mbstring at all.

12. drop the switch for the lenient mode and instead use the behavior that matches what browsers do

Agreed, but provide a debug mode which returns a log of problems.

✅ on any other point

[oliverklee]

I have now entered #461, #462 and #463 for dropping the switch for the lenient mode.

[oliverklee]

I have now entered #464 and #465 for dropping usage of the mb_* methods.

[oliverklee]

We already have the doxygen-generated diagram at https://myintervals.github.io/PHP-CSS-Parser/annotated.html, though it hasn’t been updated in a while. Should we keep generating some form of documentation?

I don't think the class documentation generated from the PHPDoc brings much addded benefit nowadays anymore (now that IDEs support PHPDoc code documentation really well). So I think we can drop this. I've added #466 for this.

What I was thinking of was a graphical UML diagram (using mermaid) so it's visible at a glance which classes inherit from or implement which, and how classes are interconnected.

[oliverklee]

Thank you, @JakeQZ and @sabberworm, for your thoughts and insights! I really enjoy working together with you on this project! ❤️

[oliverklee]

I have just converted the list above (plus updates) into a checklist and added it to the milestone. (You'll need to expand the description so see it as a checklist.)