WordPress / plugin-check

A repository for the new Plugin Check plugin from the WordPress Performance and Plugins Team.
https://wordpress.org/plugins/plugin-check/
GNU General Public License v2.0
261 stars 53 forks source link

CLI: fully structured output for easier parsing, send to a file #748

Open singerb opened 1 week ago

singerb commented 1 week ago

I'm integrating plugin-check into a testing pipeline via wp-cli, and consuming the output programmatically is a little annoying (though totally doable) at the moment. Two things that would make it easier:

swissspidy commented 1 week ago

Hi there,

If you run wp plugin check --help you'll see that there is a --format option that supports JSON as the output format. In other words, wp plugin check --format=json does exactly what you ask for. You can also pipe the output to a file if you like.

PS. If your testing is done on GitHub Actions, I'd recommend using https://github.com/WordPress/plugin-check-action directly.

singerb commented 1 week ago

If you run wp plugin check --help you'll see that there is a --format option that supports JSON as the output format. In other words, wp plugin check --format=json does exactly what you ask for.

This is exactly what I'm doing right now. The output looks like:

Status message.

Another status message.

FILE: src/my-file.php
<json blob of items for that file>
...

While yes, this is parse-able (and I'm doing so), it's not as easy as just full JSON output.

You can also pipe the output to a file if you like.

Again, I'm doing this already. It includes the status messages, as mentioned, which I would like to leave on the console and not have interleaved with the results.

PS. If your testing is done on GitHub Actions, I'd recommend using https://github.com/WordPress/plugin-check-action directly.

This is nice (and should be linked from this repo and the wordpress.org page more prominently), but not quite what I need for my use case. It's a good integration though!

swissspidy commented 1 week ago

Apologies, I misread then. Reopening.

ernilambar commented 1 week ago

Similar issue was raised when PCP was being integrated in the WP.org. Implementing there also output is parsed like you are currently doing.

May be we could add FILE value directly in the output as key so that we wont need separate line for FILE: filename

Eg: | File | Line | Column | Type | Code | Message | Docs |

swissspidy commented 1 week ago

That would be super repetitive though.

Honestly parsing the current format is not that bad, see https://github.com/WordPress/plugin-check-action/blob/0e9c1a736a7e3c8a32eaafe4db4a694f064be80b/src/main.ts#L24-L45

ernilambar commented 1 week ago

I am using following for validating the check result. It could be reference for future readers.

/**
 * Checks if given string is valid and expected output from plugin check command.
 *
 * @since 1.0.0
 *
 * @param string $input String to check for validity.
 * @return bool true if valid, otherwise false.
 */
protected function is_valid_check_result( $input ) {
    // Bail if input is empty.
    if ( empty( $input ) ) {
        return false;
    }

    // Split the input into blocks based on the pattern of "FILE:".
    $blocks = preg_split( '/(?=FILE:)/', $input, -1, PREG_SPLIT_NO_EMPTY );

    // Bail if splitted array is empty.
    if ( empty( $blocks ) || ! is_array( $blocks ) ) {
        return false;
    }

    foreach ( $blocks as $block ) {
        $block = trim( $block );

        // Split each block into a filename and a JSON string.
        $lines = explode( "\n", $block, 2 );

        if ( count( $lines ) < 2 ) {
            return false;
        }

        // Check if the first line is "FILE:" line.
        if ( ! str_starts_with( $lines[0], 'FILE:' ) ) {
            return false;
        }

        // Check if the second part is a valid JSON.
        if ( ! $this->is_valid_json_string( $lines[1] ) ) {
            return false;
        }
    }

    return true;
}
ernilambar commented 1 week ago

May be we could introduce format like json-strict which will generate output like I mentioned above (which will include "file" inside the output row). Even though it is not difficult to parse the output, I would happily welcome strict JSON format so that validation and parsing is simpler. OR introduce flag like --strict-format.

singerb commented 1 week ago

FYI for anyone thinking about picking this up, fully fixing it will AFAIK require wp-cli changes, not just plugin-check changes, based on my initial investigation/attempt to fix it myself.

On the plugin-check side, I believe a new flag like --strict-format could be added, and the presence of that used to place file information into the JSON instead of the LINE: file.php output there is now. That would simplify parsing a little, since then you just try to parse each line as JSON, and use it if it succeeds.

To add a new format type instead would require wp-cli changes, since those formats are all defined in WP_CLI\Formatter.

Because that formatter is also what's writing the output, getting an output file argument down to it would also require wp-cli changes. Alternatively, plugin-check could take over the formatting itself in this case. Something like a --structured-output=<file> flag could switch the results output into a totally plugin-check controlled flow that would e.g. write JSON line by line to the specified file.

TLDR: it's possible, but either slightly hacky (take things away from wp-cli and into plugin-check), or more involved (make wp-cli changes first, and then use them in plugin-check once released).