Skip to content

Analysis: determine whether to revive of not #2

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

Open
jrfnl opened this issue Nov 8, 2023 · 3 comments
Open

Analysis: determine whether to revive of not #2

jrfnl opened this issue Nov 8, 2023 · 3 comments

Comments

@jrfnl
Copy link
Member

jrfnl commented Nov 8, 2023

The original PHP_CodeSniffer project contained a code consistency analysis website: https://squizlabs.github.io/PHP_CodeSniffer/analysis/

That website hadn't been updated since 2018.

For now, I've removed the site from the gh-pages branch of this repo.

I'm not sure if there is still interest in the analysis website. If so, reviving it under a new (sub)domain would be an option, though the code to generate the site would probably need revision and a critical look as to which projects should be analyzed.

This ticket is intended to measure interest.

Leave a 👍🏻 if you are interested in having this analysis website revived.
Leave a comment if you want to get involved in reviving the analysis website.

Refs:

@jrfnl
Copy link
Member Author

jrfnl commented Nov 8, 2023

Also see: squizlabs/PHP_CodeSniffer#3634

@jrfnl
Copy link
Member Author

jrfnl commented Nov 22, 2023

My current line of thinking about this ticket is as follows (provided there would be enough interest):

  • Create a separate repo for the analytics project.
    This repo would effectively create a GH Pages website under a subdomain of the phpcodesniffer.com domain.
  • Move the original code used to generate the site to that repo (note: not the website, which was previously also committed).
  • Review and update that code, keeping in mind that that project does not need to comply with the PHP 5.4 minimum of the main PHPCS project.
    Things to review the code for:
  • Create a workflow to automatically generate the website which can be triggered manually (on a new PHPCS release) and also runs via a monthly/weekly cronjob to keep the site up to date automatically.

Once the initial work is done for this, it should then be (a lot) more straight-forward to keep the site running and up-to-date.

@jrfnl
Copy link
Member Author

jrfnl commented Apr 22, 2024

Some brain farts related to the analytics website:

  • It is not clear what criteria were used historically for which projects to add to the projects list.
    Might be an idea to set criteria for this going forward. I'm currently thinking: the top 500 projects in Packagist.
    This will mean that some projects (projects in the Top 500 which were previously already included) will have history and some don't, but then again, that history could be recreated anyway, if needs be.
  • It is not clear what ruleset projects were evaluated against, though I expect this will become clear once someone studies the code of the project.
    It makes sense to evaluate each project against the same ruleset containing only sniffs which record metrics. This makes the projects comparable and it allows for aggregate results.
    At the same time, it might be nice to also allow for additional sniffs a project uses in their own ruleset if those sniffs provide metrics.
  • It would be nice - though this could be a separate side-project - to have some insight into the frequency of use for sniffs. This information could be used to inform the PHPCS project for which sniffs could be candidates for removal.
    This could possibly be done by having a complete sniff list of all sniffs in PHPCS and aggregating how often each sniff is encountered in PHPCS rulesets used by those projects in the top 2000 Packagist projects which use PHPCS (via an -e explain).

jrfnl added a commit that referenced this issue Jan 23, 2025
This adds another set of dedicated tests to safeguard how XML docs which don't follow the specification are handled.

This initial set of tests for this documents the current behaviour [*]. This behaviour may not always be the desired behaviour, in which case, this will be fixed in follow-up commits.

To get these tests up and running, the following fatal errors needed to be fixed:

* Fatal error when a code comparison only contains a single code element:
    ```
    Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:253
    Stack trace:
    #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement))
    #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement))
    #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate()
    #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS()
    #4 {main}
      thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 253
    ```

* Fatal error when a code element contains no textual content:
    ```
    Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:246
    Stack trace:
    #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement))
    #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement))
    #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate()
    #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS()
    #4 {main}
      thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 246
    ```

Both of these fatals are fixed by adding defensive coding validating that there are (at least) two code blocks to the `getFormattedCodeComparisonBlock()` methods for all three generator classes.
jrfnl added a commit that referenced this issue Feb 14, 2025
This adds another set of dedicated tests to safeguard how XML docs which don't follow the specification are handled.

This initial set of tests for this documents the current behaviour [*]. This behaviour may not always be the desired behaviour, in which case, this will be fixed in follow-up commits.

To get these tests up and running, the following fatal errors needed to be fixed:

* Fatal error when a code comparison only contains a single code element:
    ```
    Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:253
    Stack trace:
    #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement))
    #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement))
    #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate()
    #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS()
    #4 {main}
      thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 253
    ```

* Fatal error when a code element contains no textual content:
    ```
    Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:246
    Stack trace:
    #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement))
    #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement))
    #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate()
    #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS()
    #4 {main}
      thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 246
    ```

Both of these fatals are fixed by adding defensive coding validating that there are (at least) two code blocks to the `getFormattedCodeComparisonBlock()` methods for all three generator classes.
jrfnl added a commit that referenced this issue Feb 14, 2025
This adds another set of dedicated tests to safeguard how XML docs which don't follow the specification are handled.

This initial set of tests for this documents the current behaviour [*]. This behaviour may not always be the desired behaviour, in which case, this will be fixed in follow-up commits.

To get these tests up and running, the following fatal errors needed to be fixed:

* Fatal error when a code comparison only contains a single code element:
    ```
    Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:253
    Stack trace:
    #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement))
    #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement))
    #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate()
    #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS()
    #4 {main}
      thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 253
    ```

* Fatal error when a code element contains no textual content:
    ```
    Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:246
    Stack trace:
    #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement))
    #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement))
    #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate()
    #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS()
    #4 {main}
      thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 246
    ```

Both of these fatals are fixed by adding defensive coding validating that there are (at least) two code blocks to the `getFormattedCodeComparisonBlock()` methods for all three generator classes.
jrfnl added a commit that referenced this issue Apr 10, 2025
This fixes the following fatal error which would occur if the `Diff` report is used in combination with caching:
```
Fatal error: Uncaught Error: Call to a member function getFixableCount() on null in path/to/PHP_CodeSniffer/src/Fixer.php:144
Stack trace:
#0 path/to/PHP_CodeSniffer/src/Reports/Diff.php(73): PHP_CodeSniffer\Fixer->fixFile()
#1 path/to/PHP_CodeSniffer/src/Reporter.php(285): PHP_CodeSniffer\Reports\Diff->generateFileReport(Array, Object(PHP_CodeSniffer\Files\LocalFile), true, 150)
#2 path/to/PHP_CodeSniffer/src/Runner.php(659): PHP_CodeSniffer\Reporter->cacheFileReport(Object(PHP_CodeSniffer\Files\LocalFile))
#3 path/to/PHP_CodeSniffer/src/Runner.php(400): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile))
#4 path/to/PHP_CodeSniffer/src/Runner.php(119): PHP_CodeSniffer\Runner->run()
#5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS()
#6 {main}
  thrown in path/to/PHP_CodeSniffer/src/Fixer.php on line 144
```
jrfnl added a commit that referenced this issue Apr 10, 2025
…report

Okay, so this is an awkward one.

If both the `Code` + the `Diff` report were requested + caching was turned on + the _order_ of the requested reports was `Code,Diff` (i.e. first the code report), the following fatal error which would occur:
```
Fatal error: Uncaught Error: Call to a member function getFixableCount() on null in path/to/PHP_CodeSniffer/src/Fixer.php:144
Stack trace:
#0 path/to/PHP_CodeSniffer/src/Reports/Diff.php(73): PHP_CodeSniffer\Fixer->fixFile()
#1 path/to/PHP_CodeSniffer/src/Reporter.php(285): PHP_CodeSniffer\Reports\Diff->generateFileReport(Array, Object(PHP_CodeSniffer\Files\LocalFile), true, 150)
#2 path/to/PHP_CodeSniffer/src/Runner.php(659): PHP_CodeSniffer\Reporter->cacheFileReport(Object(PHP_CodeSniffer\Files\LocalFile))
#3 path/to/PHP_CodeSniffer/src/Runner.php(400): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile))
#4 path/to/PHP_CodeSniffer/src/Runner.php(119): PHP_CodeSniffer\Runner->run()
#5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS()
#6 {main}
  thrown in path/to/PHP_CodeSniffer/src/Fixer.php on line 144
```

To reproduce the issue (on `master`):
1. Create a small test file like:
    ```php
    <?php

    echo 'hello'.callMe( $p );
    ```
2. Run the following command to initialize the cache:
    ```bash
    phpcs -ps ./test.php --standard=PSR2 --report=Code,Diff --cache
    ```
    All should be fine.
3. Now run the same command again and see the fatal error.

Some investigating later, it turns out that both the `Code` report, as well as the `Diff` report, re-parse the current file, with the `Diff` report initializing the fixer once the file has reparsed, but the `Code` report not doing so (as it doesn't need the fixer).
The problem with that is that the `Code` report basically leaves the `Fixer` in an invalid state, leading to the above error.

While it is up for debate whether reports should ever (be allowed to) re-parse files, for now, let's fix the fatal.

Re-evaluating the report setup should definitely be done, but should probably wait until the Reports classes have test coverage.
jrfnl added a commit that referenced this issue Apr 13, 2025
…report

Okay, so this is an awkward one.

If both the `Code` + the `Diff` report were requested + caching was turned on + the _order_ of the requested reports was `Code,Diff` (i.e. first the code report), the following fatal error which would occur:
```
Fatal error: Uncaught Error: Call to a member function getFixableCount() on null in path/to/PHP_CodeSniffer/src/Fixer.php:144
Stack trace:
#0 path/to/PHP_CodeSniffer/src/Reports/Diff.php(73): PHP_CodeSniffer\Fixer->fixFile()
#1 path/to/PHP_CodeSniffer/src/Reporter.php(285): PHP_CodeSniffer\Reports\Diff->generateFileReport(Array, Object(PHP_CodeSniffer\Files\LocalFile), true, 150)
#2 path/to/PHP_CodeSniffer/src/Runner.php(659): PHP_CodeSniffer\Reporter->cacheFileReport(Object(PHP_CodeSniffer\Files\LocalFile))
#3 path/to/PHP_CodeSniffer/src/Runner.php(400): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile))
#4 path/to/PHP_CodeSniffer/src/Runner.php(119): PHP_CodeSniffer\Runner->run()
#5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS()
#6 {main}
  thrown in path/to/PHP_CodeSniffer/src/Fixer.php on line 144
```

To reproduce the issue (on `master`):
1. Create a small test file like:
    ```php
    <?php

    echo 'hello'.callMe( $p );
    ```
2. Run the following command to initialize the cache:
    ```bash
    phpcs -ps ./test.php --standard=PSR2 --report=Code,Diff --cache
    ```
    All should be fine.
3. Now run the same command again and see the fatal error.

Some investigating later, it turns out that both the `Code` report, as well as the `Diff` report, re-parse the current file, with the `Diff` report initializing the fixer once the file has reparsed, but the `Code` report not doing so (as it doesn't need the fixer).
The problem with that is that the `Code` report basically leaves the `Fixer` in an invalid state, leading to the above error.

While it is up for debate whether reports should ever (be allowed to) re-parse files, for now, let's fix the fatal.

Re-evaluating the report setup should definitely be done, but should probably wait until the Reports classes have test coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant